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
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=52287be9a9299ac065fdb6...
commit 52287be9a9299ac065fdb676abb99f94007aa5b9 Author: Pierre Schweitzer pierre@reactos.org AuthorDate: Sun Jan 28 11:09:21 2018 +0100 Commit: Pierre Schweitzer pierre@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);
-
Pierre Schweitzer