diff options
author | David S. Miller <davem@davemloft.net> | 2008-06-12 05:22:02 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-06-12 05:22:02 -0400 |
commit | 4bb073c0e32a0862bdb5215d11af19f6c0180c98 (patch) | |
tree | 009d95592e3813346c75129bb19d140d393ca913 /drivers/net/smc911x.c | |
parent | 7afb380db43ed137b7f67e0e3c3e5afd1ecde730 (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>
Diffstat (limited to 'drivers/net/smc911x.c')
-rw-r--r-- | drivers/net/smc911x.c | 24 |
1 files changed, 5 insertions, 19 deletions
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 | ||
1034 | smc911x_phy_configure_exit: | 1033 | smc911x_phy_configure_exit: |
1035 | spin_unlock_irqrestore(&lp->lock, flags); | 1034 | spin_unlock_irqrestore(&lp->lock, flags); |
1036 | smc911x_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 | ||