Re: [PATCH v4 1/5] NFSD: filecache: add STATX_DIOALIGN and STATX_DIO_READ_ALIGN support

Chuck Lever
2025-07-24T13:17:48Z

On 7/23/25 3:56 PM, Jeff Layton wrote:
> On Wed, 2025-07-23 at 15:43 -0400, Mike Snitzer wrote:
>> On Wed, Jul 23, 2025 at 03:06:00PM -0400, Mike Snitzer wrote:
>>> On Wed, Jul 23, 2025 at 02:58:19PM -0400, Jeff Layton wrote:
>>>> On Wed, 2025-07-23 at 11:43 -0400, Mike Snitzer wrote:
>>>>> Use STATX_DIOALIGN and STATX_DIO_READ_ALIGN to get and store DIO
>>>>> alignment attributes from underlying filesystem in associated
>>>>> nfsd_file. This is done when the nfsd_file is first opened for
>>>>> a regular file.
>>>>>
>>>>> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
>>>>> ---
>>>>> fs/nfsd/filecache.c | 32 ++++++++++++++++++++++++++++++++
>>>>> fs/nfsd/filecache.h | 4 ++++
>>>>> fs/nfsd/nfsfh.c | 4 ++++
>>>>> 3 files changed, 40 insertions(+)
>>>>>
>>>>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
>>>>> index 8581c131338b..5447dba6c5da 100644
>>>>> --- a/fs/nfsd/filecache.c
>>>>> +++ b/fs/nfsd/filecache.c
>>>>> @@ -231,6 +231,9 @@ nfsd_file_alloc(struct net *net, struct inode *inode, unsigned char need,
>>>>> refcount_set(&nf->nf_ref, 1);
>>>>> nf->nf_may = need;
>>>>> nf->nf_mark = NULL;
>>>>> + nf->nf_dio_mem_align = 0;
>>>>> + nf->nf_dio_offset_align = 0;
>>>>> + nf->nf_dio_read_offset_align = 0;
>>>>> return nf;
>>>>> }
>>>>>
>>>>> @@ -1048,6 +1051,33 @@ nfsd_file_is_cached(struct inode *inode)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +static __be32
>>>>> +nfsd_file_getattr(const struct svc_fh *fhp, struct nfsd_file *nf)
>>>>> +{
>>>>> + struct inode *inode = file_inode(nf->nf_file);
>>>>> + struct kstat stat;
>>>>> + __be32 status;
>>>>> +
>>>>> + /* Currently only need to get DIO alignment info for regular files */
>>>>> + if (!S_ISREG(inode->i_mode))
>>>>> + return nfs_ok;
>>>>> +
>>>>> + status = fh_getattr(fhp, &stat);
>>>>> + if (status != nfs_ok)
>>>>> + return status;
>>>>> +
>>>>> + if (stat.result_mask & STATX_DIOALIGN) {
>>>>> + nf->nf_dio_mem_align = stat.dio_mem_align;
>>>>> + nf->nf_dio_offset_align = stat.dio_offset_align;
>>>>> + }
>>>>> + if (stat.result_mask & STATX_DIO_READ_ALIGN)
>>>>> + nf->nf_dio_read_offset_align = stat.dio_read_offset_align;
>>>>> + else
>>>>> + nf->nf_dio_read_offset_align = nf->nf_dio_offset_align;
>>>>> +
>>>>> + return status;
>>>>> +}
>>>>> +
>>>>> static __be32
>>>>> nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
>>>>> struct svc_cred *cred,
>>>>> @@ -1166,6 +1196,8 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
>>>>> }
>>>>> status = nfserrno(ret);
>>>>> trace_nfsd_file_open(nf, status);
>>>>> + if (status == nfs_ok)
>>>>> + status = nfsd_file_getattr(fhp, nf);
>>>>
>>>>
>>>> Doing a getattr alongside every open could be expensive in some
>>>> configurations (like reexported NFS). We may want to skip doing this
>>>> getattr this if O_DIRECT isn't is use. Is that possible?
>>>
>>> Good point, yes, should be easy enough. Will depend on the debugfs
>>> knobs, so will tack a patch on at the end.
>>
>> What is the best way to check for NFSD reexporting NFS?
>
>> I've done stuff like that as a side-effect of setting/checking a
>> particular flag, e.g. commit 5cca2483b9fd ("nfsd: disallow file
>> locking and delegations for NFSv4 reexport")
>>
>> But not immediately seeing a more generic way to do the check...
>>
>
> I don't think there is a reliable method, and we probably wouldn't want
> to just limit this to NFS. Other filesystems might have similar
> limitations (e.g. Ceph or Lustre).
>
> I'd probably just base this on whether support is enabled in debugfs.
> If it is, then do the getattr. If that slows down reexports then we can
> figure out what to do then. If and when we get to converting this to an
> export option, we may need to do something more elaborate, but I
> wouldn't bother for now.

Fwiw, I would expect that re-exports would prefer to utilize the NFS
server's page cache.

--
Chuck Lever