* [PATCH 1/2] SUNRPC: maintain an rpc_task signalled flag
2026-11-05 3:07 [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release Tim Menninger
@ 2026-11-05 3:07 ` Tim Menninger
2026-11-05 3:07 ` [PATCH 2/2] SUNRPC: let cancelled RPC/RDMA requests drain before release Tim Menninger
2026-11-05 4:44 ` [PATCH 0/2] " Trond Myklebust
2 siblings, 0 replies; 4+ messages in thread
From: Tim Menninger @ 2026-11-05 3:07 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey, linux-nfs, linux-kernel, Eric Badger, Jon Curley
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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] SUNRPC: let cancelled RPC/RDMA requests drain before release
2026-11-05 3:07 [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release Tim Menninger
2026-11-05 3:07 ` [PATCH 1/2] SUNRPC: maintain an rpc_task signalled flag Tim Menninger
@ 2026-11-05 3:07 ` Tim Menninger
2026-11-05 4:44 ` [PATCH 0/2] " Trond Myklebust
2 siblings, 0 replies; 4+ messages in thread
From: Tim Menninger @ 2026-11-05 3:07 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey, linux-nfs, linux-kernel, Eric Badger, Jon Curley
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release
2026-11-05 3:07 [PATCH 0/2] SUNRPC: let cancelled RPC/RDMA requests drain before release Tim Menninger
2026-11-05 3:07 ` [PATCH 1/2] SUNRPC: maintain an rpc_task signalled flag Tim Menninger
2026-11-05 3:07 ` [PATCH 2/2] SUNRPC: let cancelled RPC/RDMA requests drain before release Tim Menninger
@ 2026-11-05 4:44 ` Trond Myklebust
2 siblings, 0 replies; 4+ messages in thread
From: Trond Myklebust @ 2026-11-05 4:44 UTC (permalink / raw)
To: Tim Menninger, Anna Schumaker
Cc: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey, linux-nfs, linux-kernel, Eric Badger, Jon Curley
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
^ permalink raw reply [flat|nested] 4+ messages in thread