Message ID | 1400274296-14765-3-git-send-email-vyasevic@redhat.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2014-05-16 at 17:04 -0400, Vlad Yasevich wrote: > Currently netif_addr_lock_nested assumes that there can be only > a single nesting level between 2 devices. However, if we > have multiple devices of the same type stacked, this fails. > For example: > eth0 <-- vlan0.10 <-- vlan0.10.20 > > A more complicated configuration may stack more then one type of > device in different order. > Ex: > eth0 <-- vlan0.10 <-- macvlan0 <-- vlan1.10.20 <-- macvlan1 > > This patch adds an ndo_* function that allows each stackable > device to report its nesting level. If the device doesn't > provide this function default subclass of 1 is used. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > --- > include/linux/netdevice.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index fb912e8..9d4b1f1 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1144,6 +1144,7 @@ struct net_device_ops { > netdev_tx_t (*ndo_dfwd_start_xmit) (struct sk_buff *skb, > struct net_device *dev, > void *priv); > + int (*ndo_get_lock_subclass)(struct net_device *dev); > }; > > /** > @@ -2950,7 +2951,12 @@ static inline void netif_addr_lock(struct net_device *dev) > > static inline void netif_addr_lock_nested(struct net_device *dev) > { > - spin_lock_nested(&dev->addr_list_lock, SINGLE_DEPTH_NESTING); > + int subclass = SINGLE_DEPTH_NESTING; > + > + if (dev->netdev_ops->ndo_get_lock_subclass) > + subclass = dev->netdev_ops->ndo_get_lock_subclass(dev); > + > + spin_lock_nested(&dev->addr_list_lock, subclass); > } > Hi, I know this has already been applied. However, this commit causes a warning in include/linux/netdevice.h when W=1 In file included from ixgbe/ixgbe.h:35:0, from ixgbe/ixgbe_lib.c:29: include/linux/netdevice.h: In function ‘netif_addr_lock_nested’: include/linux/netdevice.h:2954:6: warning: variable ‘subclass’ set but not used [-Wunused-but-set-variable] int subclass = SINGLE_DEPTH_NESTING; ^ This only occurs if CONFIG_DEBUG_LOCK_ALLOC=Y, and there seems to be no easy fix for the warning, due to how spin_lock_nested is a macro that discards the second parameter when DEBUG_LOCK_ALLOC is disabled. I'm not sure how big a deal this is, considering that it only occurs at W=1, and the patch is already committed. > static inline void netif_addr_lock_bh(struct net_device *dev) Regards, Jake
From: "Keller, Jacob E" <jacob.e.keller@intel.com> Date: Wed, 4 Jun 2014 21:42:12 +0000 > This only occurs if CONFIG_DEBUG_LOCK_ALLOC=Y, and there seems to be no > easy fix for the warning, due to how spin_lock_nested is a macro that > discards the second parameter when DEBUG_LOCK_ALLOC is disabled. spin_lock_nested(), in that case, should mark the argument as unused. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-06-04 at 14:53 -0700, David Miller wrote: > From: "Keller, Jacob E" <jacob.e.keller@intel.com> > Date: Wed, 4 Jun 2014 21:42:12 +0000 > > > This only occurs if CONFIG_DEBUG_LOCK_ALLOC=Y, and there seems to be no > > easy fix for the warning, due to how spin_lock_nested is a macro that > > discards the second parameter when DEBUG_LOCK_ALLOC is disabled. > > spin_lock_nested(), in that case, should mark the argument as unused. > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Can it do that? spin_lock_nested is a macro. Thanks, Jake
On 06/04/2014 05:42 PM, Keller, Jacob E wrote: > On Fri, 2014-05-16 at 17:04 -0400, Vlad Yasevich wrote: >> Currently netif_addr_lock_nested assumes that there can be only >> a single nesting level between 2 devices. However, if we >> have multiple devices of the same type stacked, this fails. >> For example: >> eth0 <-- vlan0.10 <-- vlan0.10.20 >> >> A more complicated configuration may stack more then one type of >> device in different order. >> Ex: >> eth0 <-- vlan0.10 <-- macvlan0 <-- vlan1.10.20 <-- macvlan1 >> >> This patch adds an ndo_* function that allows each stackable >> device to report its nesting level. If the device doesn't >> provide this function default subclass of 1 is used. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >> --- >> include/linux/netdevice.h | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index fb912e8..9d4b1f1 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -1144,6 +1144,7 @@ struct net_device_ops { >> netdev_tx_t (*ndo_dfwd_start_xmit) (struct sk_buff *skb, >> struct net_device *dev, >> void *priv); >> + int (*ndo_get_lock_subclass)(struct net_device *dev); >> }; >> >> /** >> @@ -2950,7 +2951,12 @@ static inline void netif_addr_lock(struct net_device *dev) >> >> static inline void netif_addr_lock_nested(struct net_device *dev) >> { >> - spin_lock_nested(&dev->addr_list_lock, SINGLE_DEPTH_NESTING); >> + int subclass = SINGLE_DEPTH_NESTING; >> + >> + if (dev->netdev_ops->ndo_get_lock_subclass) >> + subclass = dev->netdev_ops->ndo_get_lock_subclass(dev); >> + >> + spin_lock_nested(&dev->addr_list_lock, subclass); >> } >> > > Hi, > > I know this has already been applied. However, this commit causes a > warning in include/linux/netdevice.h when W=1 > > In file included from ixgbe/ixgbe.h:35:0, > from ixgbe/ixgbe_lib.c:29: > include/linux/netdevice.h: In function ‘netif_addr_lock_nested’: > include/linux/netdevice.h:2954:6: warning: variable ‘subclass’ set but not used [-Wunused-but-set-variable] > int subclass = SINGLE_DEPTH_NESTING; > ^ > > This only occurs if CONFIG_DEBUG_LOCK_ALLOC=Y, and there seems to be no > easy fix for the warning, due to how spin_lock_nested is a macro that > discards the second parameter when DEBUG_LOCK_ALLOC is disabled. > > I'm not sure how big a deal this is, considering that it only occurs at > W=1, and the patch is already committed. > Yuck. It's actually raw_spin_lock_nested() macro that discards the subclass argument... We could do something really silly like: # define raw_spin_lock_nested(lock, subclass) \ sublcass;_raw_spin_lock(lock) That seems to get rid of the warning for me. -vlad >> static inline void netif_addr_lock_bh(struct net_device *dev) > > Regards, > Jake > > N�����r��y���b�X��ǧv�^�){.n�+���z�^�)���w*jg��������ݢj/���z�ޖ��2�ޙ���&�)ߡ�a�����G���h��j:+v���w�٥ > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index fb912e8..9d4b1f1 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1144,6 +1144,7 @@ struct net_device_ops { netdev_tx_t (*ndo_dfwd_start_xmit) (struct sk_buff *skb, struct net_device *dev, void *priv); + int (*ndo_get_lock_subclass)(struct net_device *dev); }; /** @@ -2950,7 +2951,12 @@ static inline void netif_addr_lock(struct net_device *dev) static inline void netif_addr_lock_nested(struct net_device *dev) { - spin_lock_nested(&dev->addr_list_lock, SINGLE_DEPTH_NESTING); + int subclass = SINGLE_DEPTH_NESTING; + + if (dev->netdev_ops->ndo_get_lock_subclass) + subclass = dev->netdev_ops->ndo_get_lock_subclass(dev); + + spin_lock_nested(&dev->addr_list_lock, subclass); } static inline void netif_addr_lock_bh(struct net_device *dev)
Currently netif_addr_lock_nested assumes that there can be only a single nesting level between 2 devices. However, if we have multiple devices of the same type stacked, this fails. For example: eth0 <-- vlan0.10 <-- vlan0.10.20 A more complicated configuration may stack more then one type of device in different order. Ex: eth0 <-- vlan0.10 <-- macvlan0 <-- vlan1.10.20 <-- macvlan1 This patch adds an ndo_* function that allows each stackable device to report its nesting level. If the device doesn't provide this function default subclass of 1 is used. Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> --- include/linux/netdevice.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)