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

[Chuck Lever <cel@kernel.org>]

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.


>> 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?


>> 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.

-- 
Chuck Lever