Re: NFSD: Add io_cache_{read,write} controls to debugfs

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Sep 05, 2025 at 02:45:37PM -0400, Chuck Lever wrote:
> On 9/5/25 1:32 PM, Mike Snitzer wrote:
> > On Fri, Sep 05, 2025 at 10:55:09AM -0400, Chuck Lever wrote:
> >> From: Chuck Lever <chuck.lever@oracle.com>
> > 
> > Aside from attribution, which I suspect you didn't intend to switch
> > these changes from being attributed to me, this looks good (one small
> > nit below).
> 
> I didn't intend to change the attribution, you're right. My tool
> chain's squashing always changes the patch authorship and I don't
> always remember to catch it.
> 
> I will fix that up.

Ack.

> >> Add 'io_cache_read' to NFSD's debugfs interface so that any data
> >> read by NFSD will either be:
> >> - cached using page cache (NFSD_IO_BUFFERED=0)
> >> - cached but removed from the page cache upon completion
> >>   (NFSD_IO_DONTCACHE=1).
> >>
> >> io_cache_read may be set by writing to:
> >>   /sys/kernel/debug/nfsd/io_cache_read
> >>
> >> 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=0)
> >> - cached but removed from the page cache upon completion
> >>   (NFSD_IO_DONTCACHE=1).
> >>
> >> io_cache_write may be set by writing to:
> >>   /sys/kernel/debug/nfsd/io_cache_write
> >>
> >> The default value for both settings is NFSD_IO_BUFFERED, which is
> >> NFSD's existing behavior for both read and write. Changes to these
> >> settings take immediate effect for all exports and NFS versions.
> >>
> >> If NFSD_IO_DONTCACHE is specified, all exported filesystems must
> >> implement FOP_DONTCACHE, otherwise IO flagged with RWF_DONTCACHE
> >> will fail with -EOPNOTSUPP.
> 
> Btw, I'd like NFSD to fall back to buffered I/O if the file system's
> FOP_DONTCACHE flag is not set. Do you have an objection to that?

Ah yes, now that you mention it I recall you suggesting that before.
Got lost in the shuffle but I agree that it makes sense.  Just that
like the NFSD_IO_DIRECT case, doing negative checks for every IO is
excessive. SO I left sorting out the best way to implement it as TBD,
but forgot about it.

The challenge is really that the global debugfs knobs are so
coarse-grained. Handling this type of thing will be much easier once
per-export configuration is possible.

> >> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >> ---
> >>  fs/nfsd/debugfs.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  fs/nfsd/nfsd.h    |  9 +++++
> >>  fs/nfsd/vfs.c     | 19 ++++++++++
> >>  3 files changed, 121 insertions(+)
> >>
> >> Changes from Mike's v9:
> >> - Squashed the "io controls" patches together
> >> - Removed NFSD_IO_DIRECT for the moment
> >> - Addressed a few more checkpatch.pl nits
> >>
> >> This gives a cleaner platform on which to build the direct I/O code
> >> paths, and does not expose partially implemented I/O modes to users.
> >>
> >> diff --git a/fs/nfsd/debugfs.c b/fs/nfsd/debugfs.c
> >> index 84b0c8b559dc..2b1bb716b608 100644
> >> --- a/fs/nfsd/debugfs.c
> >> +++ b/fs/nfsd/debugfs.c
> >> @@ -44,4 +131,10 @@ void nfsd_debugfs_init(void)
> >>  
> >>  	debugfs_create_file("disable-splice-read", S_IWUSR | S_IRUGO,
> >>  			    nfsd_top_dir, NULL, &nfsd_dsr_fops);
> >> +
> >> +	debugfs_create_file("io_cache_read", 0644, nfsd_top_dir, NULL,
> >> +			    &nfsd_io_cache_read_fops);
> >> +
> >> +	debugfs_create_file("io_cache_write", 0644, nfsd_top_dir, NULL,
> >> +			    &nfsd_io_cache_write_fops);
> >>  }
> > 
> > Relative to checkpatch warnings, this ^ code is what I'm aware of.
> > For consistency I stuck with "S_IWUSR | S_IRUG", whereas you honored
> > checkpatch's suggestion to use 0644.
> > 
> > Maybe update disable-splice-read to also use 0644 too? But your call!
> 
> Still debating that myself. It's a change that is not related to the
> purpose of this patch, so I probably should do that kind of clean-up
> separately.

Ack.