* [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache
@ 2026-08-28 16:17 Chuck Lever
2026-08-28 16:17 ` [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt Chuck Lever
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, Chuck Lever
Connection-oriented transports (TCP, RDMA) guarantee in-order
delivery: arrival of a new request proves every prior reply on that
connection has been received. This series exploits that property to
evict completed DRC entries as soon as the next request arrives on
the same transport, rather than waiting out the 120-second
RC_EXPIRE timeout.
Each DRC entry records its originating transport. A new
xpt_last_recv timestamp on svc_xprt marks when the most recent
request arrived. During bucket pruning, nfsd_prune_bucket_locked()
compares the entry's timestamp against xpt_last_recv; a newer
value on the same transport means the reply was delivered, and the
entry is evicted immediately.
Profiled with "perf record -e cycles -e cpu-clock -e LLC-load-misses
-e branch-misses" during an NFSv3/RDMA 4KB random-write workload.
nfsd_cache_lookup overhead dropped from 1.53% to 0.76% of CPU
cycles. 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 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 (7):
SUNRPC: Assign a unique identifier to each svc_xprt
NFSD: Track transport in DRC entries
SUNRPC: Record last-request timestamp on svc_xprt
SUNRPC: Mark connection-oriented transports on svc_xprt
NFSD: Evict completed DRC entries via implied ACK
NFSD: Add tracepoints for DRC entry eviction
NFSD: Record DRC population in lookup tracepoints
fs/nfsd/cache.h | 1 +
fs/nfsd/nfscache.c | 88 +++++++++++++++++++++++++++-----
fs/nfsd/trace.h | 49 ++++++++++++++++--
include/linux/sunrpc/svc_xprt.h | 5 +-
include/trace/events/sunrpc.h | 9 +++-
net/sunrpc/netns.h | 4 ++
net/sunrpc/sunrpc_syms.c | 3 ++
net/sunrpc/svc_xprt.c | 54 ++++++++++++++++++--
net/sunrpc/svcsock.c | 41 ++++++++++-----
net/sunrpc/xprtrdma/svc_rdma_transport.c | 6 ++-
10 files changed, 222 insertions(+), 38 deletions(-)
---
base-commit: 3a4d7c7c9466a07fa4176ce35180149709d92c22
change-id: 20260325-duplicate-reply-cache-f0fe7c7b740c
Best regards,
--
Chuck Lever <cel@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 18:37 ` Jeff Layton
2026-08-28 16:17 ` [PATCH v2 2/7] NFSD: Track transport in DRC entries Chuck Lever
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, 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()
from a cyclic IDR, 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 | 3 ++
net/sunrpc/svc_xprt.c | 47 ++++++++++++++++++++++++++++----
net/sunrpc/svcsock.c | 40 +++++++++++++++++++--------
net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +++-
7 files changed, 89 insertions(+), 21 deletions(-)
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index da2a2531e110..88d73da5f46f 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -53,6 +53,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;
@@ -159,7 +160,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..53432d759ae8 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/idr.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 idr svc_xprt_ids;
+ spinlock_t svc_xprt_ids_lock;
};
extern unsigned int sunrpc_net_id;
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index 1a3884a0376a..d9ddf5b716cc 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -58,6 +58,8 @@ 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);
+ idr_init(&sn->svc_xprt_ids);
+ spin_lock_init(&sn->svc_xprt_ids_lock);
return 0;
err_pipefs:
@@ -74,6 +76,7 @@ static __net_exit void sunrpc_exit_net(struct net *net)
{
struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
+ idr_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 40040af588fb..ca10ef965443 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;
@@ -167,7 +169,12 @@ 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;
+
+ spin_lock(&sn->svc_xprt_ids_lock);
+ idr_remove(&sn->svc_xprt_ids, xprt->xpt_id);
+ spin_unlock(&sn->svc_xprt_ids_lock);
if (test_bit(XPT_CACHE_AUTH, &xprt->xpt_flags))
svcauth_unix_info_release(xprt);
put_cred(xprt->xpt_cred);
@@ -188,13 +195,31 @@ 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
+ * value is reused once @xprt is freed.
+ *
+ * On failure, the caller has only to free @xprt's containing structure.
+ *
+ * Context: Process context. May sleep. Takes and releases the
+ * per-net svc_xprt_ids_lock.
+ *
+ * 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);
+ int id;
+
memset(xprt, 0, sizeof(*xprt));
xprt->xpt_class = xcl;
xprt->xpt_ops = xcl->xcl_ops;
@@ -206,8 +231,20 @@ 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");
+
+ idr_preload(GFP_KERNEL);
+ spin_lock(&sn->svc_xprt_ids_lock);
+ id = idr_alloc_cyclic(&sn->svc_xprt_ids, xprt, 1, 0, GFP_NOWAIT);
+ spin_unlock(&sn->svc_xprt_ids_lock);
+ idr_preload_end();
+ if (id < 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 ef7ac080fcd3..840bec53e1ee 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -802,10 +802,11 @@ static struct svc_xprt_class svc_udp_class = {
.xcl_ident = XPRT_TRANSPORT_UDP,
};
-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;
@@ -832,6 +833,7 @@ static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
default:
BUG();
}
+ return true;
}
/*
@@ -1475,12 +1477,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) {
@@ -1511,6 +1514,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)
@@ -1553,6 +1557,7 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
int pmap_register = !(flags & SVC_SOCK_ANONYMOUS);
int sendpages;
unsigned long pages;
+ int err;
sendpages = svc_sock_sendpages(serv, sock, flags);
if (sendpages < 0)
@@ -1576,8 +1581,6 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
inet = sock->sk;
if (pmap_register) {
- int err;
-
err = svc_register(serv, sock_net(sock->sk), inet->sk_family,
inet->sk_protocol,
ntohs(inet_sk(inet)->inet_sport));
@@ -1602,13 +1605,26 @@ 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:
+ /* Port zero asks rpcbind to UNSET the registration made above. */
+ if (pmap_register)
+ svc_register(serv, sock_net(sock->sk), inet->sk_family,
+ inet->sk_protocol, 0);
+ 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 093371f9d245..fabdfd891a41 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/7] NFSD: Track transport in DRC entries
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
2026-08-28 16:17 ` [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 16:17 ` [PATCH v2 3/7] SUNRPC: Record last-request timestamp on svc_xprt Chuck Lever
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/7] SUNRPC: Record last-request timestamp on svc_xprt
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
2026-08-28 16:17 ` [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt Chuck Lever
2026-08-28 16:17 ` [PATCH v2 2/7] NFSD: Track transport in DRC entries Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 16:17 ` [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports " Chuck Lever
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, 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 and update it in
svc_handle_xprt() after each successful xpo_recvfrom(). A revisited
deferred request leaves it alone, since that request arrived before
the deferral and is not fresh client activity. 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 | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 88d73da5f46f..53ed073640a6 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -78,6 +78,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 ca10ef965443..1d4a40627244 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -914,6 +914,13 @@ static void svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
clear_bit(XPT_OLD, &xprt->xpt_flags);
+ /*
+ * A deferred request is a replay of one that arrived
+ * earlier, not fresh client activity on this connection.
+ */
+ if (!rqstp->rq_deferred)
+ WRITE_ONCE(xprt->xpt_last_recv, jiffies);
+
rqstp->rq_chandle.defer = svc_defer;
if (serv->sv_stats)
--
2.55.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports on svc_xprt
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
` (2 preceding siblings ...)
2026-08-28 16:17 ` [PATCH v2 3/7] SUNRPC: Record last-request timestamp on svc_xprt Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 18:19 ` Jeff Layton
2026-08-28 16:17 ` [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK Chuck Lever
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, Chuck Lever
A TCP or RDMA svc_xprt carries one client's request stream, delivered
reliably and in order, so a later request on it indicates that earlier
replies reached the client. A UDP svc_xprt is shared by every peer and
offers no such signal.
Add an XPT_ORDERED flag, set by the TCP and RDMA init paths and clear
for UDP, so the NFSD duplicate reply cache can confine implied-ACK
eviction to transports where the signal holds.
The flag has a cost 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 whenever a client
re-binds its reserved port, so this follows from the eviction policy
rather than from RDMA.
Signed-off-by: Chuck Lever <cel@kernel.org>
---
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 +
4 files changed, 4 insertions(+)
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 53ed073640a6..ebab18e403b2 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -99,6 +99,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_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 180346e520ff..2154fbe69945 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(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 840bec53e1ee..97a592ddeb48 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1486,6 +1486,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_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 fabdfd891a41..5d39fce231a3 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_ORDERED, &cma_xprt->sc_xprt.xpt_flags);
return cma_xprt;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
` (3 preceding siblings ...)
2026-08-28 16:17 ` [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports " Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 18:38 ` Jeff Layton
2026-08-28 16:17 ` [PATCH v2 6/7] NFSD: Add tracepoints for DRC entry eviction Chuck Lever
2026-08-28 16:17 ` [PATCH v2 7/7] NFSD: Record DRC population in lookup tracepoints Chuck Lever
6 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, Chuck Lever
A completed DRC entry stays in its bucket until RC_EXPIRE elapses or
the cache exceeds max_drc_entries. Entries whose replies the client
already holds lengthen the bucket and slow 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 clients
retransmit on timeout over a shared svc_xprt, so eviction is
restricted to transports marked XPT_ORDERED. Even there the evidence
is not conclusive, since a client with several requests outstanding
sends the next before the previous reply arrives. The cache is
advisory: a premature eviction costs a miss and re-execution, the same
outcome memory pressure and RC_EXPIRE already produce.
During bucket pruning, compare each RC_DONE entry's c_timestamp
against xpt_last_recv on the transport the current request arrived
on, and evict the entry when a later request has arrived. Only that
transport is consulted, because it is the one the pruning thread holds
a reference to. Entries recorded on other transports wait for a later
lookup or a shrinker pass.
Implied ACK does not evict in age order, so the loop can no longer
stop at the first non-evictable entry. A client controls its XIDs and
the bucket is chosen by an XID hash, so an unbounded scan lets it pack
one bucket and turn every miss into a walk of the whole bucket under
cache_lock. Bound the work per call to four times the eviction limit.
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfscache.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 66 insertions(+), 12 deletions(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index b25b4f9e92f7..7a09a79a2d6e 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -87,6 +87,34 @@ 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. Only XPT_ORDERED transports qualify: a client does not
+ * retransmit within a live connection, but a UDP client retransmits on
+ * timeout and shares one svc_xprt with every other UDP peer, so a
+ * datagram from any of them would evict another client's reply.
+ *
+ * The evidence is not conclusive: a pipelined client sends its next
+ * request before @rp's reply arrives. The cache is advisory, so acting
+ * early costs no more than a miss and re-execution.
+ *
+ * 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;
+
+ 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)
@@ -257,29 +285,55 @@ 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. If
+ * @max is zero, do not limit the number of removed entries.
+ *
+ * @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;
- 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))
+ if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries)
+ goto evict;
+ if (time_before_eq(rp->c_timestamp, expiry))
+ goto evict;
+ if (rp->c_state == RC_DONE &&
+ nfsd_cacherep_implied_ack(xprt, rp))
+ goto evict;
+ /*
+ * Only implied ACK evicts out of age order, and only on an
+ * ordered transport; otherwise the first non-evictable entry
+ * ends the scan.
+ */
+ if (!xprt || !test_bit(XPT_ORDERED, &xprt->xpt_flags))
break;
+ 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. Cap the work per call;
+ * a skipped entry is reclaimed on a later prune, under
+ * pressure, or at RC_EXPIRE.
+ */
+ if (max && (freed >= max || ++visited >= max * 4))
break;
}
}
@@ -307,9 +361,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,7 +382,7 @@ 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, 0, &dispose, NULL);
spin_unlock(&b->cache_lock);
freed += nfsd_cacherep_dispose(&dispose);
@@ -501,7 +555,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);
--
2.55.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 6/7] NFSD: Add tracepoints for DRC entry eviction
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
` (4 preceding siblings ...)
2026-08-28 16:17 ` [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
2026-08-28 16:17 ` [PATCH v2 7/7] NFSD: Record DRC population in lookup tracepoints Chuck Lever
6 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, Chuck Lever
The DRC pruning path evicts entries for memory pressure, expiry, and
implied ACK, and none of these emit a trace event. There is no way to
confirm that implied-ACK eviction fires or to compare its rate with
the other paths.
Add a tracepoint for each eviction reason.
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfscache.c | 12 +++++++++---
fs/nfsd/trace.h | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 7a09a79a2d6e..dfbcc75fbb12 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -304,13 +304,19 @@ 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;
+ }
if (rp->c_state == RC_DONE &&
- nfsd_cacherep_implied_ack(xprt, rp))
+ nfsd_cacherep_implied_ack(xprt, rp)) {
+ trace_nfsd_drc_evict_implied_ack(nn, rp);
goto evict;
+ }
/*
* Only implied ACK evicts out of age order, and only on an
* ordered transport; otherwise the first non-evictable entry
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 7d7a1483109a..c8468f67abe0 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1544,6 +1544,44 @@ 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);
+DEFINE_NFSD_DRC_ENTRY_EVENT(evict_implied_ack);
+
TRACE_EVENT(nfsd_cb_args,
TP_PROTO(
const struct nfs4_client *clp,
--
2.55.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 7/7] NFSD: Record DRC population in lookup tracepoints
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
` (5 preceding siblings ...)
2026-08-28 16:17 ` [PATCH v2 6/7] NFSD: Add tracepoints for DRC entry eviction Chuck Lever
@ 2026-08-28 16:17 ` Chuck Lever
6 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 16:17 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs, 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.
Add an entries= field. The count is passed as an explicit argument
rather than read from nn->num_drc_entries in TP_fast_assign, so that
per-connection lookup paths can later supply their own depth without
changing the tracepoint format.
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 dfbcc75fbb12..b917af667a89 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -606,7 +606,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 c8468f67abe0..af3e0c3f36d2 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -1500,23 +1500,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports on svc_xprt
2026-08-28 16:17 ` [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports " Chuck Lever
@ 2026-08-28 18:19 ` Jeff Layton
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Layton @ 2026-08-28 18:19 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
> A TCP or RDMA svc_xprt carries one client's request stream, delivered
> reliably and in order, so a later request on it indicates that earlier
> replies reached the client. A UDP svc_xprt is shared by every peer and
> offers no such signal.
>
> Add an XPT_ORDERED flag, set by the TCP and RDMA init paths and clear
> for UDP, so the NFSD duplicate reply cache can confine implied-ACK
> eviction to transports where the signal holds.
>
> The flag has a cost 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 whenever a client
> re-binds its reserved port, so this follows from the eviction policy
> rather than from RDMA.
>
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> 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 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index 53ed073640a6..ebab18e403b2 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -99,6 +99,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_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 180346e520ff..2154fbe69945 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(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 840bec53e1ee..97a592ddeb48 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1486,6 +1486,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_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 fabdfd891a41..5d39fce231a3 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_ORDERED, &cma_xprt->sc_xprt.xpt_flags);
>
> return cma_xprt;
> }
It would be helpful to see how this new flag will be used other than in
the tracepoint. Maybe this patch should be folded into the following
one?
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt
2026-08-28 16:17 ` [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt Chuck Lever
@ 2026-08-28 18:37 ` Jeff Layton
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Layton @ 2026-08-28 18:37 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
> 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()
> from a cyclic IDR, 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 | 3 ++
> net/sunrpc/svc_xprt.c | 47 ++++++++++++++++++++++++++++----
> net/sunrpc/svcsock.c | 40 +++++++++++++++++++--------
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +++-
> 7 files changed, 89 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index da2a2531e110..88d73da5f46f 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -53,6 +53,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;
> @@ -159,7 +160,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..53432d759ae8 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/idr.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 idr svc_xprt_ids;
> + spinlock_t svc_xprt_ids_lock;
> };
>
> extern unsigned int sunrpc_net_id;
> diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
> index 1a3884a0376a..d9ddf5b716cc 100644
> --- a/net/sunrpc/sunrpc_syms.c
> +++ b/net/sunrpc/sunrpc_syms.c
> @@ -58,6 +58,8 @@ 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);
> + idr_init(&sn->svc_xprt_ids);
> + spin_lock_init(&sn->svc_xprt_ids_lock);
> return 0;
>
> err_pipefs:
> @@ -74,6 +76,7 @@ static __net_exit void sunrpc_exit_net(struct net *net)
> {
> struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
>
> + idr_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 40040af588fb..ca10ef965443 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;
> @@ -167,7 +169,12 @@ 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;
> +
> + spin_lock(&sn->svc_xprt_ids_lock);
> + idr_remove(&sn->svc_xprt_ids, xprt->xpt_id);
> + spin_unlock(&sn->svc_xprt_ids_lock);
> if (test_bit(XPT_CACHE_AUTH, &xprt->xpt_flags))
> svcauth_unix_info_release(xprt);
> put_cred(xprt->xpt_cred);
> @@ -188,13 +195,31 @@ 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
> + * value is reused once @xprt is freed.
> + *
> + * On failure, the caller has only to free @xprt's containing structure.
> + *
> + * Context: Process context. May sleep. Takes and releases the
> + * per-net svc_xprt_ids_lock.
> + *
> + * 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);
> + int id;
> +
> memset(xprt, 0, sizeof(*xprt));
> xprt->xpt_class = xcl;
> xprt->xpt_ops = xcl->xcl_ops;
> @@ -206,8 +231,20 @@ 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");
> +
> + idr_preload(GFP_KERNEL);
> + spin_lock(&sn->svc_xprt_ids_lock);
> + id = idr_alloc_cyclic(&sn->svc_xprt_ids, xprt, 1, 0, GFP_NOWAIT);
> + spin_unlock(&sn->svc_xprt_ids_lock);
> + idr_preload_end();
> + if (id < 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 ef7ac080fcd3..840bec53e1ee 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -802,10 +802,11 @@ static struct svc_xprt_class svc_udp_class = {
> .xcl_ident = XPRT_TRANSPORT_UDP,
> };
>
> -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;
> @@ -832,6 +833,7 @@ static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
> default:
> BUG();
> }
> + return true;
> }
>
> /*
> @@ -1475,12 +1477,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) {
> @@ -1511,6 +1514,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)
> @@ -1553,6 +1557,7 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
> int pmap_register = !(flags & SVC_SOCK_ANONYMOUS);
> int sendpages;
> unsigned long pages;
> + int err;
Why move this variable to the outer scope? ^^^
>
> sendpages = svc_sock_sendpages(serv, sock, flags);
> if (sendpages < 0)
> @@ -1576,8 +1581,6 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
> inet = sock->sk;
>
> if (pmap_register) {
> - int err;
> -
> err = svc_register(serv, sock_net(sock->sk), inet->sk_family,
> inet->sk_protocol,
> ntohs(inet_sk(inet)->inet_sport));
> @@ -1602,13 +1605,26 @@ 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:
> + /* Port zero asks rpcbind to UNSET the registration made above. */
> + if (pmap_register)
> + svc_register(serv, sock_net(sock->sk), inet->sk_family,
> + inet->sk_protocol, 0);
> + 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 093371f9d245..fabdfd891a41 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);
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK
2026-08-28 16:17 ` [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK Chuck Lever
@ 2026-08-28 18:38 ` Jeff Layton
2026-08-28 18:59 ` Chuck Lever
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2026-08-28 18:38 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
> A completed DRC entry stays in its bucket until RC_EXPIRE elapses or
> the cache exceeds max_drc_entries. Entries whose replies the client
> already holds lengthen the bucket and slow 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 clients
> retransmit on timeout over a shared svc_xprt, so eviction is
> restricted to transports marked XPT_ORDERED. Even there the evidence
> is not conclusive, since a client with several requests outstanding
> sends the next before the previous reply arrives. The cache is
> advisory: a premature eviction costs a miss and re-execution, the same
> outcome memory pressure and RC_EXPIRE already produce.
>
No, it's not. The DRC is necessary for proper function, and if we drop
non-idempotent requests prematurely, then that could cause spurious
errors.
Or am I misunderstanding what you mean by "The cache" here?
> During bucket pruning, compare each RC_DONE entry's c_timestamp
> against xpt_last_recv on the transport the current request arrived
> on, and evict the entry when a later request has arrived. Only that
> transport is consulted, because it is the one the pruning thread holds
> a reference to. Entries recorded on other transports wait for a later
> lookup or a shrinker pass.
>
> Implied ACK does not evict in age order, so the loop can no longer
> stop at the first non-evictable entry. A client controls its XIDs and
> the bucket is chosen by an XID hash, so an unbounded scan lets it pack
> one bucket and turn every miss into a walk of the whole bucket under
> cache_lock. Bound the work per call to four times the eviction limit.
>
That said, the basic approach and code itself look OK to me overall
(modulo some nits). I think you mentioned too that BSD already takes
this approach?
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> fs/nfsd/nfscache.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
> index b25b4f9e92f7..7a09a79a2d6e 100644
> --- a/fs/nfsd/nfscache.c
> +++ b/fs/nfsd/nfscache.c
> @@ -87,6 +87,34 @@ 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. Only XPT_ORDERED transports qualify: a client does not
> + * retransmit within a live connection, but a UDP client retransmits on
> + * timeout and shares one svc_xprt with every other UDP peer, so a
> + * datagram from any of them would evict another client's reply.
> + *
> + * The evidence is not conclusive: a pipelined client sends its next
> + * request before @rp's reply arrives. The cache is advisory, so acting
> + * early costs no more than a miss and re-execution.
> + *
> + * 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;
> +
> + 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)
> @@ -257,29 +285,55 @@ 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. If
> + * @max is zero, do not limit the number of removed entries.
> + *
> + * @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;
> - 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))
> + if (atomic_read(&nn->num_drc_entries) > nn->max_drc_entries)
> + goto evict;
> + if (time_before_eq(rp->c_timestamp, expiry))
> + goto evict;
> + if (rp->c_state == RC_DONE &&
> + nfsd_cacherep_implied_ack(xprt, rp))
> + goto evict;
> + /*
> + * Only implied ACK evicts out of age order, and only on an
> + * ordered transport; otherwise the first non-evictable entry
> + * ends the scan.
> + */
> + if (!xprt || !test_bit(XPT_ORDERED, &xprt->xpt_flags))
> break;
> + 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. Cap the work per call;
> + * a skipped entry is reclaimed on a later prune, under
> + * pressure, or at RC_EXPIRE.
> + */
> + if (max && (freed >= max || ++visited >= max * 4))
> break;
> }
> }
> @@ -307,9 +361,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,7 +382,7 @@ 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, 0, &dispose, NULL);
> spin_unlock(&b->cache_lock);
>
> freed += nfsd_cacherep_dispose(&dispose);
> @@ -501,7 +555,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);
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK
2026-08-28 18:38 ` Jeff Layton
@ 2026-08-28 18:59 ` Chuck Lever
2026-08-28 19:15 ` Jeff Layton
0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 18:59 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, Aug 28, 2026, at 2:38 PM, Jeff Layton wrote:
> On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
>> A completed DRC entry stays in its bucket until RC_EXPIRE elapses or
>> the cache exceeds max_drc_entries. Entries whose replies the client
>> already holds lengthen the bucket and slow 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 clients
>> retransmit on timeout over a shared svc_xprt, so eviction is
>> restricted to transports marked XPT_ORDERED. Even there the evidence
>> is not conclusive, since a client with several requests outstanding
>> sends the next before the previous reply arrives. The cache is
>> advisory: a premature eviction costs a miss and re-execution, the same
>> outcome memory pressure and RC_EXPIRE already produce.
>
> No, it's not. The DRC is necessary for proper function, and if we drop
> non-idempotent requests prematurely, then that could cause spurious
> errors.
>
> Or am I misunderstanding what you mean by "The cache" here?
Not talking about the NFSv4.1 session cache. The old DRC has
always been best-effort. It cannot be relied upon for proper
function, since the DRC is non-deterministic.
The point I'm making here is that items already get evicted
due to memory pressure or because the DRC size is capped.
In particular, if you are actively removing items that are
very unlikely to be hit, that *reduces* the likelihood that
other, perhaps more valuable, items will be evicted due to
memory pressure.
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK
2026-08-28 18:59 ` Chuck Lever
@ 2026-08-28 19:15 ` Jeff Layton
2026-08-28 20:37 ` Chuck Lever
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2026-08-28 19:15 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, 2026-08-28 at 14:59 -0400, Chuck Lever wrote:
> On Fri, Aug 28, 2026, at 2:38 PM, Jeff Layton wrote:
> > On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
> > > A completed DRC entry stays in its bucket until RC_EXPIRE elapses or
> > > the cache exceeds max_drc_entries. Entries whose replies the client
> > > already holds lengthen the bucket and slow 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 clients
> > > retransmit on timeout over a shared svc_xprt, so eviction is
> > > restricted to transports marked XPT_ORDERED. Even there the evidence
> > > is not conclusive, since a client with several requests outstanding
> > > sends the next before the previous reply arrives. The cache is
> > > advisory: a premature eviction costs a miss and re-execution, the same
> > > outcome memory pressure and RC_EXPIRE already produce.
> >
> > No, it's not. The DRC is necessary for proper function, and if we drop
> > non-idempotent requests prematurely, then that could cause spurious
> > errors.
> >
> > Or am I misunderstanding what you mean by "The cache" here?
>
> Not talking about the NFSv4.1 session cache. The old DRC has
> always been best-effort. It cannot be relied upon for proper
> function, since the DRC is non-deterministic.
>
Yes, but in this case you can potentially evict this thing well before
you actually need it:
- client does a v3 RENAME (anything non-idempotent, really)
- reply to the RENAME goes out just before a new call comes in (WRITE
or something maybe). The RENAME DRC entry's timestamp is a few jiffies
earlier so we evict it.
- connection drops before client gets the reply and client reconnects
and retransmits the v3 RENAME
...hilarity ensues? Maybe this sort of bad luck never happens in
practice, but I don't see what would prevent it.
> The point I'm making here is that items already get evicted
> due to memory pressure or because the DRC size is capped.
>
> In particular, if you are actively removing items that are
> very unlikely to be hit, that *reduces* the likelihood that
> other, perhaps more valuable, items will be evicted due to
> memory pressure.
>
Sure, but usually we evict the oldest stuff first, and we generally
keep a big enough cache to get past the point where retransmissions
occur. This patchset changes that and will evict things earlier.
Maybe that's OK in practice.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK
2026-08-28 19:15 ` Jeff Layton
@ 2026-08-28 20:37 ` Chuck Lever
0 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-28 20:37 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: Rick Macklem, linux-nfs
On Fri, Aug 28, 2026, at 3:15 PM, Jeff Layton wrote:
> On Fri, 2026-08-28 at 14:59 -0400, Chuck Lever wrote:
>> On Fri, Aug 28, 2026, at 2:38 PM, Jeff Layton wrote:
>> > On Fri, 2026-08-28 at 12:17 -0400, Chuck Lever wrote:
>> > > A completed DRC entry stays in its bucket until RC_EXPIRE elapses or
>> > > the cache exceeds max_drc_entries. Entries whose replies the client
>> > > already holds lengthen the bucket and slow 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 clients
>> > > retransmit on timeout over a shared svc_xprt, so eviction is
>> > > restricted to transports marked XPT_ORDERED. Even there the evidence
>> > > is not conclusive, since a client with several requests outstanding
>> > > sends the next before the previous reply arrives. The cache is
>> > > advisory: a premature eviction costs a miss and re-execution, the same
>> > > outcome memory pressure and RC_EXPIRE already produce.
>> >
>> > No, it's not. The DRC is necessary for proper function, and if we drop
>> > non-idempotent requests prematurely, then that could cause spurious
>> > errors.
>> >
>> > Or am I misunderstanding what you mean by "The cache" here?
>>
>> Not talking about the NFSv4.1 session cache. The old DRC has
>> always been best-effort. It cannot be relied upon for proper
>> function, since the DRC is non-deterministic.
>>
>
> Yes, but in this case you can potentially evict this thing well before
> you actually need it:
>
> - client does a v3 RENAME (anything non-idempotent, really)
> - reply to the RENAME goes out just before a new call comes in (WRITE
> or something maybe). The RENAME DRC entry's timestamp is a few jiffies
> earlier so we evict it.
> - connection drops before client gets the reply and client reconnects
> and retransmits the v3 RENAME
>
> ...hilarity ensues? Maybe this sort of bad luck never happens in
> practice, but I don't see what would prevent it.
The same thing can happen if NFSD is handling a thousand chatty
clients. With fast networks and fast storage, overrunning the DRC
is more likely than it ever has been.
I've got more patches that moderate this behavior some. I could
post those as well... I was hoping to keep the patch sets narrow
to avoid overwhelming reviewers.
>> The point I'm making here is that items already get evicted
>> due to memory pressure or because the DRC size is capped.
>>
>> In particular, if you are actively removing items that are
>> very unlikely to be hit, that *reduces* the likelihood that
>> other, perhaps more valuable, items will be evicted due to
>> memory pressure.
>>
>
> Sure, but usually we evict the oldest stuff first, and we generally
> keep a big enough cache to get past the point where retransmissions
> occur. This patchset changes that and will evict things earlier.
>
> Maybe that's OK in practice.
The reconnect case is the only one I'm really concerned about.
I'm not brushing aside your concern, but missing a non-idempotent
reply during a reconnect storm is somewhat rare, and is already
not 100% deterministic.
And we have NFSv4.1 now. For anyone who is deeply concerned
about this race, NFSv4.1 is the only total answer.
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-28 20:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28 16:17 [PATCH v2 0/7] Implied-ACK eviction for NFSD's duplicate reply cache Chuck Lever
2026-08-28 16:17 ` [PATCH v2 1/7] SUNRPC: Assign a unique identifier to each svc_xprt Chuck Lever
2026-08-28 18:37 ` Jeff Layton
2026-08-28 16:17 ` [PATCH v2 2/7] NFSD: Track transport in DRC entries Chuck Lever
2026-08-28 16:17 ` [PATCH v2 3/7] SUNRPC: Record last-request timestamp on svc_xprt Chuck Lever
2026-08-28 16:17 ` [PATCH v2 4/7] SUNRPC: Mark connection-oriented transports " Chuck Lever
2026-08-28 18:19 ` Jeff Layton
2026-08-28 16:17 ` [PATCH v2 5/7] NFSD: Evict completed DRC entries via implied ACK Chuck Lever
2026-08-28 18:38 ` Jeff Layton
2026-08-28 18:59 ` Chuck Lever
2026-08-28 19:15 ` Jeff Layton
2026-08-28 20:37 ` Chuck Lever
2026-08-28 16:17 ` [PATCH v2 6/7] NFSD: Add tracepoints for DRC entry eviction Chuck Lever
2026-08-28 16:17 ` [PATCH v2 7/7] NFSD: Record DRC population in lookup tracepoints Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox