[ros-diffs] 04/05: [NTOSKRNL] Drop the VACB lock. This has have several benefits for ReactOS Cc: - It helps reducing potential deadlocks situations in Cc - It speeds up ReactOS by reducing locks - It gets us a bit closer to Windows VACB

[Pierre Schweitzer]

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

commit 953dc72dad7117d681120e6c2423431dab824ffe
Author:     Pierre Schweitzer <pierre(a)reactos.org>
AuthorDate: Sun Apr 8 19:09:36 2018 +0200
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Sun Apr 15 22:52:53 2018 +0200

    [NTOSKRNL] Drop the VACB lock.
    This has have several benefits for ReactOS Cc:
    - It helps reducing potential deadlocks situations in Cc
    - It speeds up ReactOS by reducing locks
    - It gets us a bit closer to Windows VACB
    
    CORE-14349
---
 ntoskrnl/cc/fs.c               |  6 +++--
 ntoskrnl/cc/pin.c              |  5 -----
 ntoskrnl/cc/view.c             | 50 +++++++++++++++---------------------------
 ntoskrnl/include/internal/cc.h | 41 ++++++++++++++--------------------
 4 files changed, 38 insertions(+), 64 deletions(-)

diff --git a/ntoskrnl/cc/fs.c b/ntoskrnl/cc/fs.c
index 31b7ec6bac..cb98eed153 100644
--- a/ntoskrnl/cc/fs.c
+++ b/ntoskrnl/cc/fs.c
@@ -251,12 +251,14 @@ CcPurgeCacheSection (
 
     while (!IsListEmpty(&FreeList))
     {
+        ULONG Refs;
+
         Vacb = CONTAINING_RECORD(RemoveHeadList(&FreeList),
                                  ROS_VACB,
                                  CacheMapVacbListEntry);
         InitializeListHead(&Vacb->CacheMapVacbListEntry);
-        CcRosVacbDecRefCount(Vacb);
-        CcRosInternalFreeVacb(Vacb);
+        Refs = CcRosVacbDecRefCount(Vacb);
+        ASSERT(Refs == 0);
     }
 
     return Success;
diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index c9167834c0..d81f09d470 100644
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -215,7 +215,6 @@ CcPinRead (
 
             iBcb->Pinned = TRUE;
             iBcb->Vacb->PinCount++;
-            CcRosReleaseVacbLock(iBcb->Vacb);
 
             if (Flags & PIN_EXCLUSIVE)
             {
@@ -308,7 +307,6 @@ CcUnpinDataForThread (
     {
         ExReleaseResourceForThreadLite(&iBcb->Lock, ResourceThreadId);
         iBcb->Pinned = FALSE;
-        CcRosAcquireVacbLock(iBcb->Vacb, NULL);
         iBcb->Vacb->PinCount--;
     }
 
@@ -360,7 +358,6 @@ CcUnpinRepinnedBcb (
         IoStatus->Information = 0;
         if (WriteThrough)
         {
-            CcRosAcquireVacbLock(iBcb->Vacb, NULL);
             if (iBcb->Vacb->Dirty)
             {
                 IoStatus->Status = CcRosFlushVacb(iBcb->Vacb);
@@ -369,7 +366,6 @@ CcUnpinRepinnedBcb (
             {
                 IoStatus->Status = STATUS_SUCCESS;
             }
-            CcRosReleaseVacbLock(iBcb->Vacb);
         }
         else
         {
@@ -380,7 +376,6 @@ CcUnpinRepinnedBcb (
         {
             ExReleaseResourceLite(&iBcb->Lock);
             iBcb->Pinned = FALSE;
-            CcRosAcquireVacbLock(iBcb->Vacb, NULL);
             iBcb->Vacb->PinCount--;
             ASSERT(iBcb->Vacb->PinCount == 0);
         }
diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index cde1c2d6fa..11062095f5 100644
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -90,6 +90,11 @@ ULONG CcRosVacbDecRefCount_(PROS_VACB vacb, PCSTR file, INT line)
                  file, line, vacb, Refs, vacb->Dirty, vacb->PageOut);
     }
 
+    if (Refs == 0)
+    {
+        CcRosInternalFreeVacb(vacb);
+    }
+
     return Refs;
 }
 ULONG CcRosVacbGetRefCount_(PROS_VACB vacb, PCSTR file, INT line)
@@ -107,9 +112,6 @@ ULONG CcRosVacbGetRefCount_(PROS_VACB vacb, PCSTR file, INT line)
 }
 #endif
 
-NTSTATUS
-CcRosInternalFreeVacb(PROS_VACB Vacb);
-
 
 /* FUNCTIONS *****************************************************************/
 
@@ -187,12 +189,10 @@ CcRosFlushDirtyPages (
     PROS_VACB current;
     BOOLEAN Locked;
     NTSTATUS Status;
-    LARGE_INTEGER ZeroTimeout;
 
     DPRINT("CcRosFlushDirtyPages(Target %lu)\n", Target);
 
     (*Count) = 0;
-    ZeroTimeout.QuadPart = 0;
 
     KeEnterCriticalRegion();
     KeAcquireGuardedMutex(&ViewLock);
@@ -228,22 +228,11 @@ CcRosFlushDirtyPages (
             continue;
         }
 
-        Status = CcRosAcquireVacbLock(current,
-                                      Wait ? NULL : &ZeroTimeout);
-        if (Status != STATUS_SUCCESS)
-        {
-            current->SharedCacheMap->Callbacks->ReleaseFromLazyWrite(
-                current->SharedCacheMap->LazyWriteContext);
-            CcRosVacbDecRefCount(current);
-            continue;
-        }
-
         ASSERT(current->Dirty);
 
         /* One reference is added above */
         if (CcRosVacbGetRefCount(current) > 2)
         {
-            CcRosReleaseVacbLock(current);
             current->SharedCacheMap->Callbacks->ReleaseFromLazyWrite(
                 current->SharedCacheMap->LazyWriteContext);
             CcRosVacbDecRefCount(current);
@@ -254,7 +243,6 @@ CcRosFlushDirtyPages (
 
         Status = CcRosFlushVacb(current);
 
-        CcRosReleaseVacbLock(current);
         current->SharedCacheMap->Callbacks->ReleaseFromLazyWrite(
             current->SharedCacheMap->LazyWriteContext);
 
@@ -410,13 +398,15 @@ retry:
 
     while (!IsListEmpty(&FreeList))
     {
+        ULONG Refs;
+
         current_entry = RemoveHeadList(&FreeList);
         current = CONTAINING_RECORD(current_entry,
                                     ROS_VACB,
                                     CacheMapVacbListEntry);
         InitializeListHead(&current->CacheMapVacbListEntry);
-        CcRosVacbDecRefCount(current);
-        CcRosInternalFreeVacb(current);
+        Refs = CcRosVacbDecRefCount(current);
+        ASSERT(Refs == 0);
     }
 
     DPRINT("Evicted %lu cache pages\n", (*NrFreed));
@@ -457,8 +447,6 @@ CcRosReleaseVacb (
     Refs = CcRosVacbDecRefCount(Vacb);
     ASSERT(Refs > 0);
 
-    CcRosReleaseVacbLock(Vacb);
-
     return STATUS_SUCCESS;
 }
 
@@ -494,7 +482,6 @@ CcRosLookupVacb (
             CcRosVacbIncRefCount(current);
             KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql);
             KeReleaseGuardedMutex(&ViewLock);
-            CcRosAcquireVacbLock(current, NULL);
             return current;
         }
         if (current->FileOffset.QuadPart > FileOffset)
@@ -718,6 +705,7 @@ CcRosCreateVacb (
     PLIST_ENTRY current_entry;
     NTSTATUS Status;
     KIRQL oldIrql;
+    ULONG Refs;
 
     ASSERT(SharedCacheMap);
 
@@ -745,7 +733,6 @@ CcRosCreateVacb (
     current->MappedCount = 0;
     current->ReferenceCount = 0;
     current->PinCount = 0;
-    KeInitializeMutex(&current->Mutex, 0);
     InitializeListHead(&current->CacheMapVacbListEntry);
     InitializeListHead(&current->DirtyVacbListEntry);
     InitializeListHead(&current->VacbLruListEntry);
@@ -760,7 +747,6 @@ CcRosCreateVacb (
         return Status;
     }
 
-    CcRosAcquireVacbLock(current, NULL);
     KeAcquireGuardedMutex(&ViewLock);
 
     *Vacb = current;
@@ -792,12 +778,12 @@ CcRosCreateVacb (
                         current);
             }
 #endif
-            CcRosVacbDecRefCount(*Vacb);
-            CcRosReleaseVacbLock(*Vacb);
             KeReleaseGuardedMutex(&ViewLock);
-            CcRosInternalFreeVacb(*Vacb);
+
+            Refs = CcRosVacbDecRefCount(*Vacb);
+            ASSERT(Refs == 0);
+
             *Vacb = current;
-            CcRosAcquireVacbLock(current, NULL);
             return STATUS_SUCCESS;
         }
         if (current->FileOffset.QuadPart < FileOffset)
@@ -1111,7 +1097,6 @@ CcRosDeleteFileCache (
             KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql);
 
             current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry);
-            CcRosAcquireVacbLock(current, NULL);
             RemoveEntryList(&current->VacbLruListEntry);
             InitializeListHead(&current->VacbLruListEntry);
             if (current->Dirty)
@@ -1122,7 +1107,6 @@ CcRosDeleteFileCache (
                 DPRINT1("Freeing dirty VACB\n");
             }
             InsertHeadList(&FreeList, &current->CacheMapVacbListEntry);
-            CcRosReleaseVacbLock(current);
 
             KeAcquireSpinLock(&SharedCacheMap->CacheMapLock, &oldIrql);
         }
@@ -1136,11 +1120,13 @@ CcRosDeleteFileCache (
 
         while (!IsListEmpty(&FreeList))
         {
+            ULONG Refs;
+
             current_entry = RemoveTailList(&FreeList);
             current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry);
             InitializeListHead(&current->CacheMapVacbListEntry);
-            CcRosVacbDecRefCount(current);
-            CcRosInternalFreeVacb(current);
+            Refs = CcRosVacbDecRefCount(current);
+            ASSERT(Refs == 0);
         }
 
         OldIrql = KeAcquireQueuedSpinLock(LockQueueMasterLock);
diff --git a/ntoskrnl/include/internal/cc.h b/ntoskrnl/include/internal/cc.h
index e3c279b6f8..0dcea16f43 100644
--- a/ntoskrnl/include/internal/cc.h
+++ b/ntoskrnl/include/internal/cc.h
@@ -217,12 +217,9 @@ typedef struct _ROS_VACB
     LIST_ENTRY VacbLruListEntry;
     /* Offset in the file which this view maps. */
     LARGE_INTEGER FileOffset;
-    /* Mutex */
-    KMUTEX Mutex;
     /* Number of references. */
     volatile ULONG ReferenceCount;
     /* How many times was it pinned? */
-    _Guarded_by_(Mutex)
     LONG PinCount;
     /* Pointer to the shared cache map for the file which this view maps data for. */
     PROS_SHARED_CACHE_MAP SharedCacheMap;
@@ -470,28 +467,9 @@ VOID
 CcPerformReadAhead(
     IN PFILE_OBJECT FileObject);
 
-FORCEINLINE
 NTSTATUS
-CcRosAcquireVacbLock(
-    _Inout_ PROS_VACB Vacb,
-    _In_ PLARGE_INTEGER Timeout)
-{
-    NTSTATUS Status;
-    Status = KeWaitForSingleObject(&Vacb->Mutex,
-                                   Executive,
-                                   KernelMode,
-                                   FALSE,
-                                   Timeout);
-    return Status;
-}
-
-FORCEINLINE
-VOID
-CcRosReleaseVacbLock(
-    _Inout_ PROS_VACB Vacb)
-{
-    KeReleaseMutex(&Vacb->Mutex, FALSE);
-}
+CcRosInternalFreeVacb(
+    IN PROS_VACB Vacb);
 
 FORCEINLINE
 BOOLEAN
@@ -545,6 +523,19 @@ CcRosVacbGetRefCount_(
 
 #else
 #define CcRosVacbIncRefCount(vacb)
InterlockedIncrement((PLONG)&(vacb)->ReferenceCount)
-#define CcRosVacbDecRefCount(vacb)
InterlockedDecrement((PLONG)&(vacb)->ReferenceCount)
+FORCEINLINE
+ULONG
+CcRosVacbDecRefCount(
+    PROS_VACB vacb)
+{
+    ULONG Refs;
+
+    Refs = InterlockedDecrement((PLONG)&vacb->ReferenceCount);
+    if (Refs == 0)
+    {
+        CcRosInternalFreeVacb(vacb);
+    }
+    return Refs;
+}
 #define CcRosVacbGetRefCount(vacb)
InterlockedCompareExchange((PLONG)&(vacb)->ReferenceCount, 0, 0)
 #endif