Re: Interrupt storm mitigation needed

[Joerg Sonnenberger <joerg%britannica.bec.de@localhost>]

On Thu, Feb 12, 2015 at 05:02:28AM +0100, Tom Ivar Helbekkmo wrote:
> Joerg Sonnenberger <joerg%britannica.bec.de@localhost> writes:
> 
> > Can you add a counter to uhci.c line 1317? The question for me is
> > whether the device is creating interrupts without actually setting the
> > status to anything different or if it does provide some notification but
> > at a too high rate.
> 
> Well, that was an interesting experiment!  It is, indeed, hitting the
> "return" at that point.  In fact, many more times than the counters
> showed by "vmstat -i" indicate.  Even when I'm not noticing any
> problems, the return counter I installed climbs faster than the number
> of interrupts shown for the ioapic pin, but over the course of a couple
> of back-to-back, longish hangs, the vmstat counter increased by about a
> half million, but my return counter jumped by about 23 million!

Well, let's try the first hack of keep uhci interrupts off and just poll
via timeout.

Joerg

Index: uhci.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/sys/dev/usb/uhci.c,v
retrieving revision 1.264
diff -u -p -r1.264 uhci.c
--- uhci.c	5 Aug 2014 06:35:24 -0000	1.264
+++ uhci.c	12 Feb 2015 08:35:26 -0000
@@ -72,6 +72,11 @@ __KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.2
 /*#define UHCI_CTL_LOOP */
 
 
+Static int uhci_intr1(uhci_softc_t *);
+static void uhci_intr1_wrap(void *sc)
+{
+	uhci_intr1(sc);
+}
 
 #ifdef UHCI_DEBUG
 uhci_softc_t *thesc;
@@ -542,8 +547,11 @@ uhci_init(uhci_softc_t *sc)
 	DPRINTFN(1,("uhci_init: enabling\n"));
 
 	err =  uhci_run(sc, 1, 0);		/* and here we go... */
-	UWRITE2(sc, UHCI_INTR, UHCI_INTR_TOCRCIE | UHCI_INTR_RIE |
-		UHCI_INTR_IOCE | UHCI_INTR_SPIE);	/* enable interrupts */
+
+	callout_init(&sc->sc_simulated_interrupt, CALLOUT_MPSAFE);
+	callout_setfunc(&sc->sc_simulated_interrupt, uhci_intr1_wrap, sc);
+	callout_schedule(&sc->sc_simulated_interrupt, 1);
+
 	return err;
 }
 
@@ -720,8 +728,6 @@ uhci_resume(device_t dv, const pmf_qual_
 	UHCICMD(sc, cmd | UHCI_CMD_FGR); /* force resume */
 	usb_delay_ms_locked(&sc->sc_bus, USB_RESUME_DELAY, &sc->sc_intr_lock);
 	UHCICMD(sc, cmd & ~UHCI_CMD_EGSM); /* back to normal */
-	UWRITE2(sc, UHCI_INTR, UHCI_INTR_TOCRCIE |
-	    UHCI_INTR_RIE | UHCI_INTR_IOCE | UHCI_INTR_SPIE);
 	UHCICMD(sc, UHCI_CMD_MAXP);
 	uhci_run(sc, 1, 1); /* and start traffic again */
 	usb_delay_ms_locked(&sc->sc_bus, USB_RESUME_RECOVERY, &sc->sc_intr_lock);
@@ -735,6 +741,7 @@ uhci_resume(device_t dv, const pmf_qual_
 #endif
 
 	sc->sc_suspend = PWR_RESUME;
+	callout_schedule(&sc->sc_simulated_interrupt, 1);
 	mutex_spin_exit(&sc->sc_intr_lock);
 
 	return true;
@@ -1270,8 +1277,6 @@ uhci_remove_bulk(uhci_softc_t *sc, uhci_
 		sc->sc_bulk_end = pqh;
 }
 
-Static int uhci_intr1(uhci_softc_t *);
-
 int
 uhci_intr(void *arg)
 {
@@ -1290,8 +1295,6 @@ uhci_intr(void *arg)
 		goto done;
 	}
 
-	ret = uhci_intr1(sc);
-
  done:
 	mutex_spin_exit(&sc->sc_intr_lock);
 	return ret;
@@ -1313,8 +1316,10 @@ uhci_intr1(uhci_softc_t *sc)
 	KASSERT(mutex_owned(&sc->sc_intr_lock));
 
 	status = UREAD2(sc, UHCI_STS) & UHCI_STS_ALLINTRS;
-	if (status == 0)	/* The interrupt was not for us. */
+	if (status == 0) {	/* The interrupt was not for us. */
+		callout_schedule(&sc->sc_simulated_interrupt, 1);
 		return (0);
+	}
 
 	if (sc->sc_suspend != PWR_RESUME) {
 #ifdef DIAGNOSTIC
@@ -1322,6 +1327,7 @@ uhci_intr1(uhci_softc_t *sc)
 		       device_xname(sc->sc_dev));
 #endif
 		UWRITE2(sc, UHCI_STS, status); /* acknowledge the ints */
+		callout_schedule(&sc->sc_simulated_interrupt, 1);
 		return (0);
 	}
 
@@ -1368,6 +1374,7 @@ uhci_intr1(uhci_softc_t *sc)
 
 	DPRINTFN(15, ("%s: uhci_intr: exit\n", device_xname(sc->sc_dev)));
 
+	callout_schedule(&sc->sc_simulated_interrupt, 1);
 	return (1);
 }