[PATCH 1/2] NFSD: Use nfsd_iter_read() when ->splice_read is not zero-copy

Ameer Hamza <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.nfs
Message-ID <[email protected]>
For some files splicing a READ cannot avoid a copy: gfs2, kernfs
and the cifs direct-I/O modes use copy_splice_read() as their
->splice_read, and the VFS substitutes it for DAX files.
copy_splice_read() allocates a fresh page for every page of
payload and reads into it; nfsd_splice_actor() then installs
those pages in rq_respages, displacing Reply pages the thread
already owns. Both sets of pages are then freed.

Route these READs through nfsd_iter_read() instead. It performs
the same single copy, but into the thread's own Reply pages, so
the per-READ allocation and the displacement both disappear. On
its own this is not expected to raise throughput; it changes
which pages a Reply is built from so that the next patch can
recycle them.

9p and ceph fall back to copy_splice_read() only inside their
own ->splice_read methods, which nfsd_splice_read_is_zero_copy()
cannot detect, so they keep the splice path. nfsd_iter_read() is
the path sec=krb5i, sec=krb5p and nfsd_disable_splice_read READs
already take, and is unchanged here; the one visible difference
is that an fsnotify watcher now sees two access events per READ
instead of one, the extra one from vfs_iocb_iter_read().

Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <[email protected]>
---
 fs/nfsd/nfs4xdr.c |  4 ++--
 fs/nfsd/vfs.c     |  5 ++++-
 fs/nfsd/vfs.h     | 29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 7d1b2d6f57f20..07a4bd8feb764 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -5369,7 +5369,7 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
 	maxcount = min_t(unsigned long, read->rd_length,
 			 (xdr->buf->buflen - xdr->buf->len));
 
-	if (file->f_op->splice_read && splice_ok)
+	if (nfsd_splice_read_is_zero_copy(file) && splice_ok)
 		nfserr = nfsd4_encode_splice_read(resp, read, file, maxcount);
 	else
 		nfserr = nfsd4_encode_readv(resp, read, maxcount);
@@ -6267,7 +6267,7 @@ nfsd4_encode_read_plus_data(struct nfsd4_compoundres *resp,
 	maxcount = min_t(unsigned long, read->rd_length,
 			 (xdr->buf->buflen - xdr->buf->len));
 
-	if (file->f_op->splice_read && splice_ok)
+	if (nfsd_splice_read_is_zero_copy(file) && splice_ok)
 		nfserr = nfsd4_encode_splice_read(resp, read, file, maxcount);
 	else
 		nfserr = nfsd4_encode_readv(resp, read, maxcount);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f9131827d391e..1a5fec4cf73d3 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1176,6 +1176,9 @@ nfsd_direct_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
  *
  * Some filesystems or situations cannot use nfsd_splice_read. This
  * function is the slightly less-performant fallback for those cases.
+ * It is also the preferred path where splicing would copy anyway
+ * (see nfsd_splice_read_is_zero_copy()), because the copy then
+ * lands directly in Reply pages nfsd already owns.
  *
  * Returns nfs_ok on success, otherwise an nfserr stat value is
  * returned.
@@ -1576,7 +1579,7 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		return err;
 
 	file = nf->nf_file;
-	if (file->f_op->splice_read && nfsd_read_splice_ok(rqstp))
+	if (nfsd_splice_read_is_zero_copy(file) && nfsd_read_splice_ok(rqstp))
 		err = nfsd_splice_read(rqstp, fhp, file, offset, count, eof);
 	else
 		err = nfsd_iter_read(rqstp, fhp, nf, offset, count, 0, eof);
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index f0cb184643f2f..70bd3c6fc1880 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -149,6 +149,35 @@ __be32		nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				unsigned long *count, unsigned int base,
 				u32 *eof);
 bool		nfsd_read_splice_ok(struct svc_rqst *rqstp);
+
+/**
+ * nfsd_splice_read_is_zero_copy - check whether splice can avoid a data copy
+ * @file: file to be read from
+ *
+ * copy_splice_read() reads via ->read_iter into freshly allocated
+ * pages, exactly as nfsd_iter_read() does into pages nfsd already
+ * holds. Filesystems that implement ->splice_read with it gain
+ * nothing from the splice path, and neither do DAX files, for
+ * which the VFS substitutes copy_splice_read() no matter what
+ * the filesystem registered. The VFS substitutes it for O_DIRECT
+ * files as well, but nfsd never opens files O_DIRECT.
+ *
+ * The test is one-sided: a filesystem's own ->splice_read method
+ * may fall back to copy_splice_read() internally, as ceph and 9p
+ * do, and that cannot be detected here.
+ *
+ * Return values:
+ *   %true: splicing from @file is not known to copy
+ *   %false: splicing from @file would copy, or is not supported
+ *           at all; use nfsd_iter_read()
+ */
+static inline bool nfsd_splice_read_is_zero_copy(const struct file *file)
+{
+	return file->f_op->splice_read &&
+	       file->f_op->splice_read != copy_splice_read &&
+	       !IS_DAX(file_inode(file));
+}
+
 __be32		nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				loff_t offset, unsigned long *count,
 				u32 *eof);
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.