On Fri, 8 Dec 2006 08:06:57 +1100
Herbert Xu <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
