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
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=953dc72dad7117d681120e...
commit 953dc72dad7117d681120e6c2423431dab824ffe Author: Pierre Schweitzer pierre@reactos.org AuthorDate: Sun Apr 8 19:09:36 2018 +0200 Commit: Pierre Schweitzer pierre@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(¤t->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(¤t->Mutex, 0); InitializeListHead(¤t->CacheMapVacbListEntry); InitializeListHead(¤t->DirtyVacbListEntry); InitializeListHead(¤t->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(¤t->VacbLruListEntry); InitializeListHead(¤t->VacbLruListEntry); if (current->Dirty) @@ -1122,7 +1107,6 @@ CcRosDeleteFileCache ( DPRINT1("Freeing dirty VACB\n"); } InsertHeadList(&FreeList, ¤t->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(¤t->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
-
Pierre Schweitzer