Changeset 97921 in vbox for trunk/src/VBox/Additions

Timestamp:

Dec 30, 2022 9:15:10 PM (2 years ago)

Author:

vboxsync

Message:

Add/VBoxGuestR0LibPhysHeap.cpp: Count both kind of blocks both on heap and chunk level. This is in preparation for a workaround for VbglR0PhysHeapFree not merging with the block before the one that getting freed. Completed the vbglR0PhysHeapCheckLocked code.

Location:

trunk/src/VBox/Additions/common/VBoxGuest/lib

Files:

• VBoxGuestR0LibInternal.h (modified) (2 diffs)
• VBoxGuestR0LibPhysHeap.cpp (modified) (19 diffs)
• testcase/tstVbglR0PhysHeap-1.cpp (modified) (1 diff)

trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR0LibInternal.h

@@ -119 +119 @@
 /**
  * Global VBGL ring-0 data.
- * Lives in VbglR0Init.cpp.
+ * Lives in VBoxGuestR0LibInit.cpp.
  */
 typedef struct VBGLDATA
@@ -132 +132 @@
      * @{
      */
+    RTSEMFASTMUTEX     mutexHeap;
     VBGLPHYSHEAPBLOCK *pFreeBlocksHead;
     VBGLPHYSHEAPBLOCK *pAllocBlocksHead;
+    /** Indexed by VBGLPHYSHEAPBLOCK::fAllocated, so zero is the free ones and
+     *  one the allocated ones. */
+    int32_t            acBlocks[2];
     VBGLPHYSHEAPCHUNK *pChunkHead;
-
-    RTSEMFASTMUTEX     mutexHeap;
     /** @} */
 

trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR0LibPhysHeap.cpp

@@ -145 +145 @@
     uint32_t physAddr;
 
-    /** Number of allocated blocks in the chunk */
-    int32_t cAllocatedBlocks;
+    uint32_t uPadding1;
+
+    /** Indexed by VBGLPHYSHEAPBLOCK::fAllocated, so zero is the free ones and
+     *  one the allocated ones. */
+    int32_t acBlocks[2];
 
     /** Pointer to the next chunk. */
@@ -152 +155 @@
     /** Pointer to the previous chunk. */
     VBGLPHYSHEAPCHUNK  *pPrev;
+#if ARCH_BITS == 64
+    /** Pad the size up to 64 bytes. */
+    uintptr_t           auPadding2[3];
+#endif
 };
+#if ARCH_BITS == 64
+AssertCompileSize(VBGLPHYSHEAPCHUNK, 64);
+#endif
 
 
@@ -257 +267 @@
 
 
+/**
+ * Links @a pBlock onto the appropriate chain accoring to @a pInsertAfter or @a
+ * pBlock->fAllocated.
+ *
+ * This also update the per-chunk block counts.
+ */
 static void vbglPhysHeapInsertBlock(VBGLPHYSHEAPBLOCK *pInsertAfter, VBGLPHYSHEAPBLOCK *pBlock)
 {
+    bool const fAllocated = pBlock->fAllocated;
+
     VBGL_PH_ASSERT_MSG(pBlock->pNext == NULL, ("pBlock->pNext = %p\n", pBlock->pNext));
     VBGL_PH_ASSERT_MSG(pBlock->pPrev == NULL, ("pBlock->pPrev = %p\n", pBlock->pPrev));
@@ -277 +295 @@
         pBlock->pPrev = NULL;
 
-        if (pBlock->fAllocated)
+        if (fAllocated)
         {
             pBlock->pNext = g_vbgldata.pAllocBlocksHead;
@@ -296 +314 @@
         }
     }
+
+    /* Update the block counts: */
+    g_vbgldata.acBlocks[fAllocated]      += 1;
+    pBlock->pChunk->acBlocks[fAllocated] += 1;
+    AssertMsg(   (uint32_t)pBlock->pChunk->acBlocks[fAllocated]
+              <= pBlock->pChunk->cbSize / (sizeof(*pBlock) + sizeof(void *)),
+              ("pChunk=%p: cbSize=%#x acBlocks[%u]=%d\n",
+               pBlock->pChunk, pBlock->pChunk->cbSize, pBlock->pChunk->acBlocks[fAllocated]));
 }
 
 
 /**
- * Unlinks @a pBlock from the chain its on.
+ * Unlinks @a pBlock from the chain it's on.
+ *
+ * This also update the per-chunk block counts.
  */
 static void vbglPhysHeapExcludeBlock(VBGLPHYSHEAPBLOCK *pBlock)
 {
+    bool const fAllocated = pBlock->fAllocated;
+
     if (pBlock->pNext)
         pBlock->pNext->pPrev = pBlock->pPrev;
@@ -310 +340 @@
     if (pBlock->pPrev)
         pBlock->pPrev->pNext = pBlock->pNext;
-    else if (pBlock->fAllocated)
+    else if (fAllocated)
     {
         Assert(g_vbgldata.pAllocBlocksHead == pBlock);
@@ -323 +353 @@
     pBlock->pNext = NULL;
     pBlock->pPrev = NULL;
-}
+
+    /* Update the per-chunk counts: */
+    g_vbgldata.acBlocks[fAllocated]      -= 1;
+    pBlock->pChunk->acBlocks[fAllocated] -= 1;
+    AssertMsg(pBlock->pChunk->acBlocks[fAllocated] >= 0,
+              ("pChunk=%p: cbSize=%#x acBlocks[%u]=%d\n",
+               pBlock->pChunk, pBlock->pChunk->cbSize, pBlock->pChunk->acBlocks[fAllocated]));
+    Assert(g_vbgldata.acBlocks[fAllocated] >= 0);
+}
+
 
 static VBGLPHYSHEAPBLOCK *vbglPhysHeapChunkAlloc(uint32_t cbMinBlock)
@@ -355 +394 @@
         pChunk->cbSize           = cbChunk;
         pChunk->physAddr         = (uint32_t)PhysAddr;
-        pChunk->cAllocatedBlocks = 0;
+        pChunk->acBlocks[0]      = 0;
+        pChunk->acBlocks[1]      = 0;
         pChunk->pNext            = NULL;
         pChunk->pPrev            = NULL;
+        pChunk->uPadding1        = UINT32_C(0xADDCAAA1);
+#if ARCH_BITS == 64
+        pChunk->auPadding2[0]    = UINT64_C(0xADDCAAA3ADDCAAA2);
+        pChunk->auPadding2[1]    = UINT64_C(0xADDCAAA5ADDCAAA4);
+        pChunk->auPadding2[2]    = UINT64_C(0xADDCAAA7ADDCAAA6);
+#endif
 
         /* Initialize the free block, which now occupies entire chunk. */
@@ -532 +578 @@
         /* Insert to allocated list */
         vbglPhysHeapInsertBlock(NULL, pBlock);
-
-        /* Adjust the chunk allocated blocks counter */
-        pBlock->pChunk->cAllocatedBlocks++;
     }
 
@@ -560 +603 @@
     return physAddr;
 }
+
 
 DECLR0VBGL(void) VbglR0PhysHeapFree(void *pv)
@@ -571 +615 @@
         return;
 
-    dumpheap ("pre free");
+    dumpheap("pre free");
 
     pBlock = vbglPhysHeapData2Block(pv);
@@ -598 +642 @@
     dumpheap("post insert");
 
-    /* Adjust the chunk allocated blocks counter */
     pChunk = pBlock->pChunk;
-    pChunk->cAllocatedBlocks--;
-
-    VBGL_PH_ASSERT(pChunk->cAllocatedBlocks >= 0);
 
     /* Check if we can merge 2 free blocks. To simplify heap maintenance,
@@ -631 +671 @@
 
     /* now check if there are 2 or more free (unused) chunks */
-    if (pChunk->cAllocatedBlocks == 0)
+    if (pChunk->acBlocks[1 /*allocated*/] == 0)
     {
         VBGLPHYSHEAPCHUNK *pCurChunk;
@@ -640 +680 @@
         {
             Assert(pCurChunk->u32Signature == VBGL_PH_CHUNKSIGNATURE);
-            if (pCurChunk->cAllocatedBlocks == 0)
+            if (pCurChunk->acBlocks[1 /*allocated*/] == 0)
                 cUnusedChunks++;
         }
@@ -687 +727 @@
 }
 
+
+/**
+ * Checks the heap, caller responsible for locking.
+ *
+ * @returns VINF_SUCCESS if okay, error status if not.
+ * @param   pErrInfo    Where to return more error details, optional.
+ */
 static int vbglR0PhysHeapCheckLocked(PRTERRINFO pErrInfo)
 {
@@ -692 +739 @@
      * Scan the blocks in each chunk.
      */
-    unsigned cTotalFreeBlocks = 0;
-    unsigned cTotalUsedBlocks = 0;
-    for (VBGLPHYSHEAPCHUNK *pCurChunk = g_vbgldata.pChunkHead; pCurChunk; pCurChunk = pCurChunk->pNext)
+    unsigned acTotalBlocks[2] = { 0, 0 };
+    for (VBGLPHYSHEAPCHUNK *pCurChunk = g_vbgldata.pChunkHead, *pPrevChunk = NULL; pCurChunk; pCurChunk = pCurChunk->pNext)
     {
         AssertReturn(pCurChunk->u32Signature == VBGL_PH_CHUNKSIGNATURE,
-                     RTErrInfoSetF(pErrInfo, VERR_INVALID_MAGIC, "pCurChunk=%p: magic=%#x\n", pCurChunk, pCurChunk->u32Signature));
+                     RTErrInfoSetF(pErrInfo, VERR_INVALID_MAGIC, "pCurChunk=%p: magic=%#x", pCurChunk, pCurChunk->u32Signature));
+        AssertReturn(pCurChunk->pPrev == pPrevChunk,
+                     RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_2,
+                                   "pCurChunk=%p: pPrev=%p, expected %p", pCurChunk, pCurChunk->pPrev, pPrevChunk));
 
         uintptr_t const          uEnd        = (uintptr_t)pCurChunk + pCurChunk->cbSize;
         const VBGLPHYSHEAPBLOCK *pCurBlock   = (const VBGLPHYSHEAPBLOCK *)(pCurChunk + 1);
-        unsigned                 cUsedBlocks = 0;
+        unsigned                 acBlocks[2] = { 0, 0 };
         while ((uintptr_t)pCurBlock < uEnd)
         {
             AssertReturn(pCurBlock->u32Signature == VBGL_PH_BLOCKSIGNATURE,
                          RTErrInfoSetF(pErrInfo, VERR_INVALID_MAGIC,
-                                       "pCurBlock=%p: magic=%#x\n", pCurBlock, pCurBlock->u32Signature));
+                                       "pCurBlock=%p: magic=%#x", pCurBlock, pCurBlock->u32Signature));
             AssertReturn(pCurBlock->pChunk == pCurChunk,
                          RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_2,
-                                       "pCurBlock=%p: pChunk=%p, expected %p\n", pCurBlock, pCurBlock->pChunk, pCurChunk));
-            AssertReturn(   pCurBlock->cbDataSize >= 8
+                                       "pCurBlock=%p: pChunk=%p, expected %p", pCurBlock, pCurBlock->pChunk, pCurChunk));
+            AssertReturn(   pCurBlock->cbDataSize >= sizeof(void *)
                          && pCurBlock->cbDataSize < _128M
                          && RT_ALIGN_32(pCurBlock->cbDataSize, sizeof(void *)) == pCurBlock->cbDataSize,
                          RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_3,
-                                       "pCurBlock=%p: cbDataSize=%#x\n", pCurBlock, pCurBlock->cbDataSize));
-            if (pCurBlock->fAllocated)
-                cUsedBlocks += 1;
-            else
-                cTotalFreeBlocks += 1;
+                                       "pCurBlock=%p: cbDataSize=%#x", pCurBlock, pCurBlock->cbDataSize));
+            acBlocks[pCurBlock->fAllocated] += 1;
 
             /* advance */
@@ -725 +771 @@
         AssertReturn((uintptr_t)pCurBlock == uEnd,
                      RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_4,
-                                   "pCurBlock=%p uEnd=%p\n", pCurBlock, uEnd));
-        AssertReturn(cUsedBlocks == (uint32_t)pCurChunk->cAllocatedBlocks,
+                                   "pCurBlock=%p uEnd=%p", pCurBlock, uEnd));
+
+        acTotalBlocks[1] += acBlocks[1];
+        AssertReturn(acBlocks[1] == (uint32_t)pCurChunk->acBlocks[1],
                      RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_4,
-                                   "pCurChunk=%p: cAllocatedBlocks=%u, expected %u\n",
-                                   pCurChunk, pCurChunk->cAllocatedBlocks, cUsedBlocks));
-        cTotalUsedBlocks += cUsedBlocks;
-    }
+                                   "pCurChunk=%p: cAllocatedBlocks=%u, expected %u",
+                                   pCurChunk, pCurChunk->acBlocks[1], acBlocks[1]));
+
+        acTotalBlocks[0] += acBlocks[0];
+        AssertReturn(acBlocks[0] == (uint32_t)pCurChunk->acBlocks[0],
+                     RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_5,
+                                   "pCurChunk=%p: cFreeBlocks=%u, expected %u",
+                                   pCurChunk, pCurChunk->acBlocks[0], acBlocks[0]));
+
+        pPrevChunk = pCurChunk;
+    }
+
+    AssertReturn(acTotalBlocks[0] == (uint32_t)g_vbgldata.acBlocks[0],
+                 RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR,
+                               "g_vbgldata.acBlocks[0]=%u, expected %u", g_vbgldata.acBlocks[0], acTotalBlocks[0]));
+    AssertReturn(acTotalBlocks[1] == (uint32_t)g_vbgldata.acBlocks[1],
+                 RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR,
+                               "g_vbgldata.acBlocks[1]=%u, expected %u", g_vbgldata.acBlocks[1], acTotalBlocks[1]));
+
+    /*
+     * Count each list and check that they contain the same number of nodes.
+     */
+    VBGLPHYSHEAPBLOCK *apHeads[2] = { g_vbgldata.pFreeBlocksHead, g_vbgldata.pAllocBlocksHead };
+    for (unsigned iType = 0; iType < RT_ELEMENTS(apHeads); iType++)
+    {
+        unsigned           cBlocks    = 0;
+        VBGLPHYSHEAPBLOCK *pPrevBlock = NULL;
+        for (VBGLPHYSHEAPBLOCK *pCurBlock = apHeads[iType]; pCurBlock; pCurBlock = pCurBlock->pNext)
+        {
+            AssertReturn(pCurBlock->u32Signature == VBGL_PH_BLOCKSIGNATURE,
+                         RTErrInfoSetF(pErrInfo, VERR_INVALID_MAGIC,
+                                       "pCurBlock=%p/%u: magic=%#x", pCurBlock, iType, pCurBlock->u32Signature));
+            AssertReturn(pCurBlock->pPrev == pPrevBlock,
+                         RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_2,
+                                       "pCurBlock=%p/%u: pPrev=%p, expected %p", pCurBlock, iType, pCurBlock->pPrev, pPrevBlock));
+            AssertReturn(pCurBlock->pChunk->u32Signature == VBGL_PH_CHUNKSIGNATURE,
+                         RTErrInfoSetF(pErrInfo, VERR_INVALID_MAGIC, "pCurBlock=%p/%u: chunk (%p) magic=%#x",
+                                       pCurBlock, iType, pCurBlock->pChunk, pCurBlock->pChunk->u32Signature));
+            cBlocks++;
+            pPrevBlock = pCurBlock;
+        }
+
+        AssertReturn(cBlocks == acTotalBlocks[iType],
+                     RTErrInfoSetF(pErrInfo, VERR_INTERNAL_ERROR_3,
+                                   "iType=%u: Found %u in list, expected %u", iType, cBlocks, acTotalBlocks[iType]));
+    }
+
     return VINF_SUCCESS;
 }
+
 
 /**
@@ -739 +831 @@
  *
  * @returns Problem description on failure, NULL on success.
+ * @param   pErrInfo    Where to return more error details, optional.
  */
 DECLVBGL(int) VbglR0PhysHeapCheck(PRTERRINFO pErrInfo)
@@ -751 +844 @@
 }
 
-
 #endif /* IN_TESTCASE */
-
 
 DECLR0VBGL(int) VbglR0PhysHeapInit(void)

trunk/src/VBox/Additions/common/VBoxGuest/lib/testcase/tstVbglR0PhysHeap-1.cpp

@@ -345 +345 @@
                 s_aHistory[i].pv = NULL;
             }
+            RTTestIPrintf(RTTESTLVL_ALWAYS, "after free-all: cFreeBlocks=%u in %u chunk(s)\n", g_vbgldata.acBlocks[0], g_cChunks);
             RTTESTI_CHECK_MSG(g_cChunks == 1, ("g_cChunks=%d\n", g_cChunks));
+            RTTESTI_CHECK_MSG(g_vbgldata.acBlocks[1] == 0, ("g_vbgldata.acBlocks[1]=%d\n", g_vbgldata.acBlocks[0]));
 #if 0
             for (VBGLPHYSHEAPCHUNK *pCurChunk = g_vbgldata.pChunkHead; pCurChunk; pCurChunk = pCurChunk->pNext)