Changeset 24101 in vbox

Timestamp:

Oct 27, 2009 12:03:09 AM (15 years ago)

Author:

vboxsync

Message:

DrvSCSI,DevLsiLogic: Moved the reset request sync down to the driver where DevBusLogic can make use of it as well.

Location:

trunk

Files:

• include/VBox/pdmifs.h (modified) (1 diff)
• src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp (modified) (2 diffs)
• src/VBox/Devices/Storage/DrvSCSI.cpp (modified) (7 diffs)

trunk/include/VBox/pdmifs.h

@@ -2561 +2561 @@
      DECLR3CALLBACKMEMBER(int, pfnSCSIRequestSend, (PPDMISCSICONNECTOR pInterface, PPDMSCSIREQUEST pSCSIRequest));
 
-    /**
-     * Wait for all current requests to complete.
-     *
-     * This can only be called while the VM is running.
-     *
-     * @returns VINF_SUCCESS, VERR_NO_MEMORY, VERR_INVALID_STATE or VERR_TIMEOUT.
-     * @param   pInterface      Pointer to this interface.
-     * @param   cMillies        The timeout in milliseconds or RT_INDEFINITE_WAIT.
-     */
-     DECLR3CALLBACKMEMBER(int, pfnSyncronizeRequests, (PPDMISCSICONNECTOR pInterface, uint32_t cMillies));
-
 } PDMISCSICONNECTOR;
 

trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp

@@ -4781 +4781 @@
 }
 
-/**
- * Waits until all I/O operations on all devices are complete.
- *
- * @retruns Flag which indicates if all I/O completed in the given timeout.
- * @param   pLsiLogic    Pointer to the dveice instance to check.
- * @param   cMillis      Timeout in milliseconds to wait per device.
- */
-static bool lsilogicWaitForAsyncIOFinished(PLSILOGICSCSI pLsiLogic, unsigned cMillies)
-{
-    uint64_t u64Start;
-    unsigned i;
-
-    /*
-     * Wait for any pending async operation to finish
-     */
-    u64Start = RTTimeMilliTS();
-    for (;;)
-    {
-        /* Check every port. */
-        for (i = 0; i < RT_ELEMENTS(pLsiLogic->aDeviceStates); i++)
-            if (ASMAtomicReadU32(&pLsiLogic->aDeviceStates[i].cOutstandingRequests))
-                break;
-        if (i >= RT_ELEMENTS(pLsiLogic->aDeviceStates))
-            return true;
-
-        uint64_t cMsElapsed = RTTimeMilliTS() - u64Start;
-        if (cMsElapsed >= cMillies)
-            return false;
-
-        /* Try synchronize the request queue for this device. */
-        PPDMISCSICONNECTOR pDrvSCSIConnector = pLsiLogic->aDeviceStates[i].pDrvSCSIConnector;
-        uint32_t cMsLeft = cMillies - (uint32_t)cMsElapsed;
-        pDrvSCSIConnector->pfnSyncronizeRequests(pDrvSCSIConnector, RT_MAX(cMsLeft, 100));
-    }
-}
-
 static DECLCALLBACK(int) lsilogicLiveExec(PPDMDEVINS pDevIns, PSSMHANDLE pSSM, uint32_t uPass)
 {
@@ -5159 +5123 @@
     PLSILOGICSCSI pLsiLogic = PDMINS_2_DATA(pDevIns, PLSILOGICSCSI);
     int rc;
-
-/** @todo r=bird: this is a deadlock trap. We're EMT(0), if there are
- *        outstanding requests they may require EMT interaction because of
- *        physical write backs around lsilogicDeviceSCSIRequestCompleted...
- *
- *        I have some more generic solution for delayed suspend, reset and
- *        poweroff handling that I'm considering.  The idea is that the
- *        notification callback returns a status indicating that it didn't
- *        complete and needs to be called again or something.  EMT continues on
- *        the next device and when it's done, it processes incoming requests and
- *        does another notification round... This way we could combine the waits
- *        in the I/O controllers and reduce the time it takes to suspend a VM a
- *        wee bit...
- *
- * DrvSCSI should implement the reset notification, then we could retire this
- * fun lsilogicWaitForAsyncIOFinished code.  (The drivers are reset before the
- * device.)  The deadlock trap is still there though.
- */
-    bool fIdle = lsilogicWaitForAsyncIOFinished(pLsiLogic, 20000);
-    Assert(fIdle);
 
     rc = lsilogicHardReset(pLsiLogic);

trunk/src/VBox/Devices/Storage/DrvSCSI.cpp

@@ -89 +89 @@
     /** Queue for passing the requests to the thread. */
     PRTREQQUEUE             pQueueRequests;
+    /** Request that we've left pending on wakeup or reset. */
+    PRTREQ                  pPendingDummyReq;
     /** Release statistics: number of bytes written. */
     STAMCOUNTER             StatBytesWritten;
@@ -671 +673 @@
 
 /**
- * Request function to syncronize the request execution.
+ * Dummy request function used by drvscsiReset to wait for all pending requests
+ * to complete prior to the device reset.
  *
  * @returns VINF_SUCCESS.
@@ -716 +719 @@
 }
 
+/**
+ * Deals with any pending dummy request
+ *
+ * @returns true if no pending dummy request, false if still pending.
+ * @param   pThis               The instance data.
+ * @param   cMillies            The number of milliseconds to wait for any
+ *                              pending request to finish.
+ */
+static bool drvscsiAsyncIOLoopNoPendingDummy(PDRVSCSI pThis, uint32_t cMillies)
+{
+    if (!pThis->pPendingDummyReq)
+        return false;
+    int rc = RTReqWait(pThis->pPendingDummyReq, cMillies);
+    if (RT_FAILURE(rc))
+        return false;
+    RTReqFree(pThis->pPendingDummyReq);
+    pThis->pPendingDummyReq = NULL;
+    return true;
+}
+
 static int drvscsiAsyncIOLoopWakeup(PPDMDRVINS pDrvIns, PPDMTHREAD pThread)
 {
-    int rc;
     PDRVSCSI pThis = PDMINS_2_DATA(pDrvIns, PDRVSCSI);
     PRTREQ pReq;
+    int rc;
 
     AssertMsgReturn(pThis->pQueueRequests, ("pQueueRequests is NULL\n"), VERR_INVALID_STATE);
 
+    if (!drvscsiAsyncIOLoopNoPendingDummy(pThis, 10000 /* 10 sec */))
+    {
+        LogRel(("drvscsiAsyncIOLoopWakeup#%u: previous dummy request is still pending\n", pDrvIns->iInstance));
+        return VERR_TIMEOUT;
+    }
+
     rc = RTReqCall(pThis->pQueueRequests, &pReq, 10000 /* 10 sec. */, (PFNRT)drvscsiAsyncIOLoopWakeupFunc, 0);
-    AssertMsgRC(rc, ("Inserting request into queue failed rc=%Rrc\n", rc));
     if (RT_SUCCESS(rc))
         RTReqFree(pReq);
-    /*else: leak it */
+    else
+    {
+        pThis->pPendingDummyReq = pReq;
+        LogRel(("drvscsiAsyncIOLoopWakeup#%u: %Rrc pReq=%p\n", pDrvIns->iInstance, rc, pReq));
+    }
 
     return rc;
@@ -748 +780 @@
 
     return VINF_SUCCESS;
-}
-
-/** @copydoc PDMISCSICONNECTOR::pfnSyncronizeRequests. */
-static DECLCALLBACK(int) drvscsiSyncronizeRequests(PPDMISCSICONNECTOR pInterface, uint32_t cMillies)
-{
-    int rc;
-    PDRVSCSI pThis = PDMISCSICONNECTOR_2_DRVSCSI(pInterface);
-    PRTREQ pReq;
-
-    Assert(cMillies > 100);
-    AssertReturn(pThis->pQueueRequests, VERR_INVALID_STATE);
-
-    rc = RTReqCall(pThis->pQueueRequests, &pReq, cMillies, (PFNRT)drvscsiAsyncIOLoopSyncCallback, 0);
-    AssertMsgRC(rc, ("Inserting request into queue failed rc=%Rrc\n", rc));
-    if (RT_SUCCESS(rc))
-        RTReqFree(pReq);
-    /*else: leak it */
-
-    return rc;
 }
 
@@ -790 +803 @@
 
 /**
+ * @copydoc FNPDMDRVRESET
+ */
+static DECLCALLBACK(void) drvscsiReset(PPDMDRVINS pDrvIns)
+{
+    PDRVSCSI pThis = PDMINS_2_DATA(pDrvIns, PDRVSCSI);
+    int rc;
+
+    /*
+     * Try make sure any pending I/O has completed now.
+     */
+    if (pThis->pQueueRequests)
+    {
+        if (!drvscsiAsyncIOLoopNoPendingDummy(pThis, 20000 /*ms*/))
+        {
+            LogRel(("drvscsiReset#%u: previous dummy request is still pending\n", pDrvIns->iInstance));
+            return;
+        }
+
+        if (RTReqIsBusy(pThis->pQueueRequests))
+        {
+            PRTREQ pReq;
+            rc = RTReqCall(pThis->pQueueRequests, &pReq, 20000 /*ms*/, (PFNRT)drvscsiAsyncIOLoopSyncCallback, 0);
+            if (RT_SUCCESS(rc))
+                RTReqFree(pReq);
+            else
+            {
+                pThis->pPendingDummyReq = pReq;
+                LogRel(("drvscsiReset#%u: %Rrc pReq=%p\n", pDrvIns->iInstance, rc, pReq));
+            }
+        }
+    }
+/** @todo r=bird: this is a deadlock trap. We're EMT(0), if there are
+ *        outstanding requests they may require EMT interaction because of
+ *        physical write backs around lsilogicDeviceSCSIRequestCompleted...
+ *
+ *        I have some more generic solution for delayed suspend, reset and
+ *        poweroff handling that I'm considering.  The idea is that the
+ *        notification callback returns a status indicating that it didn't
+ *        complete and needs to be called again or something.  EMT continues on
+ *        the next device and when it's done, it processes incoming requests and
+ *        does another notification round... This way we could combine the waits
+ *        in the I/O controllers and reduce the time it takes to suspend a VM a
+ *        wee bit...
+ */
+}
+
+/**
  * Destruct a driver instance.
  *
@@ -826 +886 @@
     pDrvIns->IBase.pfnQueryInterface                    = drvscsiQueryInterface;
     pThis->ISCSIConnector.pfnSCSIRequestSend            = drvscsiRequestSend;
-    pThis->ISCSIConnector.pfnSyncronizeRequests         = drvscsiSyncronizeRequests;
 
     /*
@@ -929 +988 @@
     NULL,
     /* pfnReset */
-    NULL,
+    drvscsiReset,
     /* pfnSuspend */
     NULL,