Trying to disable the napi threaded state while the napi instance is disabled will hang forever in napi_stop_kthread(), as it waits for NAPIF_STATE_SCHED_THREADED to clear. This won't happen until the next ____napi_schedule() call, which won't happen as it's disabled. This happens in 2 instances : - Drivers that create but don't use napi instances (stmmac's rxtx napi for AF_XDP for example), here it's a driver bug - Drivers that create their napi instances in .probe(). Here, setting threaded off while the interface is down will hang : ip link set eth0 down echo 1 > /sys/class/net/eth0/threaded echo 0 > /sys/class/net/eth0/threaded -> hang Checking on netif_running() isn't enough, as napi instances may be transiently disabled when changing the MTU for example, let's use a dedicated flag to track if the instance is disabled. This allows immediately disabling the kthread when the instance is disabled. This was tested on mvpp2 (probe-time napi_add) and stmmac. Fixes: 689883de94dd ("net: stop napi kthreads when THREADED napi is disabled") Tested-by: Daniel Machon Signed-off-by: Maxime Chevallier --- V4: - Rebase on net, add Daniel's tested-by V3: https://lore.kernel.org/r/20261022075757.1164283-1-maxime.chevallier@bootlin.com - Use a new flag to keep track of the napi instance state, if it's disabled or not. It was tested on the same set of boards as before. I guess if this approach doesn't work either, we'll have to start fixing all the drivers :) V2: https://lore.kernel.org/r/20261006141119.85562-1-maxime.chevallier@bootlin.com - Different approach as per Jakub's review, just don't allow setting the napi threaded mode while interface is down. Uses the NAPI_STATE_NPSVC flag to check the napi state, but it wasn't enough. V1: https://lore.kernel.org/r/20260926194714.648819-1-maxime.chevallier@bootlin.com include/linux/netdevice.h | 2 ++ net/core/dev.c | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index c0ae442c4344..435be7880d7e 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -440,6 +440,7 @@ enum { NAPI_STATE_HAS_NOTIFIER, /* Napi has an IRQ notifier */ NAPI_STATE_THREADED_BUSY_POLL, /* The threaded NAPI poller will busy poll */ NAPI_STATE_PERCPU, /* NAPI handler is run per-CPU - incompatible with threaded mode */ + NAPI_STATE_DISABLED, /* NAPI is disabled */ }; enum { @@ -456,6 +457,7 @@ enum { NAPIF_STATE_HAS_NOTIFIER = BIT(NAPI_STATE_HAS_NOTIFIER), NAPIF_STATE_THREADED_BUSY_POLL = BIT(NAPI_STATE_THREADED_BUSY_POLL), NAPIF_STATE_PERCPU = BIT(NAPI_STATE_PERCPU), + NAPIF_STATE_DISABLED = BIT(NAPI_STATE_DISABLED), }; enum gro_result { diff --git a/net/core/dev.c b/net/core/dev.c index 87a79d6004c8..fa767d417d09 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -7381,6 +7381,13 @@ static void napi_stop_kthread(struct napi_struct *napi) { unsigned long val, new; + /* Instance is disabled, we can immediately stop the kthread */ + if (test_bit(NAPI_STATE_DISABLED, &napi->state)) { + clear_bit(NAPI_STATE_THREADED, &napi->state); + clear_bit(NAPI_STATE_THREADED_BUSY_POLL, &napi->state); + goto stop; + } + /* Wait until the napi STATE_THREADED is unset. */ while (true) { val = READ_ONCE(napi->state); @@ -7411,6 +7418,7 @@ static void napi_stop_kthread(struct napi_struct *napi) msleep(20); } +stop: kthread_stop(napi->thread); napi->thread = NULL; } @@ -7806,6 +7814,7 @@ void netif_napi_add_weight_locked(struct net_device *dev, napi->list_owner = -1; set_bit(NAPI_STATE_SCHED, &napi->state); set_bit(NAPI_STATE_NPSVC, &napi->state); + set_bit(NAPI_STATE_DISABLED, &napi->state); netif_napi_dev_list_add(dev, napi); /* default settings from sysfs are applied to all NAPIs. any per-NAPI @@ -7842,7 +7851,8 @@ void napi_disable_locked(struct napi_struct *n) val = READ_ONCE(n->state); } - new = val | NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC; + new = val | NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC | + NAPIF_STATE_DISABLED; new &= ~(NAPIF_STATE_THREADED | NAPIF_STATE_THREADED_BUSY_POLL | NAPIF_STATE_PREFER_BUSY_POLL); @@ -7887,7 +7897,8 @@ void napi_enable_locked(struct napi_struct *n) do { BUG_ON(!test_bit(NAPI_STATE_SCHED, &val)); - new = val & ~(NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC); + new = val & ~(NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC | + NAPIF_STATE_DISABLED); if (n->dev->threaded && n->thread) new |= NAPIF_STATE_THREADED; } while (!try_cmpxchg(&n->state, &val, new)); -- 2.56.0