From 36a92e6ea5823ce684dfb1886494bebc918ff6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Gardou?= Date: Fri, 5 Feb 2021 15:04:07 +0100 Subject: [PATCH] [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);