[PATCH] Avoid deadlock in smc91x driver

This patch avoids a deadlock on rtnl_sem in smc_close() when bringing down
an smc91x interface.  The semaphore is already held by devinet_ioctl() and
the pending work queue contains linkwatch_event() (scheduled by
netif_carrier_off()) which also wants rtnl_sem hence it is unsafe to call
flush_scheduled_work().

The solution is to track whether we have any pending work of our own and
wait for that instead of flushing the entire queue.

I also fixed a typo 'ence' -> 'Hence' and renamed smc_detect_phy to
smc_phy_detect in order to follow the same pattern as the other smc_phy_*
functions.

Signed-off-by: Ian Campbell <icampbell@arcom.com>
Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c
index 366e86611ced8bc831a6bc07aad5b854fe8c140b..7e6b211bb4e516dd30e1313b38084e4c91b8ab4a 100644 (file)
--- a/drivers/net/smc91x.c
+++ b/drivers/net/smc91x.c
@@ -203,7 +203,10 @@ struct smc_local {
        u32     msg_enable;
        u32     phy_type;
        struct mii_if_info mii;
+
+       /* work queue */
        struct work_struct phy_configure;
+       int     work_pending;
 
        spinlock_t lock;
 
@@ -903,7 +906,7 @@ static void smc_phy_write(struct net_device *dev, int phyaddr, int phyreg,
 /*
  * Finds and reports the PHY address
  */
-static void smc_detect_phy(struct net_device *dev)
+static void smc_phy_detect(struct net_device *dev)
 {
        struct smc_local *lp = netdev_priv(dev);
        int phyaddr;
@@ -1155,6 +1158,7 @@ static void smc_phy_configure(void *data)
 
 smc_phy_configure_exit:
        spin_unlock_irq(&lp->lock);
+       lp->work_pending = 0;
 }
 
 /*
@@ -1350,10 +1354,13 @@ static void smc_timeout(struct net_device *dev)
        /*
         * Reconfiguring the PHY doesn't seem like a bad idea here, but
         * smc_phy_configure() calls msleep() which calls schedule_timeout()
-        * which calls schedule().  Ence we use a work queue.
+        * which calls schedule().  Hence we use a work queue.
         */
-       if (lp->phy_type != 0)
-               schedule_work(&lp->phy_configure);
+       if (lp->phy_type != 0) {
+               if (schedule_work(&lp->phy_configure)) {
+                       lp->work_pending = 1;
+               }
+       }
 
        /* We can accept TX packets again */
        dev->trans_start = jiffies;
@@ -1537,7 +1544,18 @@ static int smc_close(struct net_device *dev)
        smc_shutdown(dev);
 
        if (lp->phy_type != 0) {
-               flush_scheduled_work();
+               /* We need to ensure that no calls to
+                  smc_phy_configure are pending.
+
+                  flush_scheduled_work() cannot be called because we
+                  are running with the netlink semaphore held (from
+                  devinet_ioctl()) and the pending work queue
+                  contains linkwatch_event() (scheduled by
+                  netif_carrier_off() above). linkwatch_event() also
+                  wants the netlink semaphore.
+               */
+               while(lp->work_pending)
+                       schedule();
                smc_phy_powerdown(dev, lp->mii.phy_id);
        }
 
@@ -1904,7 +1922,7 @@ static int __init smc_probe(struct net_device *dev, unsigned long ioaddr)
         * Locate the phy, if any.
         */
        if (lp->version >= (CHIP_91100 << 4))
-               smc_detect_phy(dev);
+               smc_phy_detect(dev);
 
        /* Set default parameters */
        lp->msg_enable = NETIF_MSG_LINK;