Re: [RFC PATCH net-next 1/2] net: napi: Fix interrupts permanently disabled during busy poll

Dragos Tatulea
2026-04-29T12:44:02Z

On Wed, Apr 29, 2026 at 02:13:43PM +0200, Björn Töpel wrote:
> Dragos Tatulea <dtatulea@nvidia.com> writes:
>
> > On Tue, Apr 28, 2026 at 05:38:44PM -0700, Jakub Kicinski wrote:
> >> On Tue, 28 Apr 2026 17:51:30 +0000 Dragos Tatulea wrote:
> >> > + 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);
> >>
> >> I don't think disabling IRQ is necessary?
> >> Isn't it legal to clear the bit first then schedule the timer?
> >> The timer does not own the napi instance.
> >
> > Isn't the following scenario possible (but extremely unlikely)?
> >
> > 1. busy_poll_stop(): napi timer is schedueled.
> > 2. Hard irq pre-empts busy_poll_stop() and takes an unusually long time.
> > 4. napi timer triggers (also hard irq), napi_watchdog() skips schedule
> > because NAPI_STATE_SCHED is set.
> > 5. busy_poll_stop(): NAPI_STATE_SCHED gets cleared.
>
> (Nice work finding the bug! I had to jog my memory to understand the
> gnarly busy-poll details again!)
>
> You're right that you need the save/restore if you do arm timer/clear,
> but not if you do what Jakub suggests.
>
> clear_bit(...);
> hrtimer_start();
>
Ah sorry, I didn't read it right the first time because I was too fixed
on the order.

> That would also follow the scheme in napi_schedule_done(). (Note that
> swapping arm/clear does mean that we can get a wasted-timer outcome.)
>
If it is wasted is fine. I figured that this could end up with a timer
scheduled after the napi deletion:

1. busy_poll_stop(): clears SCHED bit.
2. napi_disable() runs past SCHED wait, cancels timer and caller
deletes napi mem.
3. busy_poll_stop(): napi->timer is armed but napi is freed memory
by now.

What am I missing here?

Thanks,
Dragos