Re: [GIT PULL] NFS LOCALIO O_DIRECT changes for Linux 6.18

[Chuck Lever <chuck.lever@oracle.com>]

On 9/30/25 1:35 PM, Mike Snitzer wrote:
> On Tue, Sep 30, 2025 at 01:15:06PM -0400, Chuck Lever wrote:
>> On 9/30/25 12:26 PM, Mike Snitzer wrote:
>>> Hi Anna,
>>>
>>> Given that my NFS LOCALIO O_DIRECT changes depend on NFSD changes
>>> which will be included in the NFSD pull request for 6.18, I figured it
>>> worth proposing a post-NFSD-merge pull request for your consideration
>>> as the best way forward logistically (to ensure linux-next coverage,
>>> you could pull this in _before_ Chuck sends his pull to Linus).
>>>
>>> If you were to pull this into your NFS tree it'd bring with it Chuck's
>>> nfsd-next (commit db155b7c7c85b5 as of now) followed by my dependant
>>> NFS LOCALIO O_DIRECT changes.
>>>
>>> You'll note that I folded 3 commits from Chuck's current nfsd-testing
>>> branch into a single "NFSD: filecache: add STATX_DIOALIGN and
>>> STATX_DIO_READ_ALIGN support", the commits from cel/nfsd-testing are:
>>>
>>>  9cc8512712b11 NFSD: filecache: add STATX_DIOALIGN and STATX_DIO_READ_ALIGN support
>>>  e5107ff95c56d NFSD: Prevent a NULL pointer dereference in fh_getattr()
>>>  ed7edd1976c04 NFSD: Ignore vfs_getattr() failure in nfsd_file_get_dio_attrs()
>> I strongly prefer that the second and third patch above be permitted to
>> soak in nfsd-testing before being merged. These two modify very hot code
>> paths in NFSD, and therefore deserve proper test experience.
>>
>> Please don't push NFSD patches through other trees without at least an
>> Acked-by from the NFSD maintainers. The first patch has been acked, as
>> you requested, and is ready to be applied to the NFS client tree. That
>> Ack does not apply to the second and third patches above.
> 
> Please explain why you're in favor of NFS picking up an NFSD commit
> with known Fixes?

I didn't say "don't take the fixes." I said "don't take the fixes
without Acks and proper testing."


> And why didn't you fold your fixes into the original to begin with?

1. I had already sent 1/4 to Anna. Fixes to that patch now have to be
   treated separately.

2. As I said, the fixes need to have full test experience. 1/4 already
   has full test experience. Fixes: tags do not mean patches no longer
   need to get tested, I think you would agree.

3. The fix patches address bugs found by code review, not by test
   failures or bug reports, and are therefore not as urgent.

4. The fixes can be merged during v6.18-rc if we think there actually is
   some urgency. To me that is better form overall because it explicitly
   preserves the provenance of the changes.

5. As I stated, my Acked-by applied to 1/4 and not the other two.

6. The squashed patch needs to be posted so it gets archived properly
   along with any additional discussion.

7. It's the maintainers that get to make these decisions, not the
   contributor.

I don't see a reason why we need to waive the usual review, testing, and
merge process guidelines here, especially considering there was zero
community discussion about this beforehand.

Again: taking the fixes is fine. But please allow them to be tested
properly, and let's get maintainer Acks first.


-- 
Chuck Lever