aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid S. Miller <davem@davemloft.net>2008-06-12 05:22:02 -0400
committerDavid S. Miller <davem@davemloft.net>2008-06-12 05:22:02 -0400
commit4bb073c0e32a0862bdb5215d11af19f6c0180c98 (patch)
tree009d95592e3813346c75129bb19d140d393ca913
parent7afb380db43ed137b7f67e0e3c3e5afd1ecde730 (diff)
net: Eliminate flush_scheduled_work() calls while RTNL is held.
If the RTNL is held when we invoke flush_scheduled_work() we could deadlock. One such case is linkwatch, it is a work struct which tries to grab the RTNL semaphore. The most common case are net driver ->stop() methods. The simplest conversion is to instead use cancel_{delayed_}work_sync() explicitly on the various work struct the driver uses. This is an OK transformation because these work structs are doing things like resetting the chip, restarting link negotiation, and so forth. And if we're bringing down the device, we're about to turn the chip off and reset it anways. So if we cancel a pending work event, that's fine here. Some drivers were working around this deadlock by using a msleep() polling loop of some sort, and those cases are converted to instead use cancel_{delayed_}work_sync() as well. Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--drivers/net/bnx2.c9
-rw-r--r--drivers/net/bnx2.h1
-rw-r--r--drivers/net/ehea/ehea_main.c3
-rw-r--r--drivers/net/hamradio/baycom_epp.c2
-rw-r--r--drivers/net/smc911x.c24
-rw-r--r--drivers/net/smc91x.c17
-rw-r--r--drivers/net/tulip/tulip_core.c2
-rw-r--r--drivers/net/usb/kaweth.c2
-rw-r--r--drivers/net/wireless/hostap/hostap_main.c8
9 files changed, 21 insertions, 47 deletions
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 4b46e68183e0..367b6d462708 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -5724,14 +5724,12 @@ bnx2_reset_task(struct work_struct *work)
5724 if (!netif_running(bp->dev)) 5724 if (!netif_running(bp->dev))
5725 return; 5725 return;
5726 5726
5727 bp->in_reset_task = 1;
5728 bnx2_netif_stop(bp); 5727 bnx2_netif_stop(bp);
5729 5728
5730 bnx2_init_nic(bp); 5729 bnx2_init_nic(bp);
5731 5730
5732 atomic_set(&bp->intr_sem, 1); 5731 atomic_set(&bp->intr_sem, 1);
5733 bnx2_netif_start(bp); 5732 bnx2_netif_start(bp);
5734 bp->in_reset_task = 0;
5735} 5733}
5736 5734
5737static void 5735static void
@@ -5907,12 +5905,7 @@ bnx2_close(struct net_device *dev)
5907 struct bnx2 *bp = netdev_priv(dev); 5905 struct bnx2 *bp = netdev_priv(dev);
5908 u32 reset_code; 5906 u32 reset_code;
5909 5907
5910 /* Calling flush_scheduled_work() may deadlock because 5908 cancel_work_sync(&bp->reset_task);
5911 * linkwatch_event() may be on the workqueue and it will try to get
5912 * the rtnl_lock which we are holding.
5913 */
5914 while (bp->in_reset_task)
5915 msleep(1);
5916 5909
5917 bnx2_disable_int_sync(bp); 5910 bnx2_disable_int_sync(bp);
5918 bnx2_napi_disable(bp); 5911 bnx2_napi_disable(bp);
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index 1eaf5bb3d9c2..2377cc13bf61 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6656,7 +6656,6 @@ struct bnx2 {
6656 int current_interval; 6656 int current_interval;
6657 struct timer_list timer; 6657 struct timer_list timer;
6658 struct work_struct reset_task; 6658 struct work_struct reset_task;
6659 int in_reset_task;
6660 6659
6661 /* Used to synchronize phy accesses. */ 6660 /* Used to synchronize phy accesses. */
6662 spinlock_t phy_lock; 6661 spinlock_t phy_lock;
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index faae01dc1c4b..075fd547421e 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2605,7 +2605,8 @@ static int ehea_stop(struct net_device *dev)
2605 if (netif_msg_ifdown(port)) 2605 if (netif_msg_ifdown(port))
2606 ehea_info("disabling port %s", dev->name); 2606 ehea_info("disabling port %s", dev->name);
2607 2607
2608 flush_scheduled_work(); 2608 cancel_work_sync(&port->reset_task);
2609
2609 mutex_lock(&port->port_lock); 2610 mutex_lock(&port->port_lock);
2610 netif_stop_queue(dev); 2611 netif_stop_queue(dev);
2611 port_napi_disable(port); 2612 port_napi_disable(port);
diff --git a/drivers/net/hamradio/baycom_epp.c b/drivers/net/hamradio/baycom_epp.c
index dde9c7e6408a..00bc7fbb6b37 100644
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -959,7 +959,7 @@ static int epp_close(struct net_device *dev)
959 unsigned char tmp[1]; 959 unsigned char tmp[1];
960 960
961 bc->work_running = 0; 961 bc->work_running = 0;
962 flush_scheduled_work(); 962 cancel_delayed_work_sync(&bc->run_work);
963 bc->stat = EPP_DCDBIT; 963 bc->stat = EPP_DCDBIT;
964 tmp[0] = 0; 964 tmp[0] = 0;
965 pp->ops->epp_write_addr(pp, tmp, 1, 0); 965 pp->ops->epp_write_addr(pp, tmp, 1, 0);
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 4e2800205189..e2ee91a6ae7e 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -136,7 +136,6 @@ struct smc911x_local {
136 136
137 /* work queue */ 137 /* work queue */
138 struct work_struct phy_configure; 138 struct work_struct phy_configure;
139 int work_pending;
140 139
141 int tx_throttle; 140 int tx_throttle;
142 spinlock_t lock; 141 spinlock_t lock;
@@ -960,11 +959,11 @@ static void smc911x_phy_configure(struct work_struct *work)
960 * We should not be called if phy_type is zero. 959 * We should not be called if phy_type is zero.
961 */ 960 */
962 if (lp->phy_type == 0) 961 if (lp->phy_type == 0)
963 goto smc911x_phy_configure_exit_nolock; 962 return;
964 963
965 if (smc911x_phy_reset(dev, phyaddr)) { 964 if (smc911x_phy_reset(dev, phyaddr)) {
966 printk("%s: PHY reset timed out\n", dev->name); 965 printk("%s: PHY reset timed out\n", dev->name);
967 goto smc911x_phy_configure_exit_nolock; 966 return;
968 } 967 }
969 spin_lock_irqsave(&lp->lock, flags); 968 spin_lock_irqsave(&lp->lock, flags);
970 969
@@ -1033,8 +1032,6 @@ static void smc911x_phy_configure(struct work_struct *work)
1033 1032
1034smc911x_phy_configure_exit: 1033smc911x_phy_configure_exit:
1035 spin_unlock_irqrestore(&lp->lock, flags); 1034 spin_unlock_irqrestore(&lp->lock, flags);
1036smc911x_phy_configure_exit_nolock:
1037 lp->work_pending = 0;
1038} 1035}
1039 1036
1040/* 1037/*
@@ -1356,11 +1353,8 @@ static void smc911x_timeout(struct net_device *dev)
1356 * smc911x_phy_configure() calls msleep() which calls schedule_timeout() 1353 * smc911x_phy_configure() calls msleep() which calls schedule_timeout()
1357 * which calls schedule(). Hence we use a work queue. 1354 * which calls schedule(). Hence we use a work queue.
1358 */ 1355 */
1359 if (lp->phy_type != 0) { 1356 if (lp->phy_type != 0)
1360 if (schedule_work(&lp->phy_configure)) { 1357 schedule_work(&lp->phy_configure);
1361 lp->work_pending = 1;
1362 }
1363 }
1364 1358
1365 /* We can accept TX packets again */ 1359 /* We can accept TX packets again */
1366 dev->trans_start = jiffies; 1360 dev->trans_start = jiffies;
@@ -1531,16 +1525,8 @@ static int smc911x_close(struct net_device *dev)
1531 if (lp->phy_type != 0) { 1525 if (lp->phy_type != 0) {
1532 /* We need to ensure that no calls to 1526 /* We need to ensure that no calls to
1533 * smc911x_phy_configure are pending. 1527 * smc911x_phy_configure are pending.
1534
1535 * flush_scheduled_work() cannot be called because we
1536 * are running with the netlink semaphore held (from
1537 * devinet_ioctl()) and the pending work queue
1538 * contains linkwatch_event() (scheduled by
1539 * netif_carrier_off() above). linkwatch_event() also
1540 * wants the netlink semaphore.
1541 */ 1528 */
1542 while (lp->work_pending) 1529 cancel_work_sync(&lp->phy_configure);
1543 schedule();
1544 smc911x_phy_powerdown(dev, lp->mii.phy_id); 1530 smc911x_phy_powerdown(dev, lp->mii.phy_id);
1545 } 1531 }
1546 1532
diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c
index a188e33484e6..f2051b209da2 100644
--- a/drivers/net/smc91x.c
+++ b/drivers/net/smc91x.c
@@ -1016,15 +1016,8 @@ static void smc_phy_powerdown(struct net_device *dev)
1016 1016
1017 /* We need to ensure that no calls to smc_phy_configure are 1017 /* We need to ensure that no calls to smc_phy_configure are
1018 pending. 1018 pending.
1019
1020 flush_scheduled_work() cannot be called because we are
1021 running with the netlink semaphore held (from
1022 devinet_ioctl()) and the pending work queue contains
1023 linkwatch_event() (scheduled by netif_carrier_off()
1024 above). linkwatch_event() also wants the netlink semaphore.
1025 */ 1019 */
1026 while(lp->work_pending) 1020 cancel_work_sync(&lp->phy_configure);
1027 yield();
1028 1021
1029 bmcr = smc_phy_read(dev, phy, MII_BMCR); 1022 bmcr = smc_phy_read(dev, phy, MII_BMCR);
1030 smc_phy_write(dev, phy, MII_BMCR, bmcr | BMCR_PDOWN); 1023 smc_phy_write(dev, phy, MII_BMCR, bmcr | BMCR_PDOWN);
@@ -1161,7 +1154,6 @@ static void smc_phy_configure(struct work_struct *work)
1161smc_phy_configure_exit: 1154smc_phy_configure_exit:
1162 SMC_SELECT_BANK(lp, 2); 1155 SMC_SELECT_BANK(lp, 2);
1163 spin_unlock_irq(&lp->lock); 1156 spin_unlock_irq(&lp->lock);
1164 lp->work_pending = 0;
1165} 1157}
1166 1158
1167/* 1159/*
@@ -1389,11 +1381,8 @@ static void smc_timeout(struct net_device *dev)
1389 * smc_phy_configure() calls msleep() which calls schedule_timeout() 1381 * smc_phy_configure() calls msleep() which calls schedule_timeout()
1390 * which calls schedule(). Hence we use a work queue. 1382 * which calls schedule(). Hence we use a work queue.
1391 */ 1383 */
1392 if (lp->phy_type != 0) { 1384 if (lp->phy_type != 0)
1393 if (schedule_work(&lp->phy_configure)) { 1385 schedule_work(&lp->phy_configure);
1394 lp->work_pending = 1;
1395 }
1396 }
1397 1386
1398 /* We can accept TX packets again */ 1387 /* We can accept TX packets again */
1399 dev->trans_start = jiffies; 1388 dev->trans_start = jiffies;
diff --git a/drivers/net/tulip/tulip_core.c b/drivers/net/tulip/tulip_core.c
index 55670b5eb611..af8d2c436efd 100644
--- a/drivers/net/tulip/tulip_core.c
+++ b/drivers/net/tulip/tulip_core.c
@@ -731,7 +731,7 @@ static void tulip_down (struct net_device *dev)
731 void __iomem *ioaddr = tp->base_addr; 731 void __iomem *ioaddr = tp->base_addr;
732 unsigned long flags; 732 unsigned long flags;
733 733
734 flush_scheduled_work(); 734 cancel_work_sync(&tp->media_work);
735 735
736#ifdef CONFIG_TULIP_NAPI 736#ifdef CONFIG_TULIP_NAPI
737 napi_disable(&tp->napi); 737 napi_disable(&tp->napi);
diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index 0dcfc0310264..7c66b052f55a 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -706,7 +706,7 @@ static void kaweth_kill_urbs(struct kaweth_device *kaweth)
706 usb_kill_urb(kaweth->rx_urb); 706 usb_kill_urb(kaweth->rx_urb);
707 usb_kill_urb(kaweth->tx_urb); 707 usb_kill_urb(kaweth->tx_urb);
708 708
709 flush_scheduled_work(); 709 cancel_delayed_work_sync(&kaweth->lowmem_work);
710 710
711 /* a scheduled work may have resubmitted, 711 /* a scheduled work may have resubmitted,
712 we hit them again */ 712 we hit them again */
diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
index 20d387f6658c..f7aec9309d04 100644
--- a/drivers/net/wireless/hostap/hostap_main.c
+++ b/drivers/net/wireless/hostap/hostap_main.c
@@ -682,7 +682,13 @@ static int prism2_close(struct net_device *dev)
682 netif_device_detach(dev); 682 netif_device_detach(dev);
683 } 683 }
684 684
685 flush_scheduled_work(); 685 cancel_work_sync(&local->reset_queue);
686 cancel_work_sync(&local->set_multicast_list_queue);
687 cancel_work_sync(&local->set_tim_queue);
688#ifndef PRISM2_NO_STATION_MODES
689 cancel_work_sync(&local->info_queue);
690#endif
691 cancel_work_sync(&local->comms_qual_update);
686 692
687 module_put(local->hw_module); 693 module_put(local->hw_module);
688 694