Re: [PATCH] io_uring/napi: drop the single NAPI busy poll special case

[Jens Axboe <axboe@kernel.dk>]

On 10/25/26 5:29 PM, Cinq Prairie wrote:
> io_napi_blocking_busy_loop() has a special case for a single registered
> NAPI: it passes io_napi_busy_loop_should_end() as loop_end to
> napi_busy_loop_rcu() and runs its own loop only once, relying on the
> inner __napi_busy_loop() to spin until that predicate says to stop.
> 
> napi_busy_loop_rcu() sets NAPI_F_END_ON_RESCHED, so __napi_busy_loop()
> also returns when need_resched() becomes true. io_uring treats that
> return as the end of the busy-poll period and falls through to
> schedule_hrtimeout() for the remainder of the wait timeout. The queue
> is then serviced by the gro_flush_timeout softirq timer instead of
> being actively polled (when napi_defer_hard_irqs/gro_flush_timeout are
> set).
> 
> With two or more NAPIs, the inner loop runs for a single iteration and
> io_uring's outer loop controls the busy-poll period, honoring
> busy_poll_to. As a result, the same registration behaves differently
> depending on whether one or multiple NAPIs are registered.
> 
> Remove the special case so a single NAPI follows the same path as
> multiple NAPIs: the inner loop runs for a single iteration, while
> io_uring continues busy polling until a completion, a signal or the
> deadline. Since commit df8599ee18c0 ("io_uring/napi: cap busy_poll_to
> 10 msec"), the deadline is at most 10 ms on every path, so this does not
> extend the busy-poll period beyond what the multi-NAPI path already
> allows.
> 
> First observed on a 5.14-based distribution kernel carrying the 6.13
> io_uring NAPI backport, reproduced on io_uring-7.4 at 9c207aa58e54,
> x86-64, HZ=1000, PREEMPT_DYNAMIC (lazy), no CPU isolation, low RX
> traffic, r8169 with one RX queue, prefer_busy_poll, busy_poll_to = wait
> timeout = 10 ms, with a SCHED_FIFO task pinned to the polling CPU
> waking every 1 ms:
> 
>   before, 1 NAPI:  busy 0.5-1.5 ms per 10 ms wait, ~0.8M polls/s, CPU ~5%
>   before, 2 NAPIs: busy 10 ms per 10 ms wait,      ~7.2M polls/s, CPU ~100%
>   after,  1 NAPI:  busy 10 ms per 10 ms wait,      ~7.4M polls/s, CPU ~100%

Claude had a few comments on this one, mostly related to now
ending up with a full start/stop cycle for every poll. Details
below:

> diff --git a/io_uring/napi.c b/io_uring/napi.c
> index 7e698720cd51..5362690a9d17 100644
> --- a/io_uring/napi.c
> +++ b/io_uring/napi.c
> @@ -166,28 +166,24 @@ static bool io_napi_busy_loop_should_end(void *data,

[ ... ]

>  	list_for_each_entry_rcu(e, &ctx->napi_list, list)
> -		napi_busy_loop_rcu(e->napi_id, loop_end, loop_end_arg,
> +		napi_busy_loop_rcu(e->napi_id, NULL, NULL,
>  				   ctx->napi_prefer_busy_poll, BUSY_POLL_BUDGET);
>  	return false;
>  }

[ ... ]

> @@ -216,24 +210,12 @@ static void io_napi_blocking_busy_loop(struct io_ring_ctx *ctx,
>  				       struct io_wait_queue *iowq)
>  {
>  	unsigned long start_time = busy_loop_current_time();
> -	bool (*loop_end)(void *, unsigned long) = NULL;
> -	void *loop_end_arg = NULL;
>  	bool is_stale = false;
>  
> -	/* Singular lists use a different napi loop end check function and are
> -	 * only executed once.
> -	 */
> -	if (list_is_singular(&ctx->napi_list)) {
> -		loop_end = io_napi_busy_loop_should_end;
> -		loop_end_arg = iowq;
> -	}
> -
>  	scoped_guard(rcu) {
>  		do {
> -			is_stale = __io_napi_do_busy_loop(ctx, loop_end,
> -							  loop_end_arg);
> -		} while (!io_napi_busy_loop_should_end(iowq, start_time) &&
> -			 !loop_end_arg);
> +			is_stale = __io_napi_do_busy_loop(ctx);
> +		} while (!io_napi_busy_loop_should_end(iowq, start_time));
>  	}

Does dropping the single-NAPI path trade the early need_resched() exit
for a full busy poll start/stop cycle on every poll?

With loop_end set, __napi_busy_loop() acquires the NAPI once, spins in
its own loop, and only calls busy_poll_stop() when it is done. With
loop_end NULL it breaks out right after the first poll and tears the
busy poll state down each time:

net/core/dev.c:__napi_busy_loop() {
    ...
		if (!loop_end || loop_end(loop_end_arg, start_time))
			break;
    ...
	}
	if (napi_poll)
		busy_poll_stop(napi, have_poll_lock, flags, budget);
    ...
}

So after this change every iteration of the do/while above re-acquires
the NAPI with the cmpxchg on napi->state, polls once, and then runs
busy_poll_stop(). Without prefer_busy_poll that is a second
napi->poll() call just to re-enable the device interrupt:

net/core/dev.c:busy_poll_stop() {
    ...
	} else {
		/* Use driver poll to re-enable device interrupts. */
		rc = napi->poll(napi, budget);
    ...
}

so the interrupt is re-enabled after every poll, and once it fires and
schedules the NAPI the next cmpxchg in __napi_busy_loop() fails and that
iteration polls nothing. With prefer_busy_poll and gro_flush_timeout set
it is instead an hrtimer_start() and a clear of NAPI_STATE_SCHED per
poll.

The multi-NAPI path already behaves this way, but a single registered
NAPI is the case the tight loop was there to serve. The before numbers
in the message all cover the early exit case, so the cost of the fast
path when it is not cut short does not show up in them. Is losing that
path acceptable, given this also goes to stable?

The notes below the --- line say keeping the singular fast path and
falling back to the single-shot loop only after an early return "works,
but leaves two code paths for a case whose only benefit was the one it
was broken for". Avoiding the per-poll busy_poll_stop() looks like a
benefit that is independent of the need_resched() exit, so would the
fallback approach be the better fix here?

The notes also say re-entering the singular loop is not viable because
__napi_busy_loop() restarts its deadline on entry. It does take
start_time at entry:

net/core/dev.c:__napi_busy_loop() {
	unsigned long start_time = loop_end ? busy_loop_current_time() : 0;
    ...
}

but io_napi_busy_loop_should_end() is io_uring's own callback, so could
it check an absolute deadline stored in iowq instead of the start_time
that net core passes in? That would let the inner loop be re-entered for
the remaining budget after a need_resched() exit without overshooting
busy_poll_to.

This isn't a bug, but with the special case gone neither the single nor
the multi NAPI path checks need_resched() at all for the whole budget,
and the only preemption point is the preempt_enable() between polls
inside __napi_busy_loop(). On a kernel without preemption that is up to
10 ms with no cond_resched(). Should the message mention that the
single-NAPI case now stops yielding to a woken task on those
configurations?

-- 
Jens Axboe