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@xxxxxxxxxxxxxxxx/
> :
> 
>   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@xxxxxxxxxxxxxxx/
> :
> 
>   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@xxxxxxxxxxxxxxxx/
> 
>   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@xxxxxxxxxxxxxxxx/
> 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@xxxxxxxxxx, trond.myklebust@xxxxxxxxxxxxxxx