wifi: mt76: disable NAPI before deleting in dma_cleanup

RFC Superseded [email protected] 3 participants 6 messages Sep 16 – Sep 20

Subsystems (2)

Arm MediaTek MediaTek MT76 wireless LAN driver

lore

Share & tools

lore

[email protected]

author

2026-09-16 20:48 UTC #0 lore raw

RFC 1/2 wifi: mt76: disable NAPI before deleting in dma_cleanup

From: Ben Greear <[email protected]>

NAPI should be disabled before deleted, this stops a warning seen when trying to rmmod mt7996e.

Signed-off-by Ben Greear <[email protected]>

---

NOTE: Seems to be regression between 6.18 and 7.2, haven't looked closer yet.

 drivers/net/wireless/mediatek/mt76/dma.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/wireless/mediatek/mt76/dma.c +1 -0

index a67880a9281c..6de3f28219bc 100644
@@ -1217,6 +1217,7 @@ void mt76_dma_cleanup(struct mt76_dev *dev)
 	mt76_for_each_q_rx(dev, i) {
 		struct mt76_queue *q = &dev->q_rx[i];
 
+		napi_disable(&dev->napi[i]);
 		netif_napi_del(&dev->napi[i]);
 		mt76_dma_rx_cleanup(dev, q);

Signature · 1 line

2.42.0

[email protected]

author

2026-09-16 20:48 UTC #1 lore raw

RFC 2/2 wifi: mt76: mt7996: Clean up IRQ on module unload.

From: Ben Greear <[email protected]>

When removing mt7996e module, softirqd would go to 100% cpu usage. It appears the problem is lack of cleanup of the irq objects and proper kill of the irq_tasklet.

Clean these up, similar to how the error cleanup path works in mt7996_pci_probe.

Signed-off-by Ben Greear <[email protected]>

---

NOTE: Tested in 7.2 + modifications kernel, but applies and compiles against 7.3 linus tree. Not tested there at this point.

 drivers/net/wireless/mediatek/mt76/mt7996/init.c | 16 +++++++++++++++-
 .../net/wireless/mediatek/mt76/mt7996/mt7996.h   |  1 +
 drivers/net/wireless/mediatek/mt76/mt7996/pci.c  | 11 +++++++++--
 3 files changed, 25 insertions(+), 3 deletions(-)

drivers/net/wireless/mediatek/mt76/mt7996/init.c +15 -1

index fb635a092584..45fc76cecd04 100644
@@ -1807,6 +1807,18 @@ int mt7996_register_device(struct mt7996_dev *dev)
 	return ret;
 }
 
+static void mt7996_cleanup_irqs(struct mt7996_dev *dev)
+{
+	struct mt76_dev *mdev = &dev->mt76;
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+
+	mt7996_put_hif2(dev, dev->hif2); /* frees hif2 irq */
+
+	devm_free_irq(mdev->dev, pdev->irq, dev);
+	if (!mtk_wed_device_active(&dev->mt76.mmio.wed))
+		pci_free_irq_vectors(pdev);
+}
+
 void mt7996_unregister_device(struct mt7996_dev *dev)
 {
 	cancel_work_sync(&dev->dump_work);

@@ -1825,7 +1837,9 @@ void mt7996_unregister_device(struct mt7996_dev *dev)
 	if (mt7996_has_hwrro(dev) &&
 	    !mtk_wed_device_active(&dev->mt76.mmio.wed))
 		mt7996_rro_msdu_page_map_free(dev);
-	tasklet_disable(&dev->mt76.irq_tasklet);
+	tasklet_kill(&dev->mt76.irq_tasklet);
+
+	mt7996_cleanup_irqs(dev);
 
 	mt76_free_device(&dev->mt76);
 }

drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h +1 -0

index 2a0cdb56f822..79c57e88e107 100644
@@ -678,6 +678,7 @@ irqreturn_t mt7996_irq_handler(int irq, void *dev_instance);
 u64 __mt7996_get_tsf(struct ieee80211_hw *hw, struct mt7996_vif_link *link);
 int mt7996_register_device(struct mt7996_dev *dev);
 void mt7996_unregister_device(struct mt7996_dev *dev);
+void mt7996_put_hif2(struct mt7996_dev *dev, struct mt7996_hif *hif);
 int mt7996_vif_link_add(struct mt76_phy *mphy, struct ieee80211_vif *vif,
 			struct ieee80211_bss_conf *link_conf,
 			struct mt76_vif_link *mlink);

drivers/net/wireless/mediatek/mt76/mt7996/pci.c +9 -2

index b7d9193e042f..5c686cf66d0a 100644
@@ -53,11 +53,19 @@ static struct mt7996_hif *mt7996_pci_get_hif2(u32 idx)
 	return hif;
 }
 
-static void mt7996_put_hif2(struct mt7996_hif *hif)
+void mt7996_put_hif2(struct mt7996_dev *dev, struct mt7996_hif *hif)
 {
+	struct pci_dev *hif2_dev;
+
 	if (!hif)
 		return;
 
+	hif2_dev = container_of(hif->dev, struct pci_dev, dev);
+
+	devm_free_irq(dev->mt76.dev, hif->irq, dev);
+	if (!mtk_wed_device_active(&dev->mt76.mmio.wed_hif2))
+		pci_free_irq_vectors(hif2_dev);
+
 	put_device(hif->dev);
 }

@@ -237,7 +245,6 @@ static void mt7996_pci_remove(struct pci_dev *pdev)
 
 	mdev = pci_get_drvdata(pdev);
 	dev = container_of(mdev, struct mt7996_dev, mt76);
-	mt7996_put_hif2(dev->hif2);
 	mt7996_unregister_device(dev);
 }

Signature · 1 line

2.42.0

Shiji Yang

2026-09-18 11:04 UTC #2 lore raw

In reply to [email protected]

Hi,
This change will result in another regression. See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3aa1dcaa4f6f5ae08936491e08bd456f331f2d40

Regards,
Shiji Yang

Ben Greear

author

2026-09-18 15:23 UTC #3 lore raw

Quoted 7 lines · Shiji Yang

Hi,
This change will result in another regression. See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3aa1dcaa4f6f5ae08936491e08bd456f331f2d40
Regards,
Shiji Yang

Thanks for the pointer. Maybe we have to make the common code pay attention to the radio type since different radio driver logic is acting differently.

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Devin Wittmayer

2026-09-20 03:49 UTC #4 lore raw

In reply to [email protected]

drivers/net/wireless/mediatek/mt76/dma.c void mt76_dma_cleanup(struct mt76_dev *dev) · line 1221

Quoted 2 lines · Ben Greear

drivers/net/wireless/mediatek/mt76/dma.c +1 -0
+		napi_disable(&dev->napi[i]);
 		netif_napi_del(&dev->napi[i]);

Ben,

The warning and the softirqd spin are both real, and your 2/2 looks like the right place for them.

On 1/2 there is a snag. mt7921e and mt7925e already stop RX themselves, and they do it early:

	mt76_unregister_device()
	napi_disable() on each rx queue
	tx_token_put()
	mt792x_dma_cleanup()

It has to be early. A poll still in flight can come back with a transmit status and take an entry out of the table tx_token_put() is tearing down. By the time the shared cleanup runs RX is already stopped, so a second disable there just sits:

	task in D state, refcount -1
	napi_disable_locked <- mt76_dma_cleanup <- mt7925_pci_remove

That was on MT7927. The USB parts tear down a different way and never arrive, which is probably why this is easy to miss.

Would putting the disable next to the IRQ and tasklet work in your 2/2 cover mt7996? That keeps it where the driver already knows whether it has stopped RX.

Devin

Ben Greear

author

2026-09-20 14:49 UTC #5 lore raw

Quoted 25 lines · Devin Wittmayer

drivers/net/wireless/mediatek/mt76/dma.c void mt76_dma_cleanup(struct mt76_dev *dev) · line 1221
Quoted 2 lines · Ben Greear
drivers/net/wireless/mediatek/mt76/dma.c +1 -0
+		napi_disable(&dev->napi[i]);
  		netif_napi_del(&dev->napi[i]);
Ben,
The warning and the softirqd spin are both real, and your 2/2 looks like the right place for them.
On 1/2 there is a snag. mt7921e and mt7925e already stop RX themselves, and they do it early:
	mt76_unregister_device()
	napi_disable() on each rx queue
	tx_token_put()
	mt792x_dma_cleanup()
It has to be early. A poll still in flight can come back with a transmit status and take an entry out of the table tx_token_put() is tearing down. By the time the shared cleanup runs RX is already stopped, so a second disable there just sits:
	task in D state, refcount -1
	napi_disable_locked <- mt76_dma_cleanup <- mt7925_pci_remove

Quoted 6 lines

That was on MT7927. The USB parts tear down a different way and never arrive, which is probably why this is easy to miss.
Would putting the disable next to the IRQ and tasklet work in your 2/2 cover mt7996? That keeps it where the driver already knows whether it has stopped RX.

I have a hard time figuring out exactly how to safely tear down the mt7996 driver, especially without breaking other mt76 drivers.

But instead of just playing whack-a-mole with this,
we should figure out if sub-driver or mt76 core should be handling this, and add some comments of expected behaviour so we don't keep fixing one chipset and breaking another.

Maybe Felix has an opinion on correct path forward?

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com