01/05: [NTOSKRNL] Move the PinCount out of the VACB to the BCB

[Pierre Schweitzer]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f2849476220161e202e25…

commit f2849476220161e202e25b000d8c68a9d4123e00
Author:     Pierre Schweitzer <pierre(a)reactos.org>
AuthorDate: Fri Oct 5 19:43:10 2018 +0200
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Fri Oct 5 21:26:16 2018 +0200

    [NTOSKRNL] Move the PinCount out of the VACB to the BCB
    
    Given current ReactOS implementation, a VACB can be pinned
    several times, with different BCB. In next commits, a single
    BCB will be able to be pinned several times. That would
    lead to severe inconsistencies in counting and thus corruption.
---
 ntoskrnl/cc/pin.c              | 23 ++++++++++-------------
 ntoskrnl/cc/view.c             |  8 +++-----
 ntoskrnl/include/internal/cc.h |  4 +---
 3 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index 3c8747e76b..53d51ed824 100644
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -136,7 +136,7 @@ CcpMapData(
     iBcb->PFCB.MappedFileOffset = *FileOffset;
     iBcb->Vacb = Vacb;
     iBcb->Dirty = FALSE;
-    iBcb->Pinned = FALSE;
+    iBcb->PinCount = 0;
     iBcb->RefCount = 1;
     ExInitializeResourceLite(&iBcb->Lock);
     *pBcb = (PVOID)iBcb;
@@ -215,10 +215,9 @@ CcPinMappedData (
     }
 
     iBcb = *Bcb;
-    ASSERT(iBcb->Pinned == FALSE);
+    ASSERT(iBcb->PinCount == 0);
 
-    iBcb->Pinned = TRUE;
-    iBcb->Vacb->PinCount++;
+    iBcb->PinCount++;
 
     if (BooleanFlagOn(Flags, PIN_EXCLUSIVE))
     {
@@ -231,8 +230,7 @@ CcPinMappedData (
 
     if (!Result)
     {
-        iBcb->Pinned = FALSE;
-        iBcb->Vacb->PinCount--;
+        iBcb->PinCount--;
     }
 
     return Result;
@@ -353,11 +351,10 @@ CcUnpinDataForThread (
 
     CCTRACE(CC_API_DEBUG, "Bcb=%p ResourceThreadId=%lu\n", Bcb,
ResourceThreadId);
 
-    if (iBcb->Pinned)
+    if (iBcb->PinCount != 0)
     {
         ExReleaseResourceForThreadLite(&iBcb->Lock, ResourceThreadId);
-        iBcb->Pinned = FALSE;
-        iBcb->Vacb->PinCount--;
+        iBcb->PinCount--;
     }
 
     if (--iBcb->RefCount == 0)
@@ -365,6 +362,7 @@ CcUnpinDataForThread (
         KIRQL OldIrql;
         PROS_SHARED_CACHE_MAP SharedCacheMap;
 
+        ASSERT(iBcb->PinCount == 0);
         SharedCacheMap = iBcb->Vacb->SharedCacheMap;
         CcRosReleaseVacb(SharedCacheMap,
                          iBcb->Vacb,
@@ -432,12 +430,11 @@ CcUnpinRepinnedBcb (
             IoStatus->Status = STATUS_SUCCESS;
         }
 
-        if (iBcb->Pinned)
+        if (iBcb->PinCount != 0)
         {
             ExReleaseResourceLite(&iBcb->Lock);
-            iBcb->Pinned = FALSE;
-            iBcb->Vacb->PinCount--;
-            ASSERT(iBcb->Vacb->PinCount == 0);
+            iBcb->PinCount--;
+            ASSERT(iBcb->PinCount == 0);
         }
 
         SharedCacheMap = iBcb->Vacb->SharedCacheMap;
diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index e3c8285254..b9c4930986 100644
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -808,7 +808,6 @@ CcRosCreateVacb (
 #endif
     current->MappedCount = 0;
     current->ReferenceCount = 0;
-    current->PinCount = 0;
     InitializeListHead(&current->CacheMapVacbListEntry);
     InitializeListHead(&current->DirtyVacbListEntry);
     InitializeListHead(&current->VacbLruListEntry);
@@ -1056,16 +1055,15 @@ CcRosInternalFreeVacb (
                      NULL);
     MmUnlockAddressSpace(MmGetKernelAddressSpace());
 
-    if (Vacb->PinCount != 0 || Vacb->ReferenceCount != 0)
+    if (Vacb->ReferenceCount != 0)
     {
-        DPRINT1("Invalid free: %ld, %ld\n", Vacb->ReferenceCount,
Vacb->PinCount);
+        DPRINT1("Invalid free: %ld\n", Vacb->ReferenceCount);
         if (Vacb->SharedCacheMap->FileObject &&
Vacb->SharedCacheMap->FileObject->FileName.Length)
         {
             DPRINT1("For file: %wZ\n",
&Vacb->SharedCacheMap->FileObject->FileName);
         }
     }
 
-    ASSERT(Vacb->PinCount == 0);
     ASSERT(Vacb->ReferenceCount == 0);
     ASSERT(IsListEmpty(&Vacb->CacheMapVacbListEntry));
     ASSERT(IsListEmpty(&Vacb->DirtyVacbListEntry));
@@ -1229,7 +1227,7 @@ CcRosDeleteFileCache (
             {
                 DPRINT1("Leaking VACB %p attached to %p (%I64d)\n", current,
FileObject, current->FileOffset.QuadPart);
                 DPRINT1("There are: %d references left\n", Refs);
-                DPRINT1("Pin: %d, Map: %d\n", current->PinCount,
current->MappedCount);
+                DPRINT1("Map: %d\n", current->MappedCount);
                 DPRINT1("Dirty: %d\n", current->Dirty);
                 if (FileObject->FileName.Length != 0)
                 {
diff --git a/ntoskrnl/include/internal/cc.h b/ntoskrnl/include/internal/cc.h
index 0c12004326..16dd1f67f0 100644
--- a/ntoskrnl/include/internal/cc.h
+++ b/ntoskrnl/include/internal/cc.h
@@ -221,8 +221,6 @@ typedef struct _ROS_VACB
     LARGE_INTEGER FileOffset;
     /* Number of references. */
     volatile ULONG ReferenceCount;
-    /* How many times was it pinned? */
-    LONG PinCount;
     /* Pointer to the shared cache map for the file which this view maps data for. */
     PROS_SHARED_CACHE_MAP SharedCacheMap;
     /* Pointer to the next VACB in a chain. */
@@ -235,7 +233,7 @@ typedef struct _INTERNAL_BCB
     PUBLIC_BCB PFCB;
     PROS_VACB Vacb;
     BOOLEAN Dirty;
-    BOOLEAN Pinned;
+    ULONG PinCount;
     CSHORT RefCount; /* (At offset 0x34 on WinNT4) */
     LIST_ENTRY BcbEntry;
 } INTERNAL_BCB, *PINTERNAL_BCB;