[PATCH 3.16 057/233] USB: xhci: fix lock-inversion problem

From: Ben Hutchings
Date: Sat Sep 09 2017 - 19:18:47 EST


3.16.48-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

commit 63aea0dbab90a2461faaae357cbc8cfd6c8de9fe upstream.

With threaded interrupts, bottom-half handlers are called with
interrupts enabled. Therefore they can't safely use spin_lock(); they
have to use spin_lock_irqsave(). Lockdep warns about a violation
occurring in xhci_irq():

=========================================================
[ INFO: possible irq lock inversion dependency detected ]
4.11.0-rc8-dbg+ #1 Not tainted
---------------------------------------------------------
swapper/7/0 just changed the state of lock:
(&(&ehci->lock)->rlock){-.-...}, at: [<ffffffffa0130a69>]
ehci_hrtimer_func+0x29/0xc0 [ehci_hcd]
but this lock took another, HARDIRQ-unsafe lock in the past:
(hcd_urb_list_lock){+.....}

and interrupts could create inverse lock ordering between them.

other info that might help us debug this:
Possible interrupt unsafe locking scenario:

CPU0 CPU1
---- ----
lock(hcd_urb_list_lock);
local_irq_disable();
lock(&(&ehci->lock)->rlock);
lock(hcd_urb_list_lock);
<Interrupt>
lock(&(&ehci->lock)->rlock);
*** DEADLOCK ***

no locks held by swapper/7/0.
the shortest dependencies between 2nd lock and 1st lock:
-> (hcd_urb_list_lock){+.....} ops: 252 {
HARDIRQ-ON-W at:
__lock_acquire+0x602/0x1280
lock_acquire+0xd5/0x1c0
_raw_spin_lock+0x2f/0x40
usb_hcd_unlink_urb_from_ep+0x1b/0x60 [usbcore]
xhci_giveback_urb_in_irq.isra.45+0x70/0x1b0 [xhci_hcd]
finish_td.constprop.60+0x1d8/0x2e0 [xhci_hcd]
xhci_irq+0xdd6/0x1fa0 [xhci_hcd]
usb_hcd_irq+0x26/0x40 [usbcore]
irq_forced_thread_fn+0x2f/0x70
irq_thread+0x149/0x1d0
kthread+0x113/0x150
ret_from_fork+0x2e/0x40

This patch fixes the problem.

Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Reported-and-tested-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci-ring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2690,11 +2690,12 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
union xhci_trb *event_ring_deq;
irqreturn_t ret = IRQ_NONE;
+ unsigned long flags;
dma_addr_t deq;
u64 temp_64;
u32 status;

- spin_lock(&xhci->lock);
+ spin_lock_irqsave(&xhci->lock, flags);
/* Check if the xHC generated the interrupt, or the irq is shared */
status = readl(&xhci->op_regs->status);
if (status == 0xffffffff) {
@@ -2768,7 +2769,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd
ret = IRQ_HANDLED;

out:
- spin_unlock(&xhci->lock);
+ spin_unlock_irqrestore(&xhci->lock, flags);

return ret;
}