[ros-diffs] 02/02: [NTOSKRNL] Use interlocked operations for VACB reference counting.

[Pierre Schweitzer]

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

commit 14b05e65ffaa11a286b370e160c503fbffdc69b9
Author:     Pierre Schweitzer <pierre(a)reactos.org>
AuthorDate: Sat Mar 24 19:15:16 2018 +0100
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Sat Mar 24 19:15:58 2018 +0100

    [NTOSKRNL] Use interlocked operations for VACB reference counting.
    
    CORE-14480
    CORE-14285
---
 ntoskrnl/cc/fs.c               |  9 ++++---
 ntoskrnl/cc/view.c             | 56 +++++++++++++++++++++++++++++++-----------
 ntoskrnl/include/internal/cc.h | 18 ++++++++++----
 3 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/ntoskrnl/cc/fs.c b/ntoskrnl/cc/fs.c
index 05a9a89475..2ab7b5f8ea 100644
--- a/ntoskrnl/cc/fs.c
+++ b/ntoskrnl/cc/fs.c
@@ -207,6 +207,8 @@ CcPurgeCacheSection (
     ListEntry = SharedCacheMap->CacheMapVacbListHead.Flink;
     while (ListEntry != &SharedCacheMap->CacheMapVacbListHead)
     {
+        ULONG Refs;
+
         Vacb = CONTAINING_RECORD(ListEntry, ROS_VACB, CacheMapVacbListEntry);
         ListEntry = ListEntry->Flink;
 
@@ -225,15 +227,16 @@ CcPurgeCacheSection (
         /* Still in use, it cannot be purged, fail
          * Allow one ref: VACB is supposed to be always 1-referenced
          */
-        if ((Vacb->ReferenceCount > 1 && !Vacb->Dirty) ||
-            (Vacb->ReferenceCount > 2 && Vacb->Dirty))
+        Refs = CcRosVacbGetRefCount(Vacb);
+        if ((Refs > 1 && !Vacb->Dirty) ||
+            (Refs > 2 && Vacb->Dirty))
         {
             Success = FALSE;
             break;
         }
 
         /* This VACB is in range, so unlink it and mark for free */
-        ASSERT(Vacb->ReferenceCount == 1 || Vacb->Dirty);
+        ASSERT(Refs == 1 || Vacb->Dirty);
         RemoveEntryList(&Vacb->VacbLruListEntry);
         if (Vacb->Dirty)
         {
diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index 100e22bf9e..9cc45bdb3e 100644
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -65,25 +65,45 @@ KSPIN_LOCK CcDeferredWriteSpinLock;
 LIST_ENTRY CcCleanSharedCacheMapList;
 
 #if DBG
-VOID CcRosVacbIncRefCount_(PROS_VACB vacb, PCSTR file, INT line)
+ULONG CcRosVacbIncRefCount_(PROS_VACB vacb, PCSTR file, INT line)
 {
-    ++vacb->ReferenceCount;
+    ULONG Refs;
+
+    Refs = InterlockedIncrement((PLONG)&vacb->ReferenceCount);
     if (vacb->SharedCacheMap->Trace)
     {
         DbgPrint("(%s:%i) VACB %p ++RefCount=%lu, Dirty %u, PageOut %lu\n",
-                 file, line, vacb, vacb->ReferenceCount, vacb->Dirty,
vacb->PageOut);
+                 file, line, vacb, Refs, vacb->Dirty, vacb->PageOut);
     }
+
+    return Refs;
 }
-VOID CcRosVacbDecRefCount_(PROS_VACB vacb, PCSTR file, INT line)
+ULONG CcRosVacbDecRefCount_(PROS_VACB vacb, PCSTR file, INT line)
 {
-    ASSERT(vacb->ReferenceCount != 0);
-    --vacb->ReferenceCount;
-    ASSERT(!(vacb->ReferenceCount == 0 && vacb->Dirty));
+    ULONG Refs;
+
+    Refs = InterlockedDecrement((PLONG)&vacb->ReferenceCount);
+    ASSERT(!(Refs == 0 && vacb->Dirty));
     if (vacb->SharedCacheMap->Trace)
     {
         DbgPrint("(%s:%i) VACB %p --RefCount=%lu, Dirty %u, PageOut %lu\n",
-                 file, line, vacb, vacb->ReferenceCount, vacb->Dirty,
vacb->PageOut);
+                 file, line, vacb, Refs, vacb->Dirty, vacb->PageOut);
     }
+
+    return Refs;
+}
+ULONG CcRosVacbGetRefCount_(PROS_VACB vacb, PCSTR file, INT line)
+{
+    ULONG Refs;
+
+    Refs = InterlockedCompareExchange((PLONG)&vacb->ReferenceCount, 0, 0);
+    if (vacb->SharedCacheMap->Trace)
+    {
+        DbgPrint("(%s:%i) VACB %p ==RefCount=%lu, Dirty %u, PageOut %lu\n",
+                 file, line, vacb, Refs, vacb->Dirty, vacb->PageOut);
+    }
+
+    return Refs;
 }
 #endif
 
@@ -221,7 +241,7 @@ CcRosFlushDirtyPages (
         ASSERT(current->Dirty);
 
         /* One reference is added above */
-        if (current->ReferenceCount > 2)
+        if (CcRosVacbGetRefCount(current) > 2)
         {
             CcRosReleaseVacbLock(current);
             current->SharedCacheMap->Callbacks->ReleaseFromLazyWrite(
@@ -311,6 +331,8 @@ retry:
     current_entry = VacbLruListHead.Flink;
     while (current_entry != &VacbLruListHead)
     {
+        ULONG Refs;
+
         current = CONTAINING_RECORD(current_entry,
                                     ROS_VACB,
                                     VacbLruListEntry);
@@ -342,14 +364,14 @@ retry:
         }
 
         /* Dereference the VACB */
-        CcRosVacbDecRefCount(current);
+        Refs = CcRosVacbDecRefCount(current);
 
         /* Check if we can free this entry now */
-        if (current->ReferenceCount < 2)
+        if (Refs < 2)
         {
             ASSERT(!current->Dirty);
             ASSERT(!current->MappedCount);
-            ASSERT(current->ReferenceCount == 1);
+            ASSERT(Refs == 1);
 
             RemoveEntryList(&current->CacheMapVacbListEntry);
             RemoveEntryList(&current->VacbLruListEntry);
@@ -409,6 +431,7 @@ CcRosReleaseVacb (
     BOOLEAN Dirty,
     BOOLEAN Mapped)
 {
+    ULONG Refs;
     ASSERT(SharedCacheMap);
 
     DPRINT("CcRosReleaseVacb(SharedCacheMap 0x%p, Vacb 0x%p, Valid %u)\n",
@@ -425,13 +448,13 @@ CcRosReleaseVacb (
     {
         Vacb->MappedCount++;
     }
-    CcRosVacbDecRefCount(Vacb);
+    Refs = CcRosVacbDecRefCount(Vacb);
     if (Mapped && (Vacb->MappedCount == 1))
     {
         CcRosVacbIncRefCount(Vacb);
     }
 
-    ASSERT(Vacb->ReferenceCount > 0);
+    ASSERT(Refs > 0);
 
     CcRosReleaseVacbLock(Vacb);
 
@@ -835,6 +858,7 @@ CcRosGetVacb (
 {
     PROS_VACB current;
     NTSTATUS Status;
+    ULONG Refs;
 
     ASSERT(SharedCacheMap);
 
@@ -856,6 +880,8 @@ CcRosGetVacb (
         }
     }
 
+    Refs = CcRosVacbGetRefCount(current);
+
     KeAcquireGuardedMutex(&ViewLock);
 
     /* Move to the tail of the LRU list */
@@ -873,7 +899,7 @@ CcRosGetVacb (
     *Vacb = current;
     *BaseOffset = current->FileOffset.QuadPart;
 
-    ASSERT(current->ReferenceCount > 1);
+    ASSERT(Refs > 1);
 
     return STATUS_SUCCESS;
 }
diff --git a/ntoskrnl/include/internal/cc.h b/ntoskrnl/include/internal/cc.h
index ad939a12a9..e3c279b6f8 100644
--- a/ntoskrnl/include/internal/cc.h
+++ b/ntoskrnl/include/internal/cc.h
@@ -220,7 +220,7 @@ typedef struct _ROS_VACB
     /* Mutex */
     KMUTEX Mutex;
     /* Number of references. */
-    ULONG ReferenceCount;
+    volatile ULONG ReferenceCount;
     /* How many times was it pinned? */
     _Guarded_by_(Mutex)
     LONG PinCount;
@@ -523,20 +523,28 @@ IsPointInRange(
 #if DBG
 #define CcRosVacbIncRefCount(vacb) CcRosVacbIncRefCount_(vacb,__FILE__,__LINE__)
 #define CcRosVacbDecRefCount(vacb) CcRosVacbDecRefCount_(vacb,__FILE__,__LINE__)
+#define CcRosVacbGetRefCount(vacb) CcRosVacbGetRefCount_(vacb,__FILE__,__LINE__)
 
-VOID
+ULONG
 CcRosVacbIncRefCount_(
     PROS_VACB vacb,
     PCSTR file,
     INT line);
 
-VOID
+ULONG
 CcRosVacbDecRefCount_(
     PROS_VACB vacb,
     PCSTR file,
     INT line);
 
+ULONG
+CcRosVacbGetRefCount_(
+    PROS_VACB vacb,
+    PCSTR file,
+    INT line);
+
 #else
-#define CcRosVacbIncRefCount(vacb) (++((vacb)->ReferenceCount))
-#define CcRosVacbDecRefCount(vacb) (--((vacb)->ReferenceCount))
+#define CcRosVacbIncRefCount(vacb)
InterlockedIncrement((PLONG)&(vacb)->ReferenceCount)
+#define CcRosVacbDecRefCount(vacb)
InterlockedDecrement((PLONG)&(vacb)->ReferenceCount)
+#define CcRosVacbGetRefCount(vacb)
InterlockedCompareExchange((PLONG)&(vacb)->ReferenceCount, 0, 0)
 #endif