Home » Mailing list archives » linux-kernel » 2008 » June » 12

Re: [WARNING] local_bh_enable with irqs disabled:

Previous thread: [PATCH] proc: calculate the correct /proc/<pid> link count by Vegard Nossum on Tuesday, June 3, 2008 - 9:16 am. (3 messages)

Next thread: none
[view original message]
From: Guennadi Liakhovetski <g.liakhovetski@...>
To: <linux-kernel@...>
Cc: David Brownell <david-b@...>
Subject: [WARNING] local_bh_enable with irqs disabled:
Date: Tuesday, June 3, 2008 - 9:32 am

Hi


got this whem rmmod-ing a gpiochip driver. IRQs were disabled in

gpiochip_remove().


------------[ cut here ]------------

WARNING: at kernel/softirq.c:141 local_bh_enable+0x8c/0xac()

Modules linked in: max7301(-) pxa2xx_spi vfat fat usbhid

[<c0024bd0>] (dump_stack+0x0/0x14) from [<c0032068>] (warn_on_slowpath+0x4c/0x68)

[<c003201c>] (warn_on_slowpath+0x0/0x68) from [<c003823c>] (local_bh_enable+0x8c/0xac)

 r6:c3eac1e0 r5:c03195f0 r4:c3dde000

[<c00381b0>] (local_bh_enable+0x0/0xac) from [<c01f2764>] (sk_filter+0x34/0x94)

 r5:c3eac1e0 r4:00000000

[<c01f2730>] (sk_filter+0x0/0x94) from [<c01f55b0>] (netlink_broadcast+0x1a4/0x438)

 r5:c3db2010 r4:c3db2000

[<c01f540c>] (netlink_broadcast+0x0/0x438) from [<c0115e18>] (kobject_uevent_env+0x338/0x3ec)

[<c0115ae0>] (kobject_uevent_env+0x0/0x3ec) from [<c0115ee0>] (kobject_uevent+0x14/0x18)

[<c0115ecc>] (kobject_uevent+0x0/0x18) from [<c01538c0>] (device_del+0x13c/0x16c)

[<c0153784>] (device_del+0x0/0x16c) from [<c0153908>] (device_unregister+0x18/0x24)

 r6:40000013 r5:c3eab000 r4:c3eab000

[<c01538f0>] (device_unregister+0x0/0x24) from [<c01258b0>] (gpiochip_remove+0xa0/0x13c)

 r4:c3cfb0dc

[<c0125810>] (gpiochip_remove+0x0/0x13c) from [<bf01d0cc>] (max7301_remove+0x40/0x80 [max7301])

 r6:ffffffed r5:c3eade00 r4:c3cfb0c0

[<bf01d08c>] (max7301_remove+0x0/0x80 [max7301]) from [<c0179888>] (spi_drv_remove+0x24/0x28)

 r6:c3dde000 r5:bf01dcac r4:c3eade00

[<c0179864>] (spi_drv_remove+0x0/0x28) from [<c0155c7c>] (__device_release_driver+0x6c/0xa0)

[<c0155c10>] (__device_release_driver+0x0/0xa0) from [<c01561a8>] (driver_detach+0xcc/0xe0)

 r5:c3eade00 r4:c3eadea8

[<c01560dc>] (driver_detach+0x0/0xe0) from [<c0154ed8>] (bus_remove_driver+0x94/0xbc)

 r7:00000000 r6:c030be4c r5:bf01dcac r4:bf01dce0

[<c0154e44>] (bus_remove_driver+0x0/0xbc) from [<c0156234>] (driver_unregis...
[ message continues ]
[view original message]
From: Andrew Morton <akpm@...>
To: Guennadi Liakhovetski <g.liakhovetski@...>
Cc: <linux-kernel@...>, David Brownell <david-b@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Friday, June 6, 2008 - 1:39 am

Which kernel version are you running?


Please check 2.6.25 - its gpiochip_remove() seems innocent enough.

--
[view original message]
From: Guennadi Liakhovetski <g.liakhovetski@...>
To: Andrew Morton <akpm@...>
Cc: <linux-kernel@...>, David Brownell <david-b@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Friday, June 6, 2008 - 2:11 am

Sorry, this was a 2.6.26-rc3 based kernel with the gpio-sysfs patch from

David, e.g., http://marc.info/?l=linux-kernel&m=121107105300923&w=2,

which introduces a call to device_unregister via gpiochip_unexport(chip); 


No, that one would not have the problem.


Thanks

Guennadi

---

Guennadi Liakhovetski, Ph.D.

Freelance Open-Source Software Developer

--
[view original message]
From: David Brownell <david-b@...>
To: Guennadi Liakhovetski <g.liakhovetski@...>
Cc: Andrew Morton <akpm@...>, <linux-kernel@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Tuesday, June 10, 2008 - 8:15 pm

Right.  Obviously that wasn't tested with "rmmod" of a modular

GPIO expander ... I only have one board which supports such an

expander right now, and it's not been in use recently.


A fix is obvious; any problems with what's below?


- Dave


--- g26.orig/drivers/gpio/gpiolib.c	2008-06-10 16:56:01.000000000 -0700

+++ g26/drivers/gpio/gpiolib.c	2008-06-10 16:55:39.000000000 -0700

@@ -726,12 +726,15 @@ int gpiochip_remove(struct gpio_chip *ch

 		}

 	}

 	if (status == 0) {

-		gpiochip_unexport(chip);

 		for (id = chip->base; id < chip->base + chip->ngpio; id++)

 			gpio_desc[id].chip = NULL;

 	}


 	spin_unlock_irqrestore(&gpio_lock, flags);

+

+	if (status == 0)

+		gpiochip_unexport(chip);

+

 	return status;

 }

 EXPORT_SYMBOL_GPL(gpiochip_remove);


--
[view original message]
From: David Brownell <david-b@...>
To: Guennadi Liakhovetski <g.liakhovetski@...>
Cc: Andrew Morton <akpm@...>, <linux-kernel@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Thursday, June 12, 2008 - 2:28 am

I verified it works for me on that one board ... after I removed

the board hooks that actually *use* those GPIOs (which prevent an

rmmod) and just use it from sysfs.  The missing test case has now

been covered.


I'll send an updated patch.


--
[view original message]
From: Andrew Morton <akpm@...>
To: Guennadi Liakhovetski <g.liakhovetski@...>
Cc: <linux-kernel@...>, David Brownell <david-b@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Friday, June 6, 2008 - 2:41 am

OK, thanks.


That's quite buggy and would have generated so many runtime warnings in

a "developer" setup (rofl) that I look at Documentation/SubmitChecklist

and just weep.


I'll drop it.

--
[view original message]
From: David Brownell <david-b@...>
To: Andrew Morton <akpm@...>, Guennadi Liakhovetski <g.liakhovetski@...>
Cc: <linux-kernel@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Thursday, June 12, 2008 - 3:00 pm

That seems excessive.  I observe a locking bug with a trivial fix;

happened because *one* code path (rmmod -- not often used with GPIOs

once they work) couldn't be tested on most of my test rigs.  It would

produce *ONE* runtime warning on that code path.


Other than missing one test case, the only other significant issue

from SubmitChecklist is that the Documentation/ABI update needs to

hold up until this merges to mainline, since one part of it includes

the date where that interface became available.


So ... what else were you thinking was trouble?


- Dave


--
[view original message]
From: Andrew Morton <akpm@...>
To: David Brownell <david-b@...>
Cc: <g.liakhovetski@...>, <linux-kernel@...>
Subject: Re: [WARNING] local_bh_enable with irqs disabled:
Date: Thursday, June 12, 2008 - 3:27 pm

On Thu, 12 Jun 2008 12:00:03 -0700


The patch had a great string of sysfs operations and mutex-takings all

happening under spinlock.  Obviously all that code hadn't been tested.

I didn't take the time to sit down and analyse where it was all happening.

--
Previous thread: [PATCH] proc: calculate the correct /proc/<pid> link count by Vegard Nossum on Tuesday, June 3, 2008 - 9:16 am. (3 messages)

Next thread: none