[PATCH 0/2] SUNRPC: Recycle sent Reply pages instead of freeing them

[PATCH 0/2] SUNRPC: Recycle sent Reply pages instead of freeing them

[Ameer Hamza]

An NFS READ Reply built in pages the nfsd thread allocated has
those pages freed from softirq, when the peer's ACK arrives. Since
commit 574907741599 ("mm/page_alloc: leave IRQs enabled for per-cpu
page allocations") such frees collide with alloc_pages_bulk()'s
IRQs-on pcp window and fall back to free_one_page() under
zone->lock; on a single memory node every thread contends for the
same zone->lock and the collisions compound. Replies spliced from
page-cache folios are immune, since the page cache still holds a
reference and the ACK-time put never reaches the allocator. The
rest are not. On gfs2, kernfs, cifs direct I/O and DAX files
splicing runs copy_splice_read(), which allocates a fresh page for
every page of payload, and nfsd allocates them too when it does not
splice at all: under nfsd_disable_splice_read, and for sec=krb5i
and sec=krb5p.

Patch 1 removes a redundant allocation: on those files nfsd builds
the Reply out of pages copy_splice_read() allocated for that one
Reply and discards the pages it was already holding, so it
allocates twice for every page of payload. Reading through
nfsd_iter_read() does the same single copy straight into the pages
nfsd already owns. Patch 2 removes the free: nfsd drops its own
reference as soon as the Reply is sent, leaving the network with
the last one. Keeping that reference instead and reusing the page
once folio_ref_count() reads 1 keeps it out of the allocator. A
thread holds at most 4 MiB of pages, trimmed as its own demand
falls and freed when it exits.

How much this is worth depends on contention: on the server below
the ACK-time frees reach 43% of cycles, while on an uncontended
machine the same change removes the allocations and leaves total
CPU where it was.

The measurements below use ext4 with DAX and tmpfs so they
reproduce on a stock tree with a pmem device; ZFS is affected the
same way, its ->splice_read being copy_splice_read() as well, but
nothing here depends on it. Single-node server, 64 threads, one
nfsd thread per CPU, eight clients, 1 MiB sequential cached reads
over NFSv4.0, 60s a run, clients in network namespaces on the same
box, so vmstat counts their allocations too.

                              cycles in       pages allocated
                            free_one_page    per payload page
  ext4 dax  NFSv4            43%  ->   0%        2.97 -> 0.05
  ext4 dax  NFSv4, knob      32%  ->   0%        1.99 -> 0.05
  tmpfs     NFSv4, knob      33%  ->   0%        1.98 -> 0.05

The knob rows have nfsd_disable_splice_read set, so they reach the
same path without patch 1 and measure patch 2 alone. The first
column is free_one_page()'s share of all CPU cycles on the box, the
second is vmstat page allocations divided by the READ payload
delivered, Call buffers included.

The table is from the series backported to 6.18.38, where the
release path is adapted to that tree's combined rq_pages array.
The posted code was run only in a 20-CPU VM, not on that server.
free_one_page() falls from 24628 calls per million pages served
to 5990 and softirq time per page from 0.836 to 0.528 us, while
total CPU per page does not move: svc_reuse_scan()'s 2.5% of
cycles is paid back by the allocator work it removes. The
scan's share falls to 0.7% when late ACKs leave little to
reuse, since it stops after a pass in which no page is ready.

Both read paths pass over NFSv3 and NFSv4.2 under KASAN, lockdep
and DEBUG_VM, with xfstests -g quick and pynfs 4.0/4.1 matching
the unpatched baseline, cthon04 passing, and byte-identical
checksums over NFS/RDMA and NFSv3/UDP. All of that runs on a fast
link, where a build with the folio_ref_count() gate deliberately
removed also passes the checksum runs. Under a 40 ms ACK delay
that build corrupts data and the series stays byte-identical.

Ameer Hamza (2):
  NFSD: Use nfsd_iter_read() when ->splice_read is not zero-copy
  SUNRPC: Recycle sent Reply pages instead of freeing them

 fs/nfsd/nfs4xdr.c               |   4 +-
 fs/nfsd/vfs.c                   |   5 +-
 fs/nfsd/vfs.h                   |  29 ++++
 include/linux/sunrpc/svc.h      |  14 ++
 include/linux/sunrpc/svc_xprt.h |  14 ++
 include/trace/events/sunrpc.h   |  32 ++++-
 net/sunrpc/svc.c                | 235 +++++++++++++++++++++++++++++++-
 net/sunrpc/svc_xprt.c           |   7 +
 net/sunrpc/svcsock.c            |   2 +
 9 files changed, 335 insertions(+), 7 deletions(-)


base-commit: 76427d869120552a1a82e1f1488d9f8311827d84
-- 
2.53.0

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

[Ameer Hamza]

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 <ameer.hamza@truenas.com>
---
 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

[PATCH 2/2] SUNRPC: Recycle sent Reply pages instead of freeing them

[Ameer Hamza]

svc_rqst_release_pages() drops the thread's reference on each sent
Reply page right after the send, and svc_alloc_arg() bulk-allocates
replacements before the next RPC. Socket transports send with
MSG_SPLICE_PAGES and hold a reference of their own until transmit
completion (UDP) or until the peer's ACK covers the data (TCP), so
for a Reply built in pages the thread allocated, the last reference
is dropped from softirq at ACK time: ~257 pages per 1 MiB READ on
4 KiB pages. A Reply spliced from page-cache folios is unaffected,
since the page cache still holds a reference and nothing reaches
the allocator.

Since commit 574907741599 ("mm/page_alloc: leave IRQs enabled for
per-cpu page allocations"), alloc_pages_bulk() holds the pcp lock
across the whole batch with IRQs enabled. An ACK-time free landing
on that CPU inside the window cannot take the pcp lock, because the
free path only trylocks, so it falls back to free_one_page() under
zone->lock. The allocating CPUs then refill from the buddy more
often, under zone->lock with the pcp lock still held, so the window
lengthens and the next free collides more often. On a single memory
node this settles into a steady state with most CPU cycles in the
queued-spinlock slowpath. Commit a39f0ce0c9da ("Revert "svcrdma:
Use contiguous pages for RDMA Read sink buffers"") describes the
same terminal state.

Remove the free. svc_rqst_release_pages() now keeps the thread's
reference on pages the thread allocated, parking them in a small
per-thread array, so the transport's final put_page() no longer
enters the page allocator. A parked page refills a free slot in
this thread's buffers once folio_ref_count() reads 1, whether that
slot awaits a Call or a Reply. That is the gate the network stack
has applied to its own ACK-timed references since 2012, today in
skb_page_frag_refill(). Releasing result pages was discussed in
2021 [1][2]. The array is unordered and each scan resumes from a
persistent cursor, so pages still held by one slow connection
cannot block reuse of pages parked after them.

Reuse is opt-in per transport class and only svc_udp_class and
svc_tcp_class declare XCL_FL_REPLY_PAGE_REUSE: on a socket, every
consumer of a sent Reply page either holds its own reference for
as long as it uses the page or is done with it before
->xpo_sendto() returns, so a reference count of 1 is proof the
network is done with the page. A Reply that carries spliced-in
folios is excluded, and a page is reused only if it is order-0,
unpoisoned, not pfmemalloc, node-local and not page-cache-backed.
Otherwise pages are released exactly as before.

A thread holds at most enough pages for four maximum-size
Replies and never more than 4 MiB, so a 128-thread server holds
at most 512 MiB. A surplus is trimmed as the thread's own demand
falls; a thread that stops serving keeps what it has until
traffic resumes or it exits. For comparison, each svc_rqst
already pins two arrays of rq_maxpages pages for the life of the
thread, which is of the same order.

Measured with this work backported to 6.18.38, serving 1 MiB cached
reads to eight clients over NFSv4.0 from an ext4 export mounted
dax=always: cycles spent in free_one_page() fall from 43% to nil,
and page allocations per page of payload served from 2.97 to 0.05.
A tmpfs export with nfsd_disable_splice_read set, which reaches the
same path without the previous patch, falls from 33% to nil and
from 1.98 to 0.05. An ext4 export without DAX has no such free to
collide; its free_one_page() cycles and throughput are unchanged.

Link: https://lore.kernel.org/linux-nfs/87im7ffjp0.fsf@notabene.neil.brown.name/ [1]
Link: https://lore.kernel.org/linux-nfs/161400740732.195066.3792261943053910900.stgit@klimt.1015granger.net/ [2]
Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
 include/linux/sunrpc/svc.h      |  14 ++
 include/linux/sunrpc/svc_xprt.h |  14 ++
 include/trace/events/sunrpc.h   |  32 ++++-
 net/sunrpc/svc.c                | 235 +++++++++++++++++++++++++++++++-
 net/sunrpc/svc_xprt.c           |   7 +
 net/sunrpc/svcsock.c            |   2 +
 6 files changed, 300 insertions(+), 4 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 2db1b9ec5658d..407558b8cbdc8 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -155,6 +155,13 @@ extern u32 svc_max_payload(const struct svc_rqst *rqstp);
  * [rq_respages, rq_next_page) after each RPC. svc_alloc_arg()
  * refills only that range.
  *
+ * On a transport that has declared XCL_FL_REPLY_PAGE_REUSE,
+ * Reply pages this thread allocated and sent are moved to
+ * rq_reuse_pages instead of being released; once the transport
+ * has dropped its references, they refill released slots in
+ * place of fresh page allocations. A Reply into which
+ * nfsd_splice_actor() installed any page is released as before.
+ *
  * xdr_buf holds responses; the structure fits NFS read responses
  * (header, data pages, optional tail) and enables sharing of
  * client-side routines.
@@ -221,6 +228,9 @@ struct svc_rqst {
 	struct page *		*rq_respages;	/* Reply buffer pages */
 	struct page *		*rq_next_page; /* next reply page to use */
 	struct page *		*rq_page_end;  /* one past the last reply page */
+	struct page		**rq_reuse_pages; /* sent pages held for reuse */
+	unsigned long		rq_nreuse;	/* entries in rq_reuse_pages */
+	unsigned long		rq_reuse_cursor; /* where the last scan stopped */
 
 	struct folio_batch	rq_fbatch;
 	struct bio_vec		*rq_bvec;
@@ -276,6 +286,7 @@ enum {
 	RQ_DROPME,		/* drop current reply */
 	RQ_VICTIM,		/* Have agreed to shut down */
 	RQ_DATA,		/* request has data */
+	RQ_RES_REPLACED,	/* splice actor installed Reply pages */
 };
 
 #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
@@ -456,6 +467,9 @@ struct svc_serv *svc_create(struct svc_program *, unsigned int,
 bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
 					 struct page *page);
 void		   svc_rqst_release_pages(struct svc_rqst *rqstp);
+void		   svc_rqst_refill_pages(struct svc_rqst *rqstp,
+					 struct page **first,
+					 struct page **last);
 int		   svc_new_thread(struct svc_serv *serv, struct svc_pool *pool);
 void		   svc_exit_thread(struct svc_rqst *);
 struct svc_serv *  svc_create_pooled(struct svc_program *prog,
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index da2a2531e1106..73f9a8c9a6fbf 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -8,6 +8,7 @@
 #ifndef SUNRPC_SVC_XPRT_H
 #define SUNRPC_SVC_XPRT_H
 
+#include <linux/bits.h>
 #include <linux/sunrpc/svc.h>
 
 struct module;
@@ -37,8 +38,21 @@ struct svc_xprt_class {
 	struct list_head	xcl_list;
 	u32			xcl_max_payload;
 	int			xcl_ident;
+	unsigned long		xcl_flags;
 };
 
+/*
+ * A transport sets this flag to declare that every consumer of a
+ * sent Reply page either holds a page reference for as long as it
+ * uses the page, or is done with the page before ->xpo_sendto()
+ * returns, as sendmsg() is when it copies the payload for an
+ * egress device that lacks NETIF_F_SG. A Reply page whose
+ * folio_ref_count() has returned to one after the send is
+ * therefore no longer in use and may be reused (see
+ * svc_rqst_release_pages()).
+ */
+#define XCL_FL_REPLY_PAGE_REUSE	BIT(0)
+
 /*
  * This is embedded in an object that wants a callback before deleting
  * an xprt; intended for use by NFSv4.1, which needs to know when a
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index ff855197880de..7f5c6dc5bb35a 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1673,7 +1673,8 @@ DEFINE_SVCXDRBUF_EVENT(sendto);
 	svc_rqst_flag(USEDEFERRAL)					\
 	svc_rqst_flag(DROPME)						\
 	svc_rqst_flag(VICTIM)						\
-	svc_rqst_flag_end(DATA)
+	svc_rqst_flag(DATA)						\
+	svc_rqst_flag_end(RES_REPLACED)
 
 #undef svc_rqst_flag
 #undef svc_rqst_flag_end
@@ -2174,6 +2175,35 @@ TRACE_EVENT(svc_alloc_arg_err,
 		__entry->requested, __entry->allocated)
 );
 
+TRACE_EVENT(svc_reuse_scan,
+	TP_PROTO(
+		const struct svc_rqst *rqstp,
+		unsigned long free,
+		unsigned long filled,
+		unsigned long trimmed
+	),
+
+	TP_ARGS(rqstp, free, filled, trimmed),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, held)
+		__field(unsigned long, free)
+		__field(unsigned long, filled)
+		__field(unsigned long, trimmed)
+	),
+
+	TP_fast_assign(
+		__entry->held = rqstp->rq_nreuse;
+		__entry->free = free;
+		__entry->filled = filled;
+		__entry->trimmed = trimmed;
+	),
+
+	TP_printk("held=%lu free=%lu filled=%lu trimmed=%lu",
+		__entry->held, __entry->free, __entry->filled,
+		__entry->trimmed)
+);
+
 DECLARE_EVENT_CLASS(svc_deferred_event,
 	TP_PROTO(
 		const struct svc_deferred_req *dr
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 8297bad2b1777..ededac15771e0 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -20,6 +20,8 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/kthread.h>
+#include <linux/pagemap.h>
+#include <linux/sizes.h>
 #include <linux/slab.h>
 
 #include <linux/sunrpc/types.h>
@@ -548,6 +550,25 @@ svc_destroy(struct svc_serv **servp)
 }
 EXPORT_SYMBOL_GPL(svc_destroy);
 
+/**
+ * svc_reuse_capacity - Bound on the pages a thread may hold for reuse
+ * @rqstp: RPC transaction context
+ *
+ * Four Reply payloads, capped at 4 MiB. On a service whose own
+ * maximum payload is 4 MiB the cap is what binds, and a thread
+ * holds at most one Reply's worth. This is a policy cap, sized so
+ * that the sent-but-unacknowledged data of the connections a
+ * thread services typically fits; pages beyond it are released
+ * exactly as before, as they are when the array cannot be
+ * allocated.
+ *
+ * Return: maximum count of pages to park in rq_reuse_pages
+ */
+static inline unsigned long svc_reuse_capacity(const struct svc_rqst *rqstp)
+{
+	return min(4 * rqstp->rq_maxpages, SZ_4M / PAGE_SIZE);
+}
+
 static bool
 svc_init_buffer(struct svc_rqst *rqstp, const struct svc_serv *serv, int node)
 {
@@ -570,6 +591,15 @@ svc_init_buffer(struct svc_rqst *rqstp, const struct svc_serv *serv, int node)
 		return false;
 	}
 
+	/*
+	 * Page reuse is an optimization; a thread that cannot allocate
+	 * the array releases sent pages the way it always has.
+	 */
+	rqstp->rq_reuse_pages = kcalloc_node(svc_reuse_capacity(rqstp),
+					     sizeof(struct page *),
+					     GFP_KERNEL | __GFP_NORETRY |
+					     __GFP_NOWARN, node);
+
 	rqstp->rq_pages_nfree = rqstp->rq_maxpages;
 	rqstp->rq_next_page = rqstp->rq_respages + rqstp->rq_maxpages;
 	return true;
@@ -596,6 +626,12 @@ svc_release_buffer(struct svc_rqst *rqstp)
 				put_page(rqstp->rq_respages[i]);
 		kfree(rqstp->rq_respages);
 	}
+
+	if (rqstp->rq_reuse_pages) {
+		while (rqstp->rq_nreuse)
+			put_page(rqstp->rq_reuse_pages[--rqstp->rq_nreuse]);
+		kfree(rqstp->rq_reuse_pages);
+	}
 }
 
 static void svc_rqst_free_rcu(struct rcu_head *head)
@@ -934,6 +970,10 @@ EXPORT_SYMBOL_GPL(svc_serv_maxthreads);
  * When replacing a page in rq_respages, batch the release of the
  * replaced pages to avoid hammering the page allocator.
  *
+ * Flags the transaction so that svc_rqst_release_pages() retains
+ * none of this Reply's pages for reuse: it now carries pages this
+ * thread did not allocate.
+ *
  * Return values:
  *   %true: page replaced
  *   %false: array bounds checking failed
@@ -951,12 +991,172 @@ bool svc_rqst_replace_page(struct svc_rqst *rqstp, struct page *page)
 	if (*rqstp->rq_next_page)
 		svc_rqst_page_release(rqstp, *rqstp->rq_next_page);
 
+	/* Avoid an atomic RMW for every page of a spliced READ */
+	if (!test_bit(RQ_RES_REPLACED, &rqstp->rq_flags))
+		set_bit(RQ_RES_REPLACED, &rqstp->rq_flags);
 	get_page(page);
 	*(rqstp->rq_next_page++) = page;
 	return true;
 }
 EXPORT_SYMBOL_GPL(svc_rqst_replace_page);
 
+/*
+ * Take custody of a sent Reply page in place of dropping this
+ * thread's reference. The transport may still hold references, so
+ * the page is not reused until a scan observes ours to be the last
+ * one. That may be the scan at the end of this same release.
+ */
+static bool svc_reuse_page(struct svc_rqst *rqstp, struct page *page)
+{
+	struct folio *folio = page_folio(page);
+
+	if (!rqstp->rq_reuse_pages ||
+	    rqstp->rq_nreuse >= svc_reuse_capacity(rqstp))
+		return false;
+	/* Backstop: never hold a page-cache folio, whatever installed it */
+	if (folio_test_lru(folio) || folio_mapping(folio))
+		return false;
+	rqstp->rq_reuse_pages[rqstp->rq_nreuse++] = page;
+	return true;
+}
+
+/*
+ * Entries svc_reuse_scan() may examine beyond four per free slot.
+ * A scan resumes from rq_reuse_cursor, so this bounds one call and
+ * not overall progress.
+ */
+#define SVC_REUSE_SCAN_SLACK	64
+
+/*
+ * Pages returned to the allocator per scan once every free slot is
+ * filled. Shrinking rq_reuse_pages a few pages at a time lets it
+ * decay as demand falls, while a thread whose demand persists
+ * refills it faster than the trickle drains it.
+ */
+#define SVC_REUSE_TRIM_MAX	8
+
+/*
+ * Fill the free slots in [@first, @last) with held pages that this
+ * thread again exclusively owns. rq_reuse_pages is unordered and
+ * scanned with a cursor: peers acknowledge on independent clocks,
+ * so an ordered queue would let one slow connection block reuse of
+ * every page held after its own. A consumed entry is replaced by
+ * the last entry. The scan ends once every free slot is filled
+ * and any trim budget is spent, after a pass over rq_reuse_pages
+ * in which no entry was ready, or when the per-call examination
+ * budget is spent. Pages that are unsuitable for reuse are
+ * returned to the allocator, and so is a small surplus when
+ * @trim is set.
+ */
+static void svc_reuse_scan(struct svc_rqst *rqstp, struct page **first,
+			   struct page **last, bool trim)
+{
+	unsigned long skipped = 0, trimmed = 0, free = 0, filled = 0;
+	unsigned long i = rqstp->rq_reuse_cursor;
+	unsigned long budget;
+	struct page **slot;
+
+	if (!rqstp->rq_nreuse)
+		return;
+	for (slot = first; slot < last; slot++)
+		if (!*slot)
+			free++;
+	if (!free)
+		return;
+	budget = 4 * free + SVC_REUSE_SCAN_SLACK;
+	slot = first;
+
+	while (budget-- && skipped < rqstp->rq_nreuse) {
+		struct folio *folio;
+		struct page *page;
+
+		if (i >= rqstp->rq_nreuse)
+			i = 0;
+		page = rqstp->rq_reuse_pages[i];
+		folio = page_folio(page);
+
+		/*
+		 * A consumer holding a reference reads this page
+		 * before its fully ordered final put; the control
+		 * dependency orders the overwrite after that put.
+		 * A copying consumer is done when send returns.
+		 * skb_page_frag_refill() reuses on the same test.
+		 */
+		if (folio_ref_count(folio) != 1) {
+			i++;
+			skipped++;
+			continue;
+		}
+		skipped = 0;
+
+		/*
+		 * rq_reuse_pages is unordered, so the last entry
+		 * backfills the vacated one. @i is left alone so the
+		 * backfilled entry is examined in its turn; if the
+		 * removed entry was the last, the wrap at the top of
+		 * the loop moves @i back into range.
+		 */
+		rqstp->rq_reuse_pages[i] =
+			rqstp->rq_reuse_pages[--rqstp->rq_nreuse];
+
+		/*
+		 * memory_failure() can flag a page this thread owns
+		 * without holding a reference, so poison is checked
+		 * at reuse time. Remote and pfmemalloc pages are
+		 * released rather than reused, as the network stack
+		 * does when recycling receive buffers. A large folio
+		 * is released too: folio_ref_count() counts the whole
+		 * folio, so one subpage cannot be shown to be ours
+		 * alone.
+		 */
+		if (unlikely(folio_test_hwpoison(folio) ||
+			     folio_test_large(folio) ||
+			     folio_is_pfmemalloc(folio) ||
+			     folio_nid(folio) != numa_mem_id())) {
+			folio_put(folio);
+			continue;
+		}
+
+		while (slot < last && *slot)
+			slot++;
+		if (slot != last) {
+			*slot = page;
+			filled++;
+			continue;
+		}
+
+		/* Every free slot is filled; decay a small surplus */
+		if (trim && trimmed < SVC_REUSE_TRIM_MAX) {
+			folio_put(folio);
+			trimmed++;
+			continue;
+		}
+		rqstp->rq_reuse_pages[rqstp->rq_nreuse++] = page;
+		break;
+	}
+	rqstp->rq_reuse_cursor = i;
+	trace_svc_reuse_scan(rqstp, free, filled, trimmed);
+}
+
+/**
+ * svc_rqst_refill_pages - Fill free buffer slots from held pages
+ * @rqstp: RPC transaction context
+ * @first: first slot in the range to fill
+ * @last: one past the last slot in the range
+ *
+ * Fill the free slots in [@first, @last) with Reply pages the
+ * network has finished with, so that a thread consults the pages
+ * it already owns before asking the page allocator for more. Only
+ * as many slots are filled as one scan's budget allows.
+ * rq_reuse_pages is not trimmed here; a surplus is trimmed by
+ * svc_rqst_release_pages() instead, as each Reply is released.
+ */
+void svc_rqst_refill_pages(struct svc_rqst *rqstp,
+			   struct page **first, struct page **last)
+{
+	svc_reuse_scan(rqstp, first, last, false);
+}
+
 /**
  * svc_rqst_release_pages - Release Reply buffer pages
  * @rqstp: RPC transaction context
@@ -964,21 +1164,50 @@ EXPORT_SYMBOL_GPL(svc_rqst_replace_page);
  * Release response pages in the range [rq_respages, rq_next_page).
  * NULL entries in this range are skipped, allowing transports to
  * transfer pages to a send context before this function runs.
+ *
+ * Where possible, pages the thread allocated itself are held for
+ * reuse instead of released: the transport's final put_page() then
+ * runs against a page that still has a reference and stays out of
+ * the page allocator entirely. A Reply that contains pages
+ * installed by nfsd_splice_actor(), or one sent by a transport that
+ * has not declared its Reply-page references
+ * (XCL_FL_REPLY_PAGE_REUSE), is released as before, as is any page
+ * that does not fit within svc_reuse_capacity() or that proves to
+ * be page-cache-backed. Free slots in the released range are then
+ * refilled, as far as one scan's budget allows, from held pages
+ * the network has finished with, and a small surplus is returned
+ * to the allocator.
  */
 void svc_rqst_release_pages(struct svc_rqst *rqstp)
 {
+	struct svc_xprt *xprt = rqstp->rq_xprt;
 	struct page **pp;
+	bool hold;
+
+	if (test_bit(RQ_RES_REPLACED, &rqstp->rq_flags)) {
+		clear_bit(RQ_RES_REPLACED, &rqstp->rq_flags);
+		hold = false;
+	} else {
+		hold = xprt &&
+		       (xprt->xpt_class->xcl_flags & XCL_FL_REPLY_PAGE_REUSE);
+	}
 
 	for (pp = rqstp->rq_respages; pp < rqstp->rq_next_page; pp++) {
 		if (*pp) {
-			if (!folio_batch_add(&rqstp->rq_fbatch,
-					     page_folio(*pp)))
-				__folio_batch_release(&rqstp->rq_fbatch);
+			if (!hold || !svc_reuse_page(rqstp, *pp)) {
+				if (!folio_batch_add(&rqstp->rq_fbatch,
+						     page_folio(*pp)))
+					__folio_batch_release(&rqstp->rq_fbatch);
+			}
 			*pp = NULL;
 		}
 	}
 	if (rqstp->rq_fbatch.nr)
 		__folio_batch_release(&rqstp->rq_fbatch);
+
+	if (rqstp->rq_next_page > rqstp->rq_respages)
+		svc_reuse_scan(rqstp, rqstp->rq_respages,
+			       rqstp->rq_next_page, true);
 }
 
 /**
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 40040af588fb2..88b744c324eb0 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -690,6 +690,13 @@ static bool svc_fill_pages(struct svc_rqst *rqstp, struct page **pages,
 	unsigned long filled, ret;
 
 	for (filled = 0; filled < npages; filled = ret) {
+		/*
+		 * alloc_pages_bulk() populates only the slots that are
+		 * NULL on entry and counts the rest in its return
+		 * value, so a slot filled here is a page not allocated
+		 * below.
+		 */
+		svc_rqst_refill_pages(rqstp, pages, pages + npages);
 		ret = alloc_pages_bulk(GFP_KERNEL, npages, pages);
 		if (ret > filled)
 			/* Made progress, don't sleep yet */
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 7a423e9ee74d4..6041be33cb076 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -910,6 +910,7 @@ static struct svc_xprt_class svc_udp_class = {
 	.xcl_ops = &svc_udp_ops,
 	.xcl_max_payload = RPCSVC_MAXPAYLOAD_UDP,
 	.xcl_ident = XPRT_TRANSPORT_UDP,
+	.xcl_flags = XCL_FL_REPLY_PAGE_REUSE,
 };
 
 static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
@@ -1426,6 +1427,7 @@ static struct svc_xprt_class svc_tcp_class = {
 	.xcl_ops = &svc_tcp_ops,
 	.xcl_max_payload = RPCSVC_MAXPAYLOAD_TCP,
 	.xcl_ident = XPRT_TRANSPORT_TCP,
+	.xcl_flags = XCL_FL_REPLY_PAGE_REUSE,
 };
 
 void svc_init_xprt_sock(void)
-- 
2.53.0

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

[Chuck Lever]

Hi Ameer -

First off, thanks for posting the full series to provide good
context for your problem statement. Contention in the page
allocator is vexing, and is a problem we've been chasing for
years.

There is a lot to unpack. I'm going to focus on patch 1/2 and
set 2/2 aside for the moment. I promise we will come back to it.


On Mon, Aug 17, 2026, at 5:08 PM, Ameer Hamza wrote:
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index f0cb184643f2..70bd3c6fc188 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));
> +}

[ ... ]

The iter_read path appears to have two pre-existing problems
that will be exposed to even more file systems when 1/2 is
applied.

1. fsnotify double-counting, which you mention
2. atime is not updated, which shows up in LLM review

LLM explains it this way:

--- cut here ---

nfsd_splice_read() picks one up from the VFS on the way through:

fs/splice.c:splice_direct_to_actor() {
	...
done:
	pipe->tail = pipe->head = 0;
	file_accessed(in);
	return bytes;
	...
}

nfsd_iter_read() has no equivalent, so it leaves atime to ->read_iter.
That works out for most of what moves here -- gfs2 reaches
generic_file_read_iter(), cifs direct I/O reaches
netfs_unbuffered_read_iter(), and ext4_dax_read_iter() and
xfs_file_dax_read() both finish with file_accessed() -- but
fuse_dax_read_iter() leaves it undone:

fs/fuse/dax.c:fuse_dax_read_iter() {
	...
	ret = dax_iomap_rw(iocb, to, &fuse_iomap_ops);
	inode_unlock_shared(inode);

	/* TODO file_accessed(iocb->f_filp) */
	return ret;
}

The second runs the other way.  vfs_iocb_iter_read() emits an fsnotify
access event of its own, and nfsd_finish_read() emits a second one:

fs/nfsd/vfs.c:nfsd_finish_read() {
	...
		*count = host_err;
		fsnotify_access(file);
	...
}

splice_direct_to_actor() emits none, so the event in nfsd_finish_read()
is the only one the splice path gets, and one too many for the iter
path.

Both are reachable today without this patch.  sec=krb5i, sec=krb5p and
nfsd_disable_splice_read all reach nfsd_iter_read(), and NFSD_IO_DIRECT
reaches nfsd_direct_read(), which calls vfs_iocb_iter_read() and
nfsd_finish_read() the same way.

--- cut here ---

So what I'd like you to do is provide a backportable fix that applies
before your series to address the fsnotify and atime accounting issues.
I'm not convinced that stable will want the bulk of the 1/2 changes as
you have them here.

I don't expect this work will be a heavy lift, but let me know if it
turns into one.

-- 
Chuck Lever

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

[Christoph Hellwig]

On Tue, Aug 18, 2026 at 02:08:10AM +0500, Ameer Hamza wrote:
> 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

No.  None of the above are sensible use cases for NFSD exports.
There is no good reason to add a barely maintained special code path
thae pokes into internals for this.

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

[NeilBrown]

On Tue, 18 Aug 2026, Ameer Hamza wrote:
> 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,

I think that if we are going to do this then we should do it properly
and make it easy to detect these cases.

Could we add an FMODE flags FMODE_DONT_COPY_FOR_SPLICE which causes
copy_splice_read() to return -ENOTSUP or similar.
Then nfsd can call splice_read if it appear to exist, but set that flag.
If it fails with -ENOTSUP, fall back to iter_read.

I really don't like the approach of explicitly testing whether
f_op->splice_read is a particular value.

Thanks,
NeilBrown



>                 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 <ameer.hamza@truenas.com>
> ---
>  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
> 
> 

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

[Ameer Hamza]

On Tue, Aug 18, 2026 at 09:55:56AM -0400, Chuck Lever wrote:
> Hi Ameer -
> 
> First off, thanks for posting the full series to provide good
> context for your problem statement. Contention in the page
> allocator is vexing, and is a problem we've been chasing for
> years.
> 
> There is a lot to unpack. I'm going to focus on patch 1/2 and
> set 2/2 aside for the moment. I promise we will come back to it.

[ ... ]

> So what I'd like you to do is provide a backportable fix that applies
> before your series to address the fsnotify and atime accounting issues.
> I'm not convinced that stable will want the bulk of the 1/2 changes as
> you have them here.
> 
> I don't expect this work will be a heavy lift, but let me know if it
> turns into one.

Thanks, both issues were real. Fixes posted:

  https://lore.kernel.org/linux-nfs/20260818225715.572140-1-ameer.hamza@truenas.com/
  https://lore.kernel.org/linux-nfs/20260818225715.572140-2-ameer.hamza@truenas.com/

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

[Ameer Hamza]

On Wed, Aug 19, 2026 at 08:54:11AM +1000, NeilBrown wrote:
> On Tue, 18 Aug 2026, Ameer Hamza wrote:
> > 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,
> 
> I think that if we are going to do this then we should do it properly
> and make it easy to detect these cases.
> 
> Could we add an FMODE flags FMODE_DONT_COPY_FOR_SPLICE which causes
> copy_splice_read() to return -ENOTSUP or similar.
> Then nfsd can call splice_read if it appear to exist, but set that flag.
> If it fails with -ENOTSUP, fall back to iter_read.
> 
> I really don't like the approach of explicitly testing whether
> f_op->splice_read is a particular value.

Thanks, this indeed works better. I believe it also covers 9p and
ceph falling back to copy_splice_read() internally, which the
helper could not see. I will work it out this way for v2.

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

[Ameer Hamza]

On Tue, Aug 18, 2026 at 07:42:01AM -0700, Christoph Hellwig wrote:
> On Tue, Aug 18, 2026 at 02:08:10AM +0500, Ameer Hamza wrote:
> > 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
> 
> No.  None of the above are sensible use cases for NFSD exports.
> There is no good reason to add a barely maintained special code path
> thae pokes into internals for this.
>

Yeah, agreed on the poking into internals part. I will rework it
in v2 based on Neil's FMODE suggestion unless you have something
else in mind.