[PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[Chuck Lever]

A completed DRC entry currently stays in its bucket for 120 seconds
whether or not the client already holds the reply. On a busy server
those entries lengthen every bucket walk, and under pressure they
crowd out entries a retransmit could still hit.

This series modifies the DRC to retire an entry once there is reason
to believe its reply was delivered.

1. The transport reports delivery outright where it can. TCP now
   reports when snd_una passes the reply's sequence number, and RDMA
   reports on each Send completion.
2. Where no report will come, a later request on the same connection
   stands in as an implied ACK, since a client on a live TCP or RDMA
   connection does not retransmit within it (RFC 1813 Section 4.5).

UDP continues to use a traditional time-based eviction mechanism.

The first mechanism is the more reliable of the two, and the backup
mechanism is more of an informed guess. A pipelined client sends its
next request before the previous reply lands, and an entry evicted
in that window is lost if the connection then drops and the client
retransmits. In the current NFSD code, memory pressure and RC_EXPIRE
already open this same window. For instance, on a server with fast
networking and storage, pressure eviction already evicts entries far
younger than 120 seconds.

Two DRC capacity guards that were sized for a cache full of stale
replies can be removed, now that reply delivery reports keep the DRC
small. The commit messages for those patches explain the rationale
in detail.

v2 of this series (implied ACK only) was profiled with "perf record
-e cycles -e cpu-clock -e LLC-load-misses -e branch-misses" during
an NFSv3/RDMA 4KB random-write workload. v3 has not been re-profiled.
nfsd_cache_lookup overhead dropped from 1.53% to 0.76% of CPU
cycles during this test. The rb-tree operations (rb_erase,
rb_insert_color) that dominated LLC cache misses fell from a
combined 13.2% to 1.1% of all LLC-load-misses, because shorter-lived
entries keep the per-bucket trees small.

---
Changes in v3:
- Add the reply-acknowledged callback and its TCP and RDMA reporters
  ahead of implied ACK, which now defers to a pending report.
- Split the prune-loop restructure into its own patch.
- Move the eviction tracepoints ahead of the ack patches; the
  implied-ACK event now lands with implied ACK.
- Keep err local to the pmap_register block in svc_setup_socket()
  (per Jeff's review).
- Never move xpt_last_recv backwards when nfsd threads race.
- Fold the XPT_ORDERED patch into its consumer (per Jeff's review).
- State the re-execution exposure of implied-ACK eviction instead of
  calling the DRC advisory (per Jeff's review).
- New patch: remove the DRC request checksum and payload_misses stat.
- New patch: remove the 256k-entry cap on the DRC size.
- Link to v2: https://patch.msgid.link/20260828-duplicate-reply-cache-v2-0-25069e660a7b@kernel.org

Changes in v2:
- Print the DRC eviction tracepoints' age field as unsigned.
- Link to v1: https://patch.msgid.link/20260826-duplicate-reply-cache-v1-0-b1d51e1af5c7@kernel.org

---
Chuck Lever (12):
      SUNRPC: Assign a unique identifier to each svc_xprt
      NFSD: Track transport in DRC entries
      NFSD: Prepare bucket pruning for out-of-order eviction
      NFSD: Add tracepoints for DRC entry eviction
      NFSD: Record DRC population in lookup tracepoints
      NFSD: Add reply-acknowledged callback infrastructure
      SUNRPC: Add TCP sequence-number ACK tracking for reply delivery
      svcrdma: Fire reply-acknowledged callback on Send completion
      SUNRPC: Record last-request timestamp on svc_xprt
      NFSD: Evict unacknowledged DRC entries via implied ACK
      NFSD: Remove DRC checksum and payload_misses stat
      NFSD: Remove hard cap on duplicate reply cache size

 .../ABI/testing/procfs-nfsd-reply_cache_stats      |  11 +-
 fs/nfsd/cache.h                                    |  20 +-
 fs/nfsd/netns.h                                    |   2 -
 fs/nfsd/nfscache.c                                 | 310 +++++++++++++--------
 fs/nfsd/nfssvc.c                                   |  20 +-
 fs/nfsd/stats.h                                    |   5 -
 fs/nfsd/trace.h                                    |  81 +++++-
 include/linux/sunrpc/svc.h                         |  51 ++++
 include/linux/sunrpc/svc_rdma.h                    |   1 +
 include/linux/sunrpc/svc_xprt.h                    |   6 +-
 include/linux/sunrpc/svcsock.h                     |  10 +
 include/trace/events/sunrpc.h                      |  10 +-
 net/sunrpc/netns.h                                 |   4 +
 net/sunrpc/sunrpc_syms.c                           |   2 +
 net/sunrpc/svc.c                                   |   1 +
 net/sunrpc/svc_xprt.c                              |  53 +++-
 net/sunrpc/svcsock.c                               | 119 +++++++-
 net/sunrpc/xprtrdma/svc_rdma_sendto.c              |  14 +
 net/sunrpc/xprtrdma/svc_rdma_transport.c           |   7 +-
 19 files changed, 541 insertions(+), 186 deletions(-)
---
base-commit: abf2077ee32058e60d3f49ecab265d3c4bd953d9
change-id: 20260325-duplicate-reply-cache-f0fe7c7b740c

Best regards,
--  
Chuck Lever <cel@kernel.org>

[PATCH v3 01/12] SUNRPC: Assign a unique identifier to each svc_xprt

[Chuck Lever]

A consumer that associates state with a transport without holding a
reference cannot detect ABA collisions: once a transport is freed,
SLUB may hand out a new svc_xprt at the same address.

Allocate a per-netns identifier for each transport in svc_xprt_init()
with xa_alloc_cyclic(), which delays reuse of an identifier after its
transport is freed. svc_xprt_init() now returns a boolean, and its
callers unwind when allocation fails. The NFSD duplicate reply cache
is the first consumer.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 include/linux/sunrpc/svc_xprt.h          |  3 ++-
 include/trace/events/sunrpc.h            |  8 +++++--
 net/sunrpc/netns.h                       |  4 ++++
 net/sunrpc/sunrpc_syms.c                 |  2 ++
 net/sunrpc/svc_xprt.c                    | 39 ++++++++++++++++++++++++++++----
 net/sunrpc/svcsock.c                     | 33 +++++++++++++++++++--------
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  5 +++-
 7 files changed, 75 insertions(+), 19 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 2af222f3ea2c..c62f789e2900 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -56,6 +56,7 @@ struct svc_xprt {
 	struct svc_xprt_class	*xpt_class;
 	const struct svc_xprt_ops *xpt_ops;
 	struct kref		xpt_ref;
+	unsigned int		xpt_id;
 	ktime_t			xpt_qtime;
 	struct list_head	xpt_list;
 	struct lwq_node		xpt_ready;
@@ -162,7 +163,7 @@ static inline bool svc_xprt_is_dead(const struct svc_xprt *xprt)
 
 int	svc_reg_xprt_class(struct svc_xprt_class *);
 void	svc_unreg_xprt_class(struct svc_xprt_class *);
-void	svc_xprt_init(struct net *, struct svc_xprt_class *, struct svc_xprt *,
+bool	svc_xprt_init(struct net *, struct svc_xprt_class *, struct svc_xprt *,
 		      struct svc_serv *);
 int	svc_xprt_create_from_sa(struct svc_serv *serv, const char *xprt_name,
 				struct net *net, struct sockaddr *sap,
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index ff855197880d..180346e520ff 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1986,7 +1986,8 @@ TRACE_EVENT(svc_xprt_create_err,
 		__sockaddr(server, (x)->xpt_locallen) \
 		__sockaddr(client, (x)->xpt_remotelen) \
 		__field(unsigned long, flags) \
-		__field(unsigned int, netns_ino)
+		__field(unsigned int, netns_ino) \
+		__field(unsigned int, xpt_id)
 
 #define SVC_XPRT_ENDPOINT_ASSIGNMENTS(x) \
 		do { \
@@ -1996,13 +1997,15 @@ TRACE_EVENT(svc_xprt_create_err,
 					  (x)->xpt_remotelen); \
 			__entry->flags = (x)->xpt_flags; \
 			__entry->netns_ino = (x)->xpt_net->ns.inum; \
+			__entry->xpt_id = (x)->xpt_id; \
 		} while (0)
 
 #define SVC_XPRT_ENDPOINT_FORMAT \
-		"server=%pISpc client=%pISpc flags=%s"
+		"server=%pISpc client=%pISpc xpt_id=%u flags=%s"
 
 #define SVC_XPRT_ENDPOINT_VARARGS \
 		__get_sockaddr(server), __get_sockaddr(client), \
+		__entry->xpt_id, \
 		show_svc_xprt_flags(__entry->flags)
 
 TRACE_EVENT(svc_xprt_enqueue,
@@ -2024,6 +2027,7 @@ TRACE_EVENT(svc_xprt_enqueue,
 				  xprt->xpt_remotelen);
 		__entry->flags = flags;
 		__entry->netns_ino = xprt->xpt_net->ns.inum;
+		__entry->xpt_id = xprt->xpt_id;
 	),
 
 	TP_printk(SVC_XPRT_ENDPOINT_FORMAT, SVC_XPRT_ENDPOINT_VARARGS)
diff --git a/net/sunrpc/netns.h b/net/sunrpc/netns.h
index 4efb5f28d881..9523d860be27 100644
--- a/net/sunrpc/netns.h
+++ b/net/sunrpc/netns.h
@@ -2,6 +2,7 @@
 #ifndef __SUNRPC_NETNS_H__
 #define __SUNRPC_NETNS_H__
 
+#include <linux/xarray.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 
@@ -34,6 +35,9 @@ struct sunrpc_net {
 	atomic_t pipe_users;
 	struct proc_dir_entry *use_gssp_proc;
 	struct proc_dir_entry *gss_krb5_enctypes;
+
+	struct xarray	svc_xprt_ids;
+	u32		svc_xprt_id_next;
 };
 
 extern unsigned int sunrpc_net_id;
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index 1a3884a0376a..355404d541d4 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -58,6 +58,7 @@ static __net_init int sunrpc_init_net(struct net *net)
 	spin_lock_init(&sn->rpc_client_lock);
 	spin_lock_init(&sn->rpcb_clnt_lock);
 	mutex_init(&sn->gssp_lock);
+	xa_init_flags(&sn->svc_xprt_ids, XA_FLAGS_ALLOC1);
 	return 0;
 
 err_pipefs:
@@ -74,6 +75,7 @@ static __net_exit void sunrpc_exit_net(struct net *net)
 {
 	struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
 
+	xa_destroy(&sn->svc_xprt_ids);
 	rpc_pipefs_exit_net(net);
 	unix_gid_cache_destroy(net);
 	ip_map_cache_destroy(net);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index d5634dd6d6cc..7e8ef4832421 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -21,6 +21,8 @@
 #include <linux/netdevice.h>
 #include <trace/events/sunrpc.h>
 
+#include "netns.h"
+
 #define RPCDBG_FACILITY	RPCDBG_SVCXPRT
 
 static unsigned int svc_rpc_per_connection_limit __read_mostly;
@@ -169,7 +171,10 @@ static void svc_xprt_free(struct kref *kref)
 {
 	struct svc_xprt *xprt =
 		container_of(kref, struct svc_xprt, xpt_ref);
+	struct sunrpc_net *sn = net_generic(xprt->xpt_net, sunrpc_net_id);
 	struct module *owner = xprt->xpt_class->xcl_owner;
+
+	xa_erase(&sn->svc_xprt_ids, xprt->xpt_id);
 	if (test_bit(XPT_CACHE_AUTH, &xprt->xpt_flags))
 		svcauth_unix_info_release(xprt);
 	put_cred(xprt->xpt_cred);
@@ -190,13 +195,28 @@ void svc_xprt_put(struct svc_xprt *xprt)
 }
 EXPORT_SYMBOL_GPL(svc_xprt_put);
 
-/*
- * Called by transport drivers to initialize the transport independent
- * portion of the transport instance.
+/**
+ * svc_xprt_init - initialize transport-independent portion of a transport
+ * @net: network namespace in which the transport operates
+ * @xcl: transport class providing operations and metadata
+ * @xprt: svc_xprt to initialize
+ * @serv: RPC service that owns this transport
+ *
+ * Assigns @xprt->xpt_id, unique among the transports live in @net. The
+ * id value can be reused once @xprt is freed.
+ *
+ * Context: Process context. May sleep.
+ *
+ * Return:
+ *   %true: initialization succeeded
+ *   %false: initialization failed
  */
-void svc_xprt_init(struct net *net, struct svc_xprt_class *xcl,
+bool svc_xprt_init(struct net *net, struct svc_xprt_class *xcl,
 		   struct svc_xprt *xprt, struct svc_serv *serv)
 {
+	struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
+	u32 id;
+
 	memset(xprt, 0, sizeof(*xprt));
 	xprt->xpt_class = xcl;
 	xprt->xpt_ops = xcl->xcl_ops;
@@ -208,8 +228,17 @@ void svc_xprt_init(struct net *net, struct svc_xprt_class *xcl,
 	mutex_init(&xprt->xpt_mutex);
 	spin_lock_init(&xprt->xpt_lock);
 	set_bit(XPT_BUSY, &xprt->xpt_flags);
-	xprt->xpt_net = get_net_track(net, &xprt->ns_tracker, GFP_ATOMIC);
+	xprt->xpt_net = get_net_track(net, &xprt->ns_tracker, GFP_KERNEL);
 	strcpy(xprt->xpt_remotebuf, "uninitialized");
+
+	if (xa_alloc_cyclic(&sn->svc_xprt_ids, &id, xprt,
+			    XA_LIMIT(1, UINT_MAX), &sn->svc_xprt_id_next,
+			    GFP_KERNEL) < 0) {
+		put_net_track(xprt->xpt_net, &xprt->ns_tracker);
+		return false;
+	}
+	xprt->xpt_id = id;
+	return true;
 }
 EXPORT_SYMBOL_GPL(svc_xprt_init);
 
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index e5459d504b6a..625aebbbc6b3 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -803,10 +803,11 @@ static struct svc_xprt_class svc_udp_class = {
 	.xcl_flags = SVC_XPRT_FLAG_WSPACE_RESERVE,
 };
 
-static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
+static bool svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
 {
-	svc_xprt_init(sock_net(svsk->sk_sock->sk), &svc_udp_class,
-		      &svsk->sk_xprt, serv);
+	if (!svc_xprt_init(sock_net(svsk->sk_sock->sk), &svc_udp_class,
+			   &svsk->sk_xprt, serv))
+		return false;
 	clear_bit(XPT_CACHE_AUTH, &svsk->sk_xprt.xpt_flags);
 	svsk->sk_sk->sk_data_ready = svc_data_ready;
 	svsk->sk_sk->sk_write_space = svc_write_space;
@@ -833,6 +834,7 @@ static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
 	default:
 		BUG();
 	}
+	return true;
 }
 
 /*
@@ -1476,12 +1478,13 @@ void svc_cleanup_xprt_sock(void)
 	svc_unreg_xprt_class(&svc_udp_class);
 }
 
-static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
+static bool svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
 {
 	struct sock	*sk = svsk->sk_sk;
 
-	svc_xprt_init(sock_net(svsk->sk_sock->sk), &svc_tcp_class,
-		      &svsk->sk_xprt, serv);
+	if (!svc_xprt_init(sock_net(svsk->sk_sock->sk), &svc_tcp_class,
+			   &svsk->sk_xprt, serv))
+		return false;
 	set_bit(XPT_CACHE_AUTH, &svsk->sk_xprt.xpt_flags);
 	set_bit(XPT_CONG_CTRL, &svsk->sk_xprt.xpt_flags);
 	if (sk->sk_state == TCP_LISTEN) {
@@ -1512,6 +1515,7 @@ static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
 			svc_xprt_deferred_close(&svsk->sk_xprt);
 		}
 	}
+	return true;
 }
 
 void svc_sock_update_bufs(struct svc_serv *serv)
@@ -1603,13 +1607,22 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
 	inet->sk_user_data = svsk;
 
 	/* Initialize the socket */
-	if (sock->type == SOCK_DGRAM)
-		svc_udp_init(svsk, serv);
-	else
-		svc_tcp_init(svsk, serv);
+	if (sock->type == SOCK_DGRAM) {
+		if (!svc_udp_init(svsk, serv))
+			goto out_free;
+	} else {
+		if (!svc_tcp_init(svsk, serv))
+			goto out_free;
+	}
 
 	trace_svcsock_new(svsk, sock);
 	return svsk;
+
+out_free:
+	inet->sk_user_data = NULL;
+	kfree(svsk->sk_bvec);
+	kfree(svsk);
+	return ERR_PTR(-ENOMEM);
 }
 
 /**
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index f949601b2144..610df78f9176 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -189,7 +189,10 @@ static struct svcxprt_rdma *svc_rdma_create_xprt(struct svc_serv *serv,
 	if (!cma_xprt)
 		return NULL;
 
-	svc_xprt_init(net, &svc_rdma_class, &cma_xprt->sc_xprt, serv);
+	if (!svc_xprt_init(net, &svc_rdma_class, &cma_xprt->sc_xprt, serv)) {
+		kfree(cma_xprt);
+		return NULL;
+	}
 	INIT_LIST_HEAD(&cma_xprt->sc_accept_q);
 	INIT_LIST_HEAD(&cma_xprt->sc_rq_dto_q);
 	INIT_LIST_HEAD(&cma_xprt->sc_read_complete_q);

-- 
2.55.0

[PATCH v3 02/12] NFSD: Track transport in DRC entries

[Chuck Lever]

A cached reply carries no record of the transport its request arrived
on. Transport-aware eviction needs that association.

Record the xprt id in each entry. No behavior change.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/cache.h    | 1 +
 fs/nfsd/nfscache.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index 3bc4856e34b8..5fbf1bc37c03 100644
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -37,6 +37,7 @@ struct nfsd_cacherep {
 	unsigned char		c_state,	/* unused, inprog, done */
 				c_type,		/* status, buffer */
 				c_secure : 1;	/* req came from port < 1024 */
+	unsigned int		c_xprt;		/* svc_xprt that carried req */
 	unsigned long		c_timestamp;
 	union {
 		struct kvec	u_vec;
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 80364b91331a..b25b4f9e92f7 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -109,6 +109,7 @@ nfsd_cacherep_alloc(struct svc_rqst *rqstp, __wsum csum,
 		rp->c_key.k_vers = rqstp->rq_vers;
 		rp->c_key.k_len = rqstp->rq_arg.len;
 		rp->c_key.k_csum = csum;
+		rp->c_xprt = rqstp->rq_xprt->xpt_id;
 	}
 	return rp;
 }

-- 
2.55.0

[PATCH v3 03/12] NFSD: Prepare bucket pruning for out-of-order eviction

[Chuck Lever]

nfsd_prune_bucket_locked() evicts for memory pressure and RC_EXPIRE.
Both reasons hold for every entry older than the first one they hold
for, so the walk stops at the first entry it cannot evict. An
eviction reason that applies to one entry but not to an older one,
such as confirmation that a particular reply reached the client,
cannot be added to that loop.

Restructure the loop so each reason is a separate test that jumps
to a shared eviction label, and continue past an entry no reason
applies to. The restructured scan no longer stops at the first
entry it cannot evict, so bound the work per call to four times the
eviction limit. Pass the shrinker's remaining nr_to_scan budget as
that limit, bounding its walk of each bucket the same way.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index b25b4f9e92f7..e6504e192dfa 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -257,8 +257,8 @@ nfsd_cache_bucket_find(__be32 xid, struct nfsd_net *nn)
 }
 
 /*
- * Remove and return no more than @max expired entries in bucket @b.
- * If @max is zero, do not limit the number of removed entries.
+ * Remove and return no more than @max evictable entries in bucket @b,
+ * visiting at most 4 * @max entries. @max must not be zero.
  */
 static void
 nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
@@ -266,20 +266,31 @@ nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
 {
 	unsigned long expiry = jiffies - RC_EXPIRE;
 	struct nfsd_cacherep *rp, *tmp;
-	unsigned int freed = 0;
+	unsigned int freed = 0, visited = 0;
 
 	lockdep_assert_held(&b->cache_lock);
 
 	/* The bucket LRU is ordered oldest-first. */
 	list_for_each_entry_safe(rp, tmp, &b->lru_head, c_lru) {
-		if (atomic_read(&nn->num_drc_entries) <= nn->max_drc_entries &&
-		    time_before(expiry, rp->c_timestamp))
-			break;
+		if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries)
+			goto evict;
+		if (time_before_eq(rp->c_timestamp, expiry))
+			goto evict;
+		goto next;
 
+evict:
 		nfsd_cacherep_unlink_locked(nn, b, rp);
 		list_add(&rp->c_lru, dispose);
+		freed++;
 
-		if (max && ++freed >= max)
+next:
+		/*
+		 * A client controls its XIDs, so it can pack one bucket with
+		 * entries that are not yet evictable and turn each miss into
+		 * a full-bucket walk under cache_lock. A skipped entry is
+		 * left for a later prune.
+		 */
+		if (freed >= max || ++visited >= max * 4)
 			break;
 	}
 }
@@ -307,9 +318,9 @@ nfsd_reply_cache_count(struct shrinker *shrink, struct shrink_control *sc)
  * @shrink: our registered shrinker context
  * @sc: garbage collection parameters
  *
- * Free expired entries on each bucket's LRU list until we've released
- * nr_to_scan freed objects. Nothing will be released if the cache
- * has not exceeded it's max_drc_entries limit.
+ * Free entries on each bucket's LRU list until nr_to_scan objects have been
+ * released. Entries are evicted when they have expired or the cache exceeds
+ * its max_drc_entries limit.
  *
  * Returns the number of entries released by this call.
  */
@@ -328,11 +339,12 @@ nfsd_reply_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
 			continue;
 
 		spin_lock(&b->cache_lock);
-		nfsd_prune_bucket_locked(nn, b, 0, &dispose);
+		nfsd_prune_bucket_locked(nn, b, sc->nr_to_scan - freed,
+					 &dispose);
 		spin_unlock(&b->cache_lock);
 
 		freed += nfsd_cacherep_dispose(&dispose);
-		if (freed > sc->nr_to_scan)
+		if (freed >= sc->nr_to_scan)
 			break;
 	}
 	return freed;

-- 
2.55.0

[PATCH v3 04/12] NFSD: Add tracepoints for DRC entry eviction

[Chuck Lever]

The DRC pruning path evicts entries for memory pressure and expiry,
and neither emits a trace event, so there is no way to see which
reason is retiring entries or how old they are when it happens.
Eviction reasons that follow will need the same visibility.

Add an event class that records the cache population, the XID, the
transport, and the entry's age, and define an event for each
eviction reason.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c |  8 ++++++--
 fs/nfsd/trace.h    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index e6504e192dfa..ec8c40781cb2 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -272,10 +272,14 @@ nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
 
 	/* The bucket LRU is ordered oldest-first. */
 	list_for_each_entry_safe(rp, tmp, &b->lru_head, c_lru) {
-		if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries)
+		if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries) {
+			trace_nfsd_drc_evict_pressure(nn, rp);
 			goto evict;
-		if (time_before_eq(rp->c_timestamp, expiry))
+		}
+		if (time_before_eq(rp->c_timestamp, expiry)) {
+			trace_nfsd_drc_evict_expired(nn, rp);
 			goto evict;
+		}
 		goto next;
 
 evict:
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 2ae7f150a72c..255877e65d7c 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1557,6 +1557,43 @@ TRACE_EVENT(nfsd_drc_mismatch,
 		__entry->ingress)
 );
 
+DECLARE_EVENT_CLASS(nfsd_drc_entry_class,
+	TP_PROTO(
+		const struct nfsd_net *nn,
+		const struct nfsd_cacherep *rp
+	),
+	TP_ARGS(nn, rp),
+	TP_STRUCT__entry(
+		__field(unsigned long long, boot_time)
+		__field(unsigned int, num_drc_entries)
+		__field(u32, xid)
+		__field(unsigned int, xprt)
+		__field(unsigned long, age)
+	),
+	TP_fast_assign(
+		__entry->boot_time = nn->boot_time;
+		__entry->num_drc_entries = atomic_read(&nn->num_drc_entries);
+		__entry->xid = be32_to_cpu(rp->c_key.k_xid);
+		__entry->xprt = rp->c_xprt;
+		__entry->age = time_is_after_jiffies(rp->c_timestamp) ?
+				1 : jiffies - rp->c_timestamp;
+	),
+	TP_printk("boot_time=%16llx entries=%u xid=0x%08x xprt=%u age=%lu",
+		__entry->boot_time, __entry->num_drc_entries,
+		__entry->xid, __entry->xprt, __entry->age)
+);
+
+#define DEFINE_NFSD_DRC_ENTRY_EVENT(name)			\
+DEFINE_EVENT(nfsd_drc_entry_class, nfsd_drc_##name,		\
+	TP_PROTO(						\
+		const struct nfsd_net *nn,			\
+		const struct nfsd_cacherep *rp			\
+	),							\
+	TP_ARGS(nn, rp))
+
+DEFINE_NFSD_DRC_ENTRY_EVENT(evict_pressure);
+DEFINE_NFSD_DRC_ENTRY_EVENT(evict_expired);
+
 TRACE_EVENT(nfsd_cb_args,
 	TP_PROTO(
 		const struct nfs4_client *clp,

-- 
2.55.0

[PATCH v3 05/12] NFSD: Record DRC population in lookup tracepoints

[Chuck Lever]

nfsd_drc_found reports the outcome of a lookup but not how full the
cache was at the time, so a trace cannot show whether retransmits are
being caught while the cache runs near its cap.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c |  3 ++-
 fs/nfsd/trace.h    | 11 +++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index ec8c40781cb2..10fdc60f9bb9 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -562,7 +562,8 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
 	}
 
 out_trace:
-	trace_nfsd_drc_found(nn, rqstp, rtn);
+	trace_nfsd_drc_found(nn, atomic_read(&nn->num_drc_entries),
+			     rqstp, rtn);
 out_unlock:
 	spin_unlock(&b->cache_lock);
 out:
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 255877e65d7c..a07d3dc76f27 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1513,23 +1513,26 @@ TRACE_DEFINE_ENUM(RC_DOIT);
 TRACE_EVENT(nfsd_drc_found,
 	TP_PROTO(
 		const struct nfsd_net *nn,
+		unsigned int num_drc_entries,
 		const struct svc_rqst *rqstp,
 		int result
 	),
-	TP_ARGS(nn, rqstp, result),
+	TP_ARGS(nn, num_drc_entries, rqstp, result),
 	TP_STRUCT__entry(
 		__field(unsigned long long, boot_time)
+		__field(unsigned int, num_drc_entries)
 		__field(unsigned long, result)
 		__field(u32, xid)
 	),
 	TP_fast_assign(
 		__entry->boot_time = nn->boot_time;
+		__entry->num_drc_entries = num_drc_entries;
 		__entry->result = result;
 		__entry->xid = be32_to_cpu(rqstp->rq_xid);
 	),
-	TP_printk("boot_time=%16llx xid=0x%08x result=%s",
-		__entry->boot_time, __entry->xid,
-		show_drc_retval(__entry->result))
+	TP_printk("boot_time=%16llx entries=%u xid=0x%08x result=%s",
+		__entry->boot_time, __entry->num_drc_entries,
+		__entry->xid, show_drc_retval(__entry->result))
 
 );
 

-- 
2.55.0

[PATCH v3 06/12] NFSD: Add reply-acknowledged callback infrastructure

[Chuck Lever]

The DRC retains an entry for RC_EXPIRE whether or not the client has
received the reply. TCP and RDMA can confirm delivery, which would
allow the entry to be evicted much earlier.

Early eviction is safe because a client retransmits a request only
when its reply never arrived. Once the transport confirms the reply
reached the client, no retransmit of that XID can follow, so the
entry can never again satisfy a lookup. A reply still in flight
when a connection drops is never confirmed, so the entries a
reconnecting client retransmits against keep their full RC_EXPIRE
retention.

Add a callback and opaque private pointer on struct svc_serv so a
transport can report reply delivery without depending on NFS types.
The report carries an opaque cookie naming the entry by XID,
transport identifier, and generation. The generation guards XID
reuse: without it a stale report could mark a successor entry
delivered, evicting a reply the client never received. The handler
caps its newest-first bucket walk because a client chooses which
bucket its XIDs hash to and could otherwise pin cache_lock with an
unbounded walk.

A transport that reports on replies sets XPT_REPLY_ACK, and its
entries are marked c_ack_pending until the report arrives, so an
eviction reason that only guesses at delivery can defer to it.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/cache.h                 | 10 +++--
 fs/nfsd/nfscache.c              | 86 ++++++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfssvc.c                | 10 ++---
 fs/nfsd/trace.h                 | 34 ++++++++++++++++
 include/linux/sunrpc/svc.h      | 48 +++++++++++++++++++++++
 include/linux/sunrpc/svc_xprt.h |  1 +
 include/trace/events/sunrpc.h   |  1 +
 7 files changed, 180 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index 5fbf1bc37c03..8ad23a1fcb57 100644
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -36,8 +36,11 @@ struct nfsd_cacherep {
 	struct list_head	c_lru;
 	unsigned char		c_state,	/* unused, inprog, done */
 				c_type,		/* status, buffer */
-				c_secure : 1;	/* req came from port < 1024 */
+				c_secure : 1,	/* req came from port < 1024 */
+				c_acked : 1,	/* reply delivery confirmed */
+				c_ack_pending : 1; /* transport will report */
 	unsigned int		c_xprt;		/* svc_xprt that carried req */
+	u32			c_ack_gen;	/* uniquifies the ack cookie */
 	unsigned long		c_timestamp;
 	union {
 		struct kvec	u_vec;
@@ -80,10 +83,11 @@ enum {
 /* Checksum this amount of the request */
 #define RC_CSUMLEN		(256U)
 
+svc_ack_cookie_t nfsd_cache_ack_cookie(const struct nfsd_cacherep *rp);
 int	nfsd_drc_slab_create(void);
 void	nfsd_drc_slab_free(void);
-int	nfsd_reply_cache_init(struct nfsd_net *);
-void	nfsd_reply_cache_shutdown(struct nfsd_net *);
+int	nfsd_reply_cache_init(struct nfsd_net *, struct svc_serv *);
+void	nfsd_reply_cache_shutdown(struct nfsd_net *, struct svc_serv *);
 int	nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
 			  unsigned int len, struct nfsd_cacherep **cacherep);
 void	nfsd_cache_update(struct svc_rqst *rqstp, struct nfsd_cacherep *rp,
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 10fdc60f9bb9..b0231c659237 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -39,12 +39,15 @@ struct nfsd_drc_bucket {
 };
 
 static struct kmem_cache	*drc_slab;
+static atomic_t			drc_ack_gen;
 
 static int	nfsd_cache_append(struct svc_rqst *rqstp, struct kvec *vec);
 static unsigned long nfsd_reply_cache_count(struct shrinker *shrink,
 					    struct shrink_control *sc);
 static unsigned long nfsd_reply_cache_scan(struct shrinker *shrink,
 					   struct shrink_control *sc);
+static void	nfsd_reply_ack(void *data, const svc_ack_cookie_t *cookie,
+			       bool delivered);
 
 /*
  * Put a cap on the size of the DRC based on the amount of available
@@ -110,6 +113,9 @@ nfsd_cacherep_alloc(struct svc_rqst *rqstp, __wsum csum,
 		rp->c_key.k_len = rqstp->rq_arg.len;
 		rp->c_key.k_csum = csum;
 		rp->c_xprt = rqstp->rq_xprt->xpt_id;
+		rp->c_acked = 0;
+		rp->c_ack_pending = 0;
+		rp->c_ack_gen = 0;
 	}
 	return rp;
 }
@@ -121,6 +127,27 @@ static void nfsd_cacherep_free(struct nfsd_cacherep *rp)
 	kmem_cache_free(drc_slab, rp);
 }
 
+/*
+ * Zero is reserved so a populated cookie never compares equal to
+ * the all-zero cookie that marks an untracked reply.
+ */
+static u32 nfsd_cache_next_ack_gen(void)
+{
+	u32 gen = atomic_inc_return(&drc_ack_gen);
+
+	if (!gen)
+		gen = atomic_inc_return(&drc_ack_gen);
+	return gen;
+}
+
+svc_ack_cookie_t nfsd_cache_ack_cookie(const struct nfsd_cacherep *rp)
+{
+	return (svc_ack_cookie_t){
+		.id = ((u64)rp->c_xprt << 32) | (__force u32)rp->c_key.k_xid,
+		.gen = rp->c_ack_gen,
+	};
+}
+
 static unsigned long
 nfsd_cacherep_dispose(struct list_head *dispose)
 {
@@ -179,7 +206,7 @@ void nfsd_drc_slab_free(void)
 	kmem_cache_destroy(drc_slab);
 }
 
-int nfsd_reply_cache_init(struct nfsd_net *nn)
+int nfsd_reply_cache_init(struct nfsd_net *nn, struct svc_serv *serv)
 {
 	unsigned int hashsize;
 	unsigned int i;
@@ -210,6 +237,9 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
 	}
 	nn->drc_hashsize = hashsize;
 
+	serv->sv_reply_ack = nfsd_reply_ack;
+	serv->sv_reply_ack_data = nn;
+
 	shrinker_register(nn->nfsd_reply_cache_shrinker);
 
 	return 0;
@@ -219,7 +249,7 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
 	return -ENOMEM;
 }
 
-void nfsd_reply_cache_shutdown(struct nfsd_net *nn)
+void nfsd_reply_cache_shutdown(struct nfsd_net *nn, struct svc_serv *serv)
 {
 	struct nfsd_cacherep *rp;
 	unsigned int i;
@@ -239,6 +269,8 @@ void nfsd_reply_cache_shutdown(struct nfsd_net *nn)
 	nn->drc_hashtbl = NULL;
 	nn->drc_hashsize = 0;
 
+	serv->sv_reply_ack = NULL;
+	serv->sv_reply_ack_data = NULL;
 }
 
 static void
@@ -256,6 +288,50 @@ nfsd_cache_bucket_find(__be32 xid, struct nfsd_net *nn)
 	return &nn->drc_hashtbl[hash];
 }
 
+/*
+ * The generation match keeps a stale cookie from acknowledging a
+ * later entry that reuses the same XID and transport. The walk
+ * starts at the MRU end of the bucket LRU, where the entry for a
+ * just-sent reply sits. The visit cap bounds the time spent under
+ * cache_lock when a client packs the bucket; an entry missed under
+ * the cap is left to the other eviction reasons.
+ */
+static void nfsd_reply_ack(void *data, const svc_ack_cookie_t *cookie,
+			   bool delivered)
+{
+	struct nfsd_net *nn = data;
+	__be32 xid = (__force __be32)(u32)cookie->id;
+	unsigned int xpt_id = cookie->id >> 32;
+	unsigned int visited = 0;
+	struct nfsd_drc_bucket *b;
+	struct nfsd_cacherep *rp;
+	bool found = false;
+
+	b = nfsd_cache_bucket_find(xid, nn);
+	spin_lock(&b->cache_lock);
+	list_for_each_entry_reverse(rp, &b->lru_head, c_lru) {
+		if (++visited > 4 * TARGET_BUCKET_SIZE)
+			break;
+		if (rp->c_key.k_xid != xid)
+			continue;
+		if (rp->c_xprt != xpt_id)
+			continue;
+		if (rp->c_ack_gen != cookie->gen)
+			continue;
+		if (delivered)
+			rp->c_acked = 1;
+		rp->c_ack_pending = 0;
+		found = true;
+		break;
+	}
+	spin_unlock(&b->cache_lock);
+	if (trace_nfsd_drc_reply_acked_enabled())
+		trace_nfsd_drc_reply_acked(nn,
+					   atomic_read(&nn->num_drc_entries),
+					   be32_to_cpu(xid), xpt_id, delivered,
+					   found);
+}
+
 /*
  * Remove and return no more than @max evictable entries in bucket @b,
  * visiting at most 4 * @max entries. @max must not be zero.
@@ -272,6 +348,10 @@ nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
 
 	/* The bucket LRU is ordered oldest-first. */
 	list_for_each_entry_safe(rp, tmp, &b->lru_head, c_lru) {
+		if (rp->c_state == RC_DONE && rp->c_acked) {
+			trace_nfsd_drc_evict_acked(nn, rp);
+			goto evict;
+		}
 		if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries) {
 			trace_nfsd_drc_evict_pressure(nn, rp);
 			goto evict;
@@ -639,6 +719,8 @@ void nfsd_cache_update(struct svc_rqst *rqstp, struct nfsd_cacherep *rp,
 	nfsd_stats_drc_mem_usage_add(nn, bufsize);
 	lru_put_end(b, rp);
 	rp->c_secure = test_bit(RQ_SECURE, &rqstp->rq_flags);
+	rp->c_ack_pending = test_bit(XPT_REPLY_ACK, &rqstp->rq_xprt->xpt_flags);
+	rp->c_ack_gen = nfsd_cache_next_ack_gen();
 	rp->c_type = cachetype;
 	rp->c_state = RC_DONE;
 	spin_unlock(&b->cache_lock);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index c04ef9d180ce..d6687ffbec45 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -389,7 +389,7 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
 	if (ret)
 		goto out_lockd;
 
-	ret = nfsd_reply_cache_init(nn);
+	ret = nfsd_reply_cache_init(nn, nn->nfsd_serv);
 	if (ret)
 		goto out_filecache;
 
@@ -404,7 +404,7 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
 	return 0;
 
 out_reply_cache:
-	nfsd_reply_cache_shutdown(nn);
+	nfsd_reply_cache_shutdown(nn, nn->nfsd_serv);
 out_filecache:
 	nfsd_file_cache_shutdown_net(net);
 out_lockd:
@@ -417,7 +417,7 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
 	return ret;
 }
 
-static void nfsd_shutdown_net(struct net *net)
+static void nfsd_shutdown_net(struct net *net, struct svc_serv *serv)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
@@ -427,7 +427,7 @@ static void nfsd_shutdown_net(struct net *net)
 
 		nfsd_export_flush(net);
 		nfs4_state_shutdown_net(net);
-		nfsd_reply_cache_shutdown(nn);
+		nfsd_reply_cache_shutdown(nn, serv);
 		nfsd_file_cache_shutdown_net(net);
 		if (test_bit(NFSD_NET_LOCKD_UP, &nn->flags)) {
 			lockd_down(net);
@@ -539,7 +539,7 @@ void nfsd_destroy_serv(struct net *net)
 	 * other initialization has been done except the rpcb information.
 	 */
 	svc_xprt_destroy_all(serv, net, true);
-	nfsd_shutdown_net(net);
+	nfsd_shutdown_net(net, serv);
 	svc_destroy(&serv);
 }
 
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index a07d3dc76f27..0af3a8231fe4 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1594,9 +1594,43 @@ DEFINE_EVENT(nfsd_drc_entry_class, nfsd_drc_##name,		\
 	),							\
 	TP_ARGS(nn, rp))
 
+DEFINE_NFSD_DRC_ENTRY_EVENT(evict_acked);
 DEFINE_NFSD_DRC_ENTRY_EVENT(evict_pressure);
 DEFINE_NFSD_DRC_ENTRY_EVENT(evict_expired);
 
+TRACE_EVENT(nfsd_drc_reply_acked,
+	TP_PROTO(
+		const struct nfsd_net *nn,
+		unsigned int num_drc_entries,
+		u32 xid,
+		unsigned int xprt,
+		bool delivered,
+		bool found
+	),
+	TP_ARGS(nn, num_drc_entries, xid, xprt, delivered, found),
+	TP_STRUCT__entry(
+		__field(unsigned long long, boot_time)
+		__field(unsigned int, num_drc_entries)
+		__field(u32, xid)
+		__field(unsigned int, xprt)
+		__field(bool, delivered)
+		__field(bool, found)
+	),
+	TP_fast_assign(
+		__entry->boot_time = nn->boot_time;
+		__entry->num_drc_entries = num_drc_entries;
+		__entry->xid = xid;
+		__entry->xprt = xprt;
+		__entry->delivered = delivered;
+		__entry->found = found;
+	),
+	TP_printk("boot_time=%16llx entries=%u xid=0x%08x xprt=%u %s %s",
+		__entry->boot_time, __entry->num_drc_entries,
+		__entry->xid, __entry->xprt,
+		__entry->delivered ? "delivered" : "untracked",
+		__entry->found ? "found" : "stale")
+);
+
 TRACE_EVENT(nfsd_cb_args,
 	TP_PROTO(
 		const struct nfs4_client *clp,
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 24698856eb40..8c9e27752698 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -58,6 +58,36 @@ enum {
 	SP_TASK_STARTING,	/* Task has started but not added to idle yet */
 };
 
+/*
+ * Opaque reply-acknowledgment cookie; field contents are
+ * upper-layer-specific. An all-zero cookie marks a reply that is
+ * not tracked. Cookies are copied by value under each consumer's
+ * own serialization and no field is accessed atomically.
+ */
+typedef struct {
+	u64	id;
+	u32	gen;
+} svc_ack_cookie_t;
+
+/**
+ * svc_ack_cookie_present - report whether a reply-ack cookie is populated
+ * @cookie: cookie to test
+ *
+ * Return: true when the upper layer requested tracking for the reply.
+ */
+static inline bool svc_ack_cookie_present(const svc_ack_cookie_t *cookie)
+{
+	return cookie->id != 0 || cookie->gen != 0;
+}
+
+/*
+ * Callback to report the fate of a reply's acknowledgment to an
+ * upper layer. @delivered is true when the transport has confirmed
+ * that the reply reached the client, and false when the transport
+ * has stopped tracking the reply and no confirmation will follow.
+ */
+typedef void (*svc_ack_fn_t)(void *data, const svc_ack_cookie_t *cookie,
+			     bool delivered);
 
 /*
  * RPC service.
@@ -96,6 +126,9 @@ struct svc_serv {
 						 * connection */
 	bool			sv_bc_enabled;	/* service uses backchannel */
 #endif /* CONFIG_SUNRPC_BACKCHANNEL */
+
+	svc_ack_fn_t		sv_reply_ack;
+	void			*sv_reply_ack_data;
 };
 
 /* This is used by pool_stats to find and lock an svc */
@@ -106,6 +139,21 @@ struct svc_info {
 
 void svc_destroy(struct svc_serv **svcp);
 
+/**
+ * svc_reply_acked - report the fate of a reply's acknowledgment
+ * @serv: RPC service
+ * @cookie: opaque identifier for the reply
+ * @delivered: true if the reply reached the client, false if the
+ *	       transport will not report on this reply
+ */
+static inline void svc_reply_acked(struct svc_serv *serv,
+				   const svc_ack_cookie_t *cookie,
+				   bool delivered)
+{
+	if (serv->sv_reply_ack)
+		serv->sv_reply_ack(serv->sv_reply_ack_data, cookie, delivered);
+}
+
 /*
  * Maximum payload size supported by a kernel RPC server.
  * This is use to determine the max number of pages nfsd is
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index c62f789e2900..ec1dcb51c2a2 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -101,6 +101,7 @@ enum {
 	XPT_LOCAL,		/* connection from loopback interface */
 	XPT_KILL_TEMP,		/* call xpo_kill_temp_xprt before closing */
 	XPT_CONG_CTRL,		/* has congestion control */
+	XPT_REPLY_ACK,		/* reports reply delivery via svc_reply_acked */
 	XPT_HANDSHAKE,		/* xprt requests a handshake */
 	XPT_TLS_SESSION,	/* transport-layer security established */
 	XPT_PEER_AUTH,		/* peer has been authenticated */
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 180346e520ff..a9d42625c99b 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1931,6 +1931,7 @@ TRACE_EVENT(svc_stats_latency,
 	svc_xprt_flag(LOCAL)						\
 	svc_xprt_flag(KILL_TEMP)					\
 	svc_xprt_flag(CONG_CTRL)					\
+	svc_xprt_flag(REPLY_ACK)					\
 	svc_xprt_flag(HANDSHAKE)					\
 	svc_xprt_flag(TLS_SESSION)					\
 	svc_xprt_flag(PEER_AUTH)					\

-- 
2.55.0

[PATCH v3 07/12] SUNRPC: Add TCP sequence-number ACK tracking for reply delivery

[Chuck Lever]

A DRC entry for a reply sent over TCP lingers until RC_EXPIRE even
though the peer's TCP acknowledgment has already confirmed that the
reply arrived.

The acknowledgment confirms only that the reply reached the peer's
TCP stack. If the connection drops before the NFS client reads it,
the client retransmits the call and the evicted entry cannot answer
it. Memory pressure and RC_EXPIRE already open the same window.

When the upper layer caches a reply it stores a cookie on the
svc_rqst. After svc_tcp_sendto() transmits the reply, record the
cookie with the current write_seq in a fixed-size per-socket ring.
Before appending, drain entries whose sequence number is at or before
snd_una and report each as delivered. The send path is
single-threaded, so the ring needs no locking. sk_wmem_queued cannot
be used for this because it counts per-skb truesize, not payload
bytes.

On a kTLS session the snapshot needs one more check. A short push
inside the TLS layer leaves the rest of the record as
partially_sent_record, and tls_sw_sendmsg() still returns the full
plaintext count, so write_seq can fall short of the reply. Record the
reply only when no partially_sent_record remains after the send.

Some replies get no ring entry: the ring is full, the send fails, the
transport is already dead, or a kTLS record is still partly unsent.
Report each of these as untracked at once, so the upper layer knows no
delivery report will follow. When the socket is freed, report entries
still in the ring as undelivered. Set XPT_REPLY_ACK on TCP transports
so the upper layer expects these reports.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c             | 15 +++++---
 include/linux/sunrpc/svc.h     |  3 ++
 include/linux/sunrpc/svcsock.h | 10 +++++
 net/sunrpc/svc.c               |  1 +
 net/sunrpc/svcsock.c           | 85 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index b0231c659237..473bf825c37a 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -289,12 +289,14 @@ nfsd_cache_bucket_find(__be32 xid, struct nfsd_net *nn)
 }
 
 /*
- * The generation match keeps a stale cookie from acknowledging a
- * later entry that reuses the same XID and transport. The walk
- * starts at the MRU end of the bucket LRU, where the entry for a
- * just-sent reply sits. The visit cap bounds the time spent under
- * cache_lock when a client packs the bucket; an entry missed under
- * the cap is left to the other eviction reasons.
+ * The generation match keeps a stale cookie from acknowledging a later
+ * entry that reuses the same XID and transport.
+ *
+ * Average bucket occupancy is small (TARGET_BUCKET_SIZE), so a linear walk
+ * of the bucket LRU beats an rb-tree lookup. The walk starts at the MRU
+ * end, where the entry for a just-sent reply sits. The visit cap bounds
+ * the time spent under cache_lock when a client packs the bucket; an entry
+ * missed under the cap is left to the other eviction reasons.
  */
 static void nfsd_reply_ack(void *data, const svc_ack_cookie_t *cookie,
 			   bool delivered)
@@ -723,6 +725,7 @@ void nfsd_cache_update(struct svc_rqst *rqstp, struct nfsd_cacherep *rp,
 	rp->c_ack_gen = nfsd_cache_next_ack_gen();
 	rp->c_type = cachetype;
 	rp->c_state = RC_DONE;
+	rqstp->rq_ack_cookie = nfsd_cache_ack_cookie(rp);
 	spin_unlock(&b->cache_lock);
 	return;
 }
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 8c9e27752698..da3822c54f3e 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -315,6 +315,9 @@ struct svc_rqst {
 	unsigned int		bc_to_retries;
 	unsigned int		rq_status_counter; /* RPC processing counter */
 	void			*rq_private;	/* For use by the service thread */
+
+	svc_ack_cookie_t	rq_ack_cookie;	/* zero when no reply-ack
+						 * tracking is requested */
 };
 
 /* bits for rq_flags */
diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
index 372a00882ca6..595be631b68b 100644
--- a/include/linux/sunrpc/svcsock.h
+++ b/include/linux/sunrpc/svcsock.h
@@ -41,6 +41,16 @@ struct svc_sock {
 
 	struct page_frag_cache  sk_frag_cache;
 
+	/* TCP reply-ACK tracking (single-threaded send path) */
+#define SVC_ACK_RING_BITS	6
+#define SVC_ACK_RING_SIZE	(1 << SVC_ACK_RING_BITS)
+	struct {
+		u32		ae_pos;		/* write_seq after send */
+		svc_ack_cookie_t ae_cookie;	/* opaque DRC cookie */
+	}			sk_ack_ring[SVC_ACK_RING_SIZE];
+	unsigned int		sk_ack_head;
+	unsigned int		sk_ack_tail;
+
 	struct completion	sk_handshake_done;
 
 	/* received data */
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index f73412e123a1..33dd9cbaff42 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1488,6 +1488,7 @@ svc_process_common(struct svc_rqst *rqstp)
 
 	/* Reset the accept_stat for the RPC */
 	rqstp->rq_accept_statp = NULL;
+	rqstp->rq_ack_cookie = (svc_ack_cookie_t){};
 
 	/* Will be turned off only when NFSv4 Sessions are used */
 	set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 625aebbbc6b3..a28905181589 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -28,6 +28,7 @@
 #include <linux/file.h>
 #include <linux/freezer.h>
 #include <linux/bvec.h>
+#include <linux/circ_buf.h>
 
 #include <net/sock.h>
 #include <net/checksum.h>
@@ -36,6 +37,7 @@
 #include <net/udp.h>
 #include <net/tcp.h>
 #include <net/tcp_states.h>
+#include <net/tls.h>
 #include <net/tls_prot.h>
 #include <net/handshake.h>
 #include <linux/uaccess.h>
@@ -88,6 +90,7 @@ static void		svc_sock_free(struct svc_xprt *);
 static struct svc_xprt *svc_create_socket(struct svc_serv *, int,
 					  struct net *, struct sockaddr *,
 					  int, int);
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 static struct lock_class_key svc_key[2];
 static struct lock_class_key svc_slock_key[2];
@@ -379,6 +382,45 @@ static void svc_data_ready(struct sock *sk)
 	}
 }
 
+/*
+ * Report as delivered each recorded reply that the peer's cumulative ACK
+ * now covers.
+ */
+static void svc_tcp_ack_drain(struct svc_sock *svsk)
+{
+	struct svc_serv *serv = svsk->sk_xprt.xpt_server;
+	u32 snd_una = READ_ONCE(tcp_sk(svsk->sk_sk)->snd_una);
+
+	while (svsk->sk_ack_head != svsk->sk_ack_tail) {
+		unsigned int idx = svsk->sk_ack_tail &
+				   (SVC_ACK_RING_SIZE - 1);
+
+		if (after(svsk->sk_ack_ring[idx].ae_pos, snd_una))
+			break;
+		svc_reply_acked(serv,
+				&svsk->sk_ack_ring[idx].ae_cookie, true);
+		svsk->sk_ack_tail++;
+	}
+}
+
+/*
+ * Once the socket is freed, acknowledgments for replies still in the ring
+ * can no longer be observed.
+ */
+static void svc_tcp_ack_purge(struct svc_sock *svsk)
+{
+	struct svc_serv *serv = svsk->sk_xprt.xpt_server;
+
+	while (svsk->sk_ack_head != svsk->sk_ack_tail) {
+		unsigned int idx = svsk->sk_ack_tail &
+				   (SVC_ACK_RING_SIZE - 1);
+
+		svc_reply_acked(serv,
+				&svsk->sk_ack_ring[idx].ae_cookie, false);
+		svsk->sk_ack_tail++;
+	}
+}
+
 /*
  * INET callback when space is newly available on the socket.
  */
@@ -1392,6 +1434,19 @@ static int svc_tcp_sendmsg(struct svc_sock *svsk, struct svc_rqst *rqstp,
 	return ret;
 }
 
+/*
+ * tls_sw_sendmsg() can return the full plaintext count with part of a
+ * record still waiting for socket write space, leaving write_seq short
+ * of the reply. The TLS layer keeps that record as partially_sent_record
+ * until it is pushed.
+ */
+static bool svc_tcp_reply_queued(struct svc_sock *svsk)
+{
+	if (!test_bit(XPT_TLS_SESSION, &svsk->sk_xprt.xpt_flags))
+		return true;
+	return !READ_ONCE(tls_get_ctx(svsk->sk_sk)->partially_sent_record);
+}
+
 /**
  * svc_tcp_sendto - Send out a reply on a TCP socket
  * @rqstp: completed svc_rqst
@@ -1420,10 +1475,32 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
 	trace_svcsock_tcp_send(xprt, sent);
 	if (sent < 0 || sent != (xdr->len + sizeof(marker)))
 		goto out_close;
+
+	svc_tcp_ack_drain(svsk);
+	if (svc_ack_cookie_present(&rqstp->rq_ack_cookie)) {
+		if (svc_tcp_reply_queued(svsk) &&
+		    CIRC_SPACE(svsk->sk_ack_head, svsk->sk_ack_tail,
+			       SVC_ACK_RING_SIZE) > 0) {
+			unsigned int idx = svsk->sk_ack_head &
+					   (SVC_ACK_RING_SIZE - 1);
+
+			svsk->sk_ack_ring[idx].ae_pos =
+				tcp_sk(svsk->sk_sk)->write_seq;
+			svsk->sk_ack_ring[idx].ae_cookie =
+				rqstp->rq_ack_cookie;
+			svsk->sk_ack_head++;
+		} else {
+			svc_reply_acked(xprt->xpt_server, &rqstp->rq_ack_cookie,
+					false);
+		}
+	}
+
 	mutex_unlock(&xprt->xpt_mutex);
 	return sent;
 
 out_notconn:
+	if (svc_ack_cookie_present(&rqstp->rq_ack_cookie))
+		svc_reply_acked(xprt->xpt_server, &rqstp->rq_ack_cookie, false);
 	mutex_unlock(&xprt->xpt_mutex);
 	return -ENOTCONN;
 out_close:
@@ -1431,6 +1508,8 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
 		  xprt->xpt_server->sv_name,
 		  (sent < 0) ? "got error" : "sent",
 		  sent, xdr->len + sizeof(marker));
+	if (svc_ack_cookie_present(&rqstp->rq_ack_cookie))
+		svc_reply_acked(xprt->xpt_server, &rqstp->rq_ack_cookie, false);
 	svc_xprt_deferred_close(xprt);
 	mutex_unlock(&xprt->xpt_mutex);
 	return -EAGAIN;
@@ -1487,6 +1566,7 @@ static bool svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
 		return false;
 	set_bit(XPT_CACHE_AUTH, &svsk->sk_xprt.xpt_flags);
 	set_bit(XPT_CONG_CTRL, &svsk->sk_xprt.xpt_flags);
+	set_bit(XPT_REPLY_ACK, &svsk->sk_xprt.xpt_flags);
 	if (sk->sk_state == TCP_LISTEN) {
 		strcpy(svsk->sk_xprt.xpt_remotebuf, "listener");
 		set_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags);
@@ -1593,6 +1673,9 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
 		}
 	}
 
+	svsk->sk_ack_head = 0;
+	svsk->sk_ack_tail = 0;
+
 	svsk->sk_sock = sock;
 	svsk->sk_sk = inet;
 	svsk->sk_ostate = inet->sk_state_change;
@@ -1810,6 +1893,8 @@ static void svc_sock_free(struct svc_xprt *xprt)
 
 	trace_svcsock_free(svsk, sock);
 
+	svc_tcp_ack_purge(svsk);
+
 	tls_handshake_cancel(sock->sk);
 	if (sock->file)
 		sockfd_put(sock);

-- 
2.55.0

[PATCH v3 08/12] svcrdma: Fire reply-acknowledged callback on Send completion

[Chuck Lever]

svcrdma posts RDMA Write work requests ahead of the Send, so a
successful Send completion confirms that the transport header and the
reply payload have both reached the client.

Store rq_ack_cookie in the send context before posting and report the
reply as delivered from the Send completion handler, so the DRC can
evict the entry without waiting for RC_EXPIRE. Set XPT_REPLY_ACK on
RDMA transports so the upper layer expects these reports. A flushed
Send reports the reply as undelivered: the connection is closing, and
the entry stays cached for a retransmit on the next connection.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 include/linux/sunrpc/svc_rdma.h          |  1 +
 net/sunrpc/xprtrdma/svc_rdma_sendto.c    | 14 ++++++++++++++
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  1 +
 3 files changed, 16 insertions(+)

diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 76aa5ec4ab40..7261dd3a892e 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -248,6 +248,7 @@ struct svc_rdma_send_ctxt {
 	struct list_head	sc_write_info_list;
 	struct svc_rdma_write_info sc_reply_info;
 
+	svc_ack_cookie_t	sc_ack_cookie;
 	void			*sc_xprt_buf;
 	int			sc_page_count;
 	int			sc_cur_sge_no;
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index c09659b17351..884e2d76cb6f 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -220,6 +220,7 @@ struct svc_rdma_send_ctxt *svc_rdma_send_ctxt_get(struct svcxprt_rdma *rdma)
 	ctxt->sc_send_wr.num_sge = 0;
 	ctxt->sc_cur_sge_no = 0;
 	ctxt->sc_page_count = 0;
+	ctxt->sc_ack_cookie = (svc_ack_cookie_t){};
 	ctxt->sc_wr_chain = &ctxt->sc_send_wr;
 	ctxt->sc_sqecount = 1;
 
@@ -470,11 +471,17 @@ static void svc_rdma_wc_send(struct ib_cq *cq, struct ib_wc *wc)
 	if (unlikely(wc->status != IB_WC_SUCCESS))
 		goto flushed;
 
+	if (svc_ack_cookie_present(&ctxt->sc_ack_cookie))
+		svc_reply_acked(rdma->sc_xprt.xpt_server,
+				&ctxt->sc_ack_cookie, true);
 	trace_svcrdma_wc_send(&ctxt->sc_cid);
 	svc_rdma_send_ctxt_put(rdma, ctxt);
 	return;
 
 flushed:
+	if (svc_ack_cookie_present(&ctxt->sc_ack_cookie))
+		svc_reply_acked(rdma->sc_xprt.xpt_server,
+				&ctxt->sc_ack_cookie, false);
 	if (wc->status != IB_WC_WR_FLUSH_ERR)
 		trace_svcrdma_wc_send_err(wc, &ctxt->sc_cid);
 	else
@@ -1198,6 +1205,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
 	if (ret < 0)
 		goto put_ctxt;
 
+	sctxt->sc_ack_cookie = rqstp->rq_ack_cookie;
 	ret = svc_rdma_send_reply_msg(rdma, sctxt, rctxt, rqstp);
 	if (ret < 0)
 		goto send_err;
@@ -1207,6 +1215,12 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
 	if (ret != -E2BIG && ret != -EINVAL)
 		goto put_ctxt;
 
+	/* The sctxt is reused for the RDMA_ERROR message. Clear the
+	 * ack cookie so that message's Send completion does not
+	 * report the unsent reply as delivered.
+	 */
+	sctxt->sc_ack_cookie = (svc_ack_cookie_t){};
+
 	/* Send completion releases payload pages that were part
 	 * of previously posted RDMA Writes.
 	 */
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 610df78f9176..64e964ac941c 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -218,6 +218,7 @@ static struct svcxprt_rdma *svc_rdma_create_xprt(struct svc_serv *serv,
 	 * transports are suitable here.
 	 */
 	set_bit(XPT_CONG_CTRL, &cma_xprt->sc_xprt.xpt_flags);
+	set_bit(XPT_REPLY_ACK, &cma_xprt->sc_xprt.xpt_flags);
 
 	return cma_xprt;
 }

-- 
2.55.0

[PATCH v3 09/12] SUNRPC: Record last-request timestamp on svc_xprt

[Chuck Lever]

On a connection-oriented transport, the arrival of a later request
suggests the client received the prior reply. It is not proof: a
client with several requests outstanding sends the next one without
waiting for a reply.

Add xpt_last_recv to struct svc_xprt. svc_handle_xprt() samples
jiffies before calling xpo_recvfrom() and publishes the sample after
a successful receive. The DRC reads this value as a best-effort
signal that a completed reply has been delivered.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 include/linux/sunrpc/svc_xprt.h |  1 +
 net/sunrpc/svc_xprt.c           | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index ec1dcb51c2a2..da234a078c5d 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -81,6 +81,7 @@ struct svc_xprt {
 	struct net		*xpt_net;
 	netns_tracker		ns_tracker;
 	const struct cred	*xpt_cred;
+	unsigned long		xpt_last_recv;	/* jiffies of last request */
 	struct rpc_xprt		*xpt_bc_xprt;	/* NFSv4.1 backchannel */
 	struct rpc_xprt_switch	*xpt_bc_xps;	/* NFSv4.1 backchannel */
 };
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 7e8ef4832421..f57dac54c8dc 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -230,6 +230,7 @@ bool svc_xprt_init(struct net *net, struct svc_xprt_class *xcl,
 	set_bit(XPT_BUSY, &xprt->xpt_flags);
 	xprt->xpt_net = get_net_track(net, &xprt->ns_tracker, GFP_KERNEL);
 	strcpy(xprt->xpt_remotebuf, "uninitialized");
+	xprt->xpt_last_recv = jiffies;
 
 	if (xa_alloc_cyclic(&sn->svc_xprt_ids, &id, xprt,
 			    XA_LIMIT(1, UINT_MAX), &sn->svc_xprt_id_next,
@@ -901,6 +902,8 @@ static void svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 		svc_xprt_received(xprt);
 	} else if (svc_xprt_reserve_slot(rqstp, xprt)) {
 		/* XPT_DATA|XPT_DEFERRED case: */
+		unsigned long recv_time = jiffies;
+
 		rqstp->rq_deferred = svc_deferred_dequeue(xprt);
 		if (rqstp->rq_deferred)
 			len = svc_deferred_recv(rqstp);
@@ -916,6 +919,17 @@ static void svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 
 		clear_bit(XPT_OLD, &xprt->xpt_flags);
 
+		/*
+		 * A deferred request arrived before its deferral, so its
+		 * replay does not advance the timestamp. recv_time was
+		 * sampled before xpo_recvfrom() released XPT_BUSY.
+		 * time_after() keeps a slow thread from pushing the
+		 * timestamp past requests received since then.
+		 */
+		if (!rqstp->rq_deferred &&
+		    time_after(recv_time, READ_ONCE(xprt->xpt_last_recv)))
+			WRITE_ONCE(xprt->xpt_last_recv, recv_time);
+
 		rqstp->rq_chandle.defer = svc_defer;
 
 		if (serv->sv_stats)

-- 
2.55.0

[PATCH v3 10/12] NFSD: Evict unacknowledged DRC entries via implied ACK

[Chuck Lever]

A transport reports reply delivery only while it is tracking the
reply. The TCP ring has a fixed size, so a burst leaves some
replies untracked, and a kTLS reply is untracked when part of its
record is still unsent. Entries for those replies stay in their
bucket until RC_EXPIRE elapses or the cache exceeds
max_drc_entries, lengthening every lookup that hashes there.

RFC 1813 Section 4.5 observes that on a connection-oriented
transport a duplicate request arises from reconnection, not from
within a live connection. A fresh request on a live TCP or RDMA
connection therefore means the client is not retransmitting an
earlier one. UDP offers no such signal.

The signal is not conclusive. An entry can be evicted while its
reply is still in flight, and a reconnect then executes the
request again. Memory pressure and RC_EXPIRE already open the
same window.

Add an XPT_ORDERED flag marking transports that offer the signal.
During bucket pruning, evict an RC_DONE entry when the current
request's transport is XPT_ORDERED, no delivery report is pending,
and its c_timestamp predates xpt_last_recv. Only that transport is
consulted, because it is the one the pruning thread holds a
reference to.

On svcrdma, handle_connect_req() zeroes the remote port in the DRC
key so a cached reply survives a reconnect, and implied-ACK
eviction can retire such an entry while the original connection is
still up. TCP gives up the same thing when a client re-binds its
reserved port.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c                       | 45 +++++++++++++++++++++++++++++---
 fs/nfsd/trace.h                          |  1 +
 include/linux/sunrpc/svc_xprt.h          |  1 +
 include/trace/events/sunrpc.h            |  1 +
 net/sunrpc/svcsock.c                     |  1 +
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  1 +
 6 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 473bf825c37a..9f9baf7910ff 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -90,6 +90,36 @@ nfsd_hashsize(unsigned int limit)
 	return roundup_pow_of_two(limit / TARGET_BUCKET_SIZE);
 }
 
+/*
+ * A later request on @xprt is taken as evidence that the client
+ * received @rp's reply, because a client does not retransmit within
+ * a live connection. Only XPT_ORDERED transports qualify: a UDP
+ * client retransmits on timeout, and a datagram from any of the
+ * peers sharing the UDP svc_xprt would evict another client's reply.
+ *
+ * A pipelined client sends its next request before @rp's reply
+ * arrives, so while the transport is still going to report on that
+ * reply, wait for the report instead.
+ *
+ * c_timestamp is set after xpt_last_recv was recorded for @rp's own
+ * request, so a newer xpt_last_recv means a later request arrived.
+ */
+static bool nfsd_cacherep_implied_ack(struct svc_xprt *xprt,
+				      struct nfsd_cacherep *rp)
+{
+	unsigned long last_req;
+
+	if (!xprt || rp->c_xprt != xprt->xpt_id)
+		return false;
+	if (!test_bit(XPT_ORDERED, &xprt->xpt_flags))
+		return false;
+	if (rp->c_ack_pending)
+		return false;
+
+	last_req = READ_ONCE(xprt->xpt_last_recv);
+	return time_after(last_req, rp->c_timestamp);
+}
+
 static struct nfsd_cacherep *
 nfsd_cacherep_alloc(struct svc_rqst *rqstp, __wsum csum,
 		    struct nfsd_net *nn)
@@ -337,10 +367,14 @@ static void nfsd_reply_ack(void *data, const svc_ack_cookie_t *cookie,
 /*
  * Remove and return no more than @max evictable entries in bucket @b,
  * visiting at most 4 * @max entries. @max must not be zero.
+ *
+ * @xprt is the transport the current request arrived on, or NULL when the
+ * caller has none.
  */
 static void
 nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
-			 unsigned int max, struct list_head *dispose)
+			 unsigned int max, struct list_head *dispose,
+			 struct svc_xprt *xprt)
 {
 	unsigned long expiry = jiffies - RC_EXPIRE;
 	struct nfsd_cacherep *rp, *tmp;
@@ -362,6 +396,11 @@ nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
 			trace_nfsd_drc_evict_expired(nn, rp);
 			goto evict;
 		}
+		if (rp->c_state == RC_DONE &&
+		    nfsd_cacherep_implied_ack(xprt, rp)) {
+			trace_nfsd_drc_evict_implied_ack(nn, rp);
+			goto evict;
+		}
 		goto next;
 
 evict:
@@ -426,7 +465,7 @@ nfsd_reply_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
 
 		spin_lock(&b->cache_lock);
 		nfsd_prune_bucket_locked(nn, b, sc->nr_to_scan - freed,
-					 &dispose);
+					 &dispose, NULL);
 		spin_unlock(&b->cache_lock);
 
 		freed += nfsd_cacherep_dispose(&dispose);
@@ -599,7 +638,7 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
 		goto found_entry;
 	*cacherep = rp;
 	rp->c_state = RC_INPROG;
-	nfsd_prune_bucket_locked(nn, b, 3, &dispose);
+	nfsd_prune_bucket_locked(nn, b, 3, &dispose, rqstp->rq_xprt);
 	spin_unlock(&b->cache_lock);
 
 	nfsd_cacherep_dispose(&dispose);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 0af3a8231fe4..c99b2a369d0d 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1597,6 +1597,7 @@ DEFINE_EVENT(nfsd_drc_entry_class, nfsd_drc_##name,		\
 DEFINE_NFSD_DRC_ENTRY_EVENT(evict_acked);
 DEFINE_NFSD_DRC_ENTRY_EVENT(evict_pressure);
 DEFINE_NFSD_DRC_ENTRY_EVENT(evict_expired);
+DEFINE_NFSD_DRC_ENTRY_EVENT(evict_implied_ack);
 
 TRACE_EVENT(nfsd_drc_reply_acked,
 	TP_PROTO(
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index da234a078c5d..cc29a8db269b 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -103,6 +103,7 @@ enum {
 	XPT_KILL_TEMP,		/* call xpo_kill_temp_xprt before closing */
 	XPT_CONG_CTRL,		/* has congestion control */
 	XPT_REPLY_ACK,		/* reports reply delivery via svc_reply_acked */
+	XPT_ORDERED,		/* connection-oriented, in-order delivery */
 	XPT_HANDSHAKE,		/* xprt requests a handshake */
 	XPT_TLS_SESSION,	/* transport-layer security established */
 	XPT_PEER_AUTH,		/* peer has been authenticated */
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index a9d42625c99b..e793e8ea322b 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1932,6 +1932,7 @@ TRACE_EVENT(svc_stats_latency,
 	svc_xprt_flag(KILL_TEMP)					\
 	svc_xprt_flag(CONG_CTRL)					\
 	svc_xprt_flag(REPLY_ACK)					\
+	svc_xprt_flag(ORDERED)						\
 	svc_xprt_flag(HANDSHAKE)					\
 	svc_xprt_flag(TLS_SESSION)					\
 	svc_xprt_flag(PEER_AUTH)					\
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index a28905181589..94361e5a743e 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1567,6 +1567,7 @@ static bool svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
 	set_bit(XPT_CACHE_AUTH, &svsk->sk_xprt.xpt_flags);
 	set_bit(XPT_CONG_CTRL, &svsk->sk_xprt.xpt_flags);
 	set_bit(XPT_REPLY_ACK, &svsk->sk_xprt.xpt_flags);
+	set_bit(XPT_ORDERED, &svsk->sk_xprt.xpt_flags);
 	if (sk->sk_state == TCP_LISTEN) {
 		strcpy(svsk->sk_xprt.xpt_remotebuf, "listener");
 		set_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags);
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 64e964ac941c..acf437516ae1 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -219,6 +219,7 @@ static struct svcxprt_rdma *svc_rdma_create_xprt(struct svc_serv *serv,
 	 */
 	set_bit(XPT_CONG_CTRL, &cma_xprt->sc_xprt.xpt_flags);
 	set_bit(XPT_REPLY_ACK, &cma_xprt->sc_xprt.xpt_flags);
+	set_bit(XPT_ORDERED, &cma_xprt->sc_xprt.xpt_flags);
 
 	return cma_xprt;
 }

-- 
2.55.0

[PATCH v3 11/12] NFSD: Remove DRC checksum and payload_misses stat

[Chuck Lever]

nfsd_cache_csum() costs CPU on every non-idempotent request. It also
prevents the use of zero-copy RDMA receives for WRITE and SYMLINK
because the payload must be in the server's memory before the DRC
lookup can run.

Commit 01a7decf7593 ("nfsd: keep a checksum of the first 256 bytes
of request") added the checksum when a growing cache made XID
collisions easier to hit. It is the only guard against a reused
XID: two calls with the same procedure and equal-length arguments
match on every other key field.

ACK-driven eviction retires a TCP or RDMA entry once the transport
confirms delivery, so a fresh XID rarely finds a resident entry to
collide with (~100/2^32 per request). A UDP entry can wait out
RC_EXPIRE, but the Linux NFS client seeds its XIDs from
get_random_u32(), so even a rebooted client does not replay its
previous sequence. A client that repeats a live XID cannot reliably
match replies to its own calls anyway.

An acknowledged entry stays in its bucket until the next prune
visits it, and a lookup matches it in the meantime. The client
already holds that reply, so treat a call carrying its XID as a
miss: evict the entry and insert the new one in its place.

Remove nfsd_cache_csum(), RC_CSUMLEN, and k_csum, along with the
nfsd_drc_mismatch tracepoint and the payload_misses stat that
counted checksum-detected collisions. Drop the start and len
parameters from nfsd_cache_lookup(), so nfsd_dispatch() no longer
snapshots the argument stream before decoding.

The "payload misses" line disappears from
/proc/fs/nfsd/reply_cache_stats. No known userspace tool parses it.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 .../ABI/testing/procfs-nfsd-reply_cache_stats      |  11 +--
 fs/nfsd/cache.h                                    |   9 +-
 fs/nfsd/netns.h                                    |   2 -
 fs/nfsd/nfscache.c                                 | 103 +++++----------------
 fs/nfsd/nfssvc.c                                   |  10 +-
 fs/nfsd/stats.h                                    |   5 -
 fs/nfsd/trace.h                                    |  24 -----
 7 files changed, 28 insertions(+), 136 deletions(-)

diff --git a/Documentation/ABI/testing/procfs-nfsd-reply_cache_stats b/Documentation/ABI/testing/procfs-nfsd-reply_cache_stats
index 57ed5f8e6597..7a22ac7c1ccd 100644
--- a/Documentation/ABI/testing/procfs-nfsd-reply_cache_stats
+++ b/Documentation/ABI/testing/procfs-nfsd-reply_cache_stats
@@ -19,19 +19,16 @@ Description:
 		cache misses             s64     Requests not found in cache
 		not cached               s64     Idempotent requests that
 		                                 bypass the cache
-		payload misses           s64     XID matched but request
-		                                 checksum did not
 		longest chain len        u32     Longest hash chain observed
 		cachesize at longest     u32     Cache size when longest
 		                                 chain was recorded
 		=======================  ======  ==========================
 
 		Counter fields (cache hits, cache misses, not cached,
-		payload misses, mem usage) are maintained with per-cpu
-		counters and may briefly show stale values under
-		concurrent load. There is no way to reset these
-		counters; consumers should compute rates by sampling
-		over time.
+		mem usage) are maintained with per-cpu counters and
+		may briefly show stale values under concurrent load.
+		There is no way to reset these counters; consumers
+		should compute rates by sampling over time.
 
 		New fields may be appended in future kernels. Parsers
 		should match on field name, not line position.
diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index 8ad23a1fcb57..6bb57d20da84 100644
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -22,9 +22,7 @@ struct nfsd_net;
  */
 struct nfsd_cacherep {
 	struct {
-		/* Keep often-read xid, csum in the same cache line: */
 		__be32			k_xid;
-		__wsum			k_csum;
 		u32			k_proc;
 		u32			k_prot;
 		u32			k_vers;
@@ -80,16 +78,13 @@ enum {
 /* Cache entries expire after this time period */
 #define RC_EXPIRE		(120 * HZ)
 
-/* Checksum this amount of the request */
-#define RC_CSUMLEN		(256U)
-
 svc_ack_cookie_t nfsd_cache_ack_cookie(const struct nfsd_cacherep *rp);
 int	nfsd_drc_slab_create(void);
 void	nfsd_drc_slab_free(void);
 int	nfsd_reply_cache_init(struct nfsd_net *, struct svc_serv *);
 void	nfsd_reply_cache_shutdown(struct nfsd_net *, struct svc_serv *);
-int	nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
-			  unsigned int len, struct nfsd_cacherep **cacherep);
+int	nfsd_cache_lookup(struct svc_rqst *rqstp,
+			  struct nfsd_cacherep **cacherep);
 void	nfsd_cache_update(struct svc_rqst *rqstp, struct nfsd_cacherep *rp,
 			  int cachetype, __be32 *statp);
 int	nfsd_reply_cache_stats_show(struct seq_file *m, void *v);
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 0ce7da20aba3..30231b9027dd 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -39,8 +39,6 @@ enum nfsd_net_flag {
 };
 
 enum {
-	/* cache misses due only to checksum comparison failures */
-	NFSD_STATS_PAYLOAD_MISSES,
 	/* amount of memory (in bytes) currently consumed by the DRC */
 	NFSD_STATS_DRC_MEM_USAGE,
 	NFSD_STATS_RC_HITS,		/* repcache hits */
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 9f9baf7910ff..4ab6595a0bcd 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -13,10 +13,8 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/sunrpc/addr.h>
-#include <linux/highmem.h>
 #include <linux/log2.h>
 #include <linux/hash.h>
-#include <net/checksum.h>
 
 #include "nfsd.h"
 #include "nfserr.h"
@@ -121,8 +119,7 @@ static bool nfsd_cacherep_implied_ack(struct svc_xprt *xprt,
 }
 
 static struct nfsd_cacherep *
-nfsd_cacherep_alloc(struct svc_rqst *rqstp, __wsum csum,
-		    struct nfsd_net *nn)
+nfsd_cacherep_alloc(struct svc_rqst *rqstp, struct nfsd_net *nn)
 {
 	struct nfsd_cacherep *rp;
 
@@ -141,7 +138,6 @@ nfsd_cacherep_alloc(struct svc_rqst *rqstp, __wsum csum,
 		rp->c_key.k_prot = rqstp->rq_prot;
 		rp->c_key.k_vers = rqstp->rq_vers;
 		rp->c_key.k_len = rqstp->rq_arg.len;
-		rp->c_key.k_csum = csum;
 		rp->c_xprt = rqstp->rq_xprt->xpt_id;
 		rp->c_acked = 0;
 		rp->c_ack_pending = 0;
@@ -475,68 +471,10 @@ nfsd_reply_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
 	return freed;
 }
 
-/**
- * nfsd_cache_csum - Checksum incoming NFS Call arguments
- * @buf: buffer containing a whole RPC Call message
- * @start: starting byte of the NFS Call header
- * @remaining: size of the NFS Call header, in bytes
- *
- * Compute a weak checksum of the leading bytes of an NFS procedure
- * call header to help verify that a retransmitted Call matches an
- * entry in the duplicate reply cache.
- *
- * To avoid assumptions about how the RPC message is laid out in
- * @buf and what else it might contain (eg, a GSS MIC suffix), the
- * caller passes us the exact location and length of the NFS Call
- * header.
- *
- * Returns a 32-bit checksum value, as defined in RFC 793.
- */
-static __wsum nfsd_cache_csum(struct xdr_buf *buf, unsigned int start,
-			      unsigned int remaining)
-{
-	unsigned int base, len;
-	struct xdr_buf subbuf;
-	__wsum csum = 0;
-	void *p;
-	int idx;
-
-	if (remaining > RC_CSUMLEN)
-		remaining = RC_CSUMLEN;
-	if (xdr_buf_subsegment(buf, &subbuf, start, remaining))
-		return csum;
-
-	/* rq_arg.head first */
-	if (subbuf.head[0].iov_len) {
-		len = min_t(unsigned int, subbuf.head[0].iov_len, remaining);
-		csum = csum_partial(subbuf.head[0].iov_base, len, csum);
-		remaining -= len;
-	}
-
-	/* Continue into page array */
-	idx = subbuf.page_base / PAGE_SIZE;
-	base = subbuf.page_base & ~PAGE_MASK;
-	while (remaining) {
-		p = page_address(subbuf.pages[idx]) + base;
-		len = min_t(unsigned int, PAGE_SIZE - base, remaining);
-		csum = csum_partial(p, len, csum);
-		remaining -= len;
-		base = 0;
-		++idx;
-	}
-	return csum;
-}
-
 static int
 nfsd_cache_key_cmp(const struct nfsd_cacherep *key,
-		   const struct nfsd_cacherep *rp, struct nfsd_net *nn)
+		   const struct nfsd_cacherep *rp)
 {
-	if (key->c_key.k_xid == rp->c_key.k_xid &&
-	    key->c_key.k_csum != rp->c_key.k_csum) {
-		nfsd_stats_payload_misses_inc(nn);
-		trace_nfsd_drc_mismatch(nn, key, rp);
-	}
-
 	return memcmp(&key->c_key, &rp->c_key, sizeof(key->c_key));
 }
 
@@ -560,7 +498,7 @@ nfsd_cache_insert(struct nfsd_drc_bucket *b, struct nfsd_cacherep *key,
 		parent = *p;
 		rp = rb_entry(parent, struct nfsd_cacherep, c_node);
 
-		cmp = nfsd_cache_key_cmp(key, rp, nn);
+		cmp = nfsd_cache_key_cmp(key, rp);
 		if (cmp < 0)
 			p = &parent->rb_left;
 		else if (cmp > 0)
@@ -589,28 +527,23 @@ nfsd_cache_insert(struct nfsd_drc_bucket *b, struct nfsd_cacherep *key,
 /**
  * nfsd_cache_lookup - Find an entry in the duplicate reply cache
  * @rqstp: Incoming Call to find
- * @start: starting byte in @rqstp->rq_arg of the NFS Call header
- * @len: size of the NFS Call header, in bytes
  * @cacherep: OUT: DRC entry for this request
  *
- * Try to find an entry matching the current call in the cache. When none
- * is found, we try to grab the oldest expired entry off the LRU list. If
- * a suitable one isn't there, then drop the cache_lock and allocate a
- * new one, then search again in case one got inserted while this thread
- * didn't hold the lock.
+ * Preallocate a cache entry for the current call, then attempt to
+ * insert it.  If an existing entry matches, the preallocated entry
+ * is freed and the cached reply is returned.
  *
  * Return values:
  *   %RC_DOIT: Process the request normally
  *   %RC_REPLY: Reply from cache
  *   %RC_DROPIT: Do not process the request further
  */
-int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
-		      unsigned int len, struct nfsd_cacherep **cacherep)
+int nfsd_cache_lookup(struct svc_rqst *rqstp,
+		      struct nfsd_cacherep **cacherep)
 {
 	struct nfsd_net		*nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
 	struct nfsd_cacherep	*rp, *found;
-	__wsum			csum;
 	struct nfsd_drc_bucket	*b;
 	int type = ntli->ntli_cachetype;
 	LIST_HEAD(dispose);
@@ -621,21 +554,29 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
 		goto out;
 	}
 
-	csum = nfsd_cache_csum(&rqstp->rq_arg, start, len);
-
 	/*
 	 * Since the common case is a cache miss followed by an insert,
 	 * preallocate an entry.
 	 */
-	rp = nfsd_cacherep_alloc(rqstp, csum, nn);
+	rp = nfsd_cacherep_alloc(rqstp, nn);
 	if (!rp)
 		goto out;
 
 	b = nfsd_cache_bucket_find(rqstp->rq_xid, nn);
 	spin_lock(&b->cache_lock);
 	found = nfsd_cache_insert(b, rp, nn);
-	if (found != rp)
-		goto found_entry;
+	if (found != rp) {
+		/*
+		 * The client already holds the reply for an acknowledged
+		 * entry, so a call carrying its XID is a new call.
+		 */
+		if (!found->c_acked)
+			goto found_entry;
+		trace_nfsd_drc_evict_acked(nn, found);
+		nfsd_cacherep_unlink_locked(nn, b, found);
+		list_add(&found->c_lru, &dispose);
+		nfsd_cache_insert(b, rp, nn);
+	}
 	*cacherep = rp;
 	rp->c_state = RC_INPROG;
 	nfsd_prune_bucket_locked(nn, b, 3, &dispose, rqstp->rq_xprt);
@@ -804,8 +745,6 @@ int nfsd_reply_cache_stats_show(struct seq_file *m, void *v)
 		   percpu_counter_sum_positive(&nn->counter[NFSD_STATS_RC_MISSES]));
 	seq_printf(m, "not cached:            %lld\n",
 		   percpu_counter_sum_positive(&nn->counter[NFSD_STATS_RC_NOCACHE]));
-	seq_printf(m, "payload misses:        %lld\n",
-		   percpu_counter_sum_positive(&nn->counter[NFSD_STATS_PAYLOAD_MISSES]));
 	seq_printf(m, "longest chain len:     %u\n", nn->longest_chain);
 	seq_printf(m, "cachesize at longest:  %u\n", nn->longest_chain_cachesize);
 	return 0;
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index d6687ffbec45..bc3e0e046bc2 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1004,7 +1004,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	const struct svc_procedure *proc = rqstp->rq_procinfo;
 	__be32 *statp = rqstp->rq_accept_statp;
 	struct nfsd_cacherep *rp;
-	unsigned int start, len;
 	__be32 *nfs_reply;
 
 	/*
@@ -1013,13 +1012,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	 */
 	ntli->ntli_cachetype = proc->pc_cachetype;
 
-	/*
-	 * ->pc_decode advances the argument stream past the NFS
-	 * Call header, so grab the header's starting location and
-	 * size now for the call to nfsd_cache_lookup().
-	 */
-	start = xdr_stream_pos(&rqstp->rq_arg_stream);
-	len = xdr_stream_remaining(&rqstp->rq_arg_stream);
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
@@ -1033,7 +1025,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
 
 	rp = NULL;
-	switch (nfsd_cache_lookup(rqstp, start, len, &rp)) {
+	switch (nfsd_cache_lookup(rqstp, &rp)) {
 	case RC_DOIT:
 		break;
 	case RC_REPLY:
diff --git a/fs/nfsd/stats.h b/fs/nfsd/stats.h
index aabfbb1a9c71..4a556dfbf64a 100644
--- a/fs/nfsd/stats.h
+++ b/fs/nfsd/stats.h
@@ -97,11 +97,6 @@ static inline void nfsd_stats_io_write_add(struct nfsd_net *nn,
 					 amount);
 }
 
-static inline void nfsd_stats_payload_misses_inc(struct nfsd_net *nn)
-{
-	percpu_counter_inc(&nn->counter[NFSD_STATS_PAYLOAD_MISSES]);
-}
-
 /**
  * nfsd_stats_drc_mem_usage_add - Add memory used by a cache item
  * @nn: target network namespace
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index c99b2a369d0d..b9d89a1e1c2e 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1536,30 +1536,6 @@ TRACE_EVENT(nfsd_drc_found,
 
 );
 
-TRACE_EVENT(nfsd_drc_mismatch,
-	TP_PROTO(
-		const struct nfsd_net *nn,
-		const struct nfsd_cacherep *key,
-		const struct nfsd_cacherep *rp
-	),
-	TP_ARGS(nn, key, rp),
-	TP_STRUCT__entry(
-		__field(unsigned long long, boot_time)
-		__field(u32, xid)
-		__field(u32, cached)
-		__field(u32, ingress)
-	),
-	TP_fast_assign(
-		__entry->boot_time = nn->boot_time;
-		__entry->xid = be32_to_cpu(key->c_key.k_xid);
-		__entry->cached = (__force u32)key->c_key.k_csum;
-		__entry->ingress = (__force u32)rp->c_key.k_csum;
-	),
-	TP_printk("boot_time=%16llx xid=0x%08x cached-csum=0x%08x ingress-csum=0x%08x",
-		__entry->boot_time, __entry->xid, __entry->cached,
-		__entry->ingress)
-);
-
 DECLARE_EVENT_CLASS(nfsd_drc_entry_class,
 	TP_PROTO(
 		const struct nfsd_net *nn,

-- 
2.55.0

[PATCH v3 12/12] NFSD: Remove hard cap on duplicate reply cache size

[Chuck Lever]

The DRC matters most during a network partition, when clients cannot
receive replies and the server must hold them for retransmission
after reconnect. Explicit and implied ACK are inactive then, leaving
only RC_EXPIRE, and a 256k entry ceiling can be too small for a large
client cohort through a long partition. During normal operation the
ACK paths keep the cache small regardless of the maximum, so the cap
constrains only the failure case, where headroom matters most.

Remove the cap and let the square-root formula govern sizing. It
scales sub-linearly with memory, and the hash table already uses
kvzalloc, so larger sizes need no physical contiguity. The limit is
unchanged at 16 GB and below. At 1 TB the formula yields 1048576
entries, four times the old cap. Even at 1 KB per entry, the worst
case the old comment assumed, a full cache is 1 GB, 0.1% of that
host's memory.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfscache.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 4ab6595a0bcd..5adf3c225a4f 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -48,34 +48,35 @@ static void	nfsd_reply_ack(void *data, const svc_ack_cookie_t *cookie,
 			       bool delivered);
 
 /*
- * Put a cap on the size of the DRC based on the amount of available
- * low memory in the machine.
+ * Set the size of the DRC based on the amount of available low
+ * memory in the machine.  The sizing formula scales with the
+ * square root of available pages, so growth is sub-linear:
  *
  *  64MB:    8192
- * 128MB:   11585
+ * 128MB:   11584
  * 256MB:   16384
- * 512MB:   23170
+ * 512MB:   23168
  *   1GB:   32768
- *   2GB:   46340
+ *   2GB:   46336
  *   4GB:   65536
- *   8GB:   92681
+ *   8GB:   92672
  *  16GB:  131072
+ *  32GB:  185344
+ *  64GB:  262144
+ * 128GB:  370688
+ * 256GB:  524288
+ * 512GB:  741440
+ *   1TB: 1048576
  *
- * ...with a hard cap of 256k entries. In the worst case, each entry will be
- * ~1k, so the above numbers should give a rough max of the amount of memory
- * used in k.
- *
- * XXX: these limits are per-container, so memory used will increase
- * linearly with number of containers.  Maybe that's OK.
+ * These limits are per-net-namespace, but the per-namespace
+ * shrinker reclaims entries under memory pressure.
  */
 static unsigned int
 nfsd_cache_size_limit(void)
 {
-	unsigned int limit;
 	unsigned long low_pages = totalram_pages() - totalhigh_pages();
 
-	limit = (16 * int_sqrt(low_pages)) << (PAGE_SHIFT-10);
-	return min_t(unsigned int, limit, 256*1024);
+	return (16 * int_sqrt(low_pages)) << (PAGE_SHIFT - 10);
 }
 
 /*

-- 
2.55.0

Re: [PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[Jeff Layton]

On Thu, 2026-09-10 at 09:54 -0400, Chuck Lever wrote:
> A completed DRC entry currently stays in its bucket for 120 seconds
> whether or not the client already holds the reply. On a busy server
> those entries lengthen every bucket walk, and under pressure they
> crowd out entries a retransmit could still hit.
> 
> This series modifies the DRC to retire an entry once there is reason
> to believe its reply was delivered.
> 
> 1. The transport reports delivery outright where it can. TCP now
>    reports when snd_una passes the reply's sequence number, and RDMA
>    reports on each Send completion.
> 2. Where no report will come, a later request on the same connection
>    stands in as an implied ACK, since a client on a live TCP or RDMA
>    connection does not retransmit within it (RFC 1813 Section 4.5).
> 
> UDP continues to use a traditional time-based eviction mechanism.
> 
> The first mechanism is the more reliable of the two, and the backup
> mechanism is more of an informed guess. A pipelined client sends its
> next request before the previous reply lands, and an entry evicted
> in that window is lost if the connection then drops and the client
> retransmits. In the current NFSD code, memory pressure and RC_EXPIRE
> already open this same window. For instance, on a server with fast
> networking and storage, pressure eviction already evicts entries far
> younger than 120 seconds.
> 
> Two DRC capacity guards that were sized for a cache full of stale
> replies can be removed, now that reply delivery reports keep the DRC
> small. The commit messages for those patches explain the rationale
> in detail.
> 
> v2 of this series (implied ACK only) was profiled with "perf record
> -e cycles -e cpu-clock -e LLC-load-misses -e branch-misses" during
> an NFSv3/RDMA 4KB random-write workload. v3 has not been re-profiled.
> nfsd_cache_lookup overhead dropped from 1.53% to 0.76% of CPU
> cycles during this test. The rb-tree operations (rb_erase,
> rb_insert_color) that dominated LLC cache misses fell from a
> combined 13.2% to 1.1% of all LLC-load-misses, because shorter-lived
> entries keep the per-bucket trees small.
> 
> ---
> Changes in v3:
> - Add the reply-acknowledged callback and its TCP and RDMA reporters
>   ahead of implied ACK, which now defers to a pending report.
> - Split the prune-loop restructure into its own patch.
> - Move the eviction tracepoints ahead of the ack patches; the
>   implied-ACK event now lands with implied ACK.
> - Keep err local to the pmap_register block in svc_setup_socket()
>   (per Jeff's review).
> - Never move xpt_last_recv backwards when nfsd threads race.
> - Fold the XPT_ORDERED patch into its consumer (per Jeff's review).
> - State the re-execution exposure of implied-ACK eviction instead of
>   calling the DRC advisory (per Jeff's review).
> - New patch: remove the DRC request checksum and payload_misses stat.
> - New patch: remove the 256k-entry cap on the DRC size.
> - Link to v2: https://patch.msgid.link/20260828-duplicate-reply-cache-v2-0-25069e660a7b@kernel.org
> 
> Changes in v2:
> - Print the DRC eviction tracepoints' age field as unsigned.
> - Link to v1: https://patch.msgid.link/20260826-duplicate-reply-cache-v1-0-b1d51e1af5c7@kernel.org
> 
> ---
> Chuck Lever (12):
>       SUNRPC: Assign a unique identifier to each svc_xprt
>       NFSD: Track transport in DRC entries
>       NFSD: Prepare bucket pruning for out-of-order eviction
>       NFSD: Add tracepoints for DRC entry eviction
>       NFSD: Record DRC population in lookup tracepoints
>       NFSD: Add reply-acknowledged callback infrastructure
>       SUNRPC: Add TCP sequence-number ACK tracking for reply delivery
>       svcrdma: Fire reply-acknowledged callback on Send completion
>       SUNRPC: Record last-request timestamp on svc_xprt
>       NFSD: Evict unacknowledged DRC entries via implied ACK
>       NFSD: Remove DRC checksum and payload_misses stat
>       NFSD: Remove hard cap on duplicate reply cache size
> 
>  .../ABI/testing/procfs-nfsd-reply_cache_stats      |  11 +-
>  fs/nfsd/cache.h                                    |  20 +-
>  fs/nfsd/netns.h                                    |   2 -
>  fs/nfsd/nfscache.c                                 | 310 +++++++++++++--------
>  fs/nfsd/nfssvc.c                                   |  20 +-
>  fs/nfsd/stats.h                                    |   5 -
>  fs/nfsd/trace.h                                    |  81 +++++-
>  include/linux/sunrpc/svc.h                         |  51 ++++
>  include/linux/sunrpc/svc_rdma.h                    |   1 +
>  include/linux/sunrpc/svc_xprt.h                    |   6 +-
>  include/linux/sunrpc/svcsock.h                     |  10 +
>  include/trace/events/sunrpc.h                      |  10 +-
>  net/sunrpc/netns.h                                 |   4 +
>  net/sunrpc/sunrpc_syms.c                           |   2 +
>  net/sunrpc/svc.c                                   |   1 +
>  net/sunrpc/svc_xprt.c                              |  53 +++-
>  net/sunrpc/svcsock.c                               | 119 +++++++-
>  net/sunrpc/xprtrdma/svc_rdma_sendto.c              |  14 +
>  net/sunrpc/xprtrdma/svc_rdma_transport.c           |   7 +-
>  19 files changed, 541 insertions(+), 186 deletions(-)
> ---
> base-commit: abf2077ee32058e60d3f49ecab265d3c4bd953d9
> change-id: 20260325-duplicate-reply-cache-f0fe7c7b740c
> 
> Best regards,
> --  
> Chuck Lever <cel@kernel.org>

I like the idea overall. I'm still a bit nervous that might see a non-
idempotent operations fail occasionally, since this is aggressively
expiring them, but let's see how it does.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[NeilBrown]

On Thu, 10 Sep 2026, Chuck Lever wrote:
> A completed DRC entry currently stays in its bucket for 120 seconds
> whether or not the client already holds the reply. On a busy server
> those entries lengthen every bucket walk, and under pressure they
> crowd out entries a retransmit could still hit.
> 
> This series modifies the DRC to retire an entry once there is reason
> to believe its reply was delivered.
> 
> 1. The transport reports delivery outright where it can. TCP now
>    reports when snd_una passes the reply's sequence number, and RDMA
>    reports on each Send completion.

As you note in the relevant patch, the TCP ack can arrive before the NFS
client has seen the reply.  But I don't think you drop the cached reply
immediately, so that probably don't matter.
The risk is that the second non-idempotent request might fail (mkdir,
but dir already exists), not that it will produce a different positive
result (technically possible, but vanishingly so).  Once we see the TCP
ACK we can be confident the client *will* see that reply (network fault
won't cause it not to) so it will mostly likely act on the first reply
and ignore the second.

> 2. Where no report will come, a later request on the same connection
>    stands in as an implied ACK, since a client on a live TCP or RDMA
>    connection does not retransmit within it (RFC 1813 Section 4.5).

I don't follow this.  Why would no report come?  TCP will always send
and ACK.
And I don't read RFC 1813 4.5 as saying a client does NOT retransmit on
a live connection, only that it DOES on a re-opened connection.
I believe that when an NFS client gets a timeout (6 minutes on TCP?) it
will resend without re-opening the TCP connection.

And as you say it is possible (I would say "likely") that a client will
have multiple requests outstanding.  Even multiple non-idempotent
requests.  So I cannot see that a later request tells us anything about
receiving an earlier reply.

A non-idempotent request often returns a new filehandle, and using the
presentation of that filehandle as evidence that the earlier reply was
seen would be sound, but I cannot see much other opportunity to draw a
connection from later requests to earlier replies.

So I'm perplexed as to why point 2 is needed, or how it is justified.

Thanks,
NeilBrown



> 
> UDP continues to use a traditional time-based eviction mechanism.
> 
> The first mechanism is the more reliable of the two, and the backup
> mechanism is more of an informed guess. A pipelined client sends its
> next request before the previous reply lands, and an entry evicted
> in that window is lost if the connection then drops and the client
> retransmits. In the current NFSD code, memory pressure and RC_EXPIRE
> already open this same window. For instance, on a server with fast
> networking and storage, pressure eviction already evicts entries far
> younger than 120 seconds.
> 
> Two DRC capacity guards that were sized for a cache full of stale
> replies can be removed, now that reply delivery reports keep the DRC
> small. The commit messages for those patches explain the rationale
> in detail.
> 
> v2 of this series (implied ACK only) was profiled with "perf record
> -e cycles -e cpu-clock -e LLC-load-misses -e branch-misses" during
> an NFSv3/RDMA 4KB random-write workload. v3 has not been re-profiled.
> nfsd_cache_lookup overhead dropped from 1.53% to 0.76% of CPU
> cycles during this test. The rb-tree operations (rb_erase,
> rb_insert_color) that dominated LLC cache misses fell from a
> combined 13.2% to 1.1% of all LLC-load-misses, because shorter-lived
> entries keep the per-bucket trees small.
> 
> ---
> Changes in v3:
> - Add the reply-acknowledged callback and its TCP and RDMA reporters
>   ahead of implied ACK, which now defers to a pending report.
> - Split the prune-loop restructure into its own patch.
> - Move the eviction tracepoints ahead of the ack patches; the
>   implied-ACK event now lands with implied ACK.
> - Keep err local to the pmap_register block in svc_setup_socket()
>   (per Jeff's review).
> - Never move xpt_last_recv backwards when nfsd threads race.
> - Fold the XPT_ORDERED patch into its consumer (per Jeff's review).
> - State the re-execution exposure of implied-ACK eviction instead of
>   calling the DRC advisory (per Jeff's review).
> - New patch: remove the DRC request checksum and payload_misses stat.
> - New patch: remove the 256k-entry cap on the DRC size.
> - Link to v2: https://patch.msgid.link/20260828-duplicate-reply-cache-v2-0-25069e660a7b@kernel.org
> 
> Changes in v2:
> - Print the DRC eviction tracepoints' age field as unsigned.
> - Link to v1: https://patch.msgid.link/20260826-duplicate-reply-cache-v1-0-b1d51e1af5c7@kernel.org
> 
> ---
> Chuck Lever (12):
>       SUNRPC: Assign a unique identifier to each svc_xprt
>       NFSD: Track transport in DRC entries
>       NFSD: Prepare bucket pruning for out-of-order eviction
>       NFSD: Add tracepoints for DRC entry eviction
>       NFSD: Record DRC population in lookup tracepoints
>       NFSD: Add reply-acknowledged callback infrastructure
>       SUNRPC: Add TCP sequence-number ACK tracking for reply delivery
>       svcrdma: Fire reply-acknowledged callback on Send completion
>       SUNRPC: Record last-request timestamp on svc_xprt
>       NFSD: Evict unacknowledged DRC entries via implied ACK
>       NFSD: Remove DRC checksum and payload_misses stat
>       NFSD: Remove hard cap on duplicate reply cache size
> 
>  .../ABI/testing/procfs-nfsd-reply_cache_stats      |  11 +-
>  fs/nfsd/cache.h                                    |  20 +-
>  fs/nfsd/netns.h                                    |   2 -
>  fs/nfsd/nfscache.c                                 | 310 +++++++++++++--------
>  fs/nfsd/nfssvc.c                                   |  20 +-
>  fs/nfsd/stats.h                                    |   5 -
>  fs/nfsd/trace.h                                    |  81 +++++-
>  include/linux/sunrpc/svc.h                         |  51 ++++
>  include/linux/sunrpc/svc_rdma.h                    |   1 +
>  include/linux/sunrpc/svc_xprt.h                    |   6 +-
>  include/linux/sunrpc/svcsock.h                     |  10 +
>  include/trace/events/sunrpc.h                      |  10 +-
>  net/sunrpc/netns.h                                 |   4 +
>  net/sunrpc/sunrpc_syms.c                           |   2 +
>  net/sunrpc/svc.c                                   |   1 +
>  net/sunrpc/svc_xprt.c                              |  53 +++-
>  net/sunrpc/svcsock.c                               | 119 +++++++-
>  net/sunrpc/xprtrdma/svc_rdma_sendto.c              |  14 +
>  net/sunrpc/xprtrdma/svc_rdma_transport.c           |   7 +-
>  19 files changed, 541 insertions(+), 186 deletions(-)
> ---
> base-commit: abf2077ee32058e60d3f49ecab265d3c4bd953d9
> change-id: 20260325-duplicate-reply-cache-f0fe7c7b740c
> 
> Best regards,
> --  
> Chuck Lever <cel@kernel.org>
> 
> 

Re: [PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[Chuck Lever]

On Fri, 11 Sep 2026, NeilBrown wrote:
> On Thu, 10 Sep 2026, Chuck Lever wrote:
> > 1. The transport reports delivery outright where it can. TCP now
> >    reports when snd_una passes the reply's sequence number, and RDMA
> >    reports on each Send completion.
>
> As you note in the relevant patch, the TCP ack can arrive before the NFS
> client has seen the reply.  But I don't think you drop the cached reply
> immediately, so that probably don't matter.

Right. The ACK marks the entry, and the next prune of that bucket
evicts it.


> > 2. Where no report will come, a later request on the same connection
> >    stands in as an implied ACK, since a client on a live TCP or RDMA
> >    connection does not retransmit within it (RFC 1813 Section 4.5).
>
> I don't follow this.  Why would no report come?  TCP will always send
> and ACK.

The cover letter was unclear. TCP always ACKs, but svc_tcp_sendto()
does not always track the reply. The per-socket ring is fixed-size,
so a burst overflows it. A kTLS send that leaves part of the record
unsent is not recorded, and neither is a failed send. Each of those
is reported as untracked right away, so the DRC entry has no report
to wait for.

Implied ACK was meant to retire those entries early. Without it, an
untracked entry is handled the way every entry is handled today. The
untracked entries just sit in their buckets for the full two minutes,
lengthening each lookup that hashes there, which is the cost this
series set out to remove.


> And I don't read RFC 1813 4.5 as saying a client does NOT retransmit on
> a live connection, only that it DOES on a re-opened connection.
> I believe that when an NFS client gets a timeout (6 minutes on TCP?) it
> will resend without re-opening the TCP connection.

Section 4.5 says that a connection break with automatic reestablishment
requires duplicate request processing. I misread that as naming the only
way a duplicate reaches a server over a connection-oriented transport,
and then reasoned that a request arriving on a live connection could not
be one. Re-reading, I see that text supports no such exclusion. It only
gives one case that a DRC must handle, and says nothing about what a
client does while the connection remains healthy.


> And as you say it is possible (I would say "likely") that a client will
> have multiple requests outstanding.  Even multiple non-idempotent
> requests.  So I cannot see that a later request tells us anything about
> receiving an earlier reply.

Agreed.


> So I'm perplexed as to why point 2 is needed, or how it is justified.

With the explicit reports in place I expect implied ACK to fire
rarely: svcrdma reports every reply, and on TCP only a ring overflow
leaves a reply untracked. Shall I drop implied ACK from v4 of the
series, measure the untracked rate under load, and size the ring from
that instead?


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[NeilBrown]

On Sat, 12 Sep 2026, Chuck Lever wrote:
> On Fri, 11 Sep 2026, NeilBrown wrote:
> > On Thu, 10 Sep 2026, Chuck Lever wrote:
> > > 1. The transport reports delivery outright where it can. TCP now
> > >    reports when snd_una passes the reply's sequence number, and RDMA
> > >    reports on each Send completion.
> >
> > As you note in the relevant patch, the TCP ack can arrive before the NFS
> > client has seen the reply.  But I don't think you drop the cached reply
> > immediately, so that probably don't matter.
> 
> Right. The ACK marks the entry, and the next prune of that bucket
> evicts it.
> 
> 
> > > 2. Where no report will come, a later request on the same connection
> > >    stands in as an implied ACK, since a client on a live TCP or RDMA
> > >    connection does not retransmit within it (RFC 1813 Section 4.5).
> >
> > I don't follow this.  Why would no report come?  TCP will always send
> > and ACK.
> 
> The cover letter was unclear. TCP always ACKs, but svc_tcp_sendto()
> does not always track the reply. The per-socket ring is fixed-size,
> so a burst overflows it. A kTLS send that leaves part of the record
> unsent is not recorded, and neither is a failed send. Each of those
> is reported as untracked right away, so the DRC entry has no report
> to wait for.

I don't see the need for the per-socket ring.
When a reply is sent - record the TCP sequence number in the DRC.
When pruning the DRC, get the TCP ack number first and compare it
against sequence numbers.
Do this often enough that that entries are pruned before the sequence
wraps past them.

kTLS is clearly more subtle but there must be some way record an
approximate seq number for an incomplete send ..  maybe current seq +
header-size + queued size ??

Unsent replies will never get an ack, but presumably the client will
resend, be answered using the cached reply, and we can then get an ack
on the new connection.  I think it would be wrong to drop an unsent
reply before the timeout, unless it gets re-sent and transport-acked
before then.

I think the opaque cookie aspect of the design is a mistake.  Everything
is fully sequenced over a network connection so a sequence number
(clearly paired with xpt_id) makes more sense...
Or does RDMA not sequence replies?  If so then it can only ack a single
reply at a time, not a range of replies.  But that needn't be a barrier.

When a message is transmitted the transport hands a seq-number (and
xpt_id) to the cache.  When it gets an ack it hands a range of
seq-numbers to the cache.  For a transport that had out-of-order acks,
this range would always be of length one (that transport might need a
private atomic_t to generate the sequence numbers).  For a transport
that batches acks (like TCP) the range could be much longer.

Also, I wonder if the xpt_id should be 64bit assigned sequentially with no
xa keeping track of used one.  64bits never wraps.

I think that with this design the implied ACK would add no value.

Thanks,
NeilBrown

Re: [PATCH v3 00/12] Improve the scalability of NFSD's classic DRC

[Chuck Lever]

On Fri, Sep 11, 2026, at 7:20 PM, NeilBrown wrote:
> On Sat, 12 Sep 2026, Chuck Lever wrote:
>> On Fri, 11 Sep 2026, NeilBrown wrote:
>> > On Thu, 10 Sep 2026, Chuck Lever wrote:
>> > > 1. The transport reports delivery outright where it can. TCP now
>> > >    reports when snd_una passes the reply's sequence number, and RDMA
>> > >    reports on each Send completion.
>> >
>> > As you note in the relevant patch, the TCP ack can arrive before the NFS
>> > client has seen the reply.  But I don't think you drop the cached reply
>> > immediately, so that probably don't matter.
>> 
>> Right. The ACK marks the entry, and the next prune of that bucket
>> evicts it.
>> 
>> 
>> > > 2. Where no report will come, a later request on the same connection
>> > >    stands in as an implied ACK, since a client on a live TCP or RDMA
>> > >    connection does not retransmit within it (RFC 1813 Section 4.5).
>> >
>> > I don't follow this.  Why would no report come?  TCP will always send
>> > and ACK.
>> 
>> The cover letter was unclear. TCP always ACKs, but svc_tcp_sendto()
>> does not always track the reply. The per-socket ring is fixed-size,
>> so a burst overflows it. A kTLS send that leaves part of the record
>> unsent is not recorded, and neither is a failed send. Each of those
>> is reported as untracked right away, so the DRC entry has no report
>> to wait for.
>
> I don't see the need for the per-socket ring.
> When a reply is sent - record the TCP sequence number in the DRC.
> When pruning the DRC, get the TCP ack number first and compare it
> against sequence numbers.
> Do this often enough that that entries are pruned before the sequence
> wraps past them.
>
> kTLS is clearly more subtle but there must be some way record an
> approximate seq number for an incomplete send ..  maybe current seq +
> header-size + queued size ??
>
> Unsent replies will never get an ack, but presumably the client will
> resend, be answered using the cached reply, and we can then get an ack
> on the new connection.  I think it would be wrong to drop an unsent
> reply before the timeout, unless it gets re-sent and transport-acked
> before then.
>
> I think the opaque cookie aspect of the design is a mistake.  Everything
> is fully sequenced over a network connection so a sequence number
> (clearly paired with xpt_id) makes more sense...
> Or does RDMA not sequence replies?  If so then it can only ack a single
> reply at a time, not a range of replies.  But that needn't be a barrier.
>
> When a message is transmitted the transport hands a seq-number (and
> xpt_id) to the cache.  When it gets an ack it hands a range of
> seq-numbers to the cache.  For a transport that had out-of-order acks,
> this range would always be of length one (that transport might need a
> private atomic_t to generate the sequence numbers).  For a transport
> that batches acks (like TCP) the range could be much longer.
>
> Also, I wonder if the xpt_id should be 64bit assigned sequentially with no
> xa keeping track of used one.  64bits never wraps.
>
> I think that with this design the implied ACK would add no value.

Thanks, all makes sense to me and reduces code complexity.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)