Re: [PATCH RFC 2/3] SUNRPC: dispatch idle transports ahead of backlogged ones

On 25 Jun 2026, at 12:09, Chuck Lever wrote:

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

Excellent - if we v1, I'll add this fix.

> 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/[email protected]?part=2

I'll head over there (cool - sashiko looks smart) and give that issue attention for any v1.

Ben