Changeset 36302 in vbox for trunk/src/VBox/VMM

Timestamp:

Mar 17, 2011 12:17:18 PM (14 years ago)

Author:

vboxsync

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

70608

Message:

BlockCache: Fix race condition which can cause losing completed requests. Remove now unused variables

Location:

trunk/src/VBox/VMM

Files:

• VMMR3/PDMBlkCache.cpp (modified) (25 diffs)
• include/PDMBlkCacheInternal.h (modified) (2 diffs)

trunk/src/VBox/VMM/VMMR3/PDMBlkCache.cpp

@@ -481 +481 @@
 }
 
-DECLINLINE(int) pdmBlkCacheEnqueue(PPDMBLKCACHE pBlkCache, uint64_t off, PPDMBLKCACHEIOXFER pIoXfer)
+DECLINLINE(int) pdmBlkCacheEnqueue(PPDMBLKCACHE pBlkCache, uint64_t off, size_t cbXfer, PPDMBLKCACHEIOXFER pIoXfer)
 {
     int rc = VINF_SUCCESS;
+
+    LogFlowFunc(("%s: Enqueuing hIoXfer=%#p enmXferDir=%d\n",
+                 __FUNCTION__, pIoXfer, pIoXfer->enmXferDir));
 
     switch (pBlkCache->enmType)
@@ -491 +494 @@
             rc = pBlkCache->u.Dev.pfnXferEnqueue(pBlkCache->u.Dev.pDevIns,
                                                  pIoXfer->enmXferDir,
-                                                 off, pIoXfer->cbXfer,
+                                                 off, cbXfer,
                                                  &pIoXfer->SgBuf, pIoXfer);
             break;
@@ -499 +502 @@
             rc = pBlkCache->u.Drv.pfnXferEnqueue(pBlkCache->u.Drv.pDrvIns,
                                                  pIoXfer->enmXferDir,
-                                                 off, pIoXfer->cbXfer,
+                                                 off, cbXfer,
                                                  &pIoXfer->SgBuf, pIoXfer);
             break;
@@ -507 +510 @@
             rc = pBlkCache->u.Usb.pfnXferEnqueue(pBlkCache->u.Usb.pUsbIns,
                                                  pIoXfer->enmXferDir,
-                                                 off, pIoXfer->cbXfer,
+                                                 off, cbXfer,
                                                  &pIoXfer->SgBuf, pIoXfer);
             break;
@@ -515 +518 @@
             rc = pBlkCache->u.Int.pfnXferEnqueue(pBlkCache->u.Int.pvUser,
                                                  pIoXfer->enmXferDir,
-                                                 off, pIoXfer->cbXfer,
+                                                 off, cbXfer,
                                                  &pIoXfer->SgBuf, pIoXfer);
             break;
@@ -523 +526 @@
     }
 
+    LogFlowFunc(("%s: returns rc=%Rrc\n", __FUNCTION__, rc));
     return rc;
 }
@@ -550 +554 @@
     pIoXfer->SgSeg.pvSeg = pEntry->pbData;
     pIoXfer->SgSeg.cbSeg = pEntry->cbData;
-    pIoXfer->cbXfer      = pEntry->cbData;
     pIoXfer->enmXferDir  = PDMBLKCACHEXFERDIR_READ;
     RTSgBufInit(&pIoXfer->SgBuf, &pIoXfer->SgSeg, 1);
 
-    return pdmBlkCacheEnqueue(pBlkCache, pEntry->Core.Key, pIoXfer);
+    return pdmBlkCacheEnqueue(pBlkCache, pEntry->Core.Key, pEntry->cbData, pIoXfer);
 }
 
@@ -581 +584 @@
     pIoXfer->SgSeg.pvSeg = pEntry->pbData;
     pIoXfer->SgSeg.cbSeg = pEntry->cbData;
-    pIoXfer->cbXfer      = pEntry->cbData;
     pIoXfer->enmXferDir  = PDMBLKCACHEXFERDIR_WRITE;
     RTSgBufInit(&pIoXfer->SgBuf, &pIoXfer->SgSeg, 1);
 
-    return pdmBlkCacheEnqueue(pBlkCache, pEntry->Core.Key, pIoXfer);
+    return pdmBlkCacheEnqueue(pBlkCache, pEntry->Core.Key, pEntry->cbData, pIoXfer);
 }
 
@@ -612 +614 @@
     pIoXfer->fIoCache    = false;
     pIoXfer->pReq        = pReq;
-    pIoXfer->cbXfer      = cbData;
     pIoXfer->enmXferDir  = enmXferDir;
     if (pSgBuf)
@@ -620 +621 @@
     }
 
-    return pdmBlkCacheEnqueue(pBlkCache, offStart, pIoXfer);
+    return pdmBlkCacheEnqueue(pBlkCache, offStart, cbData, pIoXfer);
 }
 
@@ -1854 +1855 @@
 }
 
-static PPDMBLKCACHEREQ pdmBlkCacheReqAlloc(size_t cbXfer, void *pvUser)
+static PPDMBLKCACHEREQ pdmBlkCacheReqAlloc(void *pvUser)
 {
     PPDMBLKCACHEREQ pReq = (PPDMBLKCACHEREQ)RTMemAlloc(sizeof(PDMBLKCACHEREQ));
@@ -1861 +1862 @@
     {
         pReq->pvUser = pvUser;
-        pReq->cbXfer = cbXfer;
         pReq->rcReq  = VINF_SUCCESS;
         pReq->cXfersPending = 0;
@@ -1905 +1905 @@
 
 static bool pdmBlkCacheReqUpdate(PPDMBLKCACHE pBlkCache, PPDMBLKCACHEREQ pReq,
-                                 size_t cbComplete, int rcReq, bool fCallHandler)
+                                 int rcReq, bool fCallHandler)
 {
     if (RT_FAILURE(rcReq))
         ASMAtomicCmpXchgS32(&pReq->rcReq, rcReq, VINF_SUCCESS);
 
-    AssertMsg(pReq->cbXfer >= cbComplete, ("Completed more than left\n"));
     AssertMsg(pReq->cXfersPending > 0, ("No transfers are pending for this request\n"));
-    uint32_t cbOld = ASMAtomicSubU32(&pReq->cbXfer, cbComplete);
     uint32_t cXfersPending = ASMAtomicDecU32(&pReq->cXfersPending);
 
-    if (   !(cbOld - cbComplete)
-        && !cXfersPending)
+    if (!cXfersPending)
     {
         if (fCallHandler)
@@ -1923 +1920 @@
     }
 
-    LogFlowFunc(("pReq=%#p cXfersPending=%u cbXfer=%u\n", pReq, cXfersPending, (cbOld - cbComplete)));
+    LogFlowFunc(("pReq=%#p cXfersPending=%u\n", pReq, cXfersPending));
     return false;
 }
@@ -1945 +1942 @@
 
     /* Allocate new request structure. */
-    pReq = pdmBlkCacheReqAlloc(cbRead, pvUser);
+    pReq = pdmBlkCacheReqAlloc(pvUser);
     if (RT_UNLIKELY(!pReq))
         return VERR_NO_MEMORY;
@@ -2012 +2009 @@
                     /* Read as much as we can from the entry. */
                     RTSgBufCopyFromBuf(&SgBuf, pEntry->pbData + offDiff, cbToRead);
-                    ASMAtomicSubU32(&pReq->cbXfer, cbToRead);
                 }
 
@@ -2137 +2133 @@
     }
 
-    if (!pdmBlkCacheReqUpdate(pBlkCache, pReq, 0, rc, false))
+    if (!pdmBlkCacheReqUpdate(pBlkCache, pReq, rc, false))
         rc = VINF_AIO_TASK_PENDING;
 
@@ -2163 +2159 @@
 
     /* Allocate new request structure. */
-    pReq = pdmBlkCacheReqAlloc(cbWrite, pvUser);
+    pReq = pdmBlkCacheReqAlloc(pvUser);
     if (RT_UNLIKELY(!pReq))
         return VERR_NO_MEMORY;
@@ -2211 +2207 @@
                         RTSgBufCopyToBuf(&SgBuf, pEntry->pbData + offDiff,
                                          cbToWrite);
-                        ASMAtomicSubU32(&pReq->cbXfer, cbToWrite);
                     }
                     else
@@ -2244 +2239 @@
                         /* Write as much as we can into the entry and update the file. */
                         RTSgBufCopyToBuf(&SgBuf, pEntry->pbData + offDiff, cbToWrite);
-                        ASMAtomicSubU32(&pReq->cbXfer, cbToWrite);
 
                         bool fCommit = pdmBlkCacheAddDirtyEntry(pBlkCache, pEntry);
@@ -2334 +2328 @@
                 {
                     RTSgBufCopyToBuf(&SgBuf, pEntryNew->pbData, cbToWrite);
-                    ASMAtomicSubU32(&pReq->cbXfer, cbToWrite);
 
                     bool fCommit = pdmBlkCacheAddDirtyEntry(pBlkCache, pEntryNew);
@@ -2372 +2365 @@
     }
 
-    if (!pdmBlkCacheReqUpdate(pBlkCache, pReq, 0, rc, false))
+    if (!pdmBlkCacheReqUpdate(pBlkCache, pReq, rc, false))
         rc = VINF_AIO_TASK_PENDING;
 
@@ -2394 +2387 @@
 
     /* Allocate new request structure. */
-    pReq = pdmBlkCacheReqAlloc(0, pvUser);
+    pReq = pdmBlkCacheReqAlloc(pvUser);
     if (RT_UNLIKELY(!pReq))
         return VERR_NO_MEMORY;
@@ -2421 +2414 @@
     PPDMBLKCACHEREQ pReq = pWaiter->pReq;
 
-    pdmBlkCacheReqUpdate(pBlkCache, pWaiter->pReq, pWaiter->cbTransfer, rc, true);
+    pdmBlkCacheReqUpdate(pBlkCache, pWaiter->pReq, rc, true);
 
     RTMemFree(pWaiter);
@@ -2532 +2525 @@
         pdmBlkCacheIoXferCompleteEntry(pBlkCache, hIoXfer, rcIoXfer);
     else
-        pdmBlkCacheReqUpdate(pBlkCache, hIoXfer->pReq, hIoXfer->cbXfer, rcIoXfer, true);
+        pdmBlkCacheReqUpdate(pBlkCache, hIoXfer->pReq, rcIoXfer, true);
 }
 

trunk/src/VBox/VMM/include/PDMBlkCacheInternal.h

@@ -263 +263 @@
     /** Opaque user data returned on completion. */
     void             *pvUser;
-    /** Number of bytes to transfer. */
-    volatile uint32_t cbXfer;
     /** Number of pending transfers (waiting for a cache entry and passed through). */
     volatile uint32_t cXfersPending;
@@ -292 +290 @@
     /** Transfer direction. */
     PDMBLKCACHEXFERDIR enmXferDir;
-    /** Size of the transfer. */
-    size_t             cbXfer;
 } PDMBLKCACHEIOXFER;