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!
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!
On Fri, 25 May 2007 14:40:05 -0700 Greg KH <[email protected]> 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);
_
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:[email protected]]
Sent: Friday, May 25, 2007 5:00 PM
To: Greg KH
Cc: Mattia Dongili; Linux Kernel Mailing List; Hayes, Stuart; David
Brownell; [email protected]
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 <[email protected]> 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);
_
On Tue, 29 May 2007 10:14:35 -0500 <[email protected]> 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:[email protected]]
> Sent: Friday, May 25, 2007 5:00 PM
> To: Greg KH
> Cc: Mattia Dongili; Linux Kernel Mailing List; Hayes, Stuart; David
> Brownell; [email protected]
> 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 <[email protected]> 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);
>
> _
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 <[email protected]>
On Thu, May 31, 2007 at 10:26:10AM -0500, [email protected] 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 <[email protected]>
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
Hi,
I remember this one ...
On 6/7/07, Greg KH <[email protected]> wrote:
> On Thu, May 31, 2007 at 10:26:10AM -0500, [email protected] 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 <[email protected]>
>
> 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
On 6/7/07, Satyam Sharma <[email protected]> wrote:
> Hi,
>
> I remember this one ...
>
> On 6/7/07, Greg KH <[email protected]> wrote:
> > On Thu, May 31, 2007 at 10:26:10AM -0500, [email protected] 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 <[email protected]>
> >
> > 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
On Thu, May 31, 2007 at 10:26:10AM -0500, [email protected] 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 <[email protected]>
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