BUG: when using 'brctl stp'
Subsystems (3)
I get this on the latest GIT, it was also present shortly after -rc1. I have not tested with earlier kernels.
# brctl stp br0 on [ 169.672008] BUG: sleeping function called from invalid context at kernel/mutex.c:86 [ 169.672532] in_atomic():1, irqs_disabled():0 [ 169.672832] [ 169.672832] Call Trace: [ 169.673406] [<ffffffff8125db3a>] mutex_lock+0x19/0x2f [ 169.673696] [<ffffffff81067135>] __alloc_pages+0x71/0x2d3 [ 169.673996] [<ffffffff881e0abc>] :bridge:set_stp_state+0x12/0x37 [ 169.674293] [<ffffffff881e0a0a>] :bridge:store_bridge_parm+0x5f/0x79 [ 169.674587] [<ffffffff810ceb24>] sysfs_write_file+0xf2/0x134 [ 169.674879] [<ffffffff8108b3ad>] vfs_write+0xce/0x177 [ 169.675170] [<ffffffff8108b970>] sys_write+0x45/0x6e [ 169.675463] [<ffffffff8100bc8e>] system_call+0x7e/0x83 [ 169.675769] [ 169.676139] br0: starting userspace STP failed, staring kernel STP
# brctl stp br0 off [ 171.774500] BUG: sleeping function called from invalid context at kernel/mutex.c:86 [ 171.775040] in_atomic():1, irqs_disabled():0 [ 171.775327] [ 171.775328] Call Trace: [ 171.775906] [<ffffffff8125db3a>] mutex_lock+0x19/0x2f [ 171.776195] [<ffffffff81067135>] __alloc_pages+0x71/0x2d3 [ 171.776496] [<ffffffff881e0abc>] :bridge:set_stp_state+0x12/0x37 [ 171.776792] [<ffffffff881e0a0a>] :bridge:store_bridge_parm+0x5f/0x79 [ 171.777086] [<ffffffff810ceb24>] sysfs_write_file+0xf2/0x134 [ 171.777378] [<ffffffff8108b3ad>] vfs_write+0xce/0x177 [ 171.777669] [<ffffffff8108b970>] sys_write+0x45/0x6e [ 171.777958] [<ffffffff8100bc8e>] system_call+0x7e/0x83 [ 171.778250]
Daniel K.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Bridge locking for /sys/class/net/br0/bridge/stp_enabled
was wrong. Another bug in bridge utilities makes it such that this interface, meant it wasn't being used. The locking needs to be removed from set_stp_state(), the lock is already acquired down in br_stp_start()/br_stp_stop.
net/bridge/br_sysfs_br.c +0 -2
@@ -150,9 +150,7 @@ static ssize_t show_stp_state(struct devstatic void set_stp_state (struct net_bridge *br,unsigned long val) {rtnl_lock (); -spin_unlock_bh (&br->lock);br_stp_set_enabled (br, val); -spin_lock_bh (&br->lock);rtnl_unlock (); }
Quoted 5 lines · Stephen Hemminger
Bridge locking for
/sys/class/net/br0/bridge/stp_enabledwas wrong. Another bug in bridge utilities makes it such that this interface, meant it wasn't being used. The locking needs to be removed fromset_stp_state(), the lock is already acquired down in br_stp_start()/br_stp_stop.
The 'locking' in set_stp_state() is actually dropping the lock around the br_stp_set_enabled() invocation, not acquiring it:
Quoted 9 lines
net/bridge/br_sysfs_br.c +0
-2@@ -150,9 +150,7 @@ static ssize_t show_stp_state(struct devstatic void set_stp_state (struct net_bridge *br,unsigned long val) {rtnl_lock (); -spin_unlock_bh (&br->lock);br_stp_set_enabled (br, val); -spin_lock_bh (&br->lock);rtnl_unlock (); }
PATCH 1/1 bridge: sysfs locking fix.
Forget earlier patch, it is wrong...
The stp change code generates "sleeping function called from invalid context" because rtnl_lock() called with BH disabled. This fixes it by not acquiring then dropping the bridge lock.
net/bridge/br_sysfs_br.c +15 -9
@@ -147,20 +147,26 @@ static ssize_t show_stp_state(struct devreturn sprintf (buf,"%d\n" , br->stp_enabled); } -static void set_stp_state (struct net_bridge *br,unsigned long val) -{ -rtnl_lock (); -spin_unlock_bh (&br->lock); -br_stp_set_enabled (br, val); -spin_lock_bh (&br->lock); -rtnl_unlock (); -}static ssize_t store_stp_state (struct device *d,struct device_attribute *attr,const char *buf,size_t len) { -return store_bridge_parm (d, buf, len, set_stp_state); +struct net_bridge *br =to_bridge (d); +char *endp; +unsigned long val; + +if (!capable (CAP_NET_ADMIN )) +return -EPERM ; + + val =simple_strtoul (buf, &endp,0 ); +if (endp == buf) +return -EINVAL ; + +rtnl_lock (); +br_stp_set_enabled (br, val); +rtnl_unlock (); + } static DEVICE_ATTR(stp_state, S_IRUGO | S_IWUSR, show_stp_state, store_stp_state);
Re: [PATCH] bridge: sysfs locking fix.
From: Stephen Hemminger <[email protected]>
Date: Tue, 14 Aug 2007 14:50:52 +0100
Quoted 7 lines
Forget earlier patch, it is wrong...
The stp change code generates "sleeping function called from invalid context" because
rtnl_lock()called with BH disabled. This fixes it by not acquiring then dropping the bridge lock.Signed-off-by Stephen Hemminger <[email protected]>
Applied, thanks Stephen.
Re: [PATCH] bridge: sysfs locking fix.
In reply to Stephen HemmingerQuoted 33 lines · Stephen Hemminger
Forget earlier patch, it is wrong...
The stp change code generates "sleeping function called from invalid context" because
rtnl_lock()called with BH disabled. This fixes it by not acquiring then dropping the bridge lock.Signed-off-by Stephen Hemminger <[email protected]>net/bridge/br_sysfs_br.c +5
-9@@ -147,20 +147,26 @@ static ssize_t show_stp_state(struct devreturn sprintf (buf,"%d\n" , br->stp_enabled); } -static void set_stp_state (struct net_bridge *br,unsigned long val) -{ -rtnl_lock (); -spin_unlock_bh (&br->lock); -br_stp_set_enabled (br, val); -spin_lock_bh (&br->lock); -rtnl_unlock (); -}static ssize_t store_stp_state (struct device *d,struct device_attribute *attr,const char *buf,size_t len) { -return store_bridge_parm (d, buf, len, set_stp_state); +struct net_bridge *br =to_bridge (d); +char *endp; +unsigned long val; + +if (!capable (CAP_NET_ADMIN ))
net/bridge/br_sysfs_br.c static ssize_t show_stp_state(struct dev · line 169
Quoted 10 lines
net/bridge/br_sysfs_br.c +10
-0+return -EPERM ; + + val =simple_strtoul (buf, &endp,0 ); +if (endp == buf) +return -EINVAL ; + +rtnl_lock (); +br_stp_set_enabled (br, val); +rtnl_unlock (); +
Shouldn't len value be returned at the end of the function ?
Quoted 4 lines
net/bridge/br_sysfs_br.c +0
-1} static DEVICE_ATTR(stp_state, S_IRUGO | S_IWUSR, show_stp_state, store_stp_state); -