* [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
@ 2025-09-10 20:37 Samiullah Khawaja
2025-09-11 13:40 ` Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Samiullah Khawaja @ 2025-09-10 20:37 UTC (permalink / raw)
To: Jakub Kicinski, David S . Miller , Eric Dumazet, Paolo Abeni
Cc: willemb, netdev, skhawaja
napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
before stopping the kthread. But it uses test_bit with the
NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
the flag is unset.
Use the NAPI_* variant of the NAPI state bits in test_bit instead.
Tested:
./tools/testing/selftests/net/nl_netdev.py
TAP version 13
1..7
ok 1 nl_netdev.empty_check
ok 2 nl_netdev.lo_check
ok 3 nl_netdev.page_pool_check
ok 4 nl_netdev.napi_list_check
ok 5 nl_netdev.dev_set_threaded
ok 6 nl_netdev.napi_set_threaded
ok 7 nl_netdev.nsim_rxq_reset_down
# Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
./tools/testing/selftests/drivers/net/napi_threaded.py
TAP version 13
1..2
ok 1 napi_threaded.change_num_queues
ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
# Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 93a25d87b86b..8d49b2198d07 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6965,7 +6965,7 @@ static void napi_stop_kthread(struct napi_struct *napi)
* the kthread.
*/
while (true) {
- if (!test_bit(NAPIF_STATE_SCHED_THREADED, &napi->state))
+ if (!test_bit(NAPI_STATE_SCHED_THREADED, &napi->state))
break;
msleep(20);
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-10 20:37 [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread Samiullah Khawaja
@ 2025-09-11 13:40 ` Jakub Kicinski
2025-09-11 14:10 ` Samiullah Khawaja
2025-09-12 1:40 ` patchwork-bot+netdevbpf
2025-09-12 19:26 ` Simon Horman
2 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2025-09-11 13:40 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: David S . Miller , Eric Dumazet, Paolo Abeni, willemb, netdev,
mkarsten
On Wed, 10 Sep 2025 20:37:16 +0000 Samiullah Khawaja wrote:
> napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> before stopping the kthread. But it uses test_bit with the
> NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> the flag is unset.
>
> Use the NAPI_* variant of the NAPI state bits in test_bit instead.
>
> Tested:
> ./tools/testing/selftests/net/nl_netdev.py
> TAP version 13
> 1..7
> ok 1 nl_netdev.empty_check
> ok 2 nl_netdev.lo_check
> ok 3 nl_netdev.page_pool_check
> ok 4 nl_netdev.napi_list_check
> ok 5 nl_netdev.dev_set_threaded
> ok 6 nl_netdev.napi_set_threaded
> ok 7 nl_netdev.nsim_rxq_reset_down
> # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> ./tools/testing/selftests/drivers/net/napi_threaded.py
> TAP version 13
> 1..2
> ok 1 napi_threaded.change_num_queues
> ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
Is this basically addressing the bug that Martin run into?
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 93a25d87b86b..8d49b2198d07 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6965,7 +6965,7 @@ static void napi_stop_kthread(struct napi_struct *napi)
> * the kthread.
> */
> while (true) {
> - if (!test_bit(NAPIF_STATE_SCHED_THREADED, &napi->state))
> + if (!test_bit(NAPI_STATE_SCHED_THREADED, &napi->state))
> break;
>
> msleep(20);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-11 13:40 ` Jakub Kicinski
@ 2025-09-11 14:10 ` Samiullah Khawaja
2025-09-11 14:31 ` Samiullah Khawaja
0 siblings, 1 reply; 8+ messages in thread
From: Samiullah Khawaja @ 2025-09-11 14:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Eric Dumazet, Paolo Abeni, willemb, netdev,
mkarsten
On Thu, Sep 11, 2025 at 6:40 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 10 Sep 2025 20:37:16 +0000 Samiullah Khawaja wrote:
> > napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> > before stopping the kthread. But it uses test_bit with the
> > NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> > the flag is unset.
> >
> > Use the NAPI_* variant of the NAPI state bits in test_bit instead.
> >
> > Tested:
> > ./tools/testing/selftests/net/nl_netdev.py
> > TAP version 13
> > 1..7
> > ok 1 nl_netdev.empty_check
> > ok 2 nl_netdev.lo_check
> > ok 3 nl_netdev.page_pool_check
> > ok 4 nl_netdev.napi_list_check
> > ok 5 nl_netdev.dev_set_threaded
> > ok 6 nl_netdev.napi_set_threaded
> > ok 7 nl_netdev.nsim_rxq_reset_down
> > # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > ./tools/testing/selftests/drivers/net/napi_threaded.py
> > TAP version 13
> > 1..2
> > ok 1 napi_threaded.change_num_queues
> > ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> > # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>
> Is this basically addressing the bug that Martin run into?
Not really. That one was because the busy polling bit remained set
during kthread stop. Basically in this function when we unset
STATE_THREADED, we also needed to unset STATE_THREADED_BUSY_POLL.
@@ -7000,7 +7002,8 @@ static void napi_stop_kthread(struct napi_struct *napi)
*/
if ((val & NAPIF_STATE_SCHED_THREADED) ||
!(val & NAPIF_STATE_SCHED)) {
- new = val & (~NAPIF_STATE_THREADED);
+ new = val & (~(NAPIF_STATE_THREADED |
+ NAPIF_STATE_THREADED_BUSY_POLL));
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 93a25d87b86b..8d49b2198d07 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6965,7 +6965,7 @@ static void napi_stop_kthread(struct napi_struct *napi)
> > * the kthread.
> > */
> > while (true) {
> > - if (!test_bit(NAPIF_STATE_SCHED_THREADED, &napi->state))
> > + if (!test_bit(NAPI_STATE_SCHED_THREADED, &napi->state))
> > break;
> >
> > msleep(20);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-11 14:10 ` Samiullah Khawaja
@ 2025-09-11 14:31 ` Samiullah Khawaja
2025-09-11 14:38 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Samiullah Khawaja @ 2025-09-11 14:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Eric Dumazet, Paolo Abeni, willemb, netdev,
mkarsten
On Thu, Sep 11, 2025 at 7:10 AM Samiullah Khawaja <skhawaja@google.com> wrote:
>
> On Thu, Sep 11, 2025 at 6:40 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 10 Sep 2025 20:37:16 +0000 Samiullah Khawaja wrote:
> > > napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> > > before stopping the kthread. But it uses test_bit with the
> > > NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> > > the flag is unset.
> > >
> > > Use the NAPI_* variant of the NAPI state bits in test_bit instead.
> > >
> > > Tested:
> > > ./tools/testing/selftests/net/nl_netdev.py
> > > TAP version 13
> > > 1..7
> > > ok 1 nl_netdev.empty_check
> > > ok 2 nl_netdev.lo_check
> > > ok 3 nl_netdev.page_pool_check
> > > ok 4 nl_netdev.napi_list_check
> > > ok 5 nl_netdev.dev_set_threaded
> > > ok 6 nl_netdev.napi_set_threaded
> > > ok 7 nl_netdev.nsim_rxq_reset_down
> > > # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> > >
> > > ./tools/testing/selftests/drivers/net/napi_threaded.py
> > > TAP version 13
> > > 1..2
> > > ok 1 napi_threaded.change_num_queues
> > > ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> > > # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
> > >
> > > Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> > > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> >
> > Is this basically addressing the bug that Martin run into?
> Not really. That one was because the busy polling bit remained set
> during kthread stop. Basically in this function when we unset
> STATE_THREADED, we also needed to unset STATE_THREADED_BUSY_POLL.
>
> @@ -7000,7 +7002,8 @@ static void napi_stop_kthread(struct napi_struct *napi)
> */
> if ((val & NAPIF_STATE_SCHED_THREADED) ||
> !(val & NAPIF_STATE_SCHED)) {
> - new = val & (~NAPIF_STATE_THREADED);
> + new = val & (~(NAPIF_STATE_THREADED |
> + NAPIF_STATE_THREADED_BUSY_POLL));
>
Just to add to my last email: I did find this test_bit issue while
working on the fix for that other problem.
> >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 93a25d87b86b..8d49b2198d07 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -6965,7 +6965,7 @@ static void napi_stop_kthread(struct napi_struct *napi)
> > > * the kthread.
> > > */
> > > while (true) {
> > > - if (!test_bit(NAPIF_STATE_SCHED_THREADED, &napi->state))
> > > + if (!test_bit(NAPI_STATE_SCHED_THREADED, &napi->state))
> > > break;
> > >
> > > msleep(20);
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-11 14:31 ` Samiullah Khawaja
@ 2025-09-11 14:38 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2025-09-11 14:38 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: David S . Miller, Eric Dumazet, Paolo Abeni, willemb, netdev,
mkarsten
On Thu, 11 Sep 2025 07:31:11 -0700 Samiullah Khawaja wrote:
> > > Is this basically addressing the bug that Martin run into?
> > Not really. That one was because the busy polling bit remained set
> > during kthread stop. Basically in this function when we unset
> > STATE_THREADED, we also needed to unset STATE_THREADED_BUSY_POLL.
> >
> > @@ -7000,7 +7002,8 @@ static void napi_stop_kthread(struct napi_struct *napi)
> > */
> > if ((val & NAPIF_STATE_SCHED_THREADED) ||
> > !(val & NAPIF_STATE_SCHED)) {
> > - new = val & (~NAPIF_STATE_THREADED);
> > + new = val & (~(NAPIF_STATE_THREADED |
> > + NAPIF_STATE_THREADED_BUSY_POLL));
> >
> Just to add to my last email: I did find this test_bit issue while
> working on the fix for that other problem.
I see, makes sense. I was curious whether your previous posting would
work with just this fix. I guess we'll wait until next week 'cause
this fix didn't make it to today's PR in time :(
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-10 20:37 [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread Samiullah Khawaja
2025-09-11 13:40 ` Jakub Kicinski
@ 2025-09-12 1:40 ` patchwork-bot+netdevbpf
2025-09-12 19:26 ` Simon Horman
2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-09-12 1:40 UTC (permalink / raw)
To: Samiullah Khawaja; +Cc: kuba, davem, edumazet, pabeni, willemb, netdev
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 10 Sep 2025 20:37:16 +0000 you wrote:
> napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> before stopping the kthread. But it uses test_bit with the
> NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> the flag is unset.
>
> Use the NAPI_* variant of the NAPI state bits in test_bit instead.
>
> [...]
Here is the summary with links:
- [net] net: Use NAPI_* in test_bit when stopping napi kthread
https://git.kernel.org/netdev/net/c/247981eecd3d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-10 20:37 [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread Samiullah Khawaja
2025-09-11 13:40 ` Jakub Kicinski
2025-09-12 1:40 ` patchwork-bot+netdevbpf
@ 2025-09-12 19:26 ` Simon Horman
2025-09-12 19:27 ` Simon Horman
2 siblings, 1 reply; 8+ messages in thread
From: Simon Horman @ 2025-09-12 19:26 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Jakub Kicinski, David S . Miller , Eric Dumazet, Paolo Abeni,
willemb, netdev
On Wed, Sep 10, 2025 at 08:37:16PM +0000, Samiullah Khawaja wrote:
> napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> before stopping the kthread. But it uses test_bit with the
> NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> the flag is unset.
>
> Use the NAPI_* variant of the NAPI state bits in test_bit instead.
I think it would be useful to mention the difference between
NAPI_STATE_SCHED_THREADED and NAPIF_STATE_SCHED_THREADED.
For me, that would be that one is a bit number, while
the other is a mask with only the corresponding bit set.
>
> Tested:
> ./tools/testing/selftests/net/nl_netdev.py
> TAP version 13
> 1..7
> ok 1 nl_netdev.empty_check
> ok 2 nl_netdev.lo_check
> ok 3 nl_netdev.page_pool_check
> ok 4 nl_netdev.napi_list_check
> ok 5 nl_netdev.dev_set_threaded
> ok 6 nl_netdev.napi_set_threaded
> ok 7 nl_netdev.nsim_rxq_reset_down
> # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> ./tools/testing/selftests/drivers/net/napi_threaded.py
> TAP version 13
> 1..2
> ok 1 napi_threaded.change_num_queues
> ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
With the above addressed, feel free to add:
Reviewed-by: Simon Horman <horms@kernel.org>
...
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread
2025-09-12 19:26 ` Simon Horman
@ 2025-09-12 19:27 ` Simon Horman
0 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2025-09-12 19:27 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Jakub Kicinski, David S . Miller , Eric Dumazet, Paolo Abeni,
willemb, netdev
On Fri, Sep 12, 2025 at 08:26:03PM +0100, Simon Horman wrote:
> On Wed, Sep 10, 2025 at 08:37:16PM +0000, Samiullah Khawaja wrote:
> > napi_stop_kthread waits for the NAPI_STATE_SCHED_THREADED to be unset
> > before stopping the kthread. But it uses test_bit with the
> > NAPIF_STATE_SCHED_THREADED and that might stop the kthread early before
> > the flag is unset.
> >
> > Use the NAPI_* variant of the NAPI state bits in test_bit instead.
>
> I think it would be useful to mention the difference between
> NAPI_STATE_SCHED_THREADED and NAPIF_STATE_SCHED_THREADED.
>
> For me, that would be that one is a bit number, while
> the other is a mask with only the corresponding bit set.
>
> >
> > Tested:
> > ./tools/testing/selftests/net/nl_netdev.py
> > TAP version 13
> > 1..7
> > ok 1 nl_netdev.empty_check
> > ok 2 nl_netdev.lo_check
> > ok 3 nl_netdev.page_pool_check
> > ok 4 nl_netdev.napi_list_check
> > ok 5 nl_netdev.dev_set_threaded
> > ok 6 nl_netdev.napi_set_threaded
> > ok 7 nl_netdev.nsim_rxq_reset_down
> > # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > ./tools/testing/selftests/drivers/net/napi_threaded.py
> > TAP version 13
> > 1..2
> > ok 1 napi_threaded.change_num_queues
> > ok 2 napi_threaded.enable_dev_threaded_disable_napi_threaded
> > # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled")
> > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>
> With the above addressed, feel free to add:
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> ...
I now see that this patch is present in net as
commit 247981eecd3d ("net: Use NAPI_* in test_bit when stopping napi kthread")
So I think we can safely ignore the comments in my previous email.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-12 19:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-10 20:37 [PATCH net] net: Use NAPI_* in test_bit when stopping napi kthread Samiullah Khawaja
2025-09-11 13:40 ` Jakub Kicinski
2025-09-11 14:10 ` Samiullah Khawaja
2025-09-11 14:31 ` Samiullah Khawaja
2025-09-11 14:38 ` Jakub Kicinski
2025-09-12 1:40 ` patchwork-bot+netdevbpf
2025-09-12 19:26 ` Simon Horman
2025-09-12 19:27 ` Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).