There's a deadlock in isoio() on a multiprocessor, arising  from a
conflict in order of acquiring locks.  (Not just theory: I've seen
it happen on my dual-processor Epia system.)  Consider this scenario:

isoio() on cpu1:
1 isoio calls sleep(&e->wr, isoready, e) /sys/src/9/pc/usbuhci.c:1173
2	sleep calls lock(&e->wr) /sys/src/9/port/proc.c:740
3	sleep calls isoready(e) /sys/src/9/port/proc.c:755
4		isoready calls ilock(&ctlr->activends) /sys/src/9/pc/usbuhci.c:1093
5		isoready calls iunlock(&ctlr->activends) /sys/src/9/pc/usbuhci.c:1095
6	sleep calls unlock(&e->wr) /sys/src/9/port/proc.c:762

interrupt() on cpu0:
7 interrupt calls ilock(&ctlr->activends) /sys/src/9/pc/usbuhci.c:1043
8 interrupt calls cleaniso(e, ) /sys/src/9/pc/usbuhci.c:1052
9	cleaniso calls wakeup(&e->wr) /sys/src/9/pc/usbuhci.c:1015
10		wakeup calls lock(&e->wr) /sys/src/9/port/proc.c:864
11		wakeup calls unlock(&e->wr) /sys/src/9/port/proc.c:876
12 interrupt calls iunlock(&ctlr->activends) /sys/src/9/pc/usbuhci.c:1055

An interleaving of events containing 2 - 7 - 10 - 4 leads to deadlock.

I developed a fix which involved locking the individual endpoints instead
of locking ctlr->activends, but when I checked it with Sape he pointed
out that a simple revision to isoreadyx() would mean that it can be
called without locking at all.  So here's a simple patch based on Sape's
suggestion.  (Tested by playing audio for 24 hours on the above dual-
processor system, without incident.)

-- Richard Miller

Reference: /n/sources/patch/applied/usbuhci-iso-deadlock
Date: Sat Jul  8 16:05:25 CES 2006
Signed-off-by: miller@hamnavoe.com

--- /sys/src/9/pc/usbuhci.c	Sat Jul  8 16:05:15 2006
+++ /sys/src/9/pc/usbuhci.c	Sat Jul  8 16:05:10 2006
@@ -9,6 +9,7 @@
 #include	"usb.h"
 
 #define XPRINT if(debug)print
+#define XXPRINT if(0)print
 
 static int Chatty = 0;
 static int debug = 0;
@@ -1044,22 +1045,22 @@
 	for(e = ctlr->activends.f; e != nil; e = e->activef) {
 		x = e->private;
 		if(!e->iso && x->epq != nil) {
-			XPRINT("cleanq(ctlr, x->epq, 0, 0)\n");
+			XXPRINT("cleanq(ctlr, x->epq, 0, 0)\n");
 			cleanq(ctlr, x->epq, 0, 0);
 		}
 		if(e->iso) {
-			XPRINT("cleaniso(e)\n");
+			XXPRINT("cleaniso(e)\n");
 			cleaniso(e, frnum);
 		}
 	}
 	iunlock(&ctlr->activends);
-	XPRINT("cleanq(ctlr, ctlr->ctlq, 0, 0)\n");
+	XXPRINT("cleanq(ctlr, ctlr->ctlq, 0, 0)\n");
 	cleanq(ctlr, ctlr->ctlq, 0, 0);
-	XPRINT("cleanq(ctlr, ctlr->bulkq, 0, Vf)\n");
+	XXPRINT("cleanq(ctlr, ctlr->bulkq, 0, Vf)\n");
 	cleanq(ctlr, ctlr->bulkq, 0, Vf);
-	XPRINT("clean recvq\n");
+	XXPRINT("clean recvq\n");
 	for (q = ctlr->recvq->next; q; q = q->hlink) {
-		XPRINT("cleanq(ctlr, q, 0, Vf)\n");
+		XXPRINT("cleanq(ctlr, q, 0, Vf)\n");
 		cleanq(ctlr, q, 0, Vf);
 	}
 }
@@ -1074,26 +1075,13 @@
 }
 
 static int
-isoreadyx(Endptx *x)
+isoready(void *a)
 {
-	return x->etd == nil || (x->etd != x->xtd && (x->etd->status & Active) == 0);
-}
-
-static int
-isoready(void *arg)
-{
-	int ret;
-	Ctlr *ctlr;
-	Endpt *e;
 	Endptx *x;
+	TD *etd;
 
-	e = arg;
-	ctlr = e->dev->uh->ctlr;
-	x = e->private;
-	ilock(&ctlr->activends);
-	ret = isoreadyx(x);
-	iunlock(&ctlr->activends);
-	return ret;
+	x = a;
+	return (etd = x->etd) == nil || (etd != x->xtd && (etd->status & Active) == 0);
 }
 
 static long
@@ -1167,10 +1155,10 @@
 				e->off = 0;
 			}else{
 				/* New td, make sure it's ready */
-				while (isoreadyx(x) == 0){
+				while (isoready(x) == 0){
 					isolock = 0;
 					iunlock(&ctlr->activends);
-					sleep(&e->wr, isoready, e);
+					sleep(&e->wr, isoready, x);
 					ilock(&ctlr->activends);
 					isolock = 1;
 				}