[ros-diffs] [reactos] 01/01: [NTOSKRNL] Don't mark VACB dirty on unpin

[Pierre Schweitzer]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8b6696fcdb5897d41a169…

commit 8b6696fcdb5897d41a1692e1bf43f464c4ef3996
Author:     Pierre Schweitzer <pierre(a)reactos.org>
AuthorDate: Wed Apr 17 22:23:57 2019 +0200
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Wed Apr 17 22:35:19 2019 +0200

    [NTOSKRNL] Don't mark VACB dirty on unpin
    
    This could happen if BCB was marked dirty previously.
    Marking VACB dirty on unpin could lead to a double write of
    the VACB, even if clean.
    Indeed, now that setting BCB dirty leads to marking VACB
    dirty, the VACB can be flushed in between by the lazy-writer.
    The BCB state is not reset on VACB flush, contrary to the VACB state.
    Thus, on unpin even if the VACB was already flushed, we were
    setting back the dirty state, leading the VACB to be flushed again.
    
    This could bring a small performance downgrade. Though it remains
    limited since this is mostly used for FS metadata.
    Possibly it could lead to metadata corruption, but this is likely
    less possible.
    
    CORE-15954
---
 ntoskrnl/cc/pin.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index 775b105a2f..044458f7b4 100644
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -169,10 +169,15 @@ CcpDereferenceBcb(
         KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
 
         ASSERT(Bcb->PinCount == 0);
+        /*
+         * Don't mark dirty, if it was dirty,
+         * the VACB was already marked as such
+         * following the call to CcSetDirtyPinnedData
+         */
         CcRosReleaseVacb(SharedCacheMap,
                          Bcb->Vacb,
                          TRUE,
-                         Bcb->Dirty,
+                         FALSE,
                          FALSE);
 
         ExDeleteResourceLite(&Bcb->Lock);
@@ -682,10 +687,15 @@ CcUnpinRepinnedBcb (
             ASSERT(iBcb->PinCount == 0);
         }
 
+        /*
+         * Don't mark dirty, if it was dirty,
+         * the VACB was already marked as such
+         * following the call to CcSetDirtyPinnedData
+         */
         CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
                          iBcb->Vacb,
                          TRUE,
-                         iBcb->Dirty,
+                         FALSE,
                          FALSE);
 
         ExDeleteResourceLite(&iBcb->Lock);