lock validator: false positive in vlan_dev.c

lock validator: false positive in vlan_dev.c

[deweerdt]

Hi,

Assigning an inet address to a vlanized interface triggered the following BUG
from the lock validator (kernel is 2.6.17-rc6-mm2):

[176619.872000] ====================================
[176619.872000] [ BUG: possible deadlock detected! ]
[176619.872000] ------------------------------------
[176619.872000] ifconfig/13283 is trying to acquire lock:
[176619.872000]  (&dev->xmit_lock){-+..}, at: [<c02f7471>] dev_mc_add+0x31/0x11d
[176619.872000]
[176619.872000] but task is already holding lock:
[176619.872000]  (&dev->xmit_lock){-+..}, at: [<c02f7471>] dev_mc_add+0x31/0x11d
[176619.872000]
[176619.872000] which could potentially lead to deadlocks!
[176619.872000]
[176619.872000] other info that might help us debug this:
[176619.872000] 2 locks held by ifconfig/13283:
[176619.872000]  #0:  (rtnl_mutex){--..}, at: [<c036c722>] mutex_lock+0x8/0xa
[176619.872000]  #1:  (&dev->xmit_lock){-+..}, at: [<c02f7471>]
dev_mc_add+0x31/0x11d
[176619.872000]
[176619.872000] stack backtrace:
[176619.872000]  [<c0103da1>] show_trace_log_lvl+0x136/0x150
[176619.872000]  [<c0104f12>] show_trace+0x1b/0x1d
[176619.872000]  [<c0104f3a>] dump_stack+0x26/0x28
[176619.872000]  [<c0137823>] __lock_acquire+0x2ea/0xd37
[176619.872000]  [<c0138684>] lock_acquire+0x60/0x75
[176619.872000]  [<c036dc5d>] _spin_lock_bh+0x4a/0x58
[176619.872000]  [<c02f7471>] dev_mc_add+0x31/0x11d
[176619.872000]  [<f8b5ed9e>] vlan_dev_set_multicast_list+0xf5/0x309 [8021q]
[176619.872000]  [<c02f7346>] __dev_mc_upload+0x26/0x28
[176619.872000]  [<c02f7504>] dev_mc_add+0xc4/0x11d
[176619.872000]  [<c033cd14>] igmp_group_added+0x10c/0x111
[176619.872000]  [<c033d5ff>] ip_mc_inc_group+0x1a2/0x234
[176619.872000]  [<c033d6b4>] ip_mc_up+0x23/0x66
[176619.872000]  [<c0339392>] inetdev_init+0x117/0x158
[176619.872000]  [<c033ae6f>] devinet_ioctl+0x5db/0x67c
[176619.872000]  [<c033b1cb>] inet_ioctl+0x7b/0x95
[176619.872000]  [<c02e8caa>] sock_ioctl+0xb1/0x252
[176619.872000]  [<c018561a>] do_ioctl+0x2a/0x80
[176619.872000]  [<c01856c2>] vfs_ioctl+0x52/0x2db
[176619.872000]  [<c01859b7>] sys_ioctl+0x6c/0x79
[176619.872000]  [<c036e80d>] sysenter_past_esp+0x56/0x8d
[176619.872000]  [<b7f76410>] 0xb7f76410

The following steps are needed to reproduce the BUG:
vconfig add eth0 2
ifconfig eth0.2 172.31.23.22

This seels to be a false positive because the first dev->xmit_lock held is the
one from the vlan interface and the second is the one belonging to the real
interface.

I considered adding dev_mc_add(..., int nesting) ->net_tx_lock_bh(..., int
nesting) -> spin_lock_bh_nested(..., int nesting) but the number of calling
places is important, so I wondered if someone would come up with a better
solution.

Thanks,
Frederik

[patch] lockdep annotate vlan net device as being a special class

[Arjan van de Ven]

On Thu, 2006-06-15 at 16:40 +0200, deweerdt@free.fr wrote:
> Hi,
> 
> Assigning an inet address to a vlanized interface triggered the following BUG
> from the lock validator (kernel is 2.6.17-rc6-mm2):

ok below is a real working (cross my fingers) patch against the current
-mm tree:

vlan network devices have devices nesting below it, and are a special
"super class" of normal network devices; split their locks off into a
separate class since they always nest.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 net/8021q/vlan.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6.17-lockdep/net/8021q/vlan.c
===================================================================
--- linux-2.6.17-lockdep.orig/net/8021q/vlan.c
+++ linux-2.6.17-lockdep/net/8021q/vlan.c
@@ -364,6 +364,14 @@ static void vlan_transfer_operstate(cons
 	}
 }
 
+/*
+ * vlan network devices have devices nesting below it, and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key vlan_netdev_xmit_lock_key;
+
+
 /*  Attach a VLAN device to a mac address (ie Ethernet Card).
  *  Returns the device that was created, or NULL if there was
  *  an error of some kind.
@@ -460,6 +468,8 @@ static struct net_device *register_vlan_
 		    
 	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
 			       vlan_setup);
+
+	lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
 	if (new_dev == NULL)
 		goto out_unlock;
 

Re: [patch] lockdep annotate vlan net device as being a special class

[Peter Staubach]

Arjan van de Ven wrote:

>On Thu, 2006-06-15 at 16:40 +0200, deweerdt@free.fr wrote:
>  
>
>>Hi,
>>
>>Assigning an inet address to a vlanized interface triggered the following BUG
>>from the lock validator (kernel is 2.6.17-rc6-mm2):
>>    
>>
>
>ok below is a real working (cross my fingers) patch against the current
>-mm tree:
>
>vlan network devices have devices nesting below it, and are a special
>"super class" of normal network devices; split their locks off into a
>separate class since they always nest.
>
>Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
>Signed-off-by: Ingo Molnar <mingo@elte.hu>
>---
> net/8021q/vlan.c |   10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>Index: linux-2.6.17-lockdep/net/8021q/vlan.c
>===================================================================
>--- linux-2.6.17-lockdep.orig/net/8021q/vlan.c
>+++ linux-2.6.17-lockdep/net/8021q/vlan.c
>@@ -364,6 +364,14 @@ static void vlan_transfer_operstate(cons
> 	}
> }
> 
>+/*
>+ * vlan network devices have devices nesting below it, and are a special
>+ * "super class" of normal network devices; split their locks off into a
>+ * separate class since they always nest.
>+ */
>+static struct lock_class_key vlan_netdev_xmit_lock_key;
>+
>+
> /*  Attach a VLAN device to a mac address (ie Ethernet Card).
>  *  Returns the device that was created, or NULL if there was
>  *  an error of some kind.
>@@ -460,6 +468,8 @@ static struct net_device *register_vlan_
> 		    
> 	new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
> 			       vlan_setup);
>+
>+	lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
> 	if (new_dev == NULL)
> 		goto out_unlock;
>

Shouldn't this test for new_dev being NULL _before_ it gets used?

    Thanx...

       ps

Re: [patch] lockdep annotate vlan net device as being a special class

[Frederik Deweerdt]

On Tue, Jun 27, 2006 at 10:09:08AM -0400, Peter Staubach wrote:
> Shouldn't this test for new_dev being NULL _before_ it gets used?
Indeed, it should. But the lock is inited in register_netdevice anyway.
I tested (compile+boot+ifconfig) the following patch to mm3, and the trace went
away.

Regards,
Frederik

Signed-Off-By: Frederik Deweerdt <frederik.deweerdt@gmail.com>

--- v2.6.17-mm3/net/8021q/vlan.c        2006-06-27 19:02:14.000000000 +0200
+++ v2.6.17-mm3~mod/net/8021q/vlan.c    2006-06-27 18:50:48.000000000 +0200
@@ -469,7 +469,6 @@ static struct net_device *register_vlan_
        new_dev = alloc_netdev(sizeof(struct vlan_dev_info), name,
                               vlan_setup);

-       lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
        if (new_dev == NULL)
                goto out_unlock;

@@ -528,6 +527,8 @@ static struct net_device *register_vlan_
        if (register_netdevice(new_dev))
                goto out_free_newdev;

+       lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
+
        new_dev->iflink = real_dev->ifindex;
        vlan_transfer_operstate(real_dev, new_dev);
        linkwatch_fire_event(new_dev); /* _MUST_ call rfc2863_policy() */