Messages in this thread | | | Date | Mon, 12 Nov 2012 20:31:54 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [REGRESSION] 3.7-rc3+git hard lockup on CPU after inserting/removing USB stick |
| |
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@intel.com> > > > 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@intel.com> > > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > > > > > 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
| |