[PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release

[PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release

[Tim Menninger]

Restarting a data server's NFS engine during sustained pNFS/flexfiles
READ I/O over RPC/RDMA can leave the client repeatedly reconnecting
after the server becomes available again. I reproduced this on Linux
6.8.0, 6.18.34, and 7.4-rc1.

Cancelling an RPC task does not establish that the server has finished
accessing its advertised Read or Write chunks. Releasing the request can
invalidate those registrations before the server finishes using them. A
subsequent remote access can fault the QP, disrupting other RPCs and
driving further layout cancellation and transport recovery.

This series lets eligible cancelled RPC/RDMA requests remain available
for normal Reply-driven cleanup instead of immediately releasing them.

Patch 1 records the first RPC error and any (possibly subsequent) signal
indication in one atomic state. This allows a signal to interrupt
optional draining even when an earlier cancellation error is already
recorded, without overwriting that error. Accepted RPC restarts clear
both together atomically, leaving the behavior from commit 5bbd6e863b15
("SUNRPC: Prevent looping due to rpc_signal_task() races") in tact.

Patch 2 adds transport-controlled deferred release for cancelled
asynchronous tasks. RPC/RDMA opts in while registered MRs remain on the
same live connection. Wake callbacks retain their transport status, and
waiting uses the existing request timeout and transport timer.  After
normal no-reply timeout handling, an already-cancelled task exits rather
than repeatedly re-entering draining with an expired timeout.  A request
whose Reply completed before release no longer triggers its previously
requested conditional disconnect.

Testing:

This series is based on v7.4-rc1.

There is a patch in
https://lore.kernel.org/all/20261105000131.2512336-1-tmenninger@everpuredata.com/:

  pNFS: avoid repeatedly cancelling I/O for the same lseg

which is required to cleanly observe the transport-recovery behavior.
Without this, my client livelocks after NFS restart.

There are also two strict prerequisite patches in
https://lore.kernel.org/all/20261105020249.2607367-1-tmenninger@purestorage.com/:

  SUNRPC: add request-scoped disconnects when cancelling tasks
  NFSv4/flexfiles: limit transport disconnects when cancelling I/O

Those patches are required for applying this series and are required to
observe full recovery after DS restart.

Finally, I applied the patch in
https://lore.kernel.org/all/20260823134640.3083136-1-tmenninger@everpuredata.com/

  xprtrdma: serialize unmap_sync with xprt_disconnect

to avoid a separate disconnect-time deadlock. The described reconnect
storm makes this otherwise rare condition easier to encounter.

The principal workload used one client, one MDS, and one DS, with
RPC/RDMA, nconnect=16, sustained direct READ I/O, 160 threads, iodepth
64, and 1 MiB I/O.

With this series in the tested configuration, restarting the DS's NFS
engine restores full throughput and normal operation in approximately 20
seconds, matching the expected recovery interval.  The continuous
post-recovery reconnect loop is no longer observed.

Related experiments:

While not the direct cause, this is exacerbated by commit b739a5bd9d9f
("NFSv4/flexfiles: Cancel I/O if the layout is recalled or revoked").
Skipping this behavior for RDMA xprt suffices to avoid the issue during
DS/NFS restart, but the underlying issue remains that stale rkeys can
access invalidated MRs.

In antoher experiment, fencing MRs during teardown and waiting for all
request-release waiters before disconnecting sufficed to eliminate
protection error messages in dmesg during recovery, but does not slow
down the xprt disconnect feedback loop.

When applied, a separately posted layoutreturn-seqid change at
https://lore.kernel.org/all/20260921235520.502987-1-tmenninger@everpuredata.com/
leaves some workload ranks slow after recovery even though the reconnect
loop is gone. The cause of that interaction is not yet established, and
I've asked to hold off merging that submission.

Tim Menninger (2):
  SUNRPC: maintain an rpc_task signalled flag
  SUNRPC: let cancelled RPC/RDMA requests drain before release

 fs/nfs/nfs4proc.c               |  12 ++-
 fs/nfs/pnfs.c                   |  11 ++-
 include/linux/sunrpc/sched.h    |  21 +++++-
 include/linux/sunrpc/xprt.h     |   3 +
 include/trace/events/sunrpc.h   |   1 +
 net/sunrpc/clnt.c               |   2 +-
 net/sunrpc/sched.c              |  53 ++++++++++---
 net/sunrpc/xprt.c               | 129 +++++++++++++++++++++++++++++++-
 net/sunrpc/xprtrdma/transport.c |  33 ++++++++
 9 files changed, 241 insertions(+), 24 deletions(-)


base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
prerequisite-patch-id: 2e8f35e38c760c42d04307b131ce55f80df66fef
prerequisite-patch-id: e7030ad56461efdd80f97ab1e0790fd4593805a7
prerequisite-patch-id: 8be1a2d2043bc7c018bb8e85af50ef438786a2aa
prerequisite-patch-id: 17803adca3ad2c820fc719d6976327e5981adafe
-- 
2.53.0

[PATCH 1/2] SUNRPC: maintain an rpc_task signalled flag

[Tim Menninger]

Commit 5bbd6e863b15a ("SUNRPC: Prevent looping due to rpc_signal_task()
races") removed the RPC_TASK_SIGNALLED runstate flag because maintaining
it independently from the errno in tk_rpc_status introduced races
between an errno being recorded and the task being in a SIGNALLED state.
A signalled task is inferred by tk_rpc_status containing -ERESTARTSYS.

This reintroduces the concept by coupling a "task signalled" bit with
the errno. The tk_rpc_status int becomes a tk_rpc_error_state atomic64_t
whose low 32 bits are a signed errno representing the first observed RPC
error, as tk_rpc_status previously held, and bit 32 is 1 if the task is
signalled and 0 otherwise.

With this change, we can record an RPC status and later escalate the
status to "signalled" without overwriting (and losing) the first
recorded error.

Signed-off-by: Tim Menninger <tmenninger@everpuredata.com>
---
 fs/nfs/nfs4proc.c            | 12 ++++++++----
 fs/nfs/pnfs.c                | 11 +++++++----
 include/linux/sunrpc/sched.h | 21 +++++++++++++++++++--
 net/sunrpc/clnt.c            |  2 +-
 net/sunrpc/sched.c           | 34 +++++++++++++++++++++++++++-------
 5 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e1018b03c1cbd..1face07f409b4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -688,9 +688,10 @@ nfs4_async_handle_exception(struct rpc_task *task, struct nfs_server *server,
 {
 	struct nfs_client *clp = server->nfs_client;
 	int ret;
+	int task_rpc_status = rpc_task_rpc_status(task);
 
-	if ((task->tk_rpc_status == -ENETDOWN ||
-	     task->tk_rpc_status == -ENETUNREACH) &&
+	if ((task_rpc_status == -ENETDOWN ||
+	     task_rpc_status == -ENETUNREACH) &&
 	    task->tk_flags & RPC_TASK_NETUNREACH_FATAL) {
 		exception->delay = 0;
 		exception->recovering = 0;
@@ -9845,12 +9846,15 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs4_layoutreturn *lrp = calldata;
 	struct nfs_server *server;
+	int task_rpc_status;
 
 	if (!nfs41_sequence_process(task, &lrp->res.seq_res))
 		return;
 
-	if (task->tk_rpc_status < 0) {
-		switch (task->tk_rpc_status) {
+	task_rpc_status = rpc_task_rpc_status(task);
+
+	if (task_rpc_status < 0) {
+		switch (task_rpc_status) {
 		case -EACCES:
 		case -EIO:
 		case -EKEYEXPIRED:
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index a057a06318c76..c51654e9d32eb 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1687,6 +1687,7 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 {
 	struct nfs4_layoutreturn_args *arg = *argpp;
 	int retval = -EAGAIN;
+	int task_rpc_status;
 
 	if (!arg)
 		return 0;
@@ -1696,15 +1697,17 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 		retval = 0;
 		break;
 	case -NFS4ERR_NOMATCHING_LAYOUT:
+		task_rpc_status = rpc_task_rpc_status(task);
+
 		/* Was there an RPC level error? If not, retry */
-		if (task->tk_rpc_status == 0)
+		if (task_rpc_status == 0)
 			break;
 		/*
 		 * Is there a fatal network level error?
 		 * If so release the layout, but flag the error.
 		 */
-		if ((task->tk_rpc_status == -ENETDOWN ||
-		     task->tk_rpc_status == -ENETUNREACH) &&
+		if ((task_rpc_status == -ENETDOWN ||
+		     task_rpc_status == -ENETUNREACH) &&
 		    task->tk_flags & RPC_TASK_NETUNREACH_FATAL) {
 			*ret = 0;
 			(*respp)->lrs_present = 0;
@@ -1714,7 +1717,7 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 		/* If the call was not sent, let caller handle it */
 		if (!RPC_WAS_SENT(task))
 			return 0;
-		switch (task->tk_rpc_status) {
+		switch (task_rpc_status) {
 		default:
 			/*
 			 * Defer the layoutreturn if it was due
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index dbaad8b5f181c..db322e26703a4 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -10,6 +10,7 @@
 #ifndef _LINUX_SUNRPC_SCHED_H_
 #define _LINUX_SUNRPC_SCHED_H_
 
+#include <linux/atomic.h>
 #include <linux/timer.h>
 #include <linux/ktime.h>
 #include <linux/sunrpc/types.h>
@@ -92,7 +93,10 @@ struct rpc_task {
 
 	pid_t			tk_owner;	/* Process id for batching tasks */
 
-	int			tk_rpc_status;	/* Result of last RPC operation */
+	atomic64_t		tk_rpc_error_state;
+						/* Low 32 bits: first recorded RPC error.
+						 * Bit 32: signal requested.
+						 */
 	unsigned short		tk_flags;	/* misc flags */
 	unsigned short		tk_timeouts;	/* maj timeouts */
 	unsigned short		tk_pid;		/* debugging aid */
@@ -158,6 +162,18 @@ enum {
 	RPC_TASK_MSG_PIN_WAIT,
 };
 
+#define RPC_TASK_ERR_STATE_SIGNALLED	(1LL << 32)
+
+static inline s64 rpc_task_read_error_state(const struct rpc_task *task)
+{
+	return atomic64_read(&task->tk_rpc_error_state);
+}
+
+static inline int rpc_task_rpc_status(const struct rpc_task *task)
+{
+	return (int)(s32)rpc_task_read_error_state(task);
+}
+
 #define rpc_test_and_set_running(t) \
 				test_and_set_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
 #define rpc_clear_running(t)	clear_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
@@ -168,7 +184,8 @@ enum {
 
 #define RPC_IS_ACTIVATED(t)	test_bit(RPC_TASK_ACTIVE, &(t)->tk_runstate)
 
-#define RPC_SIGNALLED(t)	(READ_ONCE(task->tk_rpc_status) == -ERESTARTSYS)
+#define RPC_SIGNALLED(t)	(!!(atomic64_read(&(t)->tk_rpc_error_state) & \
+				    RPC_TASK_ERR_STATE_SIGNALLED))
 
 /*
  * Task priorities.
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 5c33bf3fb4c34..cf17dfff57e90 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1758,7 +1758,7 @@ static int
 __rpc_restart_call(struct rpc_task *task, void (*action)(struct rpc_task *))
 {
 	task->tk_status = 0;
-	task->tk_rpc_status = 0;
+	atomic64_set(&task->tk_rpc_error_state, 0);
 	task->tk_action = action;
 	return 1;
 }
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 016f16ca57799..81890327d81a9 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -67,9 +67,17 @@ EXPORT_SYMBOL_GPL(rpc_task_gfp_mask);
 
 bool rpc_task_set_rpc_status(struct rpc_task *task, int rpc_status)
 {
-	if (cmpxchg(&task->tk_rpc_status, 0, rpc_status) == 0)
-		return true;
-	return false;
+	s64 old = atomic64_read(&task->tk_rpc_error_state);
+	s64 new;
+
+	for (;;) {
+		if ((s32)old)
+			return false;
+		new = (old & RPC_TASK_ERR_STATE_SIGNALLED) |
+		      ((s64)(u32)rpc_status);
+		if (atomic64_try_cmpxchg(&task->tk_rpc_error_state, &old, new))
+			return true;
+	}
 }
 
 unsigned long
@@ -857,12 +865,24 @@ void rpc_exit_task(struct rpc_task *task)
 void rpc_signal_task(struct rpc_task *task)
 {
 	struct rpc_wait_queue *queue;
+	s64 old, new;
 
 	if (!RPC_IS_ACTIVATED(task))
 		return;
 
-	if (!rpc_task_set_rpc_status(task, -ERESTARTSYS))
-		return;
+	old = atomic64_read(&task->tk_rpc_error_state);
+	for (;;) {
+		if (old & RPC_TASK_ERR_STATE_SIGNALLED)
+			return;
+
+		new = old | RPC_TASK_ERR_STATE_SIGNALLED;
+		if (!((s32)old))
+			new |= (s64)(u32)-ERESTARTSYS;
+
+		if (atomic64_try_cmpxchg(&task->tk_rpc_error_state, &old, new))
+			break;
+	}
+
 	trace_rpc_task_signalled(task, task->tk_action);
 	queue = READ_ONCE(task->tk_waitqueue);
 	if (queue)
@@ -928,7 +948,7 @@ static void __rpc_execute(struct rpc_task *task)
 		do_action = task->tk_action;
 		/* Tasks with an RPC error status should exit */
 		if (do_action && do_action != rpc_exit_task &&
-		    (status = READ_ONCE(task->tk_rpc_status)) != 0) {
+		    (status = rpc_task_rpc_status(task)) != 0) {
 			task->tk_status = status;
 			do_action = rpc_exit_task;
 		}
@@ -970,7 +990,7 @@ static void __rpc_execute(struct rpc_task *task)
 			continue;
 		}
 		/* Wake up any task that has an exit status */
-		if (READ_ONCE(task->tk_rpc_status) != 0) {
+		if (rpc_task_rpc_status(task) != 0) {
 			rpc_wake_up_task_queue_locked(queue, task);
 			spin_unlock(&queue->lock);
 			continue;
-- 
2.53.0

[PATCH 2/2] SUNRPC: let cancelled RPC/RDMA requests drain before release

[Tim Menninger]

During pNFS/flexfiles recovery, cancellation can release a posted
RPC/RDMA request before the server has finished accessing its Read or
Write chunks. xprt_rdma_free() can then invalidate advertised rkeys
while the peer still uses them. Resulting QP errors disrupt other RPCs
and can sustain a cancellation/reconnect loop after the data server
recovers. This was observed on Linux 6.8.0, 6.18.34, and 7.3-rc4.

Allow transports to defer exit of cancelled asynchronous tasks while
their requests can still complete normally. RPC/RDMA opts in while
registered MRs remain on the same live connection. Keep these requests
available for reply-driven cleanup, preserving the cancellation error
while wake callbacks process their transport status.

Use the existing request timeout and transport timer. After normal
no-reply timeout handling, finish cancellation instead of re-entering
draining. Suppress a requested conditional disconnect when the reply
completed before release. Synchronous and signalled tasks skip the
optional wait; remaining reclamation uses the existing release path.

Signed-off-by: Tim Menninger <tmenninger@everpuredata.com>
---
 include/linux/sunrpc/xprt.h     |   3 +
 include/trace/events/sunrpc.h   |   1 +
 net/sunrpc/sched.c              |  21 +++++-
 net/sunrpc/xprt.c               | 129 +++++++++++++++++++++++++++++++-
 net/sunrpc/xprtrdma/transport.c |  33 ++++++++
 5 files changed, 180 insertions(+), 7 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index ad8ecf8ccfb48..8b5b99ee34207 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -177,6 +177,7 @@ struct rpc_xprt_ops {
 	void		(*wait_for_reply_request)(struct rpc_task *task);
 	void		(*timer)(struct rpc_xprt *xprt, struct rpc_task *task);
 	void		(*release_request)(struct rpc_task *task);
+	bool		(*release_needs_drain)(struct rpc_rqst *req);
 	void		(*close)(struct rpc_xprt *xprt);
 	void		(*destroy)(struct rpc_xprt *xprt);
 	void		(*set_connect_timeout)(struct rpc_xprt *xprt,
@@ -395,6 +396,8 @@ bool			xprt_prepare_transmit(struct rpc_task *task);
 void			xprt_request_enqueue_transmit(struct rpc_task *task);
 int			xprt_request_enqueue_receive(struct rpc_task *task);
 void			xprt_request_wait_receive(struct rpc_task *task);
+bool			xprt_request_release_needs_wait(struct rpc_task *task);
+void			xprt_request_wait_for_drain(struct rpc_task *task);
 void			xprt_request_dequeue_xprt(struct rpc_task *task);
 bool			xprt_request_need_retransmit(struct rpc_task *task);
 void			xprt_transmit(struct rpc_task *task);
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index ff855197880de..263e8f6520ee3 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -410,6 +410,7 @@ DEFINE_RPC_RUNNING_EVENT(sync_wake);
 DEFINE_RPC_RUNNING_EVENT(complete);
 DEFINE_RPC_RUNNING_EVENT(timeout);
 DEFINE_RPC_RUNNING_EVENT(signalled);
+DEFINE_RPC_RUNNING_EVENT(exit_wait);
 DEFINE_RPC_RUNNING_EVENT(end);
 DEFINE_RPC_RUNNING_EVENT(call_done);
 
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 81890327d81a9..f1806f530fab7 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -932,6 +932,8 @@ static void __rpc_execute(struct rpc_task *task)
 	int task_is_async = RPC_IS_ASYNC(task);
 	int status = 0;
 	unsigned long pflags = current->flags;
+	bool task_draining;
+	s64 error_state;
 
 	WARN_ON_ONCE(RPC_IS_QUEUED(task));
 	if (RPC_IS_QUEUED(task))
@@ -939,6 +941,7 @@ static void __rpc_execute(struct rpc_task *task)
 
 	for (;;) {
 		void (*do_action)(struct rpc_task *);
+		task_draining = false;
 
 		/*
 		 * Perform the next FSM step or a pending callback.
@@ -949,8 +952,13 @@ static void __rpc_execute(struct rpc_task *task)
 		/* Tasks with an RPC error status should exit */
 		if (do_action && do_action != rpc_exit_task &&
 		    (status = rpc_task_rpc_status(task)) != 0) {
-			task->tk_status = status;
-			do_action = rpc_exit_task;
+			if (xprt_request_release_needs_wait(task)) {
+				task_draining = true;
+				do_action = xprt_request_wait_for_drain;
+			} else {
+				task->tk_status = status;
+				do_action = rpc_exit_task;
+			}
 		}
 		/* Callbacks override all actions */
 		if (task->tk_callback) {
@@ -989,8 +997,13 @@ static void __rpc_execute(struct rpc_task *task)
 			spin_unlock(&queue->lock);
 			continue;
 		}
-		/* Wake up any task that has an exit status */
-		if (rpc_task_rpc_status(task) != 0) {
+		/* Wake up any task that has an exit status. A task that has
+		 * deliberately gone to sleep to drain its request is exempted,
+		 * unless the task was signalled (i.e. urgently terminated).
+		 */
+		error_state = rpc_task_read_error_state(task);
+		if ((error_state & RPC_TASK_ERR_STATE_SIGNALLED) ||
+		    ((s32)error_state != 0 && !task_draining)) {
 			rpc_wake_up_task_queue_locked(queue, task);
 			spin_unlock(&queue->lock);
 			continue;
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 66eca8e4690b6..cf24cb52667da 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1237,6 +1237,7 @@ static void xprt_timer(struct rpc_task *task)
 {
 	struct rpc_rqst *req = task->tk_rqstp;
 	struct rpc_xprt *xprt = req->rq_xprt;
+	int status;
 
 	if (task->tk_status != -ETIMEDOUT)
 		return;
@@ -1245,6 +1246,13 @@ static void xprt_timer(struct rpc_task *task)
 	if (!req->rq_reply_bytes_recvd) {
 		if (xprt->ops->timer)
 			xprt->ops->timer(xprt, task);
+
+		/* An already-cancelled task should not drain again */
+		if (xprt->ops->release_needs_drain) {
+			status = rpc_task_rpc_status(task);
+			if (status)
+				rpc_exit(task, status);
+		}
 	} else
 		task->tk_status = 0;
 }
@@ -1528,6 +1536,114 @@ xprt_request_dequeue_xprt(struct rpc_task *task)
 	}
 }
 
+/**
+ * xprt_request_release_needs_wait - check whether request release must wait
+ * @task: RPC task being terminated
+ *
+ * Returns true if the request is pinned or its transport resources must
+ * drain before release. Removes pending transmissions when safe to do so.
+ *
+ * Context: Called from RPC task execution context. Acquires and releases
+ *          xprt->queue_lock. The transport's ->release_needs_drain() must not
+ *          sleep.
+ */
+bool xprt_request_release_needs_wait(struct rpc_task *task)
+{
+	struct rpc_rqst *req = task->tk_rqstp;
+	struct rpc_xprt *xprt;
+	bool defer = false;
+
+	if (!RPC_IS_ASYNC(task) || RPC_SIGNALLED(task))
+		return false;
+
+	if (!req)
+		return false;
+	xprt = req->rq_xprt;
+	if (!xprt->ops->release_needs_drain)
+		return false;
+
+	/* Has not been transmitted yet, no need to defer */
+	if (!test_bit(RPC_TASK_NEED_RECV, &task->tk_runstate))
+		return false;
+
+	spin_lock(&xprt->queue_lock);
+	if (xprt_is_pinned_rqst(req))
+		defer = true;
+	else if (test_bit(RPC_TASK_NEED_XMIT, &task->tk_runstate))
+		xprt_request_dequeue_transmit_locked(task);
+	else
+		defer = xprt->ops->release_needs_drain(req);
+	spin_unlock(&xprt->queue_lock);
+
+	return defer;
+}
+
+/**
+ * xprt_request_wait_for_drain - wait until a failed request can be released
+ * @task: RPC task whose request may still be in flight
+ *
+ * Like xprt_request_wait_receive(), but for a task that has already
+ * been cancelled. The task keeps its request on the receive queue and
+ * sleeps on the pending queue until the reply arrives or the connection
+ * is lost. The wait is bounded by the request's retransmit timeout, at
+ * which point the transport's timer callback runs exactly as it would
+ * for an uncancelled request.
+ *
+ * Context: Called from RPC task execution context. May sleep while waiting
+ *          for a pinned request, with xprt->queue_lock released. Holds
+ *          queue_lock when checking request state and queuing the task.
+ */
+void xprt_request_wait_for_drain(struct rpc_task *task)
+{
+	struct rpc_rqst *req = task->tk_rqstp;
+	struct rpc_xprt *xprt = req->rq_xprt;
+
+	trace_rpc_task_exit_wait(task, task->tk_action);
+
+	/*
+	 * Drop any send ownership handed to this task before waiting.
+	 */
+	xprt_release_write(xprt, task);
+
+	/*
+	 * Signals stop optional reply draining, but do not interrupt an
+	 * in-progress wait for request pins to be released.
+	 */
+	spin_lock(&xprt->queue_lock);
+	while (xprt_is_pinned_rqst(req) && !RPC_SIGNALLED(task)) {
+		set_bit(RPC_TASK_MSG_PIN_WAIT, &task->tk_runstate);
+		spin_unlock(&xprt->queue_lock);
+		xprt_wait_on_pinned_rqst(req);
+		spin_lock(&xprt->queue_lock);
+		clear_bit(RPC_TASK_MSG_PIN_WAIT, &task->tk_runstate);
+	}
+
+	if (xprt_is_pinned_rqst(req) || RPC_SIGNALLED(task))
+		goto out_unlock;
+
+	/*
+	 * The pinned operation may have completed or failed while we waited.
+	 * Recheck whether the request needs to drain or can be dequeued.
+	 */
+	if (test_bit(RPC_TASK_NEED_XMIT, &task->tk_runstate)) {
+		xprt_request_dequeue_transmit_locked(task);
+	} else if (test_bit(RPC_TASK_NEED_RECV, &task->tk_runstate) &&
+		   xprt->ops->release_needs_drain(req) &&
+		   !RPC_SIGNALLED(task)) {
+		rpc_sleep_on_timeout(&xprt->pending, task, xprt_timer,
+				     xprt_request_timeout(req));
+
+		if (RPC_SIGNALLED(task))
+			rpc_wake_up_queued_task(&xprt->pending, task);
+		else if (xprt_request_retransmit_after_disconnect(task))
+			rpc_wake_up_queued_task_set_status(&xprt->pending, task,
+							   -ENOTCONN);
+	}
+
+out_unlock:
+	spin_unlock(&xprt->queue_lock);
+}
+
 /**
  * xprt_request_prepare - prepare an encoded request for transport
  * @req: pointer to rpc_rqst
@@ -1609,11 +1725,13 @@ xprt_request_transmit(struct rpc_rqst *req, struct rpc_task *snd_task)
 	unsigned int connect_cookie;
 	int is_retrans = RPC_WAS_SENT(task);
 	int status;
+	s64 error_state;
 
 	if (test_bit(XPRT_CLOSE_WAIT, &xprt->state))
 		return -ENOTCONN;
 
 	if (!req->rq_bytes_sent) {
+		error_state = rpc_task_read_error_state(task);
 		if (xprt_request_data_received(task)) {
 			status = 0;
 			goto out_dequeue;
@@ -1623,10 +1741,14 @@ xprt_request_transmit(struct rpc_rqst *req, struct rpc_task *snd_task)
 			status = -EBADMSG;
 			goto out_dequeue;
 		}
-		if (RPC_SIGNALLED(task)) {
+		if (error_state & RPC_TASK_ERR_STATE_SIGNALLED) {
 			status = -ERESTARTSYS;
 			goto out_dequeue;
 		}
+		if (xprt->ops->release_needs_drain && (s32)error_state != 0) {
+			status = -ECANCELED;
+			goto out_dequeue;
+		}
 	}
 
 	/*
@@ -1695,7 +1817,7 @@ xprt_transmit(struct rpc_task *task)
 		xprt_pin_rqst(next);
 		spin_unlock(&xprt->queue_lock);
 		status = xprt_request_transmit(next, task);
-		if (status == -EBADMSG && next != req)
+		if ((status == -EBADMSG || status == -ECANCELED) && next != req)
 			status = 0;
 		spin_lock(&xprt->queue_lock);
 		xprt_unpin_rqst(next);
@@ -2061,7 +2183,8 @@ void xprt_release(struct rpc_task *task)
 	req->rq_disconnect_on_release = false;
 	spin_unlock(&xprt->queue_lock);
 
-	if (unlikely(disconnect && req->rq_ntrans > 0))
+	if (unlikely(disconnect && req->rq_ntrans > 0 &&
+		     !(xprt->ops->release_needs_drain && req->rq_reply_bytes_recvd)))
 		xprt_conditional_disconnect(xprt, req->rq_connect_cookie);
 
 	spin_lock(&xprt->transport_lock);
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index e38637c2ec787..ed521d9aa8953 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -434,6 +434,35 @@ xprt_rdma_timer(struct rpc_xprt *xprt, struct rpc_task *task)
 	xprt_force_disconnect(xprt);
 }
 
+/**
+ * xprt_rdma_release_needs_drain - decide whether a cancelled request must drain
+ * @rqst: transmitted RPC request whose task has been cancelled
+ *
+ * A Call posted on the current connection may have advertised chunks that the
+ * server is still reading or writing. Invalidating those MRs before the Reply
+ * arrives provokes a remote access error that tears down the QP. Keep a
+ * cancelled request available for reply-driven MR cleanup while its connection
+ * remains live. A false result leaves any remaining reclamation to the normal
+ * release path.
+ *
+ * Context: Called with xprt->queue_lock held and @rqst unpinned.
+ *          Must not sleep.
+ *
+ * Returns true if @rqst still has registered memory on the current connection.
+ */
+static bool
+xprt_rdma_release_needs_drain(struct rpc_rqst *rqst)
+{
+	struct rpc_xprt *xprt = rqst->rq_xprt;
+	struct rpcrdma_req *req = rpcr_to_rdmar(rqst);
+
+	if (!xprt_connected(xprt) ||
+	    rqst->rq_connect_cookie != READ_ONCE(xprt->connect_cookie))
+		return false;
+
+	return !list_empty(&req->rl_registered);
+}
+
 /**
  * xprt_rdma_set_connect_timeout - set timeouts for establishing a connection
  * @xprt: controlling transport instance
@@ -655,6 +684,9 @@ xprt_rdma_free(struct rpc_task *task)
 	struct rpc_rqst *rqst = task->tk_rqstp;
 	struct rpcrdma_req *req = rpcr_to_rdmar(rqst);
 
+	/* Graceful draining can be skipped or end before all MRs are reclaimed.
+	 * Complete any remaining cleanup through the normal release path.
+	 */
 	if (unlikely(!list_empty(&req->rl_registered))) {
 		trace_xprtrdma_mrs_zap(task);
 		frwr_unmap_sync(rpcx_to_rdmax(rqst->rq_xprt), req);
@@ -795,6 +827,7 @@ static const struct rpc_xprt_ops xprt_rdma_procs = {
 	.release_request	= xprt_release_rqst_cong,       /* ditto */
 	.wait_for_reply_request	= xprt_wait_for_reply_request_def, /* ditto */
 	.timer			= xprt_rdma_timer,
+	.release_needs_drain	= xprt_rdma_release_needs_drain,
 	.rpcbind		= rpcb_getport_async,	/* sunrpc/rpcb_clnt.c */
 	.set_port		= xprt_rdma_set_port,
 	.connect		= xprt_rdma_connect,
-- 
2.53.0

Re: [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release

[Trond Myklebust]

On Thu, 2026-11-05 at 03:07 +0000, Tim Menninger wrote:
> Restarting a data server's NFS engine during sustained pNFS/flexfiles
> READ I/O over RPC/RDMA can leave the client repeatedly reconnecting
> after the server becomes available again. I reproduced this on Linux
> 6.8.0, 6.18.34, and 7.4-rc1.
> 
> Cancelling an RPC task does not establish that the server has
> finished
> accessing its advertised Read or Write chunks. Releasing the request
> can
> invalidate those registrations before the server finishes using them.
> A
> subsequent remote access can fault the QP, disrupting other RPCs and
> driving further layout cancellation and transport recovery.
> 
> This series lets eligible cancelled RPC/RDMA requests remain
> available
> for normal Reply-driven cleanup instead of immediately releasing
> them.
> 
> Patch 1 records the first RPC error and any (possibly subsequent)
> signal
> indication in one atomic state. This allows a signal to interrupt
> optional draining even when an earlier cancellation error is already
> recorded, without overwriting that error. Accepted RPC restarts clear
> both together atomically, leaving the behavior from commit
> 5bbd6e863b15
> ("SUNRPC: Prevent looping due to rpc_signal_task() races") in tact.
> 
> Patch 2 adds transport-controlled deferred release for cancelled
> asynchronous tasks. RPC/RDMA opts in while registered MRs remain on
> the
> same live connection. Wake callbacks retain their transport status,
> and
> waiting uses the existing request timeout and transport timer.  After
> normal no-reply timeout handling, an already-cancelled task exits
> rather
> than repeatedly re-entering draining with an expired timeout.  A
> request
> whose Reply completed before release no longer triggers its
> previously
> requested conditional disconnect.
> 
> Testing:
> 
> This series is based on v7.4-rc1.
> 
> There is a patch in
> https://lore.kernel.org/all/20261105000131.2512336-1-tmenninger@everpuredata.com/
> :
> 
>   pNFS: avoid repeatedly cancelling I/O for the same lseg
> 
> which is required to cleanly observe the transport-recovery behavior.
> Without this, my client livelocks after NFS restart.
> 
> There are also two strict prerequisite patches in
> https://lore.kernel.org/all/20261105020249.2607367-1-tmenninger@purestorage.com/
> :
> 
>   SUNRPC: add request-scoped disconnects when cancelling tasks
>   NFSv4/flexfiles: limit transport disconnects when cancelling I/O
> 
> Those patches are required for applying this series and are required
> to
> observe full recovery after DS restart.
> 
> Finally, I applied the patch in
> https://lore.kernel.org/all/20260823134640.3083136-1-tmenninger@everpuredata.com/
> 
>   xprtrdma: serialize unmap_sync with xprt_disconnect
> 
> to avoid a separate disconnect-time deadlock. The described reconnect
> storm makes this otherwise rare condition easier to encounter.
> 
> The principal workload used one client, one MDS, and one DS, with
> RPC/RDMA, nconnect=16, sustained direct READ I/O, 160 threads,
> iodepth
> 64, and 1 MiB I/O.
> 
> With this series in the tested configuration, restarting the DS's NFS
> engine restores full throughput and normal operation in approximately
> 20
> seconds, matching the expected recovery interval.  The continuous
> post-recovery reconnect loop is no longer observed.
> 
> Related experiments:
> 
> While not the direct cause, this is exacerbated by commit
> b739a5bd9d9f
> ("NFSv4/flexfiles: Cancel I/O if the layout is recalled or revoked").
> Skipping this behavior for RDMA xprt suffices to avoid the issue
> during
> DS/NFS restart, but the underlying issue remains that stale rkeys can
> access invalidated MRs.
> 
> In antoher experiment, fencing MRs during teardown and waiting for
> all
> request-release waiters before disconnecting sufficed to eliminate
> protection error messages in dmesg during recovery, but does not slow
> down the xprt disconnect feedback loop.
> 
> When applied, a separately posted layoutreturn-seqid change at
> https://lore.kernel.org/all/20260921235520.502987-1-tmenninger@everpuredata.com/
> leaves some workload ranks slow after recovery even though the
> reconnect
> loop is gone. The cause of that interaction is not yet established,
> and
> I've asked to hold off merging that submission.
> 
> Tim Menninger (2):
>   SUNRPC: maintain an rpc_task signalled flag
>   SUNRPC: let cancelled RPC/RDMA requests drain before release
> 
>  fs/nfs/nfs4proc.c               |  12 ++-
>  fs/nfs/pnfs.c                   |  11 ++-
>  include/linux/sunrpc/sched.h    |  21 +++++-
>  include/linux/sunrpc/xprt.h     |   3 +
>  include/trace/events/sunrpc.h   |   1 +
>  net/sunrpc/clnt.c               |   2 +-
>  net/sunrpc/sched.c              |  53 ++++++++++---
>  net/sunrpc/xprt.c               | 129
> +++++++++++++++++++++++++++++++-
>  net/sunrpc/xprtrdma/transport.c |  33 ++++++++
>  9 files changed, 241 insertions(+), 24 deletions(-)
> 
> 
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
> prerequisite-patch-id: 2e8f35e38c760c42d04307b131ce55f80df66fef
> prerequisite-patch-id: e7030ad56461efdd80f97ab1e0790fd4593805a7
> prerequisite-patch-id: 8be1a2d2043bc7c018bb8e85af50ef438786a2aa
> prerequisite-patch-id: 17803adca3ad2c820fc719d6976327e5981adafe

This needs to be reworked. RDMA-specific error handling belongs in the
RDMA layer, and not in rpc_execute().

I'd suggest using the 'wait_for_reply_request' hook to install a custom
task->tk_callback that replaces xprt_timer, and does whatever error
handling that's needed when being woken up from the 'pending' state.

I'd also like to understand why we need to introduce a 64-bit atomic
signalling flag. Under which circumstances do we want to set this new
"signalled" status?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release

[Chuck Lever]

On 11/5/26 3:07 AM, Tim Menninger wrote:
> Cancelling an RPC task does not establish that the server has finished
> accessing its advertised Read or Write chunks. Releasing the request can
> invalidate those registrations before the server finishes using them. A
> subsequent remote access can fault the QP, disrupting other RPCs and
> driving further layout cancellation and transport recovery.
>
> This series lets eligible cancelled RPC/RDMA requests remain available
> for normal Reply-driven cleanup instead of immediately releasing them.

Apologies in advance for the length of this response.

My first concern on reading the cover was the client's memory
protection model. An RPC/RDMA Requester invalidates the MRs a request
registered before it releases the request, so that the Responder can
never see the contents of memory that has moved on to another use. I
wanted to be sure the series avoids the connection drops without
weakening that model.

RFC 8166 attaches the protection obligation to reuse, not to the
upper layer's authorization state. Section 4.4.1 requires that the
Requester protect a region from Responder access before the upper
layer may touch the data. Section 8.1.3 requires invalidation before
a terminated RPC's regions are released for reuse. Section 8.1.5
permits a Requester to invalidate on cancel, and gives the reason as
stopping a late Reply from altering memory after it has been reused.
That section then describes cancel-then-invalidate churn as a
denial-of-service exposure, because the Responder's replies terminate
the connection. That churn is the storm this series is addressing.

One might ask whether a layout revocation obliges the NFS client to
invalidate at once to fence the DS. The pNFS layouts were not written
with RPC/RDMA in mind, but the SCSI and NVMe layouts are the closest
analogue, since the host registers its buffers for the storage
controller much as a Requester advertises Write chunks. RFC 8154
Section 2.4.9 and RFC 9561 Section 2.2.3 put the fence on the MDS,
which preempts the client's reservation at the device, and that fence
covers in-flight I/O. The client learns it was fenced from a
Reservation Conflict on its outstanding commands. No host driver
unmaps a buffer under an outstanding command on a live connection.
The flexfiles layout over RPC/RDMA has no reservation, so the MDS
has no way to abort in-flight DS access, and the only fence the NFS
client has is a disconnect.

When these patches are applied, cancel now means "wait for the Reply
anyway, up to the request timeout". The task holds its slot, its MRs,
and its ULP references, including lsegs, for that whole window. I
expect the held lsegs are why the lseg patch is needed to avoid the
livelock. It looks like the request's RPC/RDMA credit is held for
that period as well. That might introduce a deadlock opportunity.

My other concern is complexity. __rpc_execute() already juggles
tk_rpc_status, the signalled state, queued callbacks, and the wakeup
of tasks that acquired an exit status while asleep, and that loop is
hard enough to reason about as it stands. This series adds a third
exit path, a drain, with its own exemption from the wakeup check, a
drain function that re-derives the request state under queue_lock
through three nested conditions, a special case in xprt_timer(), a new
-ECANCELED result from xprt_request_transmit(), and a condition on the
conditional disconnect in xprt_release(). Each of those is a new place
where a cancellation, a signal, a timeout, and a Reply can interleave,
and the interactions are spread across sched.c, xprt.c, and
transport.c.

A handful of specific complaints:

- xprt_request_release_needs_wait() takes the RPC_TASK_NEED_XMIT
  branch before consulting the transport. It dequeues the transmit
  and returns false, so a request with MRs still on rl_registered from
  an earlier transmission is released without draining. On RDMA a
  retransmit normally follows a disconnect, so the connect cookie
  would not match anyway, but the branch order means the transport is
  never asked. The commit message or a comment should carry that
  argument.

- A request whose marshal registered MRs but whose Send was never
  posted has RPC_TASK_NEED_RECV set and a live connection. It drains
  for a full timeout waiting for a Reply that cannot arrive, then
  xprt_rdma_timer forces a disconnect. The drain turns a cheap local
  failure into a connection drop.

- Widening every rpc_task by an atomic64 and converting the hot-path
  reads of tk_rpc_status is a lot of surface for one abort bit. Is
  there a reason the signalled state cannot live in tk_runstate with
  a documented ordering against tk_rpc_status? The race that
  5bbd6e863b15 fixed was the flag and the errno disagreeing, so the
  argument for the combined word needs to be made against that
  specific race.


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