Re-work Cache->Cursor handling in CacheDestroySlab()

Since the advent of lazy slab spilling in commit 7f8b622668fb ("Improve the
performance of the slab allocator"), if the cursor slab is being destroyed,
it no longer means it is the only slab in the last. Remove the ASSERTions
and simply set the new cursor to the current cursor's Flink. This will
either be the next slab which (because slabs are kept in decreasing order
of occupancy) will also be empty, or it will be the list anchor (which
indicates that all slabs in the list are full). Call CacheAudit() after
slab removal to re-verify these invariants.

Also clean up a typo in a commit comment in CacheAudit().

Signed-off-by: Owen Smith <owen.smith@cloud.com>
[Re-worked original patch]
Signed-off-by: Paul Durrant <pdurrant@amazon.com>

diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c
index 6229aeddefc2d0fec7946cb641a94e203fb7234f..b67c5dd7526d82ac865c7e1e1ced2d7c5035e480 100644 (file)
--- a/src/xenbus/cache.c
+++ b/src/xenbus/cache.c
@@ -360,7 +360,7 @@ CacheAudit(
     PLIST_ENTRY         ListEntry;
 
     //
-    // The cursror should point at the first slab that is not fully
+    // The cursor should point at the first slab that is not fully
     // occupied.
     //
     for (ListEntry = Cache->SlabList.Flink;
@@ -478,21 +478,19 @@ CacheDestroySlab(
     Cache->Count -= CacheMaskSize(Slab->Allocated);
 
     //
-    // The only reason the cursor should be pointing at this slab is
-    // if it is the only one in the list.
+    // The cursor slab should always be the first slab in the list that is not
+    // fully occupied. If we are destroying it then clearly it is empty, but
+    // it may be one of several empty slabs. Set the cursor to the current
+    // cursor's Flink so that it will either point at the next empty slab, or
+    // the list anchor if there are no more empty slabs.
     //
-    if (Cache->Cursor == &Slab->ListEntry) {
-        ASSERT3P(Slab->ListEntry.Flink, ==, &Cache->SlabList);
-        ASSERT3P(Slab->ListEntry.Blink, ==, &Cache->SlabList);
-        Cache->Cursor = &Cache->SlabList;
-    }
+    if (Cache->Cursor == &Slab->ListEntry)
+        Cache->Cursor = Slab->ListEntry.Flink;
 
     RemoveEntryList(&Slab->ListEntry);
+    CacheAudit(Cache);
 
-    ASSERT(IMPLY(Cache->Cursor == &Cache->SlabList,
-                 IsListEmpty(&Cache->SlabList)));
-
-    Index = CacheMaskSize(Slab->Allocated);
+    Index = CacheMaskSize(Slab->Constructed);
     while (--Index >= 0) {
         PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size];