Changeset 64029 in vbox

Timestamp:

Sep 26, 2016 9:22:02 PM (8 years ago)

Author:

vboxsync

Message:

Storage/DrvVD: Don't recurse into trying to allocate I/O buffers again for waiting requests from drvvdMediaExIoReqBufFree() or we might see stale list data

File:

• trunk/src/VBox/Devices/Storage/DrvVD.cpp (modified) (22 diffs)

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

@@ -2708 +2708 @@
 static void drvvdMediaExIoReqRetire(PVBOXDISK pThis, PPDMMEDIAEXIOREQINT pIoReq, int rcReq, bool fUpNotify)
 {
-    int rc;
+    LogFlowFunc(("pThis=%#p pIoReq=%#p rcReq=%Rrc fUpNotify=%RTbool\n",
+                 pThis, pIoReq, rcReq, fUpNotify));
+
     bool fXchg = ASMAtomicCmpXchgU32((volatile uint32_t *)&pIoReq->enmState, VDIOREQSTATE_COMPLETING, VDIOREQSTATE_ACTIVE);
     if (fXchg)
@@ -2719 +2721 @@
 
     ASMAtomicXchgU32((volatile uint32_t *)&pIoReq->enmState, VDIOREQSTATE_COMPLETED);
-    drvvdMediaExIoReqBufFree(pThis, pIoReq);
+    //drvvdMediaExIoReqBufFree(pThis, pIoReq);
 
     /*
@@ -2794 +2796 @@
     if (fUpNotify)
     {
-        rc = pThis->pDrvMediaExPort->pfnIoReqCompleteNotify(pThis->pDrvMediaExPort,
-                                                            pIoReq, &pIoReq->abAlloc[0], rcReq);
+        int rc = pThis->pDrvMediaExPort->pfnIoReqCompleteNotify(pThis->pDrvMediaExPort,
+                                                                pIoReq, &pIoReq->abAlloc[0], rcReq);
         AssertRC(rc);
     }
+
+    LogFlowFunc(("returns\n"));
 }
 
@@ -2811 +2815 @@
 static int drvvdMediaExIoReqCompleteWorker(PVBOXDISK pThis, PPDMMEDIAEXIOREQINT pIoReq, int rcReq, bool fUpNotify)
 {
+    LogFlowFunc(("pThis=%#p pIoReq=%#p rcReq=%Rrc fUpNotify=%RTbool\n",
+                 pThis, pIoReq, rcReq, fUpNotify));
+
     /*
      * For a read we need to sync the memory before continuing to process
@@ -2865 +2872 @@
     }
 
+    LogFlowFunc(("returns %Rrc\n", rcReq));
     return rcReq;
 }
@@ -2881 +2889 @@
 DECLINLINE(int) drvvdMediaExIoReqBufAlloc(PVBOXDISK pThis, PPDMMEDIAEXIOREQINT pIoReq, size_t cb)
 {
+    LogFlowFunc(("pThis=%#p pIoReq=%#p cb=%zu\n", pThis, pIoReq, cb));
+
     int rc = IOBUFMgrAllocBuf(pThis->hIoBufMgr, &pIoReq->ReadWrite.IoBuf, cb, &pIoReq->ReadWrite.cbIoBuf);
     if (rc == VERR_NO_MEMORY)
     {
+        LogFlowFunc(("Could not allocate memory for request, deferring\n"));
         RTCritSectEnter(&pThis->CritSectIoReqsIoBufWait);
         RTListAppend(&pThis->LstIoReqIoBufWait, &pIoReq->NdLstWait);
@@ -2891 +2902 @@
     }
     else
+    {
+        LogFlowFunc(("Allocated %zu bytes of memory\n", pIoReq->ReadWrite.cbIoBuf));
         Assert(pIoReq->ReadWrite.cbIoBuf > 0);
-
+    }
+
+    LogFlowFunc(("returns %Rrc\n", rc));
     return rc;
 }
@@ -2909 +2924 @@
     int rc = VINF_SUCCESS;
 
+    LogFlowFunc(("pThis=%#p pIoReq=%#p cbReqIo=%zu pcbReqIo=%#p\n", pThis, pIoReq, cbReqIo, pcbReqIo));
+
     Assert(cbReqIo > 0);
 
@@ -2939 +2956 @@
     *pcbReqIo = cbReqIo;
 
+    LogFlowFunc(("returns %Rrc *pcbReqIo=%zu\n", rc, *pcbReqIo));
     return rc;
 }
@@ -2957 +2975 @@
     Assert(cbReqIo > 0);
 
+    LogFlowFunc(("pThis=%#p pIoReq=%#p cbReqIo=%zu pcbReqIo=%#p\n", pThis, pIoReq, cbReqIo, pcbReqIo));
+
     if (   pThis->fAsyncIOSupported
         && !(pIoReq->fFlags & PDMIMEDIAEX_F_SYNC))
@@ -2985 +3005 @@
     *pcbReqIo = cbReqIo;
 
+    LogFlowFunc(("returns %Rrc *pcbReqIo=%zu\n", rc, *pcbReqIo));
     return rc;
 }
@@ -2999 +3020 @@
     int rc = VINF_SUCCESS;
 
+    LogFlowFunc(("pThis=%#p pIoReq=%#p\n", pThis, pIoReq));
+
     if (   pThis->fAsyncIOSupported
         && !(pIoReq->fFlags & PDMIMEDIAEX_F_SYNC))
@@ -3020 +3043 @@
     }
 
+    LogFlowFunc(("returns %Rrc\n", rc));
     return rc;
 }
@@ -3034 +3058 @@
     int rc = VINF_SUCCESS;
 
+    LogFlowFunc(("pThis=%#p pIoReq=%#p\n", pThis, pIoReq));
+
     if (   pThis->fAsyncIOSupported
         && !(pIoReq->fFlags & PDMIMEDIAEX_F_SYNC))
@@ -3056 +3082 @@
     }
 
+    LogFlowFunc(("returns %Rrc\n", rc));
     return rc; 
 }
@@ -3071 +3098 @@
     int rc = VINF_SUCCESS;
 
+    LogFlowFunc(("pThis=%#p pIoReq=%#p fUpNotify=%RTbool\n", pThis, pIoReq, fUpNotify));
+
     Assert(pIoReq->enmType == PDMMEDIAEXIOREQTYPE_READ || pIoReq->enmType == PDMMEDIAEXIOREQTYPE_WRITE);
 
@@ -3111 +3140 @@
     }
 
+    LogFlowFunc(("returns %Rrc\n", rc));
     return rc;
 }
@@ -3124 +3154 @@
 DECLINLINE(void) drvvdMediaExIoReqBufFree(PVBOXDISK pThis, PPDMMEDIAEXIOREQINT pIoReq)
 {
+    LogFlowFunc(("pThis=%#p pIoReq=%#p{.cbIoBuf=%zu}\n", pThis, pIoReq, pIoReq->ReadWrite.cbIoBuf));
+
     if (   pIoReq->enmType == PDMMEDIAEXIOREQTYPE_READ
         || pIoReq->enmType == PDMMEDIAEXIOREQTYPE_WRITE)
@@ -3129 +3161 @@
         IOBUFMgrFreeBuf(&pIoReq->ReadWrite.IoBuf);
 
-        if (ASMAtomicReadU32(&pThis->cIoReqsWaiting) > 0)
+        uint32_t cIoReqsWaiting = ASMAtomicXchgU32(&pThis->cIoReqsWaiting, 0);
+        if (cIoReqsWaiting > 0)
         {
             /* Try to process as many requests as possible. */
@@ -3137 +3170 @@
             RTListForEachSafe(&pThis->LstIoReqIoBufWait, pIoReqCur, pIoReqNext, PDMMEDIAEXIOREQINT, NdLstWait)
             {
+                LogFlowFunc(("Found I/O request %#p on waiting list, trying to allocate buffer of size %zu bytes\n",
+                             pIoReqCur, pIoReqCur->ReadWrite.cbReq));
+
                 /* Allocate a suitable I/O buffer for this request. */
                 int rc = IOBUFMgrAllocBuf(pThis->hIoBufMgr, &pIoReqCur->ReadWrite.IoBuf, pIoReqCur->ReadWrite.cbReq,
@@ -3144 +3180 @@
                     Assert(pIoReq->ReadWrite.cbIoBuf > 0);
 
-                    ASMAtomicDecU32(&pThis->cIoReqsWaiting);
+                    cIoReqsWaiting--;
                     RTListNodeRemove(&pIoReqCur->NdLstWait);
 
@@ -3164 +3200 @@
             }
             RTCritSectLeave(&pThis->CritSectIoReqsIoBufWait);
-        }
-    }
+
+            ASMAtomicAddU32(&pThis->cIoReqsWaiting, cIoReqsWaiting);
+        }
+    }
+
+    LogFlowFunc(("returns\n"));
 }