[PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[Mikhail Gavrilov]

Commit 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
made mt76_dma_cleanup() disable every RX NAPI instance before deleting
it.  mt7921e_unregister_device() and mt7925e_unregister_device() already
disable the very same instances and only afterwards call
mt792x_dma_cleanup() -> mt76_dma_cleanup(), so each instance is now
disabled twice.

napi_disable() is not idempotent: on return it leaves NAPIF_STATE_SCHED
and NAPIF_STATE_NPSVC set, so a second call without an intervening
napi_enable() spins in usleep_range() forever, waiting for bits that
nobody will clear:

  task:modprobe        state:D stack:25720 pid:7954  tgid:7954
  Call Trace:
   <TASK>
   __schedule+0x11b8/0x26d0
   schedule+0xe7/0x2f0
   schedule_hrtimeout_range_clock+0x218/0x330
   usleep_range_state+0x133/0x1b0
   napi_disable_locked+0x37d/0x5f0
   napi_disable+0x43/0x80
   mt76_dma_cleanup+0x2b4/0x860 [mt76]
   mt7921_pci_remove+0x17f/0x350 [mt7921e]
   pci_device_remove+0xb6/0x1e0
   device_release_driver_internal+0x38d/0x540
   driver_detach+0xd0/0x1b0
   bus_remove_driver+0x127/0x2d0
   pci_unregister_driver+0x2a/0x280
   __do_sys_delete_module+0x36a/0x5b0
   do_syscall_64+0x11c/0x6d0
   entry_SYSCALL_64_after_hwframe+0x76/0x7e
   </TASK>

mt7921_pci_shutdown() and mt7925_pci_shutdown() reuse the remove path,
so the same deadlock is hit on every reboot and poweroff.  It is silent:
the stuck task keeps sleeping and rescheduling, so neither the hung task
detector nor the lockup detectors fire, and the last line on the console
is "systemd-shutdown[1]: Rebooting."

Drop the driver-side loops and rely on mt76_dma_cleanup() instead.
mt792x_dma_cleanup() stops and resets the WFDMA engine before calling
it, so RX is already quiesced when the NAPI instances are disabled
there.

Fixes: 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
Reported-by: Bert Karwatzki <spasswolf@web.de>
Closes: https://lore.kernel.org/all/20260724151419.26014-1-spasswolf@web.de/
Suggested-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---

I hit the same hang on an ASRock B650I Lightning WiFi with MT7922
(mt7921e) and bisected it to the same commit.  mt7921e has the same
duplicated napi_disable() as mt7925e, so this covers both; it is
Nicolas' suggestion from [1] extended to mt7921e and with the now
unused 'int i' removed.
 
Tested on 7.2.0-rc5 with KASAN and lockdep enabled, MT7922 / mt7921e.
Before the patch 'modprobe -r mt7921e' hangs (backtrace above, taken
with sysrq-w) and the machine never gets past "Rebooting.".  With the
patch 'modprobe -r mt7921e' and 'modprobe mt7921e' both complete and
the interface comes back up, reboot and poweroff work again, and no
__netif_napi_del_locked() or page_pool_disable_direct_recycling()
warnings are logged.
 
[1] https://lore.kernel.org/all/59c03ab1-ec4a-411b-9b7e-8183ecf3e02b@green-communications.fr/

 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 3 ---
 drivers/net/wireless/mediatek/mt76/mt7925/pci.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index 7728c5ae6791..65d09d13304f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -43,7 +43,6 @@ static int mt7921e_init_reset(struct mt792x_dev *dev)
 
 static void mt7921e_unregister_device(struct mt792x_dev *dev)
 {
-	int i;
 	struct mt76_connac_pm *pm = &dev->pm;
 	struct ieee80211_hw *hw = mt76_hw(dev);
 
@@ -52,8 +51,6 @@ static void mt7921e_unregister_device(struct mt792x_dev *dev)
 
 	cancel_work_sync(&dev->init_work);
 	mt76_unregister_device(&dev->mt76);
-	mt76_for_each_q_rx(&dev->mt76, i)
-		napi_disable(&dev->mt76.napi[i]);
 	cancel_delayed_work_sync(&pm->ps_work);
 	cancel_work_sync(&pm->wake_work);
 	cancel_work_sync(&dev->reset_work);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
index ea64303283ed..eb1bd60e3d9f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
@@ -36,7 +36,6 @@ static int mt7925e_init_reset(struct mt792x_dev *dev)
 
 static void mt7925e_unregister_device(struct mt792x_dev *dev)
 {
-	int i;
 	struct mt76_connac_pm *pm = &dev->pm;
 	struct ieee80211_hw *hw = mt76_hw(dev);
 
@@ -45,8 +44,6 @@ static void mt7925e_unregister_device(struct mt792x_dev *dev)
 
 	cancel_work_sync(&dev->init_work);
 	mt76_unregister_device(&dev->mt76);
-	mt76_for_each_q_rx(&dev->mt76, i)
-		napi_disable(&dev->mt76.napi[i]);
 	cancel_delayed_work_sync(&pm->ps_work);
 	cancel_work_sync(&pm->wake_work);
 	cancel_work_sync(&dev->reset_work);
-- 
2.55.0

Re: [PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[Bert Karwatzki]

Am Dienstag, dem 28.07.2026 um 05:20 +0500 schrieb Mikhail Gavrilov:
> Commit 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
> made mt76_dma_cleanup() disable every RX NAPI instance before deleting
> it.  mt7921e_unregister_device() and mt7925e_unregister_device() already
> disable the very same instances and only afterwards call
> mt792x_dma_cleanup() -> mt76_dma_cleanup(), so each instance is now
> disabled twice.
> 
> napi_disable() is not idempotent: on return it leaves NAPIF_STATE_SCHED
> and NAPIF_STATE_NPSVC set, so a second call without an intervening
> napi_enable() spins in usleep_range() forever, waiting for bits that
> nobody will clear:
> 
>   task:modprobe        state:D stack:25720 pid:7954  tgid:7954
>   Call Trace:
>    <TASK>
>    __schedule+0x11b8/0x26d0
>    schedule+0xe7/0x2f0
>    schedule_hrtimeout_range_clock+0x218/0x330
>    usleep_range_state+0x133/0x1b0
>    napi_disable_locked+0x37d/0x5f0
>    napi_disable+0x43/0x80
>    mt76_dma_cleanup+0x2b4/0x860 [mt76]
>    mt7921_pci_remove+0x17f/0x350 [mt7921e]
>    pci_device_remove+0xb6/0x1e0
>    device_release_driver_internal+0x38d/0x540
>    driver_detach+0xd0/0x1b0
>    bus_remove_driver+0x127/0x2d0
>    pci_unregister_driver+0x2a/0x280
>    __do_sys_delete_module+0x36a/0x5b0
>    do_syscall_64+0x11c/0x6d0
>    entry_SYSCALL_64_after_hwframe+0x76/0x7e
>    </TASK>
> 
> mt7921_pci_shutdown() and mt7925_pci_shutdown() reuse the remove path,
> so the same deadlock is hit on every reboot and poweroff.  It is silent:
> the stuck task keeps sleeping and rescheduling, so neither the hung task
> detector nor the lockup detectors fire, and the last line on the console
> is "systemd-shutdown[1]: Rebooting."
> 
> Drop the driver-side loops and rely on mt76_dma_cleanup() instead.
> mt792x_dma_cleanup() stops and resets the WFDMA engine before calling
> it, so RX is already quiesced when the NAPI instances are disabled
> there.
> 
> Fixes: 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
> Reported-by: Bert Karwatzki <spasswolf@web.de>
> Closes: https://lore.kernel.org/all/20260724151419.26014-1-spasswolf@web.de/
> Suggested-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
> 
> I hit the same hang on an ASRock B650I Lightning WiFi with MT7922
> (mt7921e) and bisected it to the same commit.  mt7921e has the same
> duplicated napi_disable() as mt7925e, so this covers both; it is
> Nicolas' suggestion from [1] extended to mt7921e and with the now
> unused 'int i' removed.
>  
> Tested on 7.2.0-rc5 with KASAN and lockdep enabled, MT7922 / mt7921e.
> Before the patch 'modprobe -r mt7921e' hangs (backtrace above, taken
> with sysrq-w) and the machine never gets past "Rebooting.".  With the
> patch 'modprobe -r mt7921e' and 'modprobe mt7921e' both complete and
> the interface comes back up, reboot and poweroff work again, and no
> __netif_napi_del_locked() or page_pool_disable_direct_recycling()
> warnings are logged.
>  
> [1] https://lore.kernel.org/all/59c03ab1-ec4a-411b-9b7e-8183ecf3e02b@green-communications.fr/
> 
>  drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 3 ---
>  drivers/net/wireless/mediatek/mt76/mt7925/pci.c | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> index 7728c5ae6791..65d09d13304f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> @@ -43,7 +43,6 @@ static int mt7921e_init_reset(struct mt792x_dev *dev)
>  
>  static void mt7921e_unregister_device(struct mt792x_dev *dev)
>  {
> -	int i;
>  	struct mt76_connac_pm *pm = &dev->pm;
>  	struct ieee80211_hw *hw = mt76_hw(dev);
>  
> @@ -52,8 +51,6 @@ static void mt7921e_unregister_device(struct mt792x_dev *dev)
>  
>  	cancel_work_sync(&dev->init_work);
>  	mt76_unregister_device(&dev->mt76);
> -	mt76_for_each_q_rx(&dev->mt76, i)
> -		napi_disable(&dev->mt76.napi[i]);
>  	cancel_delayed_work_sync(&pm->ps_work);
>  	cancel_work_sync(&pm->wake_work);
>  	cancel_work_sync(&dev->reset_work);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> index ea64303283ed..eb1bd60e3d9f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> @@ -36,7 +36,6 @@ static int mt7925e_init_reset(struct mt792x_dev *dev)
>  
>  static void mt7925e_unregister_device(struct mt792x_dev *dev)
>  {
> -	int i;
>  	struct mt76_connac_pm *pm = &dev->pm;
>  	struct ieee80211_hw *hw = mt76_hw(dev);
>  
> @@ -45,8 +44,6 @@ static void mt7925e_unregister_device(struct mt792x_dev *dev)
>  
>  	cancel_work_sync(&dev->init_work);
>  	mt76_unregister_device(&dev->mt76);
> -	mt76_for_each_q_rx(&dev->mt76, i)
> -		napi_disable(&dev->mt76.napi[i]);
>  	cancel_delayed_work_sync(&pm->ps_work);
>  	cancel_work_sync(&pm->wake_work);
>  	cancel_work_sync(&dev->reset_work);

I tested this patch applied to next-20260727 with mt7925 and
rebooting works fine again.

Tested-By: Bert Karwatzki <spasswolf@web.de>

Bert Karwatzki

Re: [PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[Mikhail Gavrilov]

On Tue, Jul 28, 2026 at 5:49 PM Bert Karwatzki <spasswolf@web.de> wrote:
>
> I tested this patch applied to next-20260727 with mt7925 and
> rebooting works fine again.
>
> Tested-By: Bert Karwatzki <spasswolf@web.de>


Thanks for testing!

Repeating the trailer in canonical spelling so that tooling picks it up:

Tested-by: Bert Karwatzki <spasswolf@web.de>

-- 
Thanks,
Mikhail

Re: [PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[Eric Biggers]

On Tue, Jul 28, 2026 at 05:20:48AM +0500, Mikhail Gavrilov wrote:
> Commit 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
> made mt76_dma_cleanup() disable every RX NAPI instance before deleting
> it.  mt7921e_unregister_device() and mt7925e_unregister_device() already
> disable the very same instances and only afterwards call
> mt792x_dma_cleanup() -> mt76_dma_cleanup(), so each instance is now
> disabled twice.
> 
> napi_disable() is not idempotent: on return it leaves NAPIF_STATE_SCHED
> and NAPIF_STATE_NPSVC set, so a second call without an intervening
> napi_enable() spins in usleep_range() forever, waiting for bits that
> nobody will clear:
> 
>   task:modprobe        state:D stack:25720 pid:7954  tgid:7954
>   Call Trace:
>    <TASK>
>    __schedule+0x11b8/0x26d0
>    schedule+0xe7/0x2f0
>    schedule_hrtimeout_range_clock+0x218/0x330
>    usleep_range_state+0x133/0x1b0
>    napi_disable_locked+0x37d/0x5f0
>    napi_disable+0x43/0x80
>    mt76_dma_cleanup+0x2b4/0x860 [mt76]
>    mt7921_pci_remove+0x17f/0x350 [mt7921e]
>    pci_device_remove+0xb6/0x1e0
>    device_release_driver_internal+0x38d/0x540
>    driver_detach+0xd0/0x1b0
>    bus_remove_driver+0x127/0x2d0
>    pci_unregister_driver+0x2a/0x280
>    __do_sys_delete_module+0x36a/0x5b0
>    do_syscall_64+0x11c/0x6d0
>    entry_SYSCALL_64_after_hwframe+0x76/0x7e
>    </TASK>
> 
> mt7921_pci_shutdown() and mt7925_pci_shutdown() reuse the remove path,
> so the same deadlock is hit on every reboot and poweroff.  It is silent:
> the stuck task keeps sleeping and rescheduling, so neither the hung task
> detector nor the lockup detectors fire, and the last line on the console
> is "systemd-shutdown[1]: Rebooting."
> 
> Drop the driver-side loops and rely on mt76_dma_cleanup() instead.
> mt792x_dma_cleanup() stops and resets the WFDMA engine before calling
> it, so RX is already quiesced when the NAPI instances are disabled
> there.
> 
> Fixes: 13b7e6a96a00 ("wifi: mt76: Disable napi when removing device")
> Reported-by: Bert Karwatzki <spasswolf@web.de>
> Closes: https://lore.kernel.org/all/20260724151419.26014-1-spasswolf@web.de/
> Suggested-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

While this patch fixes the shutdown hang for me too on a system using
mt7925e, Sashiko found that this patch introduces a use-after-free
because NAPI is now being disabled too late:
https://sashiko.dev/#/patchset/20260728002048.19351-1-mikhail.v.gavrilov%40gmail.com
Should 13b7e6a96a00 be reverted, then fixed in another way such as
calling napi_disable() in mt7915_unregister_device()?

- Eric

Re: [PATCH mt76] wifi: mt76: mt792x: drop redundant napi_disable() in unregister path

[Mikhail Gavrilov]

On Thu, Jul 30, 2026 at 10:06 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> While this patch fixes the shutdown hang for me too on a system using
> mt7925e, Sashiko found that this patch introduces a use-after-free
> because NAPI is now being disabled too late:
> https://sashiko.dev/#/patchset/20260728002048.19351-1-mikhail.v.gavrilov%40gmail.com
> Should 13b7e6a96a00 be reverted, then fixed in another way such as
> calling napi_disable() in mt7915_unregister_device()?
>

You are right, and so is the report.  I went through the path and it is real:

  mt792x_poll_rx()
    mt7921_queue_rx_skb()
      case PKT_TYPE_TXRX_NOTIFY:          /* mmio only, so PCIe */
        mt7921_mac_tx_free()
          mt76_token_release()            /* idr_remove() under token_lock */

and with my patch that can now run concurrently with, a few lines up in
mt7921e_unregister_device():

  mt76_connac2_tx_token_put()
    ...
    idr_destroy(&dev->token)              /* outside token_lock */

The early napi_disable() loops I removed were what kept those two apart.

My justification in the commit message was that RX is harmless once
mac80211 has stopped the hw, because mt76_rx() drops everything with
MT76_STATE_RUNNING clear.  That only covers the normal data path -
PKT_TYPE_TXRX_NOTIFY is dispatched in mt7921_queue_rx_skb() before
mt76_rx() is ever reached, and goes straight to the token IDR.  So the
window is not benign and the patch is wrong.  Please drop it.

I agree with your suggestion.  The disable belongs in the drivers that
delete the NAPI instances, where each one can pick a point that is safe
for its own teardown order, rather than in the shared mt76_dma_cleanup()
where it is forced to happen after the driver has already freed state
that the RX poll can still touch.  mt7921e/mt7925e already did it, and
early enough; mt7915 and friends simply never did.

Unless Nicolas would rather do it himself, I can send a v2 series:

  1) Revert "wifi: mt76: Disable napi when removing device", restoring
     reboot, poweroff and module unload on mt7921e/mt7925e
  2) wifi: mt76: mt7915: disable napi before deleting it, for the
     original __netif_napi_del_locked() and
     page_pool_disable_direct_recycling() warnings

I would keep 2) to mt7915 for now, since that is what was reported and
tested.  mt7603, mt7615, mt7996 and mt76x0/mt76x2 have the same gap in
their unregister paths and can follow separately.

One more item from the same report, not caused by this patch but worth
someone's attention: mt7921e/mt7925e call tasklet_disable() on
mt76.irq_tasklet without a matching tasklet_kill() before
mt76_free_device(), which can leave a scheduled tasklet pointing at
freed memory.

Thanks for catching this.

-- 
Thanks,
Mikhail