Hi,
Just booted into my shiny new #everything and got:
[ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
[ 40.220119] in_atomic():1, irqs_disabled():0
[ 40.230514] Call Trace:
[ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
[ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
[ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
[ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
[ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
[ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
[ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
[ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
[ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
[ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
[ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
[ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
[ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
[ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
[ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
[ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
[ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
[ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
I haven't found yet where the atomic is entered...
johannes
Michael Buesch wrote:
> On Sunday 16 December 2007 17:20:19 Johannes Berg wrote:
>> Hi,
>>
>> Just booted into my shiny new #everything and got:
>>
>> [ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
>> [ 40.220119] in_atomic():1, irqs_disabled():0
>> [ 40.230514] Call Trace:
>> [ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
>> [ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
>> [ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
>> [ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
>> [ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
>> [ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
>> [ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
>> [ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
>> [ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
>> [ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
>> [ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
>> [ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
>> [ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
>> [ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
>> [ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
>> [ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
>> [ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
>> [ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
>>
>> I haven't found yet where the atomic is entered...
>>
>> johannes
>>
>
> I have no idea where we enter atomic state.
> I am using a slightly older kernel but also with the patch Larry
> mentioned applied. This doesn't happen here.
> Could you please try putting an WARN_ON(in_atomic()) assertion into
> b43_probe? Though, I'm pretty sure it won't trigger.
> I'd rather say mac80211 is aquireing some spinlock in the
> register_hw codepath.
>
> Larry, note that this is not related to the b43 LEDs code.
> This assertion triggers at the place where we register the
> mac80211 TX, RX and radio LED triggers. The place where b43
> initialized rfkill and leds is not in the b43_probe stage.
I do not see this problem with my BCM4311/2 on x86_64 hardware.
Larry
> I have no idea where we enter atomic state.
Uh huh...
drivers/leds/led-triggers.c:led_trigger_register:
| read_lock(&leds_list_lock);
| list_for_each_entry(led_cdev, &leds_list, node) {
| down_write(&led_cdev->trigger_lock);
introduced in
commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
Author: Richard Purdie <[email protected]>
Date: Sat Nov 10 13:29:04 2007 +0000
leds: Fix led trigger locking bugs
I guess the read_lock needs to be a mutex/rw semaphore.
johannes
On Sunday 16 December 2007 21:43:19 Richard Purdie wrote:
> On Sun, 2007-12-16 at 21:38 +0100, Johannes Berg wrote:
> > > > drivers/leds/led-triggers.c:led_trigger_register:
> > > >
> > > > | read_lock(&leds_list_lock);
> > > > | list_for_each_entry(led_cdev, &leds_list, node) {
> > > > | down_write(&led_cdev->trigger_lock);
> > > >
> > > > introduced in
> > > >
> > > > commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
> > > > Author: Richard Purdie <[email protected]>
> > > > Date: Sat Nov 10 13:29:04 2007 +0000
> > > >
> > > > leds: Fix led trigger locking bugs
> > > >
> > > > I guess the read_lock needs to be a mutex/rw semaphore.
> > >
> > > Uh, yes. Was this patch tested at all?
> >
> > Not with default triggers, I guess. The code in question only triggers
> > when a default is assigned to any LED I think.
>
> Amongst other things it was tested on the Zaurus which has default
> triggers. What is the problem you're seeing?
You are scheduling (down_write) while holding a spinlock (leds_list_lock).
--
Greetings Michael.
On Sun, 2007-12-16 at 21:38 +0100, Johannes Berg wrote:
> > > drivers/leds/led-triggers.c:led_trigger_register:
> > >
> > > | read_lock(&leds_list_lock);
> > > | list_for_each_entry(led_cdev, &leds_list, node) {
> > > | down_write(&led_cdev->trigger_lock);
> > >
> > > introduced in
> > >
> > > commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
> > > Author: Richard Purdie <[email protected]>
> > > Date: Sat Nov 10 13:29:04 2007 +0000
> > >
> > > leds: Fix led trigger locking bugs
> > >
> > > I guess the read_lock needs to be a mutex/rw semaphore.
> >
> > Uh, yes. Was this patch tested at all?
>
> Not with default triggers, I guess. The code in question only triggers
> when a default is assigned to any LED I think.
Amongst other things it was tested on the Zaurus which has default
triggers. What is the problem you're seeing?
Richard
On Sun, 2007-12-16 at 22:32 +0000, Richard Purdie wrote:
> On Sun, 2007-12-16 at 23:17 +0100, Johannes Berg wrote:
> > > Eeek, yes, its the LED core's fault.
> > > dc47206e552c0850ad11f7e9a1fca0a3c92f5d65 is needed for various reasons
> > > but that means the patch below is also needed. Its been compile tested,
> > > once its runtime tested I'll push asap.
> >
> > Any reason you're using an rw semaphore rather than a simple mutex?
> > Nothing here is a hot-path, is it, and the led registrations are
> > mutually exclusive anyway, no?
>
> With a rw lock, the for loop of the trigger registration can happen in
> parallel with other trigger registrations. This case isn't a hot path as
> such but it keeps consistent style with other parts of the LED core
> where rw locks are used in hot paths.
Ok, just curious really, nothing that I really care about :)
johannes
> Not with default triggers, I guess. The code in question only triggers
> when a default is assigned to any LED I think.
Come to think of it, it only triggers if the LED-trigger is registered
before the LED, unrelated of whether we have defaults or not.
johannes
On Sun, Dec 16, 2007 at 09:19:34AM -0800, Larry Finger wrote:
> Johannes Berg wrote:
>> Hi,
>>
>> Just booted into my shiny new #everything and got:
>>
>> [ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
>> [ 40.220119] in_atomic():1, irqs_disabled():0
>> [ 40.230514] Call Trace:
>> [ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
>> [ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
>> [ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
>> [ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
>> [ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
>> [ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
>> [ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
>> [ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
>> [ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
>> [ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
>> [ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
>> [ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
>> [ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
>> [ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
>> [ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
>> [ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
>> [ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
>> [ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
>>
>> I haven't found yet where the atomic is entered...
>
> Is your system "vanilla" wireless-2.6, or does it include the LED switch
> patch (http://marc.info/?l=linux-wireless&m=119763729709783&w=2)?
Not sure what you mean by "vanilla"...that patch is in
wireless-2.6#everything as of last night.
There are not any new mac80211 patches in there yet -- does your fix
depend on a pending mac80211 patch?
FWIW, I don't see anything obvious in a quick code inspection.
I presume that Larry and Michael tested this on the previous iteration
of wireless-2.6 which was based on 2.6.24-rc3. Any chance that
something in the rc3->rc5 difference accounts for this?
John
--
John W. Linville
[email protected]
On Sunday 16 December 2007 20:59:36 Johannes Berg wrote:
>
> > I have no idea where we enter atomic state.
>
> Uh huh...
>
> drivers/leds/led-triggers.c:led_trigger_register:
>
> | read_lock(&leds_list_lock);
> | list_for_each_entry(led_cdev, &leds_list, node) {
> | down_write(&led_cdev->trigger_lock);
>
> introduced in
>
> commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
> Author: Richard Purdie <[email protected]>
> Date: Sat Nov 10 13:29:04 2007 +0000
>
> leds: Fix led trigger locking bugs
>
> I guess the read_lock needs to be a mutex/rw semaphore.
Uh, yes. Was this patch tested at all?
--
Greetings Michael.
Johannes Berg wrote:
> Hi,
>
> Just booted into my shiny new #everything and got:
>
> [ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
> [ 40.220119] in_atomic():1, irqs_disabled():0
> [ 40.230514] Call Trace:
> [ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
> [ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
> [ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
> [ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
> [ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
> [ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
> [ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
> [ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
> [ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
> [ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
> [ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
> [ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
> [ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
> [ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
> [ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
> [ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
> [ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
> [ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
>
> I haven't found yet where the atomic is entered...
Is your system "vanilla" wireless-2.6, or does it include the LED switch patch
(http://marc.info/?l=linux-wireless&m=119763729709783&w=2)?
Larry
> > drivers/leds/led-triggers.c:led_trigger_register:
> >
> > | read_lock(&leds_list_lock);
> > | list_for_each_entry(led_cdev, &leds_list, node) {
> > | down_write(&led_cdev->trigger_lock);
> >
> > introduced in
> >
> > commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
> > Author: Richard Purdie <[email protected]>
> > Date: Sat Nov 10 13:29:04 2007 +0000
> >
> > leds: Fix led trigger locking bugs
> >
> > I guess the read_lock needs to be a mutex/rw semaphore.
>
> Uh, yes. Was this patch tested at all?
Not with default triggers, I guess. The code in question only triggers
when a default is assigned to any LED I think.
johannes
> Is your system "vanilla" wireless-2.6, or does it include the LED switch patch
> (http://marc.info/?l=linux-wireless&m=119763729709783&w=2)?
It's "vanilla" wireless-2.6#everything but that patch is present as
commit 81a3136e64abd1a1d93a48c742e4fd50c3565d2a afaict
johannes
On Sun, 2007-12-16 at 21:57 +0100, Michael Buesch wrote:
> On Sunday 16 December 2007 21:43:19 Richard Purdie wrote:
> > On Sun, 2007-12-16 at 21:38 +0100, Johannes Berg wrote:
> > > > > drivers/leds/led-triggers.c:led_trigger_register:
> > > > >
> > > > > | read_lock(&leds_list_lock);
> > > > > | list_for_each_entry(led_cdev, &leds_list, node) {
> > > > > | down_write(&led_cdev->trigger_lock);
> > > > >
> > > > > introduced in
> > > > >
> > > > > commit dc47206e552c0850ad11f7e9a1fca0a3c92f5d65
> > > > > Author: Richard Purdie <[email protected]>
> > > > > Date: Sat Nov 10 13:29:04 2007 +0000
> > > > >
> > > > > leds: Fix led trigger locking bugs
> > > > >
> > > > > I guess the read_lock needs to be a mutex/rw semaphore.
> > > >
> > > > Uh, yes. Was this patch tested at all?
> > >
> > > Not with default triggers, I guess. The code in question only triggers
> > > when a default is assigned to any LED I think.
> >
> > Amongst other things it was tested on the Zaurus which has default
> > triggers. What is the problem you're seeing?
>
> You are scheduling (down_write) while holding a spinlock (leds_list_lock).
Eeek, yes, its the LED core's fault.
dc47206e552c0850ad11f7e9a1fca0a3c92f5d65 is needed for various reasons
but that means the patch below is also needed. Its been compile tested,
once its runtime tested I'll push asap.
Richard
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index ba8b04b..64c66b3 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -106,9 +106,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
goto err_out;
/* add to the list of leds */
- write_lock(&leds_list_lock);
+ down_write(&leds_list_lock);
list_add_tail(&led_cdev->node, &leds_list);
- write_unlock(&leds_list_lock);
+ up_write(&leds_list_lock);
#ifdef CONFIG_LEDS_TRIGGERS
init_rwsem(&led_cdev->trigger_lock);
@@ -155,9 +155,9 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
device_unregister(led_cdev->dev);
- write_lock(&leds_list_lock);
+ down_write(&leds_list_lock);
list_del(&led_cdev->node);
- write_unlock(&leds_list_lock);
+ up_write(&leds_list_lock);
}
EXPORT_SYMBOL_GPL(led_classdev_unregister);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 9b015f9..5d1ca10 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -14,11 +14,11 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/module.h>
-#include <linux/spinlock.h>
+#include <linux/rwsem.h>
#include <linux/leds.h>
#include "leds.h"
-DEFINE_RWLOCK(leds_list_lock);
+DECLARE_RWSEM(leds_list_lock);
LIST_HEAD(leds_list);
EXPORT_SYMBOL_GPL(leds_list);
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 0bdb786..13c9026 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -169,7 +169,7 @@ int led_trigger_register(struct led_trigger *trigger)
up_write(&triggers_list_lock);
/* Register with any LEDs that have this as a default trigger */
- read_lock(&leds_list_lock);
+ down_read(&leds_list_lock);
list_for_each_entry(led_cdev, &leds_list, node) {
down_write(&led_cdev->trigger_lock);
if (!led_cdev->trigger && led_cdev->default_trigger &&
@@ -177,7 +177,7 @@ int led_trigger_register(struct led_trigger *trigger)
led_trigger_set(led_cdev, trigger);
up_write(&led_cdev->trigger_lock);
}
- read_unlock(&leds_list_lock);
+ up_read(&leds_list_lock);
return 0;
}
@@ -212,14 +212,14 @@ void led_trigger_unregister(struct led_trigger *trigger)
up_write(&triggers_list_lock);
/* Remove anyone actively using this trigger */
- read_lock(&leds_list_lock);
+ down_read(&leds_list_lock);
list_for_each_entry(led_cdev, &leds_list, node) {
down_write(&led_cdev->trigger_lock);
if (led_cdev->trigger == trigger)
led_trigger_set(led_cdev, NULL);
up_write(&led_cdev->trigger_lock);
}
- read_unlock(&leds_list_lock);
+ up_read(&leds_list_lock);
}
void led_trigger_unregister_simple(struct led_trigger *trigger)
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index f2f3884..12b6fe9 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -14,6 +14,7 @@
#define __LEDS_H_INCLUDED
#include <linux/device.h>
+#include <linux/rwsem.h>
#include <linux/leds.h>
static inline void led_set_brightness(struct led_classdev *led_cdev,
@@ -26,7 +27,7 @@ static inline void led_set_brightness(struct led_classdev *led_cdev,
led_cdev->brightness_set(led_cdev, value);
}
-extern rwlock_t leds_list_lock;
+extern struct rw_semaphore leds_list_lock;
extern struct list_head leds_list;
#ifdef CONFIG_LEDS_TRIGGERS
> Amongst other things it was tested on the Zaurus which has default
> triggers. What is the problem you're seeing?
Odd. The problem is this:
> [ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
> [ 40.220119] in_atomic():1, irqs_disabled():0
> [ 40.230514] Call Trace:
> [ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
> [ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
> [ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
> [ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
> [ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
> [ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
> [ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
> [ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
> [ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
> [ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
> [ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
> [ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
> [ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
> [ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
> [ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
> [ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
> [ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
> [ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
johannes
On Sunday 16 December 2007 17:20:19 Johannes Berg wrote:
> Hi,
>
> Just booted into my shiny new #everything and got:
>
> [ 40.209739] BUG: sleeping function called from invalid context at kernel/rwsem.c:47
> [ 40.220119] in_atomic():1, irqs_disabled():0
> [ 40.230514] Call Trace:
> [ 40.240799] [eec2fc90] [c0009198] show_stack+0x4c/0x1ac (unreliable)
> [ 40.251247] [eec2fcd0] [c0027a3c] __might_sleep+0xd0/0xf0
> [ 40.261620] [eec2fce0] [c004c1d0] down_write+0x24/0x64
> [ 40.271974] [eec2fcf0] [c026347c] led_trigger_register+0xbc/0x118
> [ 40.282273] [eec2fd10] [f250246c] ieee80211_led_init+0x68/0x160 [mac80211]
> [ 40.292837] [eec2fd30] [f24eceb0] ieee80211_register_hw+0x200/0x324 [mac80211]
> [ 40.303295] [eec2fd40] [f2523234] b43_probe+0x9a8/0xa04 [b43]
> [ 40.313766] [eec2fda0] [f20b06e8] ssb_device_probe+0x50/0xac [ssb]
> [ 40.324152] [eec2fdb0] [c0214518] driver_probe_device+0xb8/0x1e8
> [ 40.334541] [eec2fdd0] [c021487c] __driver_attach+0xf8/0x124
> [ 40.344833] [eec2fdf0] [c0213674] bus_for_each_dev+0x58/0x94
> [ 40.355100] [eec2fe20] [c0214328] driver_attach+0x24/0x34
> [ 40.365321] [eec2fe30] [c0213af4] bus_add_driver+0x98/0x208
> [ 40.375513] [eec2fe50] [c0214b44] driver_register+0x58/0xa0
> [ 40.385771] [eec2fe60] [f20aff58] __ssb_driver_register+0x2c/0x3c [ssb]
> [ 40.396017] [eec2fe70] [f103e034] b43_init+0x34/0xe8 [b43]
> [ 40.406253] [eec2fe80] [c00587e4] sys_init_module+0x154/0x176c
> [ 40.416498] [eec2ff40] [c0012054] ret_from_syscall+0x0/0x38
>
> I haven't found yet where the atomic is entered...
>
> johannes
>
I have no idea where we enter atomic state.
I am using a slightly older kernel but also with the patch Larry
mentioned applied. This doesn't happen here.
Could you please try putting an WARN_ON(in_atomic()) assertion into
b43_probe? Though, I'm pretty sure it won't trigger.
I'd rather say mac80211 is aquireing some spinlock in the
register_hw codepath.
Larry, note that this is not related to the b43 LEDs code.
This assertion triggers at the place where we register the
mac80211 TX, RX and radio LED triggers. The place where b43
initialized rfkill and leds is not in the b43_probe stage.
--
Greetings Michael.
> There are not any new mac80211 patches in there yet -- does your fix
> depend on a pending mac80211 patch?
It shouldn't. There weren't any relevant patches to mac80211 that I'm
aware of.
> FWIW, I don't see anything obvious in a quick code inspection.
Me neither.
> I presume that Larry and Michael tested this on the previous iteration
> of wireless-2.6 which was based on 2.6.24-rc3. Any chance that
> something in the rc3->rc5 difference accounts for this?
Probably. I think lockdep might tell you where the atomic was entered
but I don't have it on this system.
johannes
On Sun, 2007-12-16 at 23:17 +0100, Johannes Berg wrote:
> > Eeek, yes, its the LED core's fault.
> > dc47206e552c0850ad11f7e9a1fca0a3c92f5d65 is needed for various reasons
> > but that means the patch below is also needed. Its been compile tested,
> > once its runtime tested I'll push asap.
>
> Any reason you're using an rw semaphore rather than a simple mutex?
> Nothing here is a hot-path, is it, and the led registrations are
> mutually exclusive anyway, no?
With a rw lock, the for loop of the trigger registration can happen in
parallel with other trigger registrations. This case isn't a hot path as
such but it keeps consistent style with other parts of the LED core
where rw locks are used in hot paths.
It could be replaced with a mutex instead though...
Richard
> Eeek, yes, its the LED core's fault.
> dc47206e552c0850ad11f7e9a1fca0a3c92f5d65 is needed for various reasons
> but that means the patch below is also needed. Its been compile tested,
> once its runtime tested I'll push asap.
Any reason you're using an rw semaphore rather than a simple mutex?
Nothing here is a hot-path, is it, and the led registrations are
mutually exclusive anyway, no?
johannes