Messages in this thread Patch in this message | | | Date | Fri, 8 Dec 2006 15:19:02 -0800 | From | Andrew Morton <> | Subject | Re: [patch] net: dev_watchdog() locking fix |
| |
On Fri, 8 Dec 2006 08:06:57 +1100 Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Dec 07, 2006 at 09:55:22PM +0100, Ingo Molnar wrote: > > > > fallout of the recent big networking merge i guess. Tested fix below. > > David, Herbert, do you agree with it, or is it a false positive? > > I agree that this is a bug, but the fix is in the wrong spot. The > dev_watchdog function already runs in softirq context so it doesn't > need to disable BH. > > You can almost be guaranteed that if netpoll is involved in a bug > then it should be fixed :) > > In this case, it's taking the tx lock in process context which is > not allowed. So it should disable BH before taking the tx lock. >
Like this?
/* don't get messages out of order, and no recursion */ if (skb_queue_len(&npinfo->txq) == 0 && npinfo->poll_owner != smp_processor_id()) { local_bh_disable(); /* Where's netif_tx_trylock_bh()? */ if (netif_tx_trylock(dev)) { /* try until next clock tick */ for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; tries > 0; --tries) { if (!netif_queue_stopped(dev)) status = dev->hard_start_xmit(skb, dev);
if (status == NETDEV_TX_OK) break;
/* tickle device maybe there is some cleanup */ netpoll_poll(np);
udelay(USEC_PER_POLL); } netif_tx_unlock(dev); } local_bh_enable(); }
--- a/net/core/netpoll.c~netpoll-locking-fix +++ a/net/core/netpoll.c @@ -242,22 +242,26 @@ static void netpoll_send_skb(struct netp /* don't get messages out of order, and no recursion */ if (skb_queue_len(&npinfo->txq) == 0 && - npinfo->poll_owner != smp_processor_id() && - netif_tx_trylock(dev)) { - /* try until next clock tick */ - for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; tries > 0; --tries) { - if (!netif_queue_stopped(dev)) - status = dev->hard_start_xmit(skb, dev); + npinfo->poll_owner != smp_processor_id()) { + local_bh_disable(); /* Where's netif_tx_trylock_bh()? */ + if (netif_tx_trylock(dev)) { + /* try until next clock tick */ + for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; + tries > 0; --tries) { + if (!netif_queue_stopped(dev)) + status = dev->hard_start_xmit(skb, dev); - if (status == NETDEV_TX_OK) - break; + if (status == NETDEV_TX_OK) + break; - /* tickle device maybe there is some cleanup */ - netpoll_poll(np); + /* tickle device maybe there is some cleanup */ + netpoll_poll(np); - udelay(USEC_PER_POLL); + udelay(USEC_PER_POLL); + } + netif_tx_unlock(dev); } - netif_tx_unlock(dev); + local_bh_enable(); } if (status != NETDEV_TX_OK) { _ - 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/
| |