Changeset 38676 in vbox for trunk/src/VBox

Timestamp:

Sep 7, 2011 1:27:22 PM (13 years ago)

Author:

vboxsync

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

73893

Message:

VDI/Discard: Don't read and check a block if the whole block is discarded anyway, saves I/O and CPU cycles

File:

• trunk/src/VBox/Storage/VDI.cpp (modified) (2 diffs)

trunk/src/VBox/Storage/VDI.cpp

@@ -887 +887 @@
     }
 
+    return rc;
+}
+
+/**
+ * Internal: Discard a whole block from the image filling the created hole with
+ * data from another block.
+ *
+ * @returns VBox status code.
+ * @param   pImage    VDI image instance data.
+ * @param   uBlock    The block to discard.
+ * @param   pvBlock   Memory to use for the I/O.
+ */
+static int vdiDiscardBlock(PVDIIMAGEDESC pImage, unsigned uBlock, void *pvBlock)
+{
+    int rc = VINF_SUCCESS;
+    uint64_t cbImage;
+    unsigned idxLastBlock = getImageBlocksAllocated(&pImage->Header) - 1;
+    unsigned uBlockLast = pImage->paBlocksRev[idxLastBlock];
+    VDIIMAGEBLOCKPOINTER ptrBlockDiscard = pImage->paBlocks[uBlock];
+
+    LogFlowFunc(("pImage=%#p uBlock=%u pvBlock=%#p\n",
+                 pImage, uBlock, pvBlock));
+
+    pImage->paBlocksRev[idxLastBlock] = VDI_IMAGE_BLOCK_FREE;
+
+    do
+    {
+        /*
+         * The block is empty, remove it.
+         * Read the last block of the image first.
+         */
+        if (idxLastBlock != ptrBlockDiscard)
+        {
+            uint64_t u64Offset;
+
+            LogFlowFunc(("Moving block [%u]=%u into [%u]=%u\n",
+                         uBlockLast, idxLastBlock, uBlock, pImage->paBlocks[uBlock]));
+
+            u64Offset = (uint64_t)idxLastBlock * pImage->cbTotalBlockData + pImage->offStartData;
+            rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage, u64Offset,
+                                       pvBlock, pImage->cbTotalBlockData, NULL);
+            if (RT_FAILURE(rc))
+                break;
+
+            /* Write to the now unallocated block. */
+            u64Offset = (uint64_t)ptrBlockDiscard * pImage->cbTotalBlockData + pImage->offStartData;
+            rc = vdIfIoIntFileWriteSync(pImage->pIfIo, pImage->pStorage, u64Offset,
+                                        pvBlock, pImage->cbTotalBlockData, NULL);
+            if (RT_FAILURE(rc))
+                break;
+
+            /* Update block and reverse block tables. */
+            pImage->paBlocks[uBlockLast] = ptrBlockDiscard;
+            pImage->paBlocksRev[ptrBlockDiscard] = uBlockLast;
+            rc = vdiUpdateBlockInfo(pImage, uBlockLast);
+            if (RT_FAILURE(rc))
+                break;
+        }
+        else
+            LogFlowFunc(("Discard last block [%u]=%u\n", uBlock, pImage->paBlocks[uBlock]));
+
+        pImage->paBlocks[uBlock] = VDI_IMAGE_BLOCK_ZERO;
+
+        /* Update the block pointers. */
+        setImageBlocksAllocated(&pImage->Header, idxLastBlock);
+        rc = vdiUpdateBlockInfo(pImage, uBlock);
+        if (RT_FAILURE(rc))
+            break;
+
+        /* Set new file size. */
+        rc = vdIfIoIntFileGetSize(pImage->pIfIo, pImage->pStorage, &cbImage);
+        if (RT_FAILURE(rc))
+            break;
+
+        LogFlowFunc(("Set new size %llu\n", cbImage - pImage->cbTotalBlockData));
+        rc = vdIfIoIntFileSetSize(pImage->pIfIo, pImage->pStorage, cbImage - pImage->cbTotalBlockData);
+    } while (0);
+
+    LogFlowFunc(("returns rc=%Rrc\n", rc));
     return rc;
 }
@@ -2646 +2725 @@
                 break;
             }
-            pbBlockData = (uint8_t *)pvBlock + pImage->offStartBlockData;
-
-            uint64_t u64Offset = (uint64_t)pImage->paBlocks[uBlock] * pImage->cbTotalBlockData + pImage->offStartData;
-            rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage, u64Offset,
-                                       pvBlock, pImage->cbTotalBlockData, NULL);
-            if (RT_FAILURE(rc))
-                break;
-
-            /* Clear data. */
-            memset(pbBlockData + offDiscard , 0, cbDiscard);
-
-            Assert(!(cbDiscard % 4));
-            Assert(cbDiscard * 8 <= UINT32_MAX);
-            if (ASMBitFirstSet((volatile void *)pbBlockData, getImageBlockSize(&pImage->Header) * 8) == -1)
-            {
-                uint64_t cbImage;
-                unsigned idxLastBlock = getImageBlocksAllocated(&pImage->Header) - 1;
-                unsigned uBlockLast = pImage->paBlocksRev[idxLastBlock];
-                VDIIMAGEBLOCKPOINTER ptrBlockDiscard = pImage->paBlocks[uBlock];
-
-                pImage->paBlocksRev[idxLastBlock] = VDI_IMAGE_BLOCK_FREE;
-
+
+            if (!cbPreAllocated && !cbPostAllocated)
+            {
                 /*
-                 * The block is empty, remove it.
-                 * Read the last block of the image first.
+                 * Discarding a whole block, don't check for allocated sectors.
+                 * It is possible to just remove the whole block which avoids
+                 * one read and checking the whole block for data.
                  */
-                if (idxLastBlock != ptrBlockDiscard)
+                rc = vdiDiscardBlock(pImage, uBlock, pvBlock);
+            }
+            else
+            {
+                /* Read data. */
+                pbBlockData = (uint8_t *)pvBlock + pImage->offStartBlockData;
+
+                uint64_t u64Offset = (uint64_t)pImage->paBlocks[uBlock] * pImage->cbTotalBlockData + pImage->offStartData;
+                rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage, u64Offset,
+                                           pvBlock, pImage->cbTotalBlockData, NULL);
+                if (RT_FAILURE(rc))
+                    break;
+
+                /* Clear data. */
+                memset(pbBlockData + offDiscard , 0, cbDiscard);
+
+                Assert(!(cbDiscard % 4));
+                Assert(cbDiscard * 8 <= UINT32_MAX);
+                if (ASMBitFirstSet((volatile void *)pbBlockData, getImageBlockSize(&pImage->Header) * 8) == -1)
+                    rc = vdiDiscardBlock(pImage, uBlock, pvBlock);
+                else if (fDiscard & VD_DISCARD_MARK_UNUSED)
                 {
-                    LogFlowFunc(("Moving block [%u]=%u into [%u]=%u\n",
-                                 uBlockLast, idxLastBlock, uBlock, pImage->paBlocks[uBlock]));
-
-                    u64Offset = (uint64_t)idxLastBlock * pImage->cbTotalBlockData + pImage->offStartData;
-                    rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage, u64Offset,
-                                               pvBlock, pImage->cbTotalBlockData, NULL);
-                    if (RT_FAILURE(rc))
-                        break;
-
-                    /* Write to the now unallocated block. */
-                    u64Offset = (uint64_t)ptrBlockDiscard * pImage->cbTotalBlockData + pImage->offStartData;
-                    rc = vdIfIoIntFileWriteSync(pImage->pIfIo, pImage->pStorage, u64Offset,
-                                                pvBlock, pImage->cbTotalBlockData, NULL);
-                    if (RT_FAILURE(rc))
-                        break;
-
-                    /* Update block and reverse block tables. */
-                    pImage->paBlocks[uBlockLast] = ptrBlockDiscard;
-                    pImage->paBlocksRev[ptrBlockDiscard] = uBlockLast;
-                    rc = vdiUpdateBlockInfo(pImage, uBlockLast);
-                    if (RT_FAILURE(rc))
-                        break;
+                    /* Write changed data to the image. */
+                    rc = vdIfIoIntFileWriteSync(pImage->pIfIo, pImage->pStorage, u64Offset + offDiscard,
+                                                pbBlockData + offDiscard, cbDiscard, NULL);
                 }
                 else
-                    LogFlowFunc(("Discard last block [%u]=%u\n", uBlock, pImage->paBlocks[uBlock]));
-
-                pImage->paBlocks[uBlock] = VDI_IMAGE_BLOCK_ZERO;
-
-                /* Update the block pointers. */
-                setImageBlocksAllocated(&pImage->Header, idxLastBlock);
-                rc = vdiUpdateBlockInfo(pImage, uBlock);
-                if (RT_FAILURE(rc))
-                    break;
-
-                /* Set new file size. */
-                rc = vdIfIoIntFileGetSize(pImage->pIfIo, pImage->pStorage, &cbImage);
-                if (RT_FAILURE(rc))
-                    break;
-
-                LogFlowFunc(("Set new size %llu\n", cbImage - pImage->cbTotalBlockData));
-                rc = vdIfIoIntFileSetSize(pImage->pIfIo, pImage->pStorage, cbImage - pImage->cbTotalBlockData);
-            }
-            else if (fDiscard & VD_DISCARD_MARK_UNUSED)
-            {
-                /* Write changed data to the image. */
-                rc = vdIfIoIntFileWriteSync(pImage->pIfIo, pImage->pStorage, u64Offset + offDiscard,
-                                            pbBlockData + offDiscard, cbDiscard, NULL);
-            }
-            else
-            {
-                /* Block has data, create allocation bitmap. */
-                *pcbPreAllocated = cbPreAllocated;
-                *pcbPostAllocated = cbPostAllocated;
-                *ppbmAllocationBitmap = vdiAllocationBitmapCreate(pbBlockData, getImageBlockSize(&pImage->Header));
-                if (RT_UNLIKELY(!*ppbmAllocationBitmap))
-                    rc = VERR_NO_MEMORY;
-                else
-                    rc = VERR_VD_DISCARD_ALIGNMENT_NOT_MET;
-            }
-        }
+                {
+                    /* Block has data, create allocation bitmap. */
+                    *pcbPreAllocated = cbPreAllocated;
+                    *pcbPostAllocated = cbPostAllocated;
+                    *ppbmAllocationBitmap = vdiAllocationBitmapCreate(pbBlockData, getImageBlockSize(&pImage->Header));
+                    if (RT_UNLIKELY(!*ppbmAllocationBitmap))
+                        rc = VERR_NO_MEMORY;
+                    else
+                        rc = VERR_VD_DISCARD_ALIGNMENT_NOT_MET;
+                }
+            } /* if: no complete block discarded */
+        } /* if: Block is allocated. */
         /* else: nothing to do. */
     } while (0);