Re: [REGRESSION] 3.7-rc3+git hard lockup on CPU after inserting/removingUSB stick

[Thomas Gleixner]

On Mon, 12 Nov 2012, Martin Steigerwald wrote:
> Am Sonntag, 11. November 2012 schrieb Liu, Chuansheng:
> > > The first bad commit is:
> > > 
> > > commit 73d4066055e0e2830533041f4b91df8e6e5976ff
> > > Author: Chuansheng Liu <chuansheng.liu@xxxxxxxxx>
> > > Date:   Tue Sep 11 16:00:30 2012 +0800
> > > 
> > >     USB/host: Cleanup unneccessary irq disable code
> > > 
> > >     Because the IRQF_DISABLED as the flag is now a NOOP and has been
> > >     deprecated and in hardirq context the interrupt is disabled.
> > > 
> > >     so in usb/host code:
> > >     Removing the usage of flag IRQF_DISABLED;
> > >     Removing the calling local_irq save/restore actions in irq
> > >     handler usb_hcd_irq();
> > > 
> > >     Signed-off-by: liu chuansheng <chuansheng.liu@xxxxxxxxx>
> > >     Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > >     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > 
> > > 
> > > But:
> > > 
> > > This ony happens with threadirqs option!
> > > 
> > > When I remove threadirqs from kernel command line and reboot with this
> > > last bisect kernel USB sticks work.
> > > 
> > > That may explain why nobody else has seen this.
> > > 
> > > So I will try a 3.7-rc4 now, but without threadirqs enabled.
> > > 
> > Thanks your pointing out, the USB HCD irq handler is designed to
> > execute in irq handler with irq disabled.  When threadirqs is in
> > commandline, it will be executed in thread context with local irq
> > enabling, which causes this hardlockup.

No. The problem is caused by the commit above. USB with threaded
interrupt handlers worked perfectly fine in the past.
 
> > --- a/drivers/usb/core/hcd.c
> > +++ b/drivers/usb/core/hcd.c
> > @@ -2349,7 +2349,7 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd,
> >         if (hcd->driver->irq) {
> >                 snprintf(hcd->irq_descr, sizeof(hcd->irq_descr), "%s:usb%d",
> >                                 hcd->driver->description, hcd->self.busnum);
> > -               retval = request_irq(irqnum, &usb_hcd_irq, irqflags,
> > +               retval = request_irq(irqnum, &usb_hcd_irq, irqflags|IRQF_NO_THREAD,
> >                                 hcd->irq_descr, hcd);

NAK. This is exactly the wrong thing to do.

We want to be able to run that code in an handler thread. So you
removed the local_irq_save/restore() in the driver code and with
forced threaded irqs this breaks. Now setting IRQF_NO_THREAD is just
working around the problem that the above commit broke it.

There is no hard requirement to run USB interrupts in hard interrupt
context. I'd rather see the above commit reverted and then a proper
analysis done why removing local_irq_save/restore() breaks forced
threaded interrupt handlers.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/