USB: ehci: drop workaround for forced irq threading

Force-threaded interrupt handlers used to run with interrupts enabled, something which could lead to deadlocks in case a threaded handler shared a lock with code running in hard interrupt context (e.g. timer callbacks) and did not explicitly disable interrupts.

Since commit 81e2073c175b ("genirq: Disable interrupts for force threaded handlers") interrupt handlers always run with interrupts disabled on non-RT so that drivers no longer need to do handle forced threading ("threadirqs").

Drop the now obsolete workaround added by commit a1227f3c1030 ("usb:
ehci: fix deadlock when threadirqs option is used").

---
 drivers/usb/host/ehci-hcd.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

index 1926b328b6aa..403bd3d6991f 100644
@@ -705,15 +705,8 @@ 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;
 	int			bh;
-	unsigned long		flags;
 
-	/*
-	 * For threadirqs option we use spin_lock_irqsave() variant to prevent
-	 * deadlock with ehci hrtimer callback, because hrtimer callbacks run
-	 * in interrupt context even when threadirqs is specified. We can go
-	 * back to spin_lock() variant when hrtimer callbacks become threaded.
-	 */
-	spin_lock_irqsave(&ehci->lock, flags);
+	spin_lock(&ehci->lock);
 
 	status = ehci_readl(ehci, &ehci->regs->status);
 

@@ -731,7 +724,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
 
 	/* Shared IRQ? */
 	if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) {
-		spin_unlock_irqrestore(&ehci->lock, flags);
+		spin_unlock(&ehci->lock);
 		return IRQ_NONE;
 	}
 

@@ -842,7 +835,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
 
 	if (bh)
 		ehci_work (ehci);
-	spin_unlock_irqrestore(&ehci->lock, flags);
+	spin_unlock(&ehci->lock);
 	if (pcd_status)
 		usb_hcd_poll_rh_status(hcd);
 	return IRQ_HANDLED;

2.26.3

What happens on RT systems? Are they smart enough to avoid the whole problem by enabling interrupts during _all_ callbacks?

Alan Stern

tl;dr: Yes.

The referenced commit (id 81e2073c175b) disables interrupts only on !RT configs so for RT everything remains unchanged (the backports are already adjusted for the old stable trees to use the proper CONFIG_* for enabled RT).

All hrtimer callbacks run as HRTIMER_MODE_SOFT by default. The HRTIMER_MODE_HARD ones (which expire in HARDIRQ context) were audited / explicitly enabled.
The same goes irq_work.
The printk code is different compared to mainline. A printk() on RT in HARDIRQ context is printed once the HARDIRQ context is left. So the serial/console/… driver never gets a chance to acquire its lock in hardirq context.

An interrupt handler which is not forced-threaded must be marked as such and must not use any spinlock_t based locking. lockdep/might_sleep complain here already.

Sebastian

Okay, in that case: