[NTOSKRNL] Drop the VACB lock.
authorPierre Schweitzer <pierre@reactos.org>
Sun, 8 Apr 2018 17:09:36 +0000 (19:09 +0200)
committerPierre Schweitzer <pierre@reactos.org>
Sun, 15 Apr 2018 20:52:53 +0000 (22:52 +0200)
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
ntoskrnl/cc/pin.c
ntoskrnl/cc/view.c
ntoskrnl/include/internal/cc.h

index 31b7ec6..cb98eed 100644 (file)
@@ -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;
index c916783..d81f09d 100644 (file)
@@ -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);
         }
index cde1c2d..1106209 100644 (file)
@@ -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);
index e3c279b..0dcea16 100644 (file)
@@ -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