Re: [RFC PATCH net-next 2/2] net: napi: Skip poll when arming GRO timer in busy poll

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 28 Apr 2026 17:51:31 +0000 Dragos Tatulea wrote:
> From: Martin Karsten <mkarsten@uwaterloo.ca>
> 
> As referenced in the previous patch, having the GRO timer scheduled
> while poll is running can lead to issues.
> 
> Skip the extra call to napi->poll() when the GRO timer is armed. This
> removes the need for having a separate __busy_poll_stop routine and its
> code is moved directly into the relevant places in busy_poll_stop.

This needs to go in separately, the previous patch should be a Fix,
this is net-next material.

> @@ -6918,13 +6898,28 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
>  
>  	if (flags & NAPI_F_PREFER_BUSY_POLL) {
>  		napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi);
> -		if (napi->defer_hard_irqs_count) {
> -			/* Timer will be scheduled after napi poll to avoid
> -			 * firing during a slow poll which could cause the
> -			 * queue to get stuck with interrupts disabled and no
> -			 * scheduled timer.
> +		timeout = napi_get_gro_flush_timeout(napi);
> +		if (napi->defer_hard_irqs_count && timeout) {
> +			unsigned long flags;

Feels like either timeout should also be declared in closest scope or
flags at function level

> +			/* Drop prefer-busy state as in napi_complete_done(). */

sloppy comment

> +			clear_bit(NAPI_STATE_PREFER_BUSY_POLL, &napi->state);
> +			netpoll_poll_unlock(have_poll_lock);
> +
> +			/* Flush too old packets. If HZ < 1000, flush all
> +			 * packets.
> +			 */

sloppy comment

> +			gro_flush_normal(&napi->gro, HZ >= 1000);
> +			local_irq_save(flags);
> +			hrtimer_start(&napi->timer, ns_to_ktime(timeout),
> +				      HRTIMER_MODE_REL_PINNED);
> +			clear_bit(NAPI_STATE_SCHED, &napi->state);
> +			local_irq_restore(flags);
> +
> +			/* Timer started, so need for another call to
> +			 * napi->poll().
>  			 */
> -			timeout = napi_get_gro_flush_timeout(napi);
> +			goto out;

I think an else branch would do here? Let's not abuse goto


>  		}
>  	}
>  
> @@ -6938,8 +6933,12 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
>  	 */
>  	trace_napi_poll(napi, rc, budget);
>  	netpoll_poll_unlock(have_poll_lock);
> -	if (rc == budget)
> -		__busy_poll_stop(napi, timeout);
> +	if (rc == budget) {
> +		gro_normal_list(&napi->gro);
> +		__napi_schedule(napi);
> +	}
> +
> +out:
>  	bpf_net_ctx_clear(bpf_net_ctx);
>  	local_bh_enable();
>  }