Michael Chan wrote:
This fixes an rtnl deadlock problem when flush_scheduled_work() is
called from bnx2_close(). In rare cases, linkwatch_event() may be on
the workqueue from a previous close of a different device and it will
try to get the rtnl lock which is already held by dev_close().
The fix is to set a flag if we are in the reset task which is run
from the workqueue. bnx2_close() will loop until the flag is cleared.
flush_scheduled_work() is also moved to bnx2_remove_one() before the
netdev is freed.
Signed-off-by: Michael Chan <[EMAIL PROTECTED]>
diff -rup a/drivers/net/bnx2.c b/drivers/net/bnx2.c
--- a/drivers/net/bnx2.c 2005-08-19 10:46:53.000000000 -0700
+++ b/drivers/net/bnx2.c 2005-08-24 07:13:25.000000000 -0700
@@ -3975,12 +3975,17 @@ bnx2_reset_task(void *data)
{
struct bnx2 *bp = data;
+ if (!netif_running(bp->dev))
+ return;
+
+ bp->in_reset_task = 1;
bnx2_netif_stop(bp);
bnx2_init_nic(bp);
atomic_set(&bp->intr_sem, 1);
bnx2_netif_start(bp);
+ bp->in_reset_task = 0;
}
static void
@@ -4172,7 +4177,13 @@ bnx2_close(struct net_device *dev)
struct bnx2 *bp = dev->priv;
u32 reset_code;
- flush_scheduled_work();
+ /* Calling flush_scheduled_work() may deadlock because
+ * linkwatch_event() may be on the workqueue and it will try to get
+ * the rtnl_lock which we are holding.
+ */
+ while (bp->in_reset_task)
+ yield();
Three comments:
1) Since you may have to deal with the SMP case, don't you need to add
wmb() or use atomic bit tests?
2) Would prefer to follow the generic net stack and other areas of the
kernel, for the last piece of quoted code. net stack used to loop on
schedule_timeout(1) in dev_close() [net/core/dev.c], which has now been
updated to loop on msleep(1). As the code comment there notes, we're
not in a hurry here.
3) Once bnx2 is fixed, any chance you could be talked into proposing
patches for the other drivers with this problem? If you don't have
access to hardware to test, that's not a big deal. Just note that in
the patch description.
Otherwise, patch is OK.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
