Re: nfsd: add the ability to enable use of RWF_DONTCACHE for all nfsd IO

[Mike Snitzer <snitzer@kernel.org>]

On Thu, Feb 20, 2025 at 02:15:05PM -0500, Chuck Lever wrote:
> On 2/20/25 2:00 PM, Jeff Layton wrote:
> > On Thu, 2025-02-20 at 12:12 -0500, Mike Snitzer wrote:
> >> Add nfsd 'nfsd_dontcache' modparam so that "Any data read or written
> >> by nfsd will be removed from the page cache upon completion."
> >>
> >> nfsd_dontcache is disabled by default.  It may be enabled with:
> >>   echo Y > /sys/module/nfsd/parameters/nfsd_dontcache
> >>
> >> FOP_DONTCACHE must be advertised as supported by the underlying
> >> filesystem (e.g. XFS), otherwise if/when 'nfsd_dontcache' is enabled
> >> all IO will fail with -EOPNOTSUPP.
> >>
> > 
> > A little more explanation here would be good. What problem is this
> > solving? In general we don't go for tunables like this unless there is
> > just no alternative.
> > 
> > What might help me understand this is to add some documentation that
> > explains when an admin would want to enable this.
> 
> Agreed: I might know why this is interesting, since Mike and I discussed
> it during bake-a-thon. But other reviewers don't, so it would be helpful
> to provide a little more context in the patch description.

Buffered IO can bring about some serious inefficiencies due to page
reclaim -- we (at Hammerspace) have seen some really nasty pathological
cliffs (particularly with NFSD) due to buffered IO.  I agree that
all needs to be unpacked and detailed.  To be continued... ;)

But in general, Jens details the problem DONTCACHE solves in his various
commit headers associated with DONTCACHE, highlight v6.14-rc1 commits
include:

overview:
9ad6344568cc mm/filemap: change filemap_create_folio() to take a struct kiocb

read results:
8026e49bff9b mm/filemap: add read support for RWF_DONTCACHE

write results with XFS (not yet upstream):
https://git.kernel.dk/cgit/linux/commit/?h=buffered-uncached-fs.11&id=1facf34cb1b7101fa2226c1856dad651e47d916f

Mike


> 
> 
> >> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> >> ---
> >>  fs/nfsd/vfs.c | 17 ++++++++++++++++-
> >>  1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> >> index 29cb7b812d71..d7e49004e93d 100644
> >> --- a/fs/nfsd/vfs.c
> >> +++ b/fs/nfsd/vfs.c
> >> @@ -955,6 +955,11 @@ nfsd_open_verified(struct svc_fh *fhp, int may_flags, struct file **filp)
> >>  	return __nfsd_open(fhp, S_IFREG, may_flags, filp);
> >>  }
> >>  
> >> +static bool nfsd_dontcache __read_mostly = false;
> >> +module_param(nfsd_dontcache, bool, 0644);
> >> +MODULE_PARM_DESC(nfsd_dontcache,
> >> +		 "Any data read or written by nfsd will be removed from the page cache upon completion.");
> >> +
> >>  /*
> >>   * Grab and keep cached pages associated with a file in the svc_rqst
> >>   * so that they can be passed to the network sendmsg routines
> >> @@ -1084,6 +1089,7 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >>  	loff_t ppos = offset;
> >>  	struct page *page;
> >>  	ssize_t host_err;
> >> +	rwf_t flags = 0;
> >>  
> >>  	v = 0;
> >>  	total = *count;
> >> @@ -1097,9 +1103,12 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >>  	}
> >>  	WARN_ON_ONCE(v > ARRAY_SIZE(rqstp->rq_vec));
> >>  
> >> +	if (nfsd_dontcache)
> >> +		flags |= RWF_DONTCACHE;
> >> +
> >>  	trace_nfsd_read_vector(rqstp, fhp, offset, *count);
> >>  	iov_iter_kvec(&iter, ITER_DEST, rqstp->rq_vec, v, *count);
> >> -	host_err = vfs_iter_read(file, &iter, &ppos, 0);
> >> +	host_err = vfs_iter_read(file, &iter, &ppos, flags);
> >>  	return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
> >>  }
> >>  
> >> @@ -1186,6 +1195,9 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
> >>  	if (stable && !fhp->fh_use_wgather)
> >>  		flags |= RWF_SYNC;
> >>  
> >> +	if (nfsd_dontcache)
> >> +		flags |= RWF_DONTCACHE;
> >> +
> >>  	iov_iter_kvec(&iter, ITER_SOURCE, vec, vlen, *cnt);
> >>  	since = READ_ONCE(file->f_wb_err);
> >>  	if (verf)
> >> @@ -1237,6 +1249,9 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
> >>   */
> >>  bool nfsd_read_splice_ok(struct svc_rqst *rqstp)
> >>  {
> >> +	if (nfsd_dontcache) /* force the use of vfs_iter_read for reads */
> >> +		return false;
> >> +
> >>  	switch (svc_auth_flavor(rqstp)) {
> >>  	case RPC_AUTH_GSS_KRB5I:
> >>  	case RPC_AUTH_GSS_KRB5P:
> > 
> 
> 
> -- 
> Chuck Lever