Changeset 28947 in vbox

Timestamp:

Apr 30, 2010 9:17:27 PM (15 years ago)

Author:

vboxsync

Message:

AsyncCompletion: Fix an I/O hang after the manager grew. The I/O manager should check for pending requests afterwards

Location:

trunk/src/VBox/VMM

Files:

• PDMAsyncCompletionFile.cpp (modified) (6 diffs)
• PDMAsyncCompletionFileCache.cpp (modified) (2 diffs)
• PDMAsyncCompletionFileNormal.cpp (modified) (7 diffs)

trunk/src/VBox/VMM/PDMAsyncCompletionFile.cpp

@@ -21 +21 @@
 *******************************************************************************/
 #define LOG_GROUP LOG_GROUP_PDM_ASYNC_COMPLETION
-//#define DEBUG
 #include "PDMInternal.h"
 #include <VBox/pdm.h>
@@ -259 +258 @@
     PPDMASYNCCOMPLETIONTASKFILE pTaskFile = (PPDMASYNCCOMPLETIONTASKFILE)pvUser;
 
+    LogFlowFunc(("pTask=%#p pvUser=%#p rc=%Rrc\n", pTask, pvUser, rc));
+
     if (pTask->enmTransferType == PDMACTASKFILETRANSFER_FLUSH)
     {
@@ -276 +277 @@
             pdmR3AsyncCompletionCompleteTask(&pTaskFile->Core, pTaskFile->rc, true);
     }
+}
+
+DECLINLINE(void) pdmacFileEpTaskInit(PPDMASYNCCOMPLETIONTASK pTask, size_t cbTransfer)
+{
+    PPDMASYNCCOMPLETIONTASKFILE pTaskFile = (PPDMASYNCCOMPLETIONTASKFILE)pTask;
+
+    Assert((uint32_t)cbTransfer == cbTransfer && (int32_t)cbTransfer >= 0);
+    ASMAtomicWriteS32(&pTaskFile->cbTransferLeft, (int32_t)cbTransfer);
+    ASMAtomicWriteBool(&pTaskFile->fCompleted, false);
+    ASMAtomicWriteS32(&pTaskFile->rc, VINF_SUCCESS);
 }
 
@@ -290 +301 @@
     Assert(   (enmTransfer == PDMACTASKFILETRANSFER_READ)
            || (enmTransfer == PDMACTASKFILETRANSFER_WRITE));
-
-    Assert((uint32_t)cbTransfer == cbTransfer && (int32_t)cbTransfer >= 0);
-    ASMAtomicWriteS32(&pTaskFile->cbTransferLeft, (int32_t)cbTransfer);
-    ASMAtomicWriteBool(&pTaskFile->fCompleted, false);
-    ASMAtomicWriteS32(&pTaskFile->rc, VINF_SUCCESS);
 
     for (unsigned i = 0; i < cSegments; i++)
@@ -970 +976 @@
     PPDMASYNCCOMPLETIONENDPOINTFILE pEpFile = (PPDMASYNCCOMPLETIONENDPOINTFILE)pEndpoint;
 
+    LogFlowFunc(("pTask=%#p pEndpoint=%#p off=%RTfoff paSegments=%#p cSegments=%zu cbRead=%zu\n",
+                 pTask, pEndpoint, off, paSegments, cSegments, cbRead));
+
     STAM_PROFILE_ADV_START(&pEpFile->StatRead, Read);
+
+    pdmacFileEpTaskInit(pTask, cbRead);
 
     if (pEpFile->fCaching)
@@ -996 +1007 @@
 
     STAM_PROFILE_ADV_START(&pEpFile->StatWrite, Write);
+
+    pdmacFileEpTaskInit(pTask, cbWrite);
 
     if (pEpFile->fCaching)

trunk/src/VBox/VMM/PDMAsyncCompletionFileCache.cpp

@@ -1722 +1722 @@
                  pEndpoint, pEndpoint->Core.pszUri, pTask, off, paSegments, cSegments, cbRead));
 
-    pTask->cbTransferLeft = cbRead;
-    pTask->rc             = VINF_SUCCESS;
     /* Set to completed to make sure that the task is valid while we access it. */
     ASMAtomicWriteBool(&pTask->fCompleted, true);
@@ -1956 +1954 @@
                  pEndpoint, pEndpoint->Core.pszUri, pTask, off, paSegments, cSegments, cbWrite));
 
-    pTask->cbTransferLeft = cbWrite;
-    pTask->rc             = VINF_SUCCESS;
     /* Set to completed to make sure that the task is valid while we access it. */
     ASMAtomicWriteBool(&pTask->fCompleted, true);

trunk/src/VBox/VMM/PDMAsyncCompletionFileNormal.cpp

@@ -446 +446 @@
 
     pEndpoint->AioMgr.pReqsPendingTail = pTaskHead;
+    pTaskHead->pNext = NULL;
 }
 
@@ -466 +467 @@
 
     pEndpoint->AioMgr.pReqsPendingTail = pTask;
+    pTask->pNext = NULL;
 }
 
@@ -560 +562 @@
 
                     if (pTask->cbBounceBuffer)
-                        RTMemFree(pTask->pvBounceBuffer);
+                        RTMemPageFree(pTask->pvBounceBuffer, pTask->cbBounceBuffer);
 
                     pTask->fPrefetch = false;
+                    pTask->cbBounceBuffer = 0;
 
                     /* Free the lock and process pending tasks if neccessary */
@@ -1391 +1394 @@
 
             /* Call completion callback */
+            LogFlow(("Task=%#p completed with %Rrc\n", pTask, rcReq));
             pTask->pfnCompleted(pTask, pTask->pvUser, rcReq);
             pdmacFileTaskFree(pEndpoint, pTask);
@@ -1445 +1449 @@
 
     while (   (pAioMgr->enmState == PDMACEPFILEMGRSTATE_RUNNING)
-           || (pAioMgr->enmState == PDMACEPFILEMGRSTATE_SUSPENDING))
-    {
-        ASMAtomicWriteBool(&pAioMgr->fWaitingEventSem, true);
-        if (!ASMAtomicReadBool(&pAioMgr->fWokenUp))
-            rc = RTSemEventWait(pAioMgr->EventSem, RT_INDEFINITE_WAIT);
-        ASMAtomicWriteBool(&pAioMgr->fWaitingEventSem, false);
-        AssertRC(rc);
-
-        LogFlow(("Got woken up\n"));
-        ASMAtomicWriteBool(&pAioMgr->fWokenUp, false);
+           || (pAioMgr->enmState == PDMACEPFILEMGRSTATE_SUSPENDING)
+           || (pAioMgr->enmState == PDMACEPFILEMGRSTATE_GROWING))
+    {
+        if (!pAioMgr->cRequestsActive)
+        {
+            ASMAtomicWriteBool(&pAioMgr->fWaitingEventSem, true);
+            if (!ASMAtomicReadBool(&pAioMgr->fWokenUp))
+                rc = RTSemEventWait(pAioMgr->EventSem, RT_INDEFINITE_WAIT);
+            ASMAtomicWriteBool(&pAioMgr->fWaitingEventSem, false);
+            AssertRC(rc);
+
+            LogFlow(("Got woken up\n"));
+            ASMAtomicWriteBool(&pAioMgr->fWokenUp, false);
+        }
 
         /* Check for an external blocking event first. */
@@ -1463 +1471 @@
         }
 
-        if (RT_LIKELY(pAioMgr->enmState == PDMACEPFILEMGRSTATE_RUNNING))
+        if (RT_LIKELY(    pAioMgr->enmState == PDMACEPFILEMGRSTATE_RUNNING
+                      ||  pAioMgr->enmState == PDMACEPFILEMGRSTATE_GROWING))
         {
             /* We got woken up because an endpoint issued new requests. Queue them. */
@@ -1546 +1555 @@
                 AssertRC(rc);
                 Assert(pAioMgr->enmState == PDMACEPFILEMGRSTATE_RUNNING);
+
+                rc = pdmacFileAioMgrNormalCheckEndpoints(pAioMgr);
+                CHECK_RC(pAioMgr, rc);
             }
         } /* if still running */
     } /* while running */
 
+    LogFlowFunc(("rc=%Rrc\n", rc));
     return rc;
 }