Re: [PATCH v2 1/2] SUNRPC: add request-scoped disconnects when cancelling tasks

[Trond Myklebust <trondmy@kernel.org>]

On Thu, 2026-11-05 at 02:02 +0000, tmenninger@purestorage.com wrote:
> From: Tim Menninger <tmenninger@everpuredata.com>
> 
> rpc_cancel_tasks() allows callers to cancel a selected set of RPC
> tasks,
> but callers that also need to tear down the connection currently have
> to
> disconnect at the rpc_clnt level.
> 
> That is broader than necessary for clients with multiple transports.
> A
> cancelled request is associated with a specific rpc_xprt and records
> the
> connection generation on which it was transmitted in
> rq_connect_cookie.
> 
> Add rpc_cancel_tasks_and_disconnect() to cancel matching tasks and
> mark
> matching requests that remain on the transport transmit or receive
> queues
> for conditional transport disconnect when the request is released.
> 
> Store the disconnect indication in struct rpc_rqst rather than struct
> rpc_task. An RPC task can release one request and later acquire
> another,
> so task-scoped state could otherwise be consumed by a successor
> request
> and disconnect the wrong transport generation.
> 
> After cancelling matching tasks, walk the client's transports and
> mark
> matching requests belonging to that client that remain on the
> transmit
> or receive queues. An rpc_xprt may be shared by multiple rpc_clnt
> instances, so requests belonging to other clients are left untouched.
> 
> When a marked request is released, use xprt_conditional_disconnect()
> with
> its saved rq_connect_cookie. This disconnects the transport only if
> it is
> still using the same connection generation on which the request was
> sent,
> and leaves a replacement connection untouched.
> 
> Requests that were never transmitted do not require a disconnect.
> 

I'm all for optimising where it makes sense, however this is adding
another spinlock protected section in xprt_release() for something that
is clearly a rare corner case.

So how about something like the following change instead?

8<---------------------------------------------------------------
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 7fe8b91fa47c..42b0b04de659 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -2459,6 +2459,12 @@ static bool ff_layout_match_io(const struct rpc_task *task, const void *data)
 	return false;
 }
 
+static bool ff_layout_match_untransmitted_io(const struct rpc_task *task,
+					     const void *data)
+{
+	return !RPC_WAS_SENT(task) && ff_layout_match_io(task, data);
+}
+
 static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
 {
 	struct nfs4_ff_layout_segment *flseg = FF_LAYOUT_LSEG(lseg);
@@ -2484,6 +2490,9 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
 			clnt = ds_clp->cl_rpcclient;
 			if (!clnt)
 				continue;
+			rpc_cancel_tasks(clnt, -ECANCELED,
+					 ff_layout_match_untransmitted_io,
+					 lseg);
 			if (!rpc_cancel_tasks(clnt, -ECANCELED,
 					      ff_layout_match_io, lseg))
 				continue;

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