Changeset 52148 in vbox

Timestamp:

Jul 23, 2014 12:42:21 PM (11 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

95182

Message:

Devices/USB: ancel URBs on the I/O thread if available, fixes a race causing an assertion

Location:

trunk/src/VBox/Devices/USB

Files:

• VUSBDevice.cpp (modified) (5 diffs)
• VUSBInternal.h (modified) (2 diffs)
• VUSBUrb.cpp (modified) (3 diffs)
• freebsd/USBProxyDevice-freebsd.cpp (modified) (1 diff)

trunk/src/VBox/Devices/USB/VUSBDevice.cpp

@@ -1130 +1130 @@
             vusbUrbDoReapAsyncDev(pDev, RT_INDEFINITE_WAIT);
 
+        /* Process any URBs waiting to be cancelled first. */
+        int rc = RTReqQueueProcess(pDev->hReqQueueCancel, 0); /* Don't wait if there is nothing to do. */
+        Assert(RT_SUCCESS(rc) || rc == VERR_TIMEOUT);
+
         /* Woken up or there is an URB to queue. */
         PRTQUEUEATOMICITEM pHead = RTQueueAtomicRemoveAll(&pDev->QueueUrb);
@@ -1139 +1143 @@
 
             LogFlow(("%s: Queuing URB\n", pUrb->pszDesc));
-            int rc = pUrb->pUsbIns->pReg->pfnUrbQueue(pUrb->pUsbIns, pUrb);
+            rc = pUrb->pUsbIns->pReg->pfnUrbQueue(pUrb->pUsbIns, pUrb);
             if (RT_FAILURE(rc))
             {
@@ -1219 +1223 @@
         rc = rcThread;
 
+    pDev->hUrbIoThread = NIL_RTTHREAD;
+
     return rc;
 }
@@ -1292 +1298 @@
     /* Destroy I/O thread. */
     vusbDevUrbIoThreadDestroy(pDev);
+
+    /* Destroy request queue. */
+    int rc = RTReqQueueDestroy(pDev->hReqQueueCancel);
+    AssertRC(rc);
 
     /*
@@ -1730 +1740 @@
     AssertMsgReturn(pDev->paIfStates, ("RTMemAllocZ(%d) failed\n", cbIface), VERR_NO_MEMORY);
 
+    /* Setup request queue for cancelling URBs synchronously. */
+    rc = RTReqQueueCreate(&pDev->hReqQueueCancel);
+    AssertRCReturn(rc, rc);
+
     rc = vusbDevUrbIoThreadCreate(pDev);
     AssertRCReturn(rc, rc);

trunk/src/VBox/Devices/USB/VUSBInternal.h

@@ -30 +30 @@
 #include <iprt/assert.h>
 #include <iprt/queueatomic.h>
+#include <iprt/req.h>
 
 RT_C_DECLS_BEGIN
@@ -219 +220 @@
     /** Queue of URBs to submit. */
     RTQUEUEATOMIC       QueueUrb;
+    /** Request queue for cancelling URBs on the I/O thread. */
+    RTREQQUEUE          hReqQueueCancel;
     /** Flag whether the URB I/O thread should terminate. */
     bool volatile       fTerminate;

trunk/src/VBox/Devices/USB/VUSBUrb.cpp

@@ -2037 +2037 @@
 }
 
-
-/**
- * Cancels an URB with CRC failure.
- *
- * Cancelling an URB is a tricky thing. The USBProxy backend can not
- * all cancel it and we must keep the URB around until it's ripe and
- * can be reaped the normal way. However, we must complete the URB
- * now, before leaving this function. This is not nice. sigh.
- *
- * This function will cancel the URB if it's in-flight and complete
- * it. The device will in its pfnCancel method be given the chance to
- * say that the URB doesn't need reaping and should be unlinked.
- *
- * An URB which is in the cancel state after pfnCancel will remain in that
- * state and in the async list until its reaped. When it's finally reaped
- * it will be unlinked and freed without doing any completion.
- *
- * There are different modes of canceling an URB. When devices are being
- * disconnected etc., they will be completed with an error (CRC). However,
- * when the HC needs to temporarily halt communication with a device, the
- * URB/TD must be left alone if possible.
- *
+/**
+ * The worker for vusbUrbCancel() which is executed on the I/O thread.
+ *
+ * @returns nothing.
  * @param   pUrb        The URB to cancel.
- * @param   mode        The way the URB should be canceled.
- */
-void vusbUrbCancel(PVUSBURB pUrb, CANCELMODE mode)
+ * @param   enmMode     The way the URB should be canceled.
+ */
+static void vusbUrbCancelWorker(PVUSBURB pUrb, CANCELMODE enmMode)
 {
     vusbUrbAssert(pUrb);
@@ -2102 +2084 @@
     {
         AssertMsg(pUrb->enmState == VUSBURBSTATE_CANCELLED, ("Invalid state %d, pUrb=%p\n", pUrb->enmState, pUrb));
-        switch (mode)
+        switch (enmMode)
         {
             default:
@@ -2117 +2099 @@
 }
 
+/**
+ * Cancels an URB with CRC failure.
+ *
+ * Cancelling an URB is a tricky thing. The USBProxy backend can not
+ * all cancel it and we must keep the URB around until it's ripe and
+ * can be reaped the normal way. However, we must complete the URB
+ * now, before leaving this function. This is not nice. sigh.
+ *
+ * This function will cancel the URB if it's in-flight and complete
+ * it. The device will in its pfnCancel method be given the chance to
+ * say that the URB doesn't need reaping and should be unlinked.
+ *
+ * An URB which is in the cancel state after pfnCancel will remain in that
+ * state and in the async list until its reaped. When it's finally reaped
+ * it will be unlinked and freed without doing any completion.
+ *
+ * There are different modes of canceling an URB. When devices are being
+ * disconnected etc., they will be completed with an error (CRC). However,
+ * when the HC needs to temporarily halt communication with a device, the
+ * URB/TD must be left alone if possible.
+ *
+ * @param   pUrb        The URB to cancel.
+ * @param   mode        The way the URB should be canceled.
+ */
+void vusbUrbCancel(PVUSBURB pUrb, CANCELMODE mode)
+{
+    PRTREQ hReq = NULL;
+
+    if (pUrb->VUsb.pDev->hUrbIoThread != NIL_RTTHREAD)
+    {
+        int rc = RTReqQueueCallVoid(pUrb->VUsb.pDev->hReqQueueCancel, &hReq, 0,
+                                    (PFNRT)vusbUrbCancelWorker, 2, pUrb, mode);
+        Assert(RT_SUCCESS(rc) || rc == VERR_TIMEOUT);
+        vusbDevUrbIoThreadWakeup(pUrb->VUsb.pDev);
+        rc = RTReqWait(hReq, RT_INDEFINITE_WAIT);
+        AssertRC(rc);
+    }
+    else /* Call the worker directly. */
+        vusbUrbCancelWorker(pUrb, mode);
+}
+
 
 /**

trunk/src/VBox/Devices/USB/freebsd/USBProxyDevice-freebsd.cpp

@@ -673 +673 @@
  * @copydoc USBPROXYBACK::pfnUrbQueue
  */
-static DECLCALLBACK(int) usbProxyFreeBSDUrbQueue(PVUSBURB pUrb)
-{
-    PUSBPROXYDEV pProxyDev = PDMINS_2_DATA(pUrb->pUsbIns, PUSBPROXYDEV);
+static DECLCALLBACK(int) usbProxyFreeBSDUrbQueue(PUSBPROXYDEV pProxyDev, PVUSBURB pUrb)
+{
     PUSBPROXYDEVFBSD pDevFBSD = USBPROXYDEV_2_DATA(pProxyDev, PUSBPROXYDEVFBSD);
     PUSBENDPOINTFBSD pEndpointFBSD;