| Message ID | 20210715213744.GA44506@redhat |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI | expand |
On Thu, Jul 15, 2021 at 05:37:44PM -0400, David Jeffery wrote: > When MSI is used by the ehci-hcd driver, it can cause lost interrupts which > results in EHCI only continuing to work due to a polling fallback. But the > reliance of polling drastically reduces performance of any I/O through EHCI. > > Interrupts are lost as the EHCI interrupt handler does not safely handle > edge-triggered interrupts. It fails to ensure all interrupt status bits are > cleared, which works with level-triggered interrupts but not the > edge-triggered interrupts typical from using MSI. > > To fix this problem, check if the driver may have raced with the hardware > setting additional interrupt status bits and clear status until it is in a > stable state. > > Signed-off-by: David Jeffery <djeffery@redhat.com> > Tested-by: Laurence Oberman <loberman@redhat.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Alan Stern <stern@rowland.harvard.edu> > Fixes: 306c54d0edb6 usb: hcd: Try MSI interrupts on PCI devices Improper format. Add to your .gitconfig the following: [alias] one = show -s --pretty='format:%h (\"%s\")' c = show -s --pretty='format:https://git.kernel.org/torvalds/c/%h' -- With Best Regards, Andy Shevchenko
On Fri, Jul 16, 2021 at 04:36:18PM +0300, Andy Shevchenko wrote: > On Thu, Jul 15, 2021 at 05:37:44PM -0400, David Jeffery wrote: > > When MSI is used by the ehci-hcd driver, it can cause lost interrupts which > > results in EHCI only continuing to work due to a polling fallback. But the > > reliance of polling drastically reduces performance of any I/O through EHCI. > > > > Interrupts are lost as the EHCI interrupt handler does not safely handle > > edge-triggered interrupts. It fails to ensure all interrupt status bits are > > cleared, which works with level-triggered interrupts but not the > > edge-triggered interrupts typical from using MSI. > > > > To fix this problem, check if the driver may have raced with the hardware > > setting additional interrupt status bits and clear status until it is in a > > stable state. > > > > Signed-off-by: David Jeffery <djeffery@redhat.com> > > Tested-by: Laurence Oberman <loberman@redhat.com> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > > Fixes: 306c54d0edb6 usb: hcd: Try MSI interrupts on PCI devices > > Improper format. > > Add to your .gitconfig the following: > > [alias] > one = show -s --pretty='format:%h (\"%s\")' > c = show -s --pretty='format:https://git.kernel.org/torvalds/c/%h' And this [core] abbrev = 12 `git one $COMMIT` will give you a nice representation. -- With Best Regards, Andy Shevchenko
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 36f5bf6a0752..10b0365f3439 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -703,24 +703,28 @@ EXPORT_SYMBOL_GPL(ehci_setup); static irqreturn_t ehci_irq (struct usb_hcd *hcd) { struct ehci_hcd *ehci = hcd_to_ehci (hcd); - u32 status, masked_status, pcd_status = 0, cmd; + u32 status, current_status, masked_status, pcd_status = 0; + u32 cmd; int bh; spin_lock(&ehci->lock); - status = ehci_readl(ehci, &ehci->regs->status); + status = 0; + current_status = ehci_readl(ehci, &ehci->regs->status); +restart: /* e.g. cardbus physical eject */ - if (status == ~(u32) 0) { + if (current_status == ~(u32) 0) { ehci_dbg (ehci, "device removed\n"); goto dead; } + status |= current_status; /* * We don't use STS_FLR, but some controllers don't like it to * remain on, so mask it out along with the other status bits. */ - masked_status = status & (INTR_MASK | STS_FLR); + masked_status = current_status & (INTR_MASK | STS_FLR); /* Shared IRQ? */ if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) { @@ -730,6 +734,12 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) /* clear (just) interrupts */ ehci_writel(ehci, masked_status, &ehci->regs->status); + + /* For edge interrupts, don't race with an interrupt bit being raised */ + current_status = ehci_readl(ehci, &ehci->regs->status); + if (current_status & INTR_MASK) + goto restart; + cmd = ehci_readl(ehci, &ehci->regs->command); bh = 0;