Re: [PATCH RFC 2/3] SUNRPC: dispatch idle transports ahead of backlogged ones
"Chuck Lever" <[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Jun 24, 2026, at 1:04 PM, Benjamin Coddington wrote:
> A pool dispatches ready transports in FIFO order, so a connection that
> already has requests in flight sits in the same queue, on equal terms,
> as one that has been idle. A client driving many concurrent requests
> therefore delays the next request of an interactive client whose
> previous reply has already completed: the interactive request waits
> behind the busy client's backlog even though servicing it costs a
> single round trip.
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 63d1002e63e7..ec4c05094e9a 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -523,7 +523,10 @@ void svc_xprt_enqueue(struct svc_xprt *xprt)
>
> percpu_counter_inc(&pool->sp_sockets_queued);
> xprt->xpt_qtime = ktime_get();
> - lwq_enqueue(&xprt->xpt_ready, &pool->sp_xprts);
> + if (atomic_read(&xprt->xpt_nr_rqsts))
> + lwq_enqueue(&xprt->xpt_ready, &pool->sp_xprts);
> + else
> + lwq_enqueue(&xprt->xpt_ready, &pool->sp_xprts_hi);
>
> svc_pool_wake_idle_thread(pool);
> }
The classification is keyed on in-flight request count, but a deferred
close is enqueued the same way. svc_xprt_deferred_close() sets
XPT_CLOSE and calls svc_xprt_enqueue() while the slot is still held;
svc_rdma_sendto() on a send error is one such path, and
svc_check_conn_limits() is another. svc_handle_xprt() reserves the
slot before calling recvfrom and releases it only after svc_process()
returns; recvfrom clears XPT_BUSY in between (svc_rdma_recvfrom ->
svc_xprt_received). So the deferred close is enqueued with XPT_BUSY
clear and xpt_nr_rqsts != 0, and the close lands on sp_xprts. Since
svc_xprt_dequeue() drains sp_xprts_hi first, the teardown waits behind
idle-flow traffic precisely when the server is busy and wants the
resources back.
Control work isn't request work, and svc_xprt_ready() already treats
XPT_CONN|XPT_CLOSE|XPT_HANDSHAKE as a class distinct from XPT_DATA.
Routing that class to the high-priority queue regardless of the request
count keeps the two consistent:
if (xprt->xpt_flags & (BIT(XPT_CONN) | BIT(XPT_CLOSE) |
BIT(XPT_HANDSHAKE)) ||
!atomic_read(&xprt->xpt_nr_rqsts))
lwq_enqueue(&xprt->xpt_ready, &pool->sp_xprts_hi);
else
lwq_enqueue(&xprt->xpt_ready, &pool->sp_xprts);
The burst-allowance patch narrows this -- a close with credit left
rides sp_xprts_hi -- but a flow that has spent its budget still routes
its close to the bulk queue, so the gap remains for exactly the
backlogged connections most worth closing promptly.
You should also have a look at the pre-existing issue that sashiko
identified: it might be amplified by this patch, so we should
consider addressing that issue as a pre-requisite to this series.
https://sashiko.dev/#/patchset/cover.1782314746.git.bcodding-F/[email protected]?part=2
--
Chuck Lever