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

[Tim Menninger <tmenninger@everpuredata.com>]

> 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.

I agree about avoiding the additional lock acquisition in
xprt_release(). I should rework that part.

> So how about something like the following change instead?

I tested it with the same reproducer, but I still see the reconnect
behavior, without a noticeable improvement.

I think there are two separate issues here. Counting only successful
cancellations avoids triggering another disconnect merely because an
already-cancelled task still matches. Cancelling untransmitted tasks
separately also avoids disconnecting solely because of those tasks.

However, when cancellation of a transmitted task succeeds, we still call
rpc_clnt_disconnect(), which disconnects every connected transport on
that client. That can affect another transport which carried no matching
requests, or a replacement connection generation established since the
cancelled request was transmitted. Neither RPC_WAS_SENT() nor a
successful cancellation status update tells us whether those connections
carried the affected request.

What I'm trying to do is limit that blast radius. For a request with
rq_xprt == X and rq_connect_cookie == C, cancellation should not
disconnect X if its current cookie no longer matches C, nor disconnect
an unrelated transport Y on the same client.

Would retaining the request/connection-generation scope, while removing
the additional unconditional lock acquisition from xprt_release(), be a
reasonable direction?