Re: [PATCH v5 4/7] NFSD: add io_cache_write controls to debugfs interface

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Aug 08, 2025 at 01:58:54PM -0400, Chuck Lever wrote:
> On 8/7/25 12:25 PM, Mike Snitzer wrote:
> > Add 'io_cache_write' to NFSD's debugfs interface so that: Any data
> > written by NFSD will either be:
> > - cached using page cache (NFSD_IO_BUFFERED=1)
> > - cached but removed from the page cache upon completion
> >   (NFSD_IO_DONTCACHE=2).
> > - not cached (NFSD_IO_DIRECT=3)
> > 
> > io_cache_write may be set by writing to:
> >   /sys/kernel/debug/nfsd/io_cache_write
> > 
> > If NFSD_IO_DONTCACHE is specified using 2, FOP_DONTCACHE must be
> > advertised as supported by the underlying filesystem (e.g. XFS),
> > otherwise all IO flagged with RWF_DONTCACHE will fail with
> > -EOPNOTSUPP.
> > 
> > If NFSD_IO_DIRECT is specified using 3, the IO must be aligned
> > relative to the underlying block device's logical_block_size. Also the
> > memory buffer used to store the WRITE payload must be aligned relative
> > to the underlying block device's dma_alignment.
> > 
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > ---
> >  fs/nfsd/debugfs.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  fs/nfsd/nfsd.h    |  1 +
> >  fs/nfsd/vfs.c     | 16 ++++++++++++++++
> >  3 files changed, 61 insertions(+)
> > 
> > diff --git a/fs/nfsd/debugfs.c b/fs/nfsd/debugfs.c
> > index c07f71d4e84f4..872de65f0e9ac 100644
> > --- a/fs/nfsd/debugfs.c
> > +++ b/fs/nfsd/debugfs.c
> > @@ -87,6 +87,47 @@ static int nfsd_io_cache_read_set(void *data, u64 val)
> >  DEFINE_DEBUGFS_ATTRIBUTE(nfsd_io_cache_read_fops, nfsd_io_cache_read_get,
> >  			 nfsd_io_cache_read_set, "%llu\n");
> >  
> > +/*
> > + * /sys/kernel/debug/nfsd/io_cache_write
> > + *
> > + * Contents:
> > + *   %1: NFS WRITE will use buffered IO
> > + *   %2: NFS WRITE will use dontcache (buffered IO w/ dropbehind)
> > + *   %3: NFS WRITE will use direct IO
> > + *
> > + * The default value of this setting is zero (UNSPECIFIED).
> > + * This setting takes immediate effect for all NFS versions,
> > + * all exports, and in all NFSD net namespaces.
> > + */
> > +
> > +static int nfsd_io_cache_write_get(void *data, u64 *val)
> > +{
> > +	*val = nfsd_io_cache_write;
> > +	return 0;
> > +}
> > +
> > +static int nfsd_io_cache_write_set(void *data, u64 val)
> > +{
> > +	int ret = 0;
> > +
> > +	switch (val) {
> > +	case NFSD_IO_BUFFERED:
> > +	case NFSD_IO_DONTCACHE:
> > +	case NFSD_IO_DIRECT:
> > +		nfsd_io_cache_write = val;
> > +		break;
> > +	default:
> > +		nfsd_io_cache_write = NFSD_IO_UNSPECIFIED;
> > +		ret = -EINVAL;
> 
> I might be wrong, but an error return should leave the setting
> untouched, IMO. Likewise for the read setting.

OK, we should get the NFSD_IO_UNSPECIFIED by default as a side-effect
of it being 0.

So _not_ explicitly setting NFSD_IO_UNSPECIFIED as
catch-all/default/error makes sense.

Will fix, thanks.