[2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[Mattia Dongili]

Hello,

with gregkh-usb-usb-ehci-cpufreq-fix.patch removing ehci-hcd causes the
following BUG:
[  459.800033] BUG: scheduling while atomic: rmmod/0x00000001/4568
[  459.800045]  [<c0104eb3>] dump_trace+0x63/0x1ec
[  459.800055]  [<c0105056>] show_trace_log_lvl+0x1a/0x30
[  459.800066]  [<c0105c82>] show_trace+0x12/0x14
[  459.800099]  [<c0105d0f>] dump_stack+0x16/0x18
[  459.800135]  [<c02eff66>] __sched_text_start+0x56/0x7db
[  459.800142]  [<c02f0768>] wait_for_completion+0x65/0x9b
[  459.800147]  [<c0132c38>] synchronize_rcu+0x2d/0x33
[  459.800154]  [<c0138387>] synchronize_srcu+0x23/0x5f
[  459.800160]  [<c012f6bf>] srcu_notifier_chain_unregister+0x43/0x4d
[  459.800185]  [<c0287b83>] cpufreq_unregister_notifier+0x22/0x32
[  459.800203]  [<f8e85713>] ehci_stop+0x4f/0xb7 [ehci_hcd]
[  459.800248]  [<f8dc48d4>] usb_remove_hcd+0x97/0xd7 [usbcore]
[  459.800280]  [<f8dcdf1d>] usb_hcd_pci_remove+0x18/0x6a [usbcore]
[  459.800317]  [<c01f1ca3>] pci_device_remove+0x1c/0x3d
[  459.800324]  [<c024fc46>] __device_release_driver+0x74/0x90
[  459.800332]  [<c025009f>] driver_detach+0x81/0xc2
[  459.800337]  [<c024f7f9>] bus_remove_driver+0x5d/0x7c
[  459.800342]  [<c025010a>] driver_unregister+0xb/0xd
[  459.800347]  [<c01f1e1d>] pci_unregister_driver+0x13/0x65
[  459.800351]  [<f8e858f8>] ehci_hcd_cleanup+0x10/0x12 [ehci_hcd]
[  459.800360]  [<c0143b11>] sys_delete_module+0x187/0x1ae
[  459.800367]  [<c0103e62>] sysenter_past_esp+0x5f/0x85
[  459.800373]  [<ffffe410>] 0xffffe410
[  459.800384]  =======================

static void ehci_stop (struct usb_hcd *hcd)
{
	...
	spin_lock_irq(&ehci->lock);
	if (HC_IS_RUNNING (hcd->state))
		ehci_quiesce (ehci);

#ifdef CONFIG_CPU_FREQ
	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
				    CPUFREQ_TRANSITION_NOTIFIER);
#endif

-- 
mattia
:wq!

Re: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[Greg KH]

On Mon, May 21, 2007 at 11:44:37AM +0900, Mattia Dongili wrote:
> Hello,
> 
> with gregkh-usb-usb-ehci-cpufreq-fix.patch removing ehci-hcd causes the
> following BUG:

Thanks for letting me know.

Stuart, any help here?

thanks,

greg k-h



> [  459.800033] BUG: scheduling while atomic: rmmod/0x00000001/4568
> [  459.800045]  [<c0104eb3>] dump_trace+0x63/0x1ec
> [  459.800055]  [<c0105056>] show_trace_log_lvl+0x1a/0x30
> [  459.800066]  [<c0105c82>] show_trace+0x12/0x14
> [  459.800099]  [<c0105d0f>] dump_stack+0x16/0x18
> [  459.800135]  [<c02eff66>] __sched_text_start+0x56/0x7db
> [  459.800142]  [<c02f0768>] wait_for_completion+0x65/0x9b
> [  459.800147]  [<c0132c38>] synchronize_rcu+0x2d/0x33
> [  459.800154]  [<c0138387>] synchronize_srcu+0x23/0x5f
> [  459.800160]  [<c012f6bf>] srcu_notifier_chain_unregister+0x43/0x4d
> [  459.800185]  [<c0287b83>] cpufreq_unregister_notifier+0x22/0x32
> [  459.800203]  [<f8e85713>] ehci_stop+0x4f/0xb7 [ehci_hcd]
> [  459.800248]  [<f8dc48d4>] usb_remove_hcd+0x97/0xd7 [usbcore]
> [  459.800280]  [<f8dcdf1d>] usb_hcd_pci_remove+0x18/0x6a [usbcore]
> [  459.800317]  [<c01f1ca3>] pci_device_remove+0x1c/0x3d
> [  459.800324]  [<c024fc46>] __device_release_driver+0x74/0x90
> [  459.800332]  [<c025009f>] driver_detach+0x81/0xc2
> [  459.800337]  [<c024f7f9>] bus_remove_driver+0x5d/0x7c
> [  459.800342]  [<c025010a>] driver_unregister+0xb/0xd
> [  459.800347]  [<c01f1e1d>] pci_unregister_driver+0x13/0x65
> [  459.800351]  [<f8e858f8>] ehci_hcd_cleanup+0x10/0x12 [ehci_hcd]
> [  459.800360]  [<c0143b11>] sys_delete_module+0x187/0x1ae
> [  459.800367]  [<c0103e62>] sysenter_past_esp+0x5f/0x85
> [  459.800373]  [<ffffe410>] 0xffffe410
> [  459.800384]  =======================
> 
> static void ehci_stop (struct usb_hcd *hcd)
> {
> 	...
> 	spin_lock_irq(&ehci->lock);
> 	if (HC_IS_RUNNING (hcd->state))
> 		ehci_quiesce (ehci);
> 
> #ifdef CONFIG_CPU_FREQ
> 	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
> 				    CPUFREQ_TRANSITION_NOTIFIER);
> #endif
> 
> -- 
> mattia
> :wq!

Re: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[Andrew Morton]

On Fri, 25 May 2007 14:40:05 -0700 Greg KH <gregkh@suse.de> wrote:

> On Mon, May 21, 2007 at 11:44:37AM +0900, Mattia Dongili wrote:
> > Hello,
> > 
> > with gregkh-usb-usb-ehci-cpufreq-fix.patch removing ehci-hcd causes the
> > following BUG:
> 
> Thanks for letting me know.
> 
> Stuart, any help here?

pretty obvious.  cpufreq_unregister_notifier() sleeps, and that patch
causes it to be called under spinlock.

Something like this...

--- a/drivers/usb/host/ehci-hcd.c~fix-gregkh-usb-usb-ehci-cpufreq-fix
+++ a/drivers/usb/host/ehci-hcd.c
@@ -452,14 +452,14 @@ static void ehci_stop (struct usb_hcd *h
 	if (HC_IS_RUNNING (hcd->state))
 		ehci_quiesce (ehci);
 
-#ifdef CONFIG_CPU_FREQ
-	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
-				    CPUFREQ_TRANSITION_NOTIFIER);
-#endif
 	ehci_reset (ehci);
 	ehci_writel(ehci, 0, &ehci->regs->intr_enable);
 	spin_unlock_irq(&ehci->lock);
 
+#ifdef CONFIG_CPU_FREQ
+	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
+				    CPUFREQ_TRANSITION_NOTIFIER);
+#endif
 	/* let companion controllers work when we aren't */
 	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
 
_

RE: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[Stuart_Hayes]

Sorry about that.  Would it be helpful if I verified that and sent it in
signed off?

Thanks
Stuart

-----Original Message-----
From: Andrew Morton [mailto:akpm@linux-foundation.org] 
Sent: Friday, May 25, 2007 5:00 PM
To: Greg KH
Cc: Mattia Dongili; Linux Kernel Mailing List; Hayes, Stuart; David
Brownell; linux-usb-devel@lists.sourceforge.net
Subject: Re: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic:
rmmod/0x00000001/4568

On Fri, 25 May 2007 14:40:05 -0700 Greg KH <gregkh@suse.de> wrote:

> On Mon, May 21, 2007 at 11:44:37AM +0900, Mattia Dongili wrote:
> > Hello,
> > 
> > with gregkh-usb-usb-ehci-cpufreq-fix.patch removing ehci-hcd causes 
> > the following BUG:
> 
> Thanks for letting me know.
> 
> Stuart, any help here?

pretty obvious.  cpufreq_unregister_notifier() sleeps, and that patch
causes it to be called under spinlock.

Something like this...

--- a/drivers/usb/host/ehci-hcd.c~fix-gregkh-usb-usb-ehci-cpufreq-fix
+++ a/drivers/usb/host/ehci-hcd.c
@@ -452,14 +452,14 @@ static void ehci_stop (struct usb_hcd *h
 	if (HC_IS_RUNNING (hcd->state))
 		ehci_quiesce (ehci);
 
-#ifdef CONFIG_CPU_FREQ
-	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
-				    CPUFREQ_TRANSITION_NOTIFIER);
-#endif
 	ehci_reset (ehci);
 	ehci_writel(ehci, 0, &ehci->regs->intr_enable);
 	spin_unlock_irq(&ehci->lock);
 
+#ifdef CONFIG_CPU_FREQ
+	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
+				    CPUFREQ_TRANSITION_NOTIFIER);
+#endif
 	/* let companion controllers work when we aren't */
 	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
 
_

Re: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic: rmmod/0x00000001/4568

[Andrew Morton]

On Tue, 29 May 2007 10:14:35 -0500 <Stuart_Hayes@Dell.com> wrote:

> 
> Sorry about that.  Would it be helpful if I verified that and sent it in
> signed off?
> 

Yes please.  The question in my mind was "did it add a race": say, the
notifier chain gets called by some external source after we've gone and
reset the device?


> 
> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org] 
> Sent: Friday, May 25, 2007 5:00 PM
> To: Greg KH
> Cc: Mattia Dongili; Linux Kernel Mailing List; Hayes, Stuart; David
> Brownell; linux-usb-devel@lists.sourceforge.net
> Subject: Re: [2.6.22-rc1-mm1] ehci-hcd - BUG: scheduling while atomic:
> rmmod/0x00000001/4568
> 
> On Fri, 25 May 2007 14:40:05 -0700 Greg KH <gregkh@suse.de> wrote:
> 
> > On Mon, May 21, 2007 at 11:44:37AM +0900, Mattia Dongili wrote:
> > > Hello,
> > > 
> > > with gregkh-usb-usb-ehci-cpufreq-fix.patch removing ehci-hcd causes 
> > > the following BUG:
> > 
> > Thanks for letting me know.
> > 
> > Stuart, any help here?
> 
> pretty obvious.  cpufreq_unregister_notifier() sleeps, and that patch
> causes it to be called under spinlock.
> 
> Something like this...
> 
> --- a/drivers/usb/host/ehci-hcd.c~fix-gregkh-usb-usb-ehci-cpufreq-fix
> +++ a/drivers/usb/host/ehci-hcd.c
> @@ -452,14 +452,14 @@ static void ehci_stop (struct usb_hcd *h
>  	if (HC_IS_RUNNING (hcd->state))
>  		ehci_quiesce (ehci);
>  
> -#ifdef CONFIG_CPU_FREQ
> -	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
> -				    CPUFREQ_TRANSITION_NOTIFIER);
> -#endif
>  	ehci_reset (ehci);
>  	ehci_writel(ehci, 0, &ehci->regs->intr_enable);
>  	spin_unlock_irq(&ehci->lock);
>  
> +#ifdef CONFIG_CPU_FREQ
> +	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
> +				    CPUFREQ_TRANSITION_NOTIFIER);
> +#endif
>  	/* let companion controllers work when we aren't */
>  	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
>  
> _

[linux-usb-devel] [PATCH] bug removing ehci-hcd

[Stuart_Hayes]

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


I wasn't actually able to reproduce the bug myself, but I guess it is
pretty obvious that I shouldn't have called cpufreq_unregister_notifier
with a spinlock held.  I haven't been doing this long enough to know
exactly which kernel this patch should be against, so let me know if
this ins't good.  Thanks!


This patch (for the 2.6.21.3 kernel plus previously sent cpufreq
notifier patch) fixes a bug caused by calling
cpufreq_unregister_notifier (which can sleep) while holding a spinlock.

Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

[-- Attachment #2: ehci_unregister_cpufreq_notifier.patch --]
[-- Type: application/octet-stream, Size: 846 bytes --]

diff -purN linux-2.6.21.3_freqorig/drivers/usb/host/ehci-hcd.c linux-2.6.21.3_freqbugfix/drivers/usb/host/ehci-hcd.c
--- linux-2.6.21.3_freqorig/drivers/usb/host/ehci-hcd.c	2007-05-29 16:05:20.000000000 -0500
+++ linux-2.6.21.3_freqbugfix/drivers/usb/host/ehci-hcd.c	2007-05-29 15:40:41.000000000 -0500
@@ -449,14 +449,14 @@ static void ehci_stop (struct usb_hcd *h
 	/* no more interrupts ... */
 	del_timer_sync (&ehci->watchdog);
 
-	spin_lock_irq(&ehci->lock);
-	if (HC_IS_RUNNING (hcd->state))
-		ehci_quiesce (ehci);
-
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&ehci->cpufreq_transition,
 				    CPUFREQ_TRANSITION_NOTIFIER);
 #endif
+	spin_lock_irq(&ehci->lock);
+	if (HC_IS_RUNNING (hcd->state))
+		ehci_quiesce (ehci);
+
 	ehci_reset (ehci);
 	ehci_writel(ehci, 0, &ehci->regs->intr_enable);
 	spin_unlock_irq(&ehci->lock);

Re: [linux-usb-devel] [PATCH] bug removing ehci-hcd

[Greg KH]

On Thu, May 31, 2007 at 10:26:10AM -0500, Stuart_Hayes@Dell.com wrote:
> 
> I wasn't actually able to reproduce the bug myself, but I guess it is
> pretty obvious that I shouldn't have called cpufreq_unregister_notifier
> with a spinlock held.  I haven't been doing this long enough to know
> exactly which kernel this patch should be against, so let me know if
> this ins't good.  Thanks!
> 
> 
> This patch (for the 2.6.21.3 kernel plus previously sent cpufreq
> notifier patch) fixes a bug caused by calling
> cpufreq_unregister_notifier (which can sleep) while holding a spinlock.
> 
> Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

Hm, this doesn't apply to the 2.6.21.3 kernel.

Can you send both patches merged together?

And is the fix already in Linus's tree?

thanks,

greg k-h

Re: [linux-usb-devel] [PATCH] bug removing ehci-hcd

[Satyam Sharma]

Hi,

I remember this one ...

On 6/7/07, Greg KH <greg@kroah.com> wrote:
> On Thu, May 31, 2007 at 10:26:10AM -0500, Stuart_Hayes@Dell.com wrote:
> >
> > I wasn't actually able to reproduce the bug myself, but I guess it is
> > pretty obvious that I shouldn't have called cpufreq_unregister_notifier
> > with a spinlock held.  I haven't been doing this long enough to know
> > exactly which kernel this patch should be against, so let me know if
> > this ins't good.  Thanks!
> >
> >
> > This patch (for the 2.6.21.3 kernel plus previously sent cpufreq
> > notifier patch) fixes a bug caused by calling
> > cpufreq_unregister_notifier (which can sleep) while holding a spinlock.
> >
> > Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>
>
> Hm, this doesn't apply to the 2.6.21.3 kernel.

The cpufreq patches only live in -mm as of now ...

> Can you send both patches merged together?
>
> And is the fix already in Linus's tree?

Andrew seems to have already fixed this in the latest -mm
(in this very thread, funnily enough, looks like you missed it
as the subject change broke the threading :-)

[ There is a subtle difference, however, in that Andrew's
fix pushes the notifier unregistration /after/ the
spin_unlock_irq(&ehci->lock) critical section whereas Stuart
seems to be prefer doing it /before/ the corresponding
spin_lock_irq() ... ]

Satyam

Re: [linux-usb-devel] [PATCH] bug removing ehci-hcd

[Satyam Sharma]

On 6/7/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> Hi,
>
> I remember this one ...
>
> On 6/7/07, Greg KH <greg@kroah.com> wrote:
> > On Thu, May 31, 2007 at 10:26:10AM -0500, Stuart_Hayes@Dell.com wrote:
> > >
> > > I wasn't actually able to reproduce the bug myself, but I guess it is
> > > pretty obvious that I shouldn't have called cpufreq_unregister_notifier
> > > with a spinlock held.  I haven't been doing this long enough to know
> > > exactly which kernel this patch should be against, so let me know if
> > > this ins't good.  Thanks!
> > >
> > >
> > > This patch (for the 2.6.21.3 kernel plus previously sent cpufreq
> > > notifier patch) fixes a bug caused by calling
> > > cpufreq_unregister_notifier (which can sleep) while holding a spinlock.
> > >
> > > Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>
> >
> > Hm, this doesn't apply to the 2.6.21.3 kernel.
>
> The cpufreq patches only live in -mm as of now ...

Argh, excuse the noise!

> > Can you send both patches merged together?
> >
> > And is the fix already in Linus's tree?

And you clearly meant gregkh-usb-usb-ehci-cpufreq-fix.patch
here ... (btw, no, I don't see this in the latest git)

Satyam

Re: [linux-usb-devel] [PATCH] bug removing ehci-hcd

[Greg KH]

On Thu, May 31, 2007 at 10:26:10AM -0500, Stuart_Hayes@Dell.com wrote:
> 
> I wasn't actually able to reproduce the bug myself, but I guess it is
> pretty obvious that I shouldn't have called cpufreq_unregister_notifier
> with a spinlock held.  I haven't been doing this long enough to know
> exactly which kernel this patch should be against, so let me know if
> this ins't good.  Thanks!
> 
> 
> This patch (for the 2.6.21.3 kernel plus previously sent cpufreq
> notifier patch) fixes a bug caused by calling
> cpufreq_unregister_notifier (which can sleep) while holding a spinlock.
> 
> Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

Thanks, I've merged this with your previous patch.  Sorry for the
previous confusion as to where this patch was applied against (ended up
in my -stable queue...)

thanks,

greg k-h