Commit 8ed47e4e authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

eth: tg3: fix calling napi_enable() in atomic context 



tg3 has a spin lock protecting most of the config,
switch to taking netdev_lock() explicitly on enable/start
paths. Disable/stop paths seem to not be under the spin
lock (since napi_disable() already needs to sleep),
so leave that side as is.

tg3_restart_hw() releases and re-takes the spin lock,
we need to do the same because dev_close() needs to
take netdev_lock().

Fixes: 413f0271 ("net: protect NAPI enablement with netdev_lock()")
Reported-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/dcfd56bc-de32-4b11-9e19-d8bd1543745d@stanley.mountain


Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Reviewed-by: default avatarMichael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250124031841.1179756-2-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 964417a5

drivers/net/ethernet/broadcom/tg3.c

+31 −4
Original line number Diff line number Diff line
@@ -7424,7 +7424,7 @@ static void tg3_napi_enable(struct tg3 *tp) 


	for (i = 0; i < tp->irq_cnt; i++) {

		tnapi = &tp->napi[i];

		napi_enable(&tnapi->napi);

		napi_enable_locked(&tnapi->napi);

		if (tnapi->tx_buffers) {

			netif_queue_set_napi(tp->dev, txq_idx,

					     NETDEV_QUEUE_TYPE_TX,
@@ -7445,9 +7445,10 @@ static void tg3_napi_init(struct tg3 *tp) 
	int i;



	for (i = 0; i < tp->irq_cnt; i++) {

		netif_napi_add(tp->dev, &tp->napi[i].napi,

		netif_napi_add_locked(tp->dev, &tp->napi[i].napi,

				      i ? tg3_poll_msix : tg3_poll);

		netif_napi_set_irq(&tp->napi[i].napi, tp->napi[i].irq_vec);

		netif_napi_set_irq_locked(&tp->napi[i].napi,

					  tp->napi[i].irq_vec);

	}

}
@@ -11259,6 +11260,8 @@ static void tg3_timer_stop(struct tg3 *tp) 
static int tg3_restart_hw(struct tg3 *tp, bool reset_phy)

	__releases(tp->lock)

	__acquires(tp->lock)

	__releases(tp->dev->lock)

	__acquires(tp->dev->lock)

{

	int err;
@@ -11271,7 +11274,9 @@ static int tg3_restart_hw(struct tg3 *tp, bool reset_phy) 
		tg3_timer_stop(tp);

		tp->irq_sync = 0;

		tg3_napi_enable(tp);

		netdev_unlock(tp->dev);

		dev_close(tp->dev);

		netdev_lock(tp->dev);

		tg3_full_lock(tp, 0);

	}

	return err;
@@ -11299,6 +11304,7 @@ static void tg3_reset_task(struct work_struct *work) 


	tg3_netif_stop(tp);



	netdev_lock(tp->dev);

	tg3_full_lock(tp, 1);



	if (tg3_flag(tp, TX_RECOVERY_PENDING)) {
@@ -11318,12 +11324,14 @@ static void tg3_reset_task(struct work_struct *work) 
		 * call cancel_work_sync() and wait forever.

		 */

		tg3_flag_clear(tp, RESET_TASK_PENDING);

		netdev_unlock(tp->dev);

		dev_close(tp->dev);

		goto out;

	}



	tg3_netif_start(tp);

	tg3_full_unlock(tp);

	netdev_unlock(tp->dev);

	tg3_phy_start(tp);

	tg3_flag_clear(tp, RESET_TASK_PENDING);

out:
@@ -11683,9 +11691,11 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq, 
	if (err)

		goto out_ints_fini;



	netdev_lock(dev);

	tg3_napi_init(tp);



	tg3_napi_enable(tp);

	netdev_unlock(dev);



	for (i = 0; i < tp->irq_cnt; i++) {

		err = tg3_request_irq(tp, i);
@@ -12569,6 +12579,7 @@ static int tg3_set_ringparam(struct net_device *dev, 
		irq_sync = 1;

	}



	netdev_lock(dev);

	tg3_full_lock(tp, irq_sync);



	tp->rx_pending = ering->rx_pending;
@@ -12597,6 +12608,7 @@ static int tg3_set_ringparam(struct net_device *dev, 
	}



	tg3_full_unlock(tp);

	netdev_unlock(dev);



	if (irq_sync && !err)

		tg3_phy_start(tp);
@@ -12678,6 +12690,7 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam 
			irq_sync = 1;

		}



		netdev_lock(dev);

		tg3_full_lock(tp, irq_sync);



		if (epause->autoneg)
@@ -12707,6 +12720,7 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam 
		}



		tg3_full_unlock(tp);

		netdev_unlock(dev);

	}



	tp->phy_flags |= TG3_PHYFLG_USER_CONFIGURED;
@@ -13911,6 +13925,7 @@ static void tg3_self_test(struct net_device *dev, struct ethtool_test *etest, 
			data[TG3_INTERRUPT_TEST] = 1;

		}



		netdev_lock(dev);

		tg3_full_lock(tp, 0);



		tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
@@ -13922,6 +13937,7 @@ static void tg3_self_test(struct net_device *dev, struct ethtool_test *etest, 
		}



		tg3_full_unlock(tp);

		netdev_unlock(dev);



		if (irq_sync && !err2)

			tg3_phy_start(tp);
@@ -14365,6 +14381,7 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu) 


	tg3_set_mtu(dev, tp, new_mtu);



	netdev_lock(dev);

	tg3_full_lock(tp, 1);



	tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
@@ -14384,6 +14401,7 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu) 
		tg3_netif_start(tp);



	tg3_full_unlock(tp);

	netdev_unlock(dev);



	if (!err)

		tg3_phy_start(tp);
@@ -18164,6 +18182,7 @@ static int tg3_resume(struct device *device) 


	netif_device_attach(dev);



	netdev_lock(dev);

	tg3_full_lock(tp, 0);



	tg3_ape_driver_state_change(tp, RESET_KIND_INIT);
@@ -18180,6 +18199,7 @@ static int tg3_resume(struct device *device) 


out:

	tg3_full_unlock(tp);

	netdev_unlock(dev);



	if (!err)

		tg3_phy_start(tp);
@@ -18260,7 +18280,9 @@ static pci_ers_result_t tg3_io_error_detected(struct pci_dev *pdev, 
done:

	if (state == pci_channel_io_perm_failure) {

		if (netdev) {

			netdev_lock(netdev);

			tg3_napi_enable(tp);

			netdev_unlock(netdev);

			dev_close(netdev);

		}

		err = PCI_ERS_RESULT_DISCONNECT;
@@ -18314,7 +18336,9 @@ static pci_ers_result_t tg3_io_slot_reset(struct pci_dev *pdev) 


done:

	if (rc != PCI_ERS_RESULT_RECOVERED && netdev && netif_running(netdev)) {

		netdev_lock(netdev);

		tg3_napi_enable(tp);

		netdev_unlock(netdev);

		dev_close(netdev);

	}

	rtnl_unlock();
@@ -18340,12 +18364,14 @@ static void tg3_io_resume(struct pci_dev *pdev) 
	if (!netdev || !netif_running(netdev))

		goto done;



	netdev_lock(netdev);

	tg3_full_lock(tp, 0);

	tg3_ape_driver_state_change(tp, RESET_KIND_INIT);

	tg3_flag_set(tp, INIT_COMPLETE);

	err = tg3_restart_hw(tp, true);

	if (err) {

		tg3_full_unlock(tp);

		netdev_unlock(netdev);

		netdev_err(netdev, "Cannot restart hardware after reset.\n");

		goto done;

	}
@@ -18357,6 +18383,7 @@ static void tg3_io_resume(struct pci_dev *pdev) 
	tg3_netif_start(tp);



	tg3_full_unlock(tp);

	netdev_unlock(netdev);



	tg3_phy_start(tp);