https://git.reactos.org/?p=reactos.git;a=commitdiff;h=36a92e6ea5823ce684dfb1886494bebc918ff6ae

commit 36a92e6ea5823ce684dfb1886494bebc918ff6ae
Author:     Jérôme Gardou <[email protected]>
AuthorDate: Fri Feb 5 15:04:07 2021 +0100
Commit:     Jérôme Gardou <[email protected]>
CommitDate: Thu Apr 8 15:40:37 2021 +0200

    [NTOS:MM] Fix a bit the page-out/page-in logic
    
     - Do not lock the section segment when we are serving a fault for a 
process private page.
     - Do not keep the process address space lock while writing to pagefile.
     - Do not wait for an event that might never be set.
---
 ntoskrnl/cache/section/newmm.h |  10 ----
 ntoskrnl/include/internal/mm.h |   3 +-
 ntoskrnl/mm/i386/page.c        |  27 ++--------
 ntoskrnl/mm/mmfault.c          |   5 +-
 ntoskrnl/mm/rmap.c             |  47 +++++++++++------
 ntoskrnl/mm/section.c          | 111 ++++++++++++++++++-----------------------
 6 files changed, 88 insertions(+), 115 deletions(-)

diff --git a/ntoskrnl/cache/section/newmm.h b/ntoskrnl/cache/section/newmm.h
index 2efcd9cf7d5..cf40d688edf 100644
--- a/ntoskrnl/cache/section/newmm.h
+++ b/ntoskrnl/cache/section/newmm.h
@@ -17,16 +17,6 @@
 
 #define SEC_CACHE                           (0x20000000)
 
-#define MiWaitForPageEvent(Process,Address) do {                         \
-    DPRINT("MiWaitForPageEvent %p:%p #\n", Process, Address);            \
-    KeWaitForSingleObject(&MmWaitPageEvent, 0, KernelMode, FALSE, NULL); \
-} while(0)
-
-#define MiSetPageEvent(Process,Address) do {              \
-    DPRINT("MiSetPageEvent %p:%p #\n",Process, (PVOID)(Address));  \
-    KeSetEvent(&MmWaitPageEvent, IO_NO_INCREMENT, FALSE); \
-} while(0)
-
 /* We store 8 bits of location with a page association */
 #define ENTRIES_PER_ELEMENT 256
 
diff --git a/ntoskrnl/include/internal/mm.h b/ntoskrnl/include/internal/mm.h
index 074dbd4e454..d05360c28be 100644
--- a/ntoskrnl/include/internal/mm.h
+++ b/ntoskrnl/include/internal/mm.h
@@ -1376,7 +1376,8 @@ NTAPI
 MmAccessFaultSectionView(
     PMMSUPPORT AddressSpace,
     MEMORY_AREA* MemoryArea,
-    PVOID Address
+    PVOID Address,
+    BOOLEAN Locked
 );
 
 VOID
diff --git a/ntoskrnl/mm/i386/page.c b/ntoskrnl/mm/i386/page.c
index b541b5d4752..4690a9a5d15 100644
--- a/ntoskrnl/mm/i386/page.c
+++ b/ntoskrnl/mm/i386/page.c
@@ -219,6 +219,8 @@ MmGetPageTableForProcess(PEPROCESS Process, PVOID Address, 
BOOLEAN Create, PKIRQ
             PMMPDE PdeBase;
             ULONG PdeOffset = MiGetPdeOffset(Address);
 
+            ASSERT(!Create);
+
             PdeBase = MiMapPageInHyperSpace(PsGetCurrentProcess(),
                                             
PTE_TO_PFN(Process->Pcb.DirectoryTableBase[0]),
                                             OldIrql);
@@ -229,29 +231,8 @@ MmGetPageTableForProcess(PEPROCESS Process, PVOID Address, 
BOOLEAN Create, PKIRQ
             PointerPde = PdeBase + PdeOffset;
             if (PointerPde->u.Hard.Valid == 0)
             {
-                KAPC_STATE ApcState;
-                NTSTATUS Status;
-
-                if (!Create)
-                {
-                    MiUnmapPageInHyperSpace(PsGetCurrentProcess(), PdeBase, 
*OldIrql);
-                    return NULL;
-                }
-
-                KeStackAttachProcess(&Process->Pcb, &ApcState);
-
-                Status = MiDispatchFault(0x1,
-                                     MiAddressToPte(Address),
-                                     MiAddressToPde(Address),
-                                     NULL,
-                                     FALSE,
-                                     Process,
-                                     NULL,
-                                     NULL);
-
-                KeUnstackDetachProcess(&ApcState);
-                if (!NT_SUCCESS(Status))
-                    return NULL;
+                MiUnmapPageInHyperSpace(PsGetCurrentProcess(), PdeBase, 
*OldIrql);
+                return NULL;
             }
 
             Pfn = PointerPde->u.Hard.PageFrameNumber;
diff --git a/ntoskrnl/mm/mmfault.c b/ntoskrnl/mm/mmfault.c
index 75f7b584a22..410b2f558e8 100644
--- a/ntoskrnl/mm/mmfault.c
+++ b/ntoskrnl/mm/mmfault.c
@@ -77,7 +77,8 @@ MmpAccessFault(KPROCESSOR_MODE Mode,
         case MEMORY_AREA_SECTION_VIEW:
             Status = MmAccessFaultSectionView(AddressSpace,
                                               MemoryArea,
-                                              (PVOID)Address);
+                                              (PVOID)Address,
+                                              !FromMdl);
             break;
 #ifdef NEWCC
         case MEMORY_AREA_CACHE:
@@ -169,7 +170,7 @@ MmNotPresentFault(KPROCESSOR_MODE Mode,
             Status = MmNotPresentFaultSectionView(AddressSpace,
                                                   MemoryArea,
                                                   (PVOID)Address,
-                                                  FromMdl);
+                                                  !FromMdl);
             break;
 #ifdef NEWCC
         case MEMORY_AREA_CACHE:
diff --git a/ntoskrnl/mm/rmap.c b/ntoskrnl/mm/rmap.c
index a2169eeb54f..973b8342a5e 100644
--- a/ntoskrnl/mm/rmap.c
+++ b/ntoskrnl/mm/rmap.c
@@ -141,15 +141,13 @@ GetEntry:
             /* The segment is being read or something. Give up */
             MmUnlockSectionSegment(Segment);
             MmUnlockAddressSpace(AddressSpace);
-            if (Address < MmSystemRangeStart)
-            {
-                ExReleaseRundownProtection(&Process->RundownProtect);
-                ObDereferenceObject(Process);
-            }
+            ExReleaseRundownProtection(&Process->RundownProtect);
+            ObDereferenceObject(Process);
             return(STATUS_UNSUCCESSFUL);
         }
 
         /* Delete this virtual mapping in the process */
+        MmDeleteRmap(Page, Process, Address);
         MmDeleteVirtualMapping(Process, Address, &Dirty, &MapPage);
 
         /* We checked this earlier */
@@ -162,6 +160,11 @@ GetEntry:
             /* This page is private to the process */
             MmUnlockSectionSegment(Segment);
 
+            /* Attach to it, if needed */
+            ASSERT(PsGetCurrentProcess() == PsInitialSystemProcess);
+            if (Process != PsInitialSystemProcess)
+                KeAttachProcess(&Process->Pcb);
+
             /* Check if we should write it back to the page file */
             SwapEntry = MmGetSavedSwapEntryPage(Page);
 
@@ -177,14 +180,14 @@ GetEntry:
 
                     /* We can't, so let this page in the Process VM */
                     MmCreateVirtualMapping(Process, Address, Region->Protect, 
&Page, 1);
+                    MmInsertRmap(Page, Process, Address);
                     MmSetDirtyPage(Process, Address);
 
                     MmUnlockAddressSpace(AddressSpace);
-                    if (Address < MmSystemRangeStart)
-                    {
-                        ExReleaseRundownProtection(&Process->RundownProtect);
-                        ObDereferenceObject(Process);
-                    }
+                    if (Process != PsInitialSystemProcess)
+                        KeDetachProcess();
+                    ExReleaseRundownProtection(&Process->RundownProtect);
+                    ObDereferenceObject(Process);
 
                     return STATUS_UNSUCCESSFUL;
                 }
@@ -192,7 +195,18 @@ GetEntry:
 
             if (Dirty)
             {
+                SWAPENTRY Dummy;
+
+                /* Put a wait entry into the process and unlock */
+                MmCreatePageFileMapping(Process, Address, MM_WAIT_ENTRY);
+                MmUnlockAddressSpace(AddressSpace);
+
                 Status = MmWriteToSwapPage(SwapEntry, Page);
+
+                MmLockAddressSpace(AddressSpace);
+                MmDeletePageFileMapping(Process, Address, &Dummy);
+                ASSERT(Dummy == MM_WAIT_ENTRY);
+
                 if (!NT_SUCCESS(Status))
                 {
                     /* We failed at saving the content of this page. Keep it 
in */
@@ -206,9 +220,12 @@ GetEntry:
 
                     /* We can't, so let this page in the Process VM */
                     MmCreateVirtualMapping(Process, Address, Region->Protect, 
&Page, 1);
+                    MmInsertRmap(Page, Process, Address);
                     MmSetDirtyPage(Process, Address);
 
                     MmUnlockAddressSpace(AddressSpace);
+                    if (Process != PsInitialSystemProcess)
+                        KeDetachProcess();
                     ExReleaseRundownProtection(&Process->RundownProtect);
                     ObDereferenceObject(Process);
 
@@ -223,10 +240,11 @@ GetEntry:
                 MmSetSavedSwapEntryPage(Page, 0);
             }
 
-            MmUnlockAddressSpace(AddressSpace);
-
             /* We can finally let this page go */
-            MmDeleteRmap(Page, Process, Address);
+
+            MmUnlockAddressSpace(AddressSpace);
+            if (Process != PsInitialSystemProcess)
+                KeDetachProcess();
 #if DBG
             OldIrql = MiAcquirePfnLock();
             ASSERT(MmGetRmapListHeadPage(Page) == NULL);
@@ -240,9 +258,6 @@ GetEntry:
             return STATUS_SUCCESS;
         }
 
-        /* Delete this RMAP */
-        MmDeleteRmap(Page, Process, Address);
-
         /* One less mapping referencing this segment */
         Released = MmUnsharePageEntrySectionSegment(MemoryArea, Segment, 
&Offset, Dirty, TRUE, NULL);
 
diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index 3ff8098937d..88392afee7d 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -1461,7 +1461,11 @@ MmAlterViewAttributes(PMMSUPPORT AddressSpace,
                 MmGetPageFileMapping(Process, Address, &SwapEntry);
                 if (SwapEntry != MM_WAIT_ENTRY)
                     break;
-                MiWaitForPageEvent(Process, Address);
+                MmUnlockSectionSegment(Segment);
+                MmUnlockAddressSpace(AddressSpace);
+                YieldProcessor();
+                MmLockAddressSpace(AddressSpace);
+                MmLockSectionSegment(Segment);
             }
             while (TRUE);
 
@@ -1522,6 +1526,8 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
     PEPROCESS Process = MmGetAddressSpaceOwner(AddressSpace);
     SWAPENTRY SwapEntry;
 
+    ASSERT(Locked);
+
     /*
      * There is a window between taking the page fault and locking the
      * address space when another thread could load the page so we check
@@ -1577,39 +1583,6 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         return STATUS_GUARD_PAGE_VIOLATION;
     }
 
-    /*
-     * Lock the segment
-     */
-    MmLockSectionSegment(Segment);
-    Entry = MmGetPageEntrySectionSegment(Segment, &Offset);
-    /*
-     * Check if this page needs to be mapped COW
-     */
-    if ((Segment->WriteCopy) &&
-            (Region->Protect == PAGE_READWRITE ||
-             Region->Protect == PAGE_EXECUTE_READWRITE))
-    {
-        Attributes = Region->Protect == PAGE_READWRITE ? PAGE_READONLY : 
PAGE_EXECUTE_READ;
-    }
-    else
-    {
-        Attributes = Region->Protect;
-    }
-
-    /*
-     * Check if someone else is already handling this fault, if so wait
-     * for them
-     */
-    if (Entry && MM_IS_WAIT_PTE(Entry))
-    {
-        MmUnlockSectionSegment(Segment);
-        MmUnlockAddressSpace(AddressSpace);
-        MiWaitForPageEvent(NULL, NULL);
-        MmLockAddressSpace(AddressSpace);
-        DPRINT("Address 0x%p\n", Address);
-        return STATUS_MM_RESTART_OPERATION;
-    }
-
     HasSwapEntry = MmIsPageSwapEntry(Process, Address);
 
     /* See if we should use a private page */
@@ -1620,22 +1593,21 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         MmGetPageFileMapping(Process, Address, &SwapEntry);
         if (SwapEntry == MM_WAIT_ENTRY)
         {
-            MmUnlockSectionSegment(Segment);
             MmUnlockAddressSpace(AddressSpace);
-            MiWaitForPageEvent(NULL, NULL);
+            YieldProcessor();
             MmLockAddressSpace(AddressSpace);
             return STATUS_MM_RESTART_OPERATION;
         }
 
         /*
-            * Must be private page we have swapped out.
-            */
+         * Must be private page we have swapped out.
+         */
 
         /*
         * Sanity check
         */
-        MmDeletePageFileMapping(Process, Address, &SwapEntry);
-        MmUnlockSectionSegment(Segment);
+        MmDeletePageFileMapping(Process, Address, &DummyEntry);
+        ASSERT(DummyEntry == SwapEntry);
 
         /* Tell everyone else we are serving the fault. */
         MmCreatePageFileMapping(Process, Address, MM_WAIT_ENTRY);
@@ -1650,18 +1622,17 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
             KeBugCheck(MEMORY_MANAGEMENT);
         }
 
-        if (HasSwapEntry)
+        Status = MmReadFromSwapPage(SwapEntry, Page);
+        if (!NT_SUCCESS(Status))
         {
-            Status = MmReadFromSwapPage(SwapEntry, Page);
-            if (!NT_SUCCESS(Status))
-            {
-                DPRINT1("MmReadFromSwapPage failed, status = %x\n", Status);
-                KeBugCheck(MEMORY_MANAGEMENT);
-            }
+            DPRINT1("MmReadFromSwapPage failed, status = %x\n", Status);
+            KeBugCheck(MEMORY_MANAGEMENT);
         }
 
         MmLockAddressSpace(AddressSpace);
         MmDeletePageFileMapping(Process, PAddress, &DummyEntry);
+        ASSERT(DummyEntry == MM_WAIT_ENTRY);
+
         Status = MmCreateVirtualMapping(Process,
                                         PAddress,
                                         Region->Protect,
@@ -1677,8 +1648,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         /*
          * Store the swap entry for later use.
          */
-        if (HasSwapEntry)
-            MmSetSavedSwapEntryPage(Page, SwapEntry);
+        MmSetSavedSwapEntryPage(Page, SwapEntry);
 
         /*
          * Add the page to the process's working set
@@ -1687,11 +1657,15 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         /*
          * Finish the operation
          */
-        MiSetPageEvent(Process, Address);
         DPRINT("Address 0x%p\n", Address);
         return STATUS_SUCCESS;
     }
 
+    /*
+     * Lock the segment
+     */
+    MmLockSectionSegment(Segment);
+
     /*
      * Satisfying a page fault on a map of /Device/PhysicalMemory is easy
      */
@@ -1717,16 +1691,29 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         /*
          * Cleanup and release locks
          */
-        MiSetPageEvent(Process, Address);
         DPRINT("Address 0x%p\n", Address);
         return STATUS_SUCCESS;
     }
 
+    /*
+     * Check if this page needs to be mapped COW
+     */
+    if ((Segment->WriteCopy) &&
+            (Region->Protect == PAGE_READWRITE ||
+             Region->Protect == PAGE_EXECUTE_READWRITE))
+    {
+        Attributes = Region->Protect == PAGE_READWRITE ? PAGE_READONLY : 
PAGE_EXECUTE_READ;
+    }
+    else
+    {
+        Attributes = Region->Protect;
+    }
+
+
     /*
      * Get the entry corresponding to the offset within the section
      */
     Entry = MmGetPageEntrySectionSegment(Segment, &Offset);
-
     if (Entry == 0)
     {
         /*
@@ -1752,7 +1739,6 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
             if (Process)
                 MmInsertRmap(Page, Process, Address);
 
-            MiSetPageEvent(Process, Address);
             DPRINT("Address 0x%p\n", Address);
             return STATUS_SUCCESS;
         }
@@ -1792,7 +1778,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         {
             MmUnlockSectionSegment(Segment);
             MmUnlockAddressSpace(AddressSpace);
-            MiWaitForPageEvent(NULL, NULL);
+            YieldProcessor();
             MmLockAddressSpace(AddressSpace);
             return STATUS_MM_RESTART_OPERATION;
         }
@@ -1800,6 +1786,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         /*
         * Release all our locks and read in the page from disk
         */
+        MmSetPageEntrySectionSegment(Segment, &Offset, 
MAKE_SWAP_SSE(MM_WAIT_ENTRY));
         MmUnlockSectionSegment(Segment);
 
         MmUnlockAddressSpace(AddressSpace);
@@ -1826,7 +1813,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
          * that has a pending page-in.
          */
         Entry1 = MmGetPageEntrySectionSegment(Segment, &Offset);
-        if (Entry != Entry1)
+        if (Entry1 != MAKE_SWAP_SSE(MM_WAIT_ENTRY))
         {
             DPRINT1("Someone changed ppte entry while we slept (%x vs %x)\n", 
Entry, Entry1);
             KeBugCheck(MEMORY_MANAGEMENT);
@@ -1859,7 +1846,6 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         MmSetPageEntrySectionSegment(Segment, &Offset, Entry);
         MmUnlockSectionSegment(Segment);
 
-        MiSetPageEvent(Process, Address);
         DPRINT("Address 0x%p\n", Address);
         return STATUS_SUCCESS;
     }
@@ -1886,7 +1872,6 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
         MmSharePageEntrySectionSegment(Segment, &Offset);
         MmUnlockSectionSegment(Segment);
 
-        MiSetPageEvent(Process, Address);
         DPRINT("Address 0x%p\n", Address);
         return STATUS_SUCCESS;
     }
@@ -1896,7 +1881,8 @@ NTSTATUS
 NTAPI
 MmAccessFaultSectionView(PMMSUPPORT AddressSpace,
                          MEMORY_AREA* MemoryArea,
-                         PVOID Address)
+                         PVOID Address,
+                         BOOLEAN Locked)
 {
     PMM_SECTION_SEGMENT Segment;
     PFN_NUMBER OldPage;
@@ -1911,7 +1897,7 @@ MmAccessFaultSectionView(PMMSUPPORT AddressSpace,
     DPRINT("MmAccessFaultSectionView(%p, %p, %p)\n", AddressSpace, MemoryArea, 
Address);
 
     /* Make sure we have a page mapping for this address.  */
-    Status = MmNotPresentFaultSectionView(AddressSpace, MemoryArea, Address, 
TRUE);
+    Status = MmNotPresentFaultSectionView(AddressSpace, MemoryArea, Address, 
Locked);
     if (!NT_SUCCESS(Status))
     {
         /* This is invalid access ! */
@@ -2011,7 +1997,6 @@ MmAccessFaultSectionView(PMMSUPPORT AddressSpace,
     if (Process)
         MmInsertRmap(NewPage, Process, PAddress);
 
-    MiSetPageEvent(Process, Address);
     DPRINT("Address 0x%p\n", Address);
     return STATUS_SUCCESS;
 }
@@ -3383,7 +3368,7 @@ MmFreeSectionPage(PVOID Context, MEMORY_AREA* MemoryArea, 
PVOID Address,
         MmUnlockSectionSegment(Segment);
         MmUnlockAddressSpace(AddressSpace);
 
-        MiWaitForPageEvent(NULL, NULL);
+        YieldProcessor();
 
         MmLockAddressSpace(AddressSpace);
         MmLockSectionSegment(Segment);
@@ -4600,7 +4585,7 @@ MmRosFlushVirtualMemory(
         while (MM_IS_WAIT_PTE(Entry))
         {
             MmUnlockSectionSegment(Segment);
-            MiWaitForPageEvent(NULL, NULL);
+            YieldProcessor();
             MmLockSectionSegment(Segment);
             Entry = MmGetPageEntrySectionSegment(Segment, &SegmentOffset);
         }
@@ -5010,7 +4995,7 @@ MmMakePagesDirty(
         {
             MmUnlockSectionSegment(Segment);
             MmUnlockAddressSpace(AddressSpace);
-            MiWaitForPageEvent(NULL, NULL);
+            YieldProcessor();
             MmLockAddressSpace(AddressSpace);
             MmLockSectionSegment(Segment);
             Entry = MmGetPageEntrySectionSegment(Segment, &SegmentOffset);

Reply via email to