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

[Jeff Layton <jlayton@kernel.org>]

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.
-- 
Jeff Layton <jlayton@kernel.org>