Prev: bonding: make bonding support netpoll
Next: [PATCH] kbuild: Include gen_initramfs_list.sh and the file list in the .d file
From: Eric Dumazet on 12 Apr 2010 06:40 Le lundi 12 avril 2010 à 18:37 +0800, Cong Wang a écrit : > Stephen Hemminger wrote: > > There is no protection on dev->priv_flags for SMP access. > > It would better bit value in dev->state if you are using it as control flag. > > > > Then you could use > > if (unlikely(test_and_clear_bit(__IN_NETPOLL, &skb->dev->state))) > > netpoll_send_skb(...) > > > > > > Hmm, I think we can't use ->state here, it is not for this kind of purpose, > according to its comments. > > Also, I find other usages of IFF_XXX flags of ->priv_flags are also using > &, | to set or clear the flags. So there must be some other things preventing > the race... Yes, its RTNL that protects priv_flags changes, hopefully... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Cong Wang on 12 Apr 2010 06:40 Stephen Hemminger wrote: >> Index: linux-2.6/net/bridge/br_forward.c >> =================================================================== >> --- linux-2.6.orig/net/bridge/br_forward.c >> +++ linux-2.6/net/bridge/br_forward.c >> @@ -15,6 +15,7 @@ >> #include <linux/slab.h> >> #include <linux/kernel.h> >> #include <linux/netdevice.h> >> +#include <linux/netpoll.h> >> #include <linux/skbuff.h> >> #include <linux/if_vlan.h> >> #include <linux/netfilter_bridge.h> >> @@ -50,7 +51,13 @@ int br_dev_queue_push_xmit(struct sk_buf >> else { >> skb_push(skb, ETH_HLEN); >> >> - dev_queue_xmit(skb); >> +#ifdef CONFIG_NET_POLL_CONTROLLER >> + if (skb->dev->priv_flags & IFF_IN_NETPOLL) { >> + netpoll_send_skb(skb->dev->npinfo->netpoll, skb); >> + skb->dev->priv_flags &= ~IFF_IN_NETPOLL; >> + } else >> +#endif > > There is no protection on dev->priv_flags for SMP access. > It would better bit value in dev->state if you are using it as control flag. > > Then you could use > if (unlikely(test_and_clear_bit(__IN_NETPOLL, &skb->dev->state))) > netpoll_send_skb(...) > > Hmm, I think we can't use ->state here, it is not for this kind of purpose, according to its comments. Also, I find other usages of IFF_XXX flags of ->priv_flags are also using &, | to set or clear the flags. So there must be some other things preventing the race... Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 27 Apr 2010 18:30
From: Amerigo Wang <amwang(a)redhat.com> Date: Tue, 27 Apr 2010 03:55:56 -0400 > + if (p->dev->priv_flags & IFF_DISABLE_NETPOLL > + || !p->dev->netdev_ops->ndo_poll_controller) "||" goes on first line, and indentation on second line is incorrect. See my comments from patch #1. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |