Messages in this thread Patch in this message | | | Subject | Avoid deadlock in smc91x driver | From | Ian Campbell <> | Date | Thu, 25 Nov 2004 16:49:24 +0000 |
| |
Hi Jeff,
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>
Index: 2.6/drivers/net/smc91x.c =================================================================== --- 2.6.orig/drivers/net/smc91x.c 2004-11-16 09:26:52.000000000 +0000 +++ 2.6/drivers/net/smc91x.c 2004-11-25 09:49:38.830953019 +0000 @@ -203,7 +203,10 @@ 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 @@ /* * 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 @@ smc_phy_configure_exit: spin_unlock_irq(&lp->lock); + lp->work_pending = 0; } /* @@ -1350,10 +1354,13 @@ /* * 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 @@ 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 @@ * 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; -- Ian Campbell, Senior Design Engineer Web: http://www.arcom.com Arcom, Clifton Road, Direct: +44 (0)1223 403 465 Cambridge CB1 7EA, United Kingdom Phone: +44 (0)1223 411 200
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |