Re: BUG: spinlock bad magic on CPU#1, modprobe/815 - led-triggers

["Vegard Nossum" <vegard.nossum@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Wed, Jul 16, 2008 at 2:24 PM, Zdenek Kabelac
<zdenek.kabelac@gmail.com> wrote:
> Hello
>
> I'm running 2.6.26 kernel - git commit: bce7f793daec3e65ec5c5705d2457b81fe7b5725
> I've T61, 2GB, C2D, x86_64
>
>  I'm getting this oops while using 64bit kernel and running mostly 32bit system.
>
> This one happens sometimes during boot - unpredictible - usually once
> in 15 boots.
>

Thanks for the report.

It looks like the error is in led_classdev_register(); the led_cdev is
added to the leds_list *before* the led_cdev->trigger_lock is
initialized.

(This might also explain why you don't always hit it.)

On the other hand, I might be completely wrong. The following NULL
pointer dereference bothers me somewhat as it isn't entirely clear why
it appears.

I've added a couple of Ccs.

Can you try the patch I've attached?


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

[-- Attachment #2: led-class.patch --]
[-- Type: application/octet-stream, Size: 1222 bytes --]

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 559a408..44d13ac 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -113,13 +113,6 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 	if (rc)
 		goto err_out;
 
-	/* add to the list of leds */
-	down_write(&leds_list_lock);
-	list_add_tail(&led_cdev->node, &leds_list);
-	up_write(&leds_list_lock);
-
-	led_update_brightness(led_cdev);
-
 #ifdef CONFIG_LEDS_TRIGGERS
 	init_rwsem(&led_cdev->trigger_lock);
 
@@ -130,6 +123,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 	led_trigger_set_default(led_cdev);
 #endif
 
+	led_update_brightness(led_cdev);
+
+	/* add to the list of leds */
+	down_write(&leds_list_lock);
+	list_add_tail(&led_cdev->node, &leds_list);
+	up_write(&leds_list_lock);
+
 	printk(KERN_INFO "Registered led device: %s\n",
 			led_cdev->name);
 
@@ -138,7 +138,6 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 #ifdef CONFIG_LEDS_TRIGGERS
 err_out_led_list:
 	device_remove_file(led_cdev->dev, &dev_attr_brightness);
-	list_del(&led_cdev->node);
 #endif
 err_out:
 	device_unregister(led_cdev->dev);