[ros-diffs] 01/01: [NTOSKRNL] Misc fixes to Cc: - CcUnpinDataForThread() only release VACB when the last BCB reference is gone. This avoids having a valid BCB with an invalid VACB - CcRosMarkDirtyVacb() will only accept non-dirty VACB now. This avoids a major bug where a an already dirty VACB was over-dereferenced - Thanks to previous point, simplify CcRosUnmapVacb(), CcRosReleaseVacb() implementation - And only set VACB dirty once in CcSetDirtyPinnedData() - Add a few sanity checks

[Pierre Schweitzer]

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

commit 52287be9a9299ac065fdb676abb99f94007aa5b9
Author:     Pierre Schweitzer <pierre(a)reactos.org>
AuthorDate: Sun Jan 28 11:09:21 2018 +0100
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Sun Jan 28 11:14:15 2018 +0100

    [NTOSKRNL] Misc fixes to Cc:
    - CcUnpinDataForThread() only release VACB when the last BCB reference is gone. This
avoids having a valid BCB with an invalid VACB
    - CcRosMarkDirtyVacb() will only accept non-dirty VACB now. This avoids a major bug
where a an already dirty VACB was over-dereferenced
    - Thanks to previous point, simplify CcRosUnmapVacb(), CcRosReleaseVacb()
implementation
    - And only set VACB dirty once in CcSetDirtyPinnedData()
    - Add a few sanity checks
    
    With that I can again install ReactOS with 128MB RAM :-).
    
    CORE-14263
    CORE-14268
---
 ntoskrnl/cc/pin.c  | 17 +++++++++-------
 ntoskrnl/cc/view.c | 57 ++++++++++++++----------------------------------------
 2 files changed, 25 insertions(+), 49 deletions(-)

diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index 393b206a0f..955bbd0ca7 100644
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -247,7 +247,10 @@ CcSetDirtyPinnedData (
         Bcb, Lsn);
 
     iBcb->Dirty = TRUE;
-    CcRosMarkDirtyVacb(iBcb->Vacb);
+    if (!iBcb->Vacb->Dirty)
+    {
+        CcRosMarkDirtyVacb(iBcb->Vacb);
+    }
 }
 
 
@@ -284,14 +287,14 @@ CcUnpinDataForThread (
         iBcb->Vacb->PinCount--;
     }
 
-    CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
-                     iBcb->Vacb,
-                     TRUE,
-                     iBcb->Dirty,
-                     FALSE);
-
     if (--iBcb->RefCount == 0)
     {
+        CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
+                         iBcb->Vacb,
+                         TRUE,
+                         iBcb->Dirty,
+                         FALSE);
+
         ExDeleteResourceLite(&iBcb->Lock);
         ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
     }
diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index 78167505fa..e846130c79 100644
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -99,6 +99,7 @@ static void CcRosVacbIncRefCount_(PROS_VACB vacb, const char* file, int
line)
 static void CcRosVacbDecRefCount_(PROS_VACB vacb, const char* file, int line)
 {
     --vacb->ReferenceCount;
+    ASSERT(!(vacb->ReferenceCount == 0 && vacb->Dirty));
     if (vacb->SharedCacheMap->Trace)
     {
         DbgPrint("(%s:%i) VACB %p --RefCount=%lu, Dirty %u, PageOut %lu\n",
@@ -518,8 +519,6 @@ CcRosReleaseVacb (
     BOOLEAN Dirty,
     BOOLEAN Mapped)
 {
-    BOOLEAN WasDirty;
-
     ASSERT(SharedCacheMap);
 
     DPRINT("CcRosReleaseVacb(SharedCacheMap 0x%p, Vacb 0x%p, Valid %u)\n",
@@ -527,17 +526,9 @@ CcRosReleaseVacb (
 
     Vacb->Valid = Valid;
 
-    WasDirty = FALSE;
-    if (Dirty)
+    if (Dirty && !Vacb->Dirty)
     {
-        if (!Vacb->Dirty)
-        {
-            CcRosMarkDirtyVacb(Vacb);
-        }
-        else
-        {
-            WasDirty = TRUE;
-        }
+        CcRosMarkDirtyVacb(Vacb);
     }
 
     if (Mapped)
@@ -549,10 +540,6 @@ CcRosReleaseVacb (
     {
         CcRosVacbIncRefCount(Vacb);
     }
-    if (!WasDirty && Vacb->Dirty)
-    {
-        CcRosVacbIncRefCount(Vacb);
-    }
 
     CcRosReleaseVacbLock(Vacb);
 
@@ -618,16 +605,12 @@ CcRosMarkDirtyVacb (
     KeAcquireGuardedMutex(&ViewLock);
     KeAcquireSpinLock(&SharedCacheMap->CacheMapLock, &oldIrql);
 
-    if (!Vacb->Dirty)
-    {
-        InsertTailList(&DirtyVacbListHead, &Vacb->DirtyVacbListEntry);
-        CcTotalDirtyPages += VACB_MAPPING_GRANULARITY / PAGE_SIZE;
-        Vacb->SharedCacheMap->DirtyPages += VACB_MAPPING_GRANULARITY / PAGE_SIZE;
-    }
-    else
-    {
-        CcRosVacbDecRefCount(Vacb);
-    }
+    ASSERT(!Vacb->Dirty);
+
+    InsertTailList(&DirtyVacbListHead, &Vacb->DirtyVacbListEntry);
+    CcTotalDirtyPages += VACB_MAPPING_GRANULARITY / PAGE_SIZE;
+    Vacb->SharedCacheMap->DirtyPages += VACB_MAPPING_GRANULARITY / PAGE_SIZE;
+    CcRosVacbIncRefCount(Vacb);
 
     /* Move to the tail of the LRU list */
     RemoveEntryList(&Vacb->VacbLruListEntry);
@@ -658,7 +641,10 @@ CcRosMarkDirtyFile (
         KeBugCheck(CACHE_MANAGER);
     }
 
-    CcRosMarkDirtyVacb(Vacb);
+    if (!Vacb->Dirty)
+    {
+        CcRosMarkDirtyVacb(Vacb);
+    }
 
     CcRosReleaseVacbLock(Vacb);
 
@@ -673,7 +659,6 @@ CcRosUnmapVacb (
     BOOLEAN NowDirty)
 {
     PROS_VACB Vacb;
-    BOOLEAN WasDirty;
 
     ASSERT(SharedCacheMap);
 
@@ -686,26 +671,14 @@ CcRosUnmapVacb (
         return STATUS_UNSUCCESSFUL;
     }
 
-    WasDirty = FALSE;
-    if (NowDirty)
+    if (NowDirty && !Vacb->Dirty)
     {
-        if (!Vacb->Dirty)
-        {
-            CcRosMarkDirtyVacb(Vacb);
-        }
-        else
-        {
-            WasDirty = TRUE;
-        }
+        CcRosMarkDirtyVacb(Vacb);
     }
 
     Vacb->MappedCount--;
 
     CcRosVacbDecRefCount(Vacb);
-    if (!WasDirty && NowDirty)
-    {
-        CcRosVacbIncRefCount(Vacb);
-    }
     if (Vacb->MappedCount == 0)
     {
         CcRosVacbDecRefCount(Vacb);