* [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
@ 2026-11-03 18:19 Mani Tofigh via B4 Relay
2026-11-03 18:37 ` Eric Dumazet
2026-11-06 9:22 ` netdev-bot+sashiko
0 siblings, 2 replies; 6+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-03 18:19 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Andrew Lunn, Martin Karsten, Joe Damato,
Dragos Tatulea, Kuniyuki Iwashima, Stanislav Fomichev,
Sridhar Samudrala, Cosmin Ratiu, Tariq Toukan, Wang Zhan, netdev,
linux-kernel, tz2294, Mani Tofigh
From: Mani Tofigh <mt4022@columbia.edu>
A napi can be freed with its timer still queued. napi_disable()
cancels napi->timer, but napi_complete_done(), busy_poll_stop() and
napi_suspend_irqs() start it without owning the napi and can do so
right after the cancel. netif_napi_del() only waits for that start to
finish:
Task A (epoll_wait) Task B (queue reset)
napi_suspend_irqs()
rcu_read_lock()
napi = napi_by_id(id)
napi_disable_locked(napi)
hrtimer_cancel(&napi->timer)
napi_hash_del(napi)
netif_napi_del_locked(napi)
synchronize_net()
/* waits for Task A */
hrtimer_start(&napi->timer)
rcu_read_unlock()
kfree(rq)
/* timer still queued */
With netdevsim and debug objects:
[ 3.073483] ODEBUG: free active (active state 0) object: 000000001504358f object type: hrtimer hint: napi_watchdog+0x0/0xa0
[ 3.073515] WARNING: lib/debugobjects.c:629 at debug_print_object+0x1a4/0x3b0, CPU#0: napitimer/66
[ 3.079603] CPU: 0 UID: 0 PID: 66 Comm: napitimer Tainted: G W 7.3.0-17087-gd865f2677c44 #1 PREEMPT
[ 3.084822] RIP: 0010:debug_print_object+0x1ae/0x3b0
...
[ 3.103822] Call Trace:
[ 3.104408] <TASK>
[ 3.105933] debug_check_no_obj_freed+0x496/0x5f0
[ 3.109988] kfree+0x33a/0x510
[ 3.110715] netdev_rx_queue_reconfig+0x357/0x580
[ 3.111780] netdev_rx_queue_restart+0x8f/0xc0
[ 3.113924] nsim_qreset_write+0x24d/0x350
[ 3.117832] full_proxy_write+0x109/0x1d0
[ 3.118741] vfs_write+0x1e3/0x1250
[ 3.124758] __x64_sys_pwrite64+0x18c/0x250
[ 3.129645] do_syscall_64+0xfb/0x590
[ 3.131385] entry_SYSCALL_64_after_hwframe+0x77/0x7f
Without debug objects, napi_watchdog() later runs on freed memory.
After napi_disable() and a grace period none of the three can reach the
napi anymore. Two need SCHED, which napi_disable() keeps, and
napi_suspend_irqs() needs the napi hash, which napi_disable() removed it
from. So cancel the timer again after the grace period that
netif_napi_del() already waits for, and in free_netdev(), where
unregister has already waited for one. With the fix, debug objects
don't report a freed active timer anymore.
This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development. The C reproducer below however
hits it within about 1s.
Fixes: 6f8b12d661d0 ("net: napi: add hard irqs deferral feature")
Fixes: 3fcbecbdeb04 ("net: Add control functions for irq suspension")
Fixes: 58e2330bd455 ("net: napi: Avoid gro timer misfiring at end of busypoll")
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Drivers that call __netif_napi_del{,_locked}() and then their own
synchronize_net() (eg netdevsim on close, veth, virtio_net, bnxt and
gro_cells) need the same cancel. I can send that separately.
Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/napitimer.c
---
include/linux/netdevice.h | 9 +++++++++
net/core/dev.c | 5 ++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 435be7880d7e..ad3685caf814 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3013,8 +3013,13 @@ static inline void __netif_napi_del(struct napi_struct *napi)
static inline void netif_napi_del_locked(struct napi_struct *napi)
{
+ bool listed = test_bit(NAPI_STATE_LISTED, &napi->state);
+
__netif_napi_del_locked(napi);
synchronize_net();
+ /* napi->timer can be started until a grace period after napi_disable() */
+ if (listed)
+ hrtimer_cancel(&napi->timer);
}
/**
@@ -3025,8 +3030,12 @@ static inline void netif_napi_del_locked(struct napi_struct *napi)
*/
static inline void netif_napi_del(struct napi_struct *napi)
{
+ bool listed = test_bit(NAPI_STATE_LISTED, &napi->state);
+
__netif_napi_del(napi);
synchronize_net();
+ if (listed)
+ hrtimer_cancel(&napi->timer);
}
int netif_enable_cpu_rmap(struct net_device *dev, unsigned int num_irqs);
diff --git a/net/core/dev.c b/net/core/dev.c
index f32353b0467c..6d6384522a1b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -12450,9 +12450,12 @@ static void netdev_napi_exit(struct net_device *dev)
if (!list_empty(&dev->napi_list)) {
struct napi_struct *p, *n;
+ /* unregister has waited for a grace period after napi_disable() */
netdev_lock(dev);
- list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
+ list_for_each_entry_safe(p, n, &dev->napi_list, dev_list) {
+ hrtimer_cancel(&p->timer);
__netif_napi_del_locked(p);
+ }
netdev_unlock(dev);
synchronize_net();
---
base-commit: 647b81ddefc6364290e67bf33c89d54971f8995e
change-id: 20261103-napi-timer-cancel-fa0a285fd581
Best regards,
--
Mani Tofigh <mt4022@columbia.edu>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
2026-11-03 18:19 [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del() Mani Tofigh via B4 Relay
@ 2026-11-03 18:37 ` Eric Dumazet
2026-11-04 0:55 ` Martin Karsten
2026-11-05 21:40 ` Mani Tofigh
2026-11-06 9:22 ` netdev-bot+sashiko
1 sibling, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-11-03 18:37 UTC (permalink / raw)
To: mt4022
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Andrew Lunn, Martin Karsten, Joe Damato, Dragos Tatulea,
Kuniyuki Iwashima, Stanislav Fomichev, Sridhar Samudrala,
Cosmin Ratiu, Tariq Toukan, Wang Zhan, netdev, linux-kernel,
tz2294
Le mar. 3 nov. 2026 à 19:19, Mani Tofigh via B4 Relay
<devnull+mt4022.columbia.edu@kernel.org> a écrit :
>
> From: Mani Tofigh <mt4022@columbia.edu>
>
> A napi can be freed with its timer still queued. napi_disable()
> cancels napi->timer, but napi_complete_done(), busy_poll_stop() and
> napi_suspend_irqs() start it without owning the napi and can do so
> right after the cancel. netif_napi_del() only waits for that start to
> finish:
>
> Task A (epoll_wait) Task B (queue reset)
> napi_suspend_irqs()
> rcu_read_lock()
> napi = napi_by_id(id)
> napi_disable_locked(napi)
> hrtimer_cancel(&napi->timer)
> napi_hash_del(napi)
> netif_napi_del_locked(napi)
> synchronize_net()
> /* waits for Task A */
> hrtimer_start(&napi->timer)
> rcu_read_unlock()
> kfree(rq)
> /* timer still queued */
>
> With netdevsim and debug objects:
>
> [ 3.073483] ODEBUG: free active (active state 0) object: 000000001504358f object type: hrtimer hint: napi_watchdog+0x0/0xa0
> [ 3.073515] WARNING: lib/debugobjects.c:629 at debug_print_object+0x1a4/0x3b0, CPU#0: napitimer/66
> [ 3.079603] CPU: 0 UID: 0 PID: 66 Comm: napitimer Tainted: G W 7.3.0-17087-gd865f2677c44 #1 PREEMPT
> [ 3.084822] RIP: 0010:debug_print_object+0x1ae/0x3b0
> ...
> [ 3.103822] Call Trace:
> [ 3.104408] <TASK>
> [ 3.105933] debug_check_no_obj_freed+0x496/0x5f0
> [ 3.109988] kfree+0x33a/0x510
> [ 3.110715] netdev_rx_queue_reconfig+0x357/0x580
> [ 3.111780] netdev_rx_queue_restart+0x8f/0xc0
> [ 3.113924] nsim_qreset_write+0x24d/0x350
> [ 3.117832] full_proxy_write+0x109/0x1d0
> [ 3.118741] vfs_write+0x1e3/0x1250
> [ 3.124758] __x64_sys_pwrite64+0x18c/0x250
> [ 3.129645] do_syscall_64+0xfb/0x590
> [ 3.131385] entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Without debug objects, napi_watchdog() later runs on freed memory.
>
> After napi_disable() and a grace period none of the three can reach the
> napi anymore. Two need SCHED, which napi_disable() keeps, and
> napi_suspend_irqs() needs the napi hash, which napi_disable() removed it
> from. So cancel the timer again after the grace period that
> netif_napi_del() already waits for, and in free_netdev(), where
> unregister has already waited for one. With the fix, debug objects
> don't report a freed active timer anymore.
>
> This bug was identified and confirmed with the help of a deterministic
> hypervisor currently under development. The C reproducer below however
> hits it within about 1s.
>
> Fixes: 6f8b12d661d0 ("net: napi: add hard irqs deferral feature")
> Fixes: 3fcbecbdeb04 ("net: Add control functions for irq suspension")
> Fixes: 58e2330bd455 ("net: napi: Avoid gro timer misfiring at end of busypoll")
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
> ---
Bug seems real, but could we prepare something generic (in the core).
> Drivers that call __netif_napi_del{,_locked}() and then their own
> synchronize_net() (eg netdevsim on close, veth, virtio_net, bnxt and
> gro_cells) need the same cancel. I can send that separately.
The list is longer than that : bnx2x, enic, ixgbe, myri10ge, dpaa and
bnge also use __netif_napi_del{,_locked}(). I would rather not touch 11
drivers for a core implementation detail.
Also the free_netdev() part assumes napi_disable() was called before
the synchronize_net() in unregister_netdevice_many_notify(), which is
not guaranteed (ndo_uninit(), or remove() handlers calling
napi_disable() between unregister_netdev() and free_netdev()).
All callers of __netif_napi_del_locked() must already respect an RCU
grace period before freeing the napi, and the three hrtimer_start()
callers run under RCU or with BH disabled.
So I think we can solve this in the core, with a store-buffer
pairing on NAPI_STATE_LISTED: the arming side re-checks LISTED after
hrtimer_start(), and __netif_napi_del_locked() cancels the timer after
clearing LISTED. One of the two must see the other.
Something like (untested):
/* Callers do not own @n, which can be deleted concurrently.
* Pairs with test_and_clear_bit(NAPI_STATE_LISTED)
* in __netif_napi_del_locked().
*/
static void napi_timer_start(struct napi_struct *n, unsigned long timeout)
{
hrtimer_start(&n->timer, ns_to_ktime(timeout),
HRTIMER_MODE_REL_PINNED);
smp_mb();
if (unlikely(!test_bit(NAPI_STATE_LISTED, &n->state)))
hrtimer_try_to_cancel(&n->timer);
}
And in __netif_napi_del_locked() :
if (!test_and_clear_bit(NAPI_STATE_LISTED, &napi->state))
return;
/* Full barrier implied by test_and_clear_bit()
* pairs with napi_timer_start().
*/
hrtimer_cancel(&napi->timer);
napi_complete_done(), busy_poll_stop() and napi_suspend_irqs() would
use napi_timer_start().
If hrtimer_try_to_cancel() fails, napi_watchdog() is running, either
from hard irq or softirq context. It does not rearm the timer, and the
grace period the caller of __netif_napi_del_locked() has to observe
before freeing the napi also waits for it.
A timer armed between napi_disable() and __netif_napi_del_locked()
is harmless : napi_watchdog() can not grab NAPI_STATE_SCHED.
This removes the need for the 'listed' dance in netif_napi_del() and
the netdev_napi_exit() change, and covers all drivers.
The extra smp_mb() is only paid when the timer is armed, next to
an hrtimer_start() which is already much more expensive.
Can you give this a try with your reproducer ?
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
2026-11-03 18:37 ` Eric Dumazet
@ 2026-11-04 0:55 ` Martin Karsten
2026-11-05 21:43 ` Mani Tofigh
2026-11-05 21:40 ` Mani Tofigh
1 sibling, 1 reply; 6+ messages in thread
From: Martin Karsten @ 2026-11-04 0:55 UTC (permalink / raw)
To: Eric Dumazet, mt4022
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Andrew Lunn, Joe Damato, Dragos Tatulea, Kuniyuki Iwashima,
Stanislav Fomichev, Sridhar Samudrala, Cosmin Ratiu,
Tariq Toukan, Wang Zhan, netdev, linux-kernel, tz2294
On 2026-11-03 13:37, Eric Dumazet wrote:
> Le mar. 3 nov. 2026 à 19:19, Mani Tofigh via B4 Relay
> <devnull+mt4022.columbia.edu@kernel.org> a écrit :
>>
>> From: Mani Tofigh <mt4022@columbia.edu>
>>
>> A napi can be freed with its timer still queued. napi_disable()
>> cancels napi->timer, but napi_complete_done(), busy_poll_stop() and
>> napi_suspend_irqs() start it without owning the napi and can do so
>> right after the cancel. netif_napi_del() only waits for that start to
>> finish:
>>
>> Task A (epoll_wait) Task B (queue reset)
>> napi_suspend_irqs()
>> rcu_read_lock()
>> napi = napi_by_id(id)
>> napi_disable_locked(napi)
>> hrtimer_cancel(&napi->timer)
>> napi_hash_del(napi)
>> netif_napi_del_locked(napi)
>> synchronize_net()
>> /* waits for Task A */
>> hrtimer_start(&napi->timer)
>> rcu_read_unlock()
>> kfree(rq)
>> /* timer still queued */
>>
>> With netdevsim and debug objects:
>>
>> [ 3.073483] ODEBUG: free active (active state 0) object: 000000001504358f object type: hrtimer hint: napi_watchdog+0x0/0xa0
>> [ 3.073515] WARNING: lib/debugobjects.c:629 at debug_print_object+0x1a4/0x3b0, CPU#0: napitimer/66
>> [ 3.079603] CPU: 0 UID: 0 PID: 66 Comm: napitimer Tainted: G W 7.3.0-17087-gd865f2677c44 #1 PREEMPT
>> [ 3.084822] RIP: 0010:debug_print_object+0x1ae/0x3b0
>> ...
>> [ 3.103822] Call Trace:
>> [ 3.104408] <TASK>
>> [ 3.105933] debug_check_no_obj_freed+0x496/0x5f0
>> [ 3.109988] kfree+0x33a/0x510
>> [ 3.110715] netdev_rx_queue_reconfig+0x357/0x580
>> [ 3.111780] netdev_rx_queue_restart+0x8f/0xc0
>> [ 3.113924] nsim_qreset_write+0x24d/0x350
>> [ 3.117832] full_proxy_write+0x109/0x1d0
>> [ 3.118741] vfs_write+0x1e3/0x1250
>> [ 3.124758] __x64_sys_pwrite64+0x18c/0x250
>> [ 3.129645] do_syscall_64+0xfb/0x590
>> [ 3.131385] entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>
>> Without debug objects, napi_watchdog() later runs on freed memory.
>>
>> After napi_disable() and a grace period none of the three can reach the
>> napi anymore. Two need SCHED, which napi_disable() keeps, and
>> napi_suspend_irqs() needs the napi hash, which napi_disable() removed it
>> from. So cancel the timer again after the grace period that
>> netif_napi_del() already waits for, and in free_netdev(), where
>> unregister has already waited for one. With the fix, debug objects
>> don't report a freed active timer anymore.
>>
>> This bug was identified and confirmed with the help of a deterministic
>> hypervisor currently under development. The C reproducer below however
>> hits it within about 1s.
>>
>> Fixes: 6f8b12d661d0 ("net: napi: add hard irqs deferral feature")
>> Fixes: 3fcbecbdeb04 ("net: Add control functions for irq suspension")
>> Fixes: 58e2330bd455 ("net: napi: Avoid gro timer misfiring at end of busypoll")
>> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
>> ---
Thanks for reporting this. I'm very impressed by the reproducer!
> Bug seems real, but could we prepare something generic (in the core).
I agree and I believe Sashiko has found this bug earlier [1]. I didn't
have a reproducer at the time and maintainers didn't flag it, so I
didn't speak up.
I thought this could be addressed more easily, by adding to each of the
3 calls to hrtimer_start (maybe via subroutine as Eric suggests):
> static void napi_timer_start(struct napi_struct *n, unsigned long timeout)
> {
> hrtimer_start(&n->timer, ns_to_ktime(timeout),
> HRTIMER_MODE_REL_PINNED);
> smp_mb();
> if (unlikely(!test_bit(NAPI_STATE_LISTED, &n->state)))
> hrtimer_try_to_cancel(&n->timer);
> }
hrtimer_start(&n->timer, ns_to_ktime(timeout),
HRTIMER_MODE_REL_PINNED);
if (unlikely(napi_is_scheduled(n)))
hrtimer_cancel(&n->timer);
> And in __netif_napi_del_locked() :
This change is not needed then.
My reasoning is as follows: SCHED is being set in napi_disable_locked()
before hrtimer_cancel() is called. hrtimer_start() and hrtimer_cancel()
are atomic to each other and impose strict memory ordering via the timer
lock (I assume?), so the SCHED bit is propagated, if hrtimer_cancel() in
napi_disable() happens first. Memory is not yet released due to RCU. If
the napi might be scheduled for some other reason, I believe it is still
fine to cancel the timer.
I am not sure about hrtimer_try_to_cancel() vs hrtimer_cancel(). If
hrtimer_try_to_cancel() fails, the timer callback might "running", but
could it be on a different core and slow enough, so napi_disable() might
complete and memory be freed before napi_watchdog() is done?
In my tests, including the reproducer, this works, but this stuff is
always dicey...
Thanks,
Martin
[1]
https://sashiko.dev/#/patchset/20260523012247.1574691-1-mkarsten%40uwaterloo.ca
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
2026-11-04 0:55 ` Martin Karsten
@ 2026-11-05 21:43 ` Mani Tofigh
0 siblings, 0 replies; 6+ messages in thread
From: Mani Tofigh @ 2026-11-05 21:43 UTC (permalink / raw)
To: Martin Karsten, Eric Dumazet
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Andrew Lunn, Joe Damato, Dragos Tatulea, Kuniyuki Iwashima,
Stanislav Fomichev, Sridhar Samudrala, Cosmin Ratiu,
Tariq Toukan, Wang Zhan, netdev, linux-kernel, tz2294
On 11/3/26 7:55 PM, Martin Karsten wrote:
> if (unlikely(napi_is_scheduled(n)))
> hrtimer_cancel(&n->timer);
This worked here too, but I think the cancel could hit a timer that a
new owner just armed (e.g., a busy poller that clears SCHED and arms
its own timer right before our check). I'll go with Eric's version.
Thanks!
Mani
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
2026-11-03 18:37 ` Eric Dumazet
2026-11-04 0:55 ` Martin Karsten
@ 2026-11-05 21:40 ` Mani Tofigh
1 sibling, 0 replies; 6+ messages in thread
From: Mani Tofigh @ 2026-11-05 21:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Andrew Lunn, Martin Karsten, Joe Damato, Dragos Tatulea,
Kuniyuki Iwashima, Stanislav Fomichev, Sridhar Samudrala,
Cosmin Ratiu, Tariq Toukan, Wang Zhan, netdev, linux-kernel,
tz2294
On 11/3/26 1:37 PM, Eric Dumazet wrote:
> hrtimer_try_to_cancel(&n->timer);
One case I'm not sure about. If napi_watchdog() is running when
hrtimer_start() is called, the timer is queued again and
hrtimer_try_to_cancel() returns -1 without dequeuing it. With
preemptible RCU two napi_suspend_irqs() callers could both end up there
after the delete. None of the callers run in hard irq or from
napi_watchdog() so would hrtimer_cancel() be ok there?
> Can you give this a try with your reproducer ?
>
Yes, it works.
I'll send v2 with your approach.
Thanks!
Mani
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del()
2026-11-03 18:19 [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del() Mani Tofigh via B4 Relay
2026-11-03 18:37 ` Eric Dumazet
@ 2026-11-06 9:22 ` netdev-bot+sashiko
1 sibling, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-11-06 9:22 UTC (permalink / raw)
To: mt4022
Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, mkarsten,
joe, dtatulea, kuniyu, sdf.kernel, sridhar.samudrala, cratiu,
tariqt, wang.zhan, netdev, linux-kernel, tz2294
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The fix only adds the post-grace-period hrtimer_cancel() to the
inline wrappers netif_napi_del() and netif_napi_del_locked()…
- [Medium] netdev_napi_exit() (net/core/dev.c:12453-12461) runs
hrtimer_cancel(&p->timer) before __netif_napi_del_locked() and before…
--
net: cancel napi->timer after the grace period in netif_napi_del()
This patch adds a second hrtimer_cancel() of napi->timer after the
synchronize_net() in netif_napi_del() and netif_napi_del_locked(). It
targets a timer re-armed by napi_complete_done(), busy_poll_stop() or
napi_suspend_irqs() after napi_disable() has cancelled it. It also
cancels the timer in netdev_napi_exit(), relying on unregister having
already waited for a grace period.
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 435be7880d7e5..ad3685caf8147 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3013,8 +3013,13 @@ static inline void __netif_napi_del(struct napi_struct *napi)
>
> static inline void netif_napi_del_locked(struct napi_struct *napi)
> {
> + bool listed = test_bit(NAPI_STATE_LISTED, &napi->state);
> +
> __netif_napi_del_locked(napi);
> synchronize_net();
> + /* napi->timer can be started until a grace period after napi_disable() */
> + if (listed)
> + hrtimer_cancel(&napi->timer);
> }
[Severity: High]
Does this close the bug for callers that call __netif_napi_del() or
__netif_napi_del_locked() directly and then run their own grace period?
The new cancel only exists in the two inline wrappers. Those direct
callers can still free or re-initialise the napi while napi->timer is
queued.
The netdevsim driver used by the reproducer is one of them, in
nsim_del_napi():
drivers/net/netdevsim/netdev.c:nsim_del_napi() {
...
napi_disable_locked(&rq->napi);
__netif_napi_del_locked(&rq->napi);
}
synchronize_net();
...
}
Nothing cancels the timer after that synchronize_net(). Later,
nsim_queue_free() only cancels rq->napi_timer before kfree(rq). On
reopen, netif_napi_add_config_locked()->hrtimer_setup() runs on a timer
that may still be queued.
The same pattern shows up in veth_napi_del_range() and in
virtnet_free_queues(), which is followed by kfree(vi->rq). It also shows
up in bnxt, enic, bnx2x, myri10ge, dpaa and bnge.
Two callers have no synchronous point after the grace period at all:
net/core/gro_cells.c:gro_cells_destroy() {
...
napi_disable(&cell->napi);
__netif_napi_del(&cell->napi);
__skb_queue_purge(&cell->napi_skbs);
}
...
call_rcu(&defer->rcu, percpu_free_defer_callback);
...
}
ixgbe_free_q_vector() is similar: __netif_napi_del() followed by
kfree_rcu(q_vector, rcu). The note below the --- line suggests sending
the same cancel separately, but there is nowhere to put it in these two
paths. Would they need a different approach?
All of these paths share the window described in the commit message.
napi_complete_done() clears SCHED in its try_cmpxchg() before it calls
hrtimer_start(). busy_poll_stop() calls clear_bit(NAPI_STATE_SCHED)
before hrtimer_start(). napi_suspend_irqs() re-arms a napi it found
only through napi_by_id() under RCU.
The kernel-doc for __netif_napi_del() also still says:
* Warning: caller must observe RCU grace period before freeing memory
* containing @napi. Drivers might want to call this helper to combine
* all the needed RCU grace periods into a single one.
That contradicts the new comment above, and the driver comments in
virtio_net, ixgbe and bnxt repeat it. Should the documented contract be
updated as well?
The remaining gap is only described below the --- line, so it won't be
in git history. That note also leaves out ixgbe, enic, bnx2x, myri10ge,
dpaa and bnge.
[ ... ]
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f32353b0467cd..6d6384522a1bb 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -12450,9 +12450,12 @@ static void netdev_napi_exit(struct net_device *dev)
> if (!list_empty(&dev->napi_list)) {
> struct napi_struct *p, *n;
>
> + /* unregister has waited for a grace period after napi_disable() */
> netdev_lock(dev);
> - list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
> + list_for_each_entry_safe(p, n, &dev->napi_list, dev_list) {
> + hrtimer_cancel(&p->timer);
> __netif_napi_del_locked(p);
> + }
> netdev_unlock(dev);
>
> synchronize_net();
[Severity: Medium]
Does the premise in this comment hold for every device that reaches
free_netdev()? Here the cancel runs before this function's own
synchronize_net(). That is the reverse of the order used in
netif_napi_del_locked().
unregister_netdevice_many_notify() calls synchronize_net() after close
and after ndo_uninit, so napis disabled in ndo_stop or ndo_uninit are
covered. However, free_netdev() also calls netdev_napi_exit() for
devices that were never unregistered. These include
NETREG_UNINITIALIZED probe-failure unwinds and NETREG_DUMMY devices
from alloc_netdev_dummy().
For example, mal_remove() in drivers/net/ethernet/ibm/emac/mal.c does:
napi_disable(&mal->napi);
...
free_netdev(mal->dummy_dev);
There is no grace period in between. A registered device has the same
gap if its driver calls napi_disable() in the remove path after
unregister_netdev() and leaves the napi listed for free_netdev() to
delete.
On those paths, an in-flight start can arm the timer after this early
cancel:
napi_complete_done() between its try_cmpxchg() and hrtimer_start()
busy_poll_stop() between clear_bit() and hrtimer_start()
a napi_suspend_irqs() reader
The synchronize_net() below then only waits for that start to finish,
and the napi is freed with its timer queued.
In practice, the timeouts default to 0 on unregistered and dummy
devices, and netlink napi-set requires NETREG_REGISTERED. The realistic
trigger is a registered device with gro_flush_timeout, defer_hard_irqs
or irq_suspend_timeout set. I couldn't find an in-tree driver that hits
this today.
Could netdev_napi_exit() do the grace period itself, for example a
synchronize_net() before the loop, instead of relying on the caller?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103-napi-timer-cancel-v1-1-55c631ad5286%40columbia.edu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-06 9:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 18:19 [PATCH net] net: cancel napi->timer after the grace period in netif_napi_del() Mani Tofigh via B4 Relay
2026-11-03 18:37 ` Eric Dumazet
2026-11-04 0:55 ` Martin Karsten
2026-11-05 21:43 ` Mani Tofigh
2026-11-05 21:40 ` Mani Tofigh
2026-11-06 9:22 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®