* BUG: when using 'brctl stp'
@ 2007-08-10 22:15 Daniel K.
[not found] ` <20070812012037.0195b610.akpm@linux-foundation.org>
0 siblings, 1 reply; 6+ messages in thread
From: Daniel K. @ 2007-08-10 22:15 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread[parent not found: <20070812012037.0195b610.akpm@linux-foundation.org>]
* Re: BUG: when using 'brctl stp' [not found] ` <20070812012037.0195b610.akpm@linux-foundation.org> @ 2007-08-14 13:11 ` Stephen Hemminger 2007-08-14 13:18 ` Lennert Buytenhek 0 siblings, 1 reply; 6+ messages in thread From: Stephen Hemminger @ 2007-08-14 13:11 UTC (permalink / raw) To: Andrew Morton, David S. Miller; +Cc: netdev, bridge 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. Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org> --- a/net/bridge/br_sysfs_br.c 2007-07-16 14:24:18.000000000 +0100 +++ b/net/bridge/br_sysfs_br.c 2007-08-14 13:44:23.000000000 +0100 @@ -150,9 +150,7 @@ static ssize_t show_stp_state(struct dev 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(); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG: when using 'brctl stp' 2007-08-14 13:11 ` Stephen Hemminger @ 2007-08-14 13:18 ` Lennert Buytenhek 2007-08-14 13:50 ` [PATCH] bridge: sysfs locking fix Stephen Hemminger 0 siblings, 1 reply; 6+ messages in thread From: Lennert Buytenhek @ 2007-08-14 13:18 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Andrew Morton, David S. Miller, netdev, bridge On Tue, Aug 14, 2007 at 02:11:05PM +0100, Stephen Hemminger wrote: > 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. The 'locking' in set_stp_state() is actually dropping the lock around the br_stp_set_enabled() invocation, not acquiring it: > @@ -150,9 +150,7 @@ static ssize_t show_stp_state(struct dev > 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(); > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] bridge: sysfs locking fix. 2007-08-14 13:18 ` Lennert Buytenhek @ 2007-08-14 13:50 ` Stephen Hemminger 2007-08-14 20:21 ` David Miller 2007-08-24 8:18 ` Daniel Lezcano 0 siblings, 2 replies; 6+ messages in thread From: Stephen Hemminger @ 2007-08-14 13:50 UTC (permalink / raw) To: Lennert Buytenhek; +Cc: Andrew Morton, David S. Miller, netdev, bridge 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 <shemminger@linux-foundation.org> --- a/net/bridge/br_sysfs_br.c 2007-08-06 09:26:48.000000000 +0100 +++ b/net/bridge/br_sysfs_br.c 2007-08-14 14:29:52.000000000 +0100 @@ -147,20 +147,26 @@ static ssize_t show_stp_state(struct dev return 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); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: sysfs locking fix. 2007-08-14 13:50 ` [PATCH] bridge: sysfs locking fix Stephen Hemminger @ 2007-08-14 20:21 ` David Miller 2007-08-24 8:18 ` Daniel Lezcano 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2007-08-14 20:21 UTC (permalink / raw) To: shemminger; +Cc: buytenh, akpm, netdev, bridge From: Stephen Hemminger <shemminger@linux-foundation.org> Date: Tue, 14 Aug 2007 14:50:52 +0100 > 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 <shemminger@linux-foundation.org> Applied, thanks Stephen. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: sysfs locking fix. 2007-08-14 13:50 ` [PATCH] bridge: sysfs locking fix Stephen Hemminger 2007-08-14 20:21 ` David Miller @ 2007-08-24 8:18 ` Daniel Lezcano 1 sibling, 0 replies; 6+ messages in thread From: Daniel Lezcano @ 2007-08-24 8:18 UTC (permalink / raw) To: Stephen Hemminger Cc: Lennert Buytenhek, Andrew Morton, David S. Miller, netdev, bridge Stephen Hemminger wrote: > 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 <shemminger@linux-foundation.org> > > --- a/net/bridge/br_sysfs_br.c 2007-08-06 09:26:48.000000000 +0100 > +++ b/net/bridge/br_sysfs_br.c 2007-08-14 14:29:52.000000000 +0100 > @@ -147,20 +147,26 @@ static ssize_t show_stp_state(struct dev > return 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(); > + Shouldn't len value be returned at the end of the function ? > } > static DEVICE_ATTR(stp_state, S_IRUGO | S_IWUSR, show_stp_state, > store_stp_state); > - ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-08-24 8:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-10 22:15 BUG: when using 'brctl stp' Daniel K.
[not found] ` <20070812012037.0195b610.akpm@linux-foundation.org>
2007-08-14 13:11 ` Stephen Hemminger
2007-08-14 13:18 ` Lennert Buytenhek
2007-08-14 13:50 ` [PATCH] bridge: sysfs locking fix Stephen Hemminger
2007-08-14 20:21 ` David Miller
2007-08-24 8:18 ` Daniel Lezcano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).