19 May
2021
19 May
'21
10:22 p.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=7eff8a36d55c779a49b16…
commit 7eff8a36d55c779a49b16f63ecc0832683a17706
Author: Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Thu May 20 00:19:43 2021 +0200
Commit: Jérôme Gardou <jerome.gardou(a)reactos.org>
CommitDate: Thu May 20 00:19:43 2021 +0200
Revert "[NTOS:MM] Add private pages to process working sets"
This is so full of bugs, I don't know what to say.
This reverts commit 374fef2d59f39b714ff3152f1590f6af843c8bf5.
---
ntoskrnl/include/internal/mm.h | 15 ---------
ntoskrnl/mm/ARM3/pagfault.c | 66 ++++++++++--------------------------
ntoskrnl/mm/ARM3/virtual.c | 76 +++++++++++++++++++++---------------------
ntoskrnl/mm/marea.c | 12 ++++---
4 files changed, 63 insertions(+), 106 deletions(-)
diff --git a/ntoskrnl/include/internal/mm.h b/ntoskrnl/include/internal/mm.h
index a591feeec62..c3391191a17 100644
--- a/ntoskrnl/include/internal/mm.h
+++ b/ntoskrnl/include/internal/mm.h
@@ -1674,21 +1674,6 @@ VOID
NTAPI
MiInitializeWorkingSetList(_Inout_ PMMSUPPORT WorkingSet);
-_Requires_exclusive_lock_held_(Vm->WorkingSetMutex)
-VOID
-NTAPI
-MiInsertInWorkingSetList(
- _Inout_ PMMSUPPORT Vm,
- _In_ PVOID Address,
- _In_ ULONG Protection);
-
-_Requires_exclusive_lock_held_(Vm->WorkingSetMutex)
-VOID
-NTAPI
-MiRemoveFromWorkingSetList(
- _Inout_ PMMSUPPORT Vm,
- _In_ PVOID Address);
-
#ifdef __cplusplus
} // extern "C"
diff --git a/ntoskrnl/mm/ARM3/pagfault.c b/ntoskrnl/mm/ARM3/pagfault.c
index 061577671e9..87c789c1742 100644
--- a/ntoskrnl/mm/ARM3/pagfault.c
+++ b/ntoskrnl/mm/ARM3/pagfault.c
@@ -697,6 +697,16 @@ MiResolveDemandZeroFault(IN PVOID Address,
/* Increment demand zero faults */
KeGetCurrentPrcb()->MmDemandZeroCount++;
+ /* Do we have the lock? */
+ if (HaveLock)
+ {
+ /* Release it */
+ MiReleasePfnLock(OldIrql);
+
+ /* Update performance counters */
+ if (Process > HYDRA_PROCESS) Process->NumberOfPrivatePages++;
+ }
+
/* Zero the page if need be */
if (NeedZero) MiZeroPfn(PageFrameNumber);
@@ -733,19 +743,6 @@ MiResolveDemandZeroFault(IN PVOID Address,
/* Windows does these sanity checks */
ASSERT(Pfn1->u1.Event == 0);
ASSERT(Pfn1->u3.e1.PrototypePte == 0);
-
- /* Release it */
- MiReleasePfnLock(OldIrql);
-
- /* Update performance counters */
- if (Process > HYDRA_PROCESS) Process->NumberOfPrivatePages++;
- }
-
- /* Add the page to our working set, if it's not a proto PTE */
- if ((Process > HYDRA_PROCESS) && (PointerPte == MiAddressToPte(Address)))
- {
- /* FIXME: Also support session VM scenario */
- MiInsertInWorkingSetList(&Process->Vm, Address, Protection);
}
//
@@ -965,9 +962,6 @@ MiResolvePageFileFault(_In_ BOOLEAN StoreInstruction,
KeSetEvent(Pfn1->u1.Event, IO_NO_INCREMENT, FALSE);
}
- /* And we can insert this into the working set */
- MiInsertInWorkingSetList(&CurrentProcess->Vm, FaultingAddress, Protection);
-
return Status;
}
@@ -985,8 +979,6 @@ MiResolveTransitionFault(IN BOOLEAN StoreInstruction,
PMMPFN Pfn1;
MMPTE TempPte;
PMMPTE PointerToPteForProtoPage;
- ULONG Protection;
-
DPRINT("Transition fault on 0x%p with PTE 0x%p in process %s\n",
FaultingAddress, PointerPte, CurrentProcess->ImageFileName);
@@ -1080,9 +1072,8 @@ MiResolveTransitionFault(IN BOOLEAN StoreInstruction,
ASSERT(PointerPte->u.Hard.Valid == 0);
ASSERT(PointerPte->u.Trans.Prototype == 0);
ASSERT(PointerPte->u.Trans.Transition == 1);
- Protection = TempPte.u.Trans.Protection;
TempPte.u.Long = (PointerPte->u.Long & ~0xFFF) |
- (MmProtectToPteMask[Protection]) |
+ (MmProtectToPteMask[PointerPte->u.Trans.Protection]) |
MiDetermineUserGlobalPteMask(PointerPte);
/* Is the PTE writeable? */
@@ -1102,10 +1093,6 @@ MiResolveTransitionFault(IN BOOLEAN StoreInstruction,
/* Write the valid PTE */
MI_WRITE_VALID_PTE(PointerPte, TempPte);
- /* If this was a user fault, add it to the working set */
- if (CurrentProcess > HYDRA_PROCESS)
- MiInsertInWorkingSetList(&CurrentProcess->Vm, FaultingAddress,
Protection);
-
/* Return success */
return STATUS_PAGE_FAULT_TRANSITION;
}
@@ -1247,9 +1234,6 @@ MiResolveProtoPteFault(IN BOOLEAN StoreInstruction,
Pfn1 = MI_PFN_ELEMENT(PageFrameIndex);
MiInitializePfn(PageFrameIndex, PointerPte, TRUE);
- /* The caller expects us to release the PFN lock */
- MiReleasePfnLock(OldIrql);
-
/* Fix the protection */
Protection &= ~MM_WRITECOPY;
Protection |= MM_READWRITE;
@@ -1267,12 +1251,8 @@ MiResolveProtoPteFault(IN BOOLEAN StoreInstruction,
/* And finally, write the valid PTE */
MI_WRITE_VALID_PTE(PointerPte, PteContents);
- /* Add the page to our working set */
- if (Process > HYDRA_PROCESS)
- {
- /* FIXME: Also support session VM scenario */
- MiInsertInWorkingSetList(&Process->Vm, Address, Protection);
- }
+ /* The caller expects us to release the PFN lock */
+ MiReleasePfnLock(OldIrql);
return Status;
}
@@ -2231,7 +2211,6 @@ UserFault:
{
PFN_NUMBER PageFrameIndex, OldPageFrameIndex;
PMMPFN Pfn1;
- ProtectionCode = TempPte.u.Soft.Protection;
LockIrql = MiAcquirePfnLock();
@@ -2255,18 +2234,13 @@ UserFault:
/* And make a new shiny one with our page */
MiInitializePfn(PageFrameIndex, PointerPte, TRUE);
-
- MiReleasePfnLock(LockIrql);
-
TempPte.u.Hard.PageFrameNumber = PageFrameIndex;
TempPte.u.Hard.Write = 1;
TempPte.u.Hard.CopyOnWrite = 0;
MI_WRITE_VALID_PTE(PointerPte, TempPte);
- /* We can now add it to our working set */
- MiInsertInWorkingSetList(&CurrentProcess->Vm, Address,
ProtectionCode);
-
+ MiReleasePfnLock(LockIrql);
/* Return the status */
MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread);
@@ -2383,7 +2357,6 @@ UserFault:
TempPte.u.Soft.Protection = ProtectionCode;
MI_WRITE_INVALID_PTE(PointerPte, TempPte);
}
- ProtectionCode = PointerPte->u.Soft.Protection;
/* Lock the PFN database since we're going to grab a page */
OldIrql = MiAcquirePfnLock();
@@ -2415,14 +2388,14 @@ UserFault:
/* Initialize the PFN entry now */
MiInitializePfn(PageFrameIndex, PointerPte, 1);
- /* And be done with the lock */
- MiReleasePfnLock(OldIrql);
-
/* Increment the count of pages in the process */
CurrentProcess->NumberOfPrivatePages++;
/* One more demand-zero fault */
- InterlockedIncrement(&KeGetCurrentPrcb()->MmDemandZeroCount);
+ KeGetCurrentPrcb()->MmDemandZeroCount++;
+
+ /* And we're done with the lock */
+ MiReleasePfnLock(OldIrql);
/* Fault on user PDE, or fault on user PTE? */
if (PointerPte <= MiHighestUserPte)
@@ -2450,9 +2423,6 @@ UserFault:
Pfn1 = MI_PFN_ELEMENT(PageFrameIndex);
ASSERT(Pfn1->u1.Event == NULL);
- /* We can now insert it into the working set */
- MiInsertInWorkingSetList(&CurrentProcess->Vm, Address,
ProtectionCode);
-
/* Demand zero */
ASSERT(KeGetCurrentIrql() <= APC_LEVEL);
MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread);
diff --git a/ntoskrnl/mm/ARM3/virtual.c b/ntoskrnl/mm/ARM3/virtual.c
index f4d10583150..9396e18b05a 100644
--- a/ntoskrnl/mm/ARM3/virtual.c
+++ b/ntoskrnl/mm/ARM3/virtual.c
@@ -398,6 +398,9 @@ MiDeletePte(IN PMMPTE PointerPte,
PFN_NUMBER PageFrameIndex;
PMMPDE PointerPde;
+ /* PFN lock must be held */
+ MI_ASSERT_PFN_LOCK_HELD();
+
/* Capture the PTE */
TempPte = *PointerPte;
@@ -422,8 +425,6 @@ MiDeletePte(IN PMMPTE PointerPte,
/* Destroy the PTE */
MI_ERASE_PTE(PointerPte);
- KIRQL OldIrql = MiAcquirePfnLock();
-
/* Drop the reference on the page table. */
MiDecrementShareCount(MiGetPfnEntry(Pfn1->u4.PteFrame),
Pfn1->u4.PteFrame);
@@ -443,8 +444,6 @@ MiDeletePte(IN PMMPTE PointerPte,
MI_SET_PFN_DELETED(Pfn1);
MiDecrementReferenceCount(Pfn1, PageFrameIndex);
}
-
- MiReleasePfnLock(OldIrql);
return;
}
}
@@ -475,8 +474,6 @@ MiDeletePte(IN PMMPTE PointerPte,
#if (_MI_PAGING_LEVELS == 2)
}
#endif
-
- KIRQL OldIrql = MiAcquirePfnLock();
/* Drop the share count on the page table */
PointerPde = MiPteToPde(PointerPte);
MiDecrementShareCount(MiGetPfnEntry(PointerPde->u.Hard.PageFrameNumber),
@@ -484,7 +481,6 @@ MiDeletePte(IN PMMPTE PointerPte,
/* Drop the share count */
MiDecrementShareCount(Pfn1, PageFrameIndex);
- MiReleasePfnLock(OldIrql);
/* Either a fork, or this is the shared user data page */
if ((PointerPte <= MiHighestUserPte) && (PrototypePte !=
Pfn1->PteAddress))
@@ -507,9 +503,6 @@ MiDeletePte(IN PMMPTE PointerPte,
}
else
{
- /* Remove this address from the WS list */
- MiRemoveFromWorkingSetList(&CurrentProcess->Vm, VirtualAddress);
-
/* Make sure the saved PTE address is valid */
if ((PMMPTE)((ULONG_PTR)Pfn1->PteAddress & ~0x1) != PointerPte)
{
@@ -524,7 +517,6 @@ MiDeletePte(IN PMMPTE PointerPte,
/* Erase the PTE */
MI_ERASE_PTE(PointerPte);
- KIRQL OldIrql = MiAcquirePfnLock();
/* There should only be 1 shared reference count */
ASSERT(Pfn1->u2.ShareCount == 1);
@@ -534,7 +526,6 @@ MiDeletePte(IN PMMPTE PointerPte,
/* Mark the PFN for deletion and dereference what should be the last ref */
MI_SET_PFN_DELETED(Pfn1);
MiDecrementShareCount(Pfn1, PageFrameIndex);
- MiReleasePfnLock(OldIrql);
/* We should eventually do this */
//CurrentProcess->NumberOfPrivatePages--;
@@ -2348,23 +2339,24 @@ MiProtectVirtualMemory(IN PEPROCESS Process,
if ((NewAccessProtection & PAGE_NOACCESS) ||
(NewAccessProtection & PAGE_GUARD))
{
- /* Remove this from the working set */
- MiRemoveFromWorkingSetList(AddressSpace,
MiPteToAddress(PointerPte));
+ KIRQL OldIrql = MiAcquirePfnLock();
/* Mark the PTE as transition and change its protection */
PteContents.u.Hard.Valid = 0;
PteContents.u.Soft.Transition = 1;
PteContents.u.Trans.Protection = ProtectionMask;
/* Decrease PFN share count and write the PTE */
- KIRQL OldIrql = MiAcquirePfnLock();
MiDecrementShareCount(Pfn1, PFN_FROM_PTE(&PteContents));
- MiReleasePfnLock(OldIrql);
+ // FIXME: remove the page from the WS
MI_WRITE_INVALID_PTE(PointerPte, PteContents);
#ifdef CONFIG_SMP
// FIXME: Should invalidate entry in every CPU TLB
ASSERT(FALSE);
#endif
KeInvalidateTlbEntry(MiPteToAddress(PointerPte));
+
+ /* We are done for this PTE */
+ MiReleasePfnLock(OldIrql);
}
else
{
@@ -2493,44 +2485,41 @@ MiMakePdeExistAndMakeValid(IN PMMPDE PointerPde,
VOID
NTAPI
-MiProcessValidPteList(
- _Inout_ PMMSUPPORT Vm,
- _Inout_ PMMPTE *ValidPteList,
- _In_ ULONG Count)
+MiProcessValidPteList(IN PMMPTE *ValidPteList,
+ IN ULONG Count)
{
+ KIRQL OldIrql;
ULONG i;
+ MMPTE TempPte;
+ PFN_NUMBER PageFrameIndex;
+ PMMPFN Pfn1, Pfn2;
+
//
- // Loop all the PTEs in the list
+ // Acquire the PFN lock and loop all the PTEs in the list
//
+ OldIrql = MiAcquirePfnLock();
for (i = 0; i != Count; i++)
{
//
// The PTE must currently be valid
//
- MMPTE TempPte = *ValidPteList[i];
+ TempPte = *ValidPteList[i];
ASSERT(TempPte.u.Hard.Valid == 1);
- //
- // We can now remove this addres from the working set
- //
- MiRemoveFromWorkingSetList(Vm, MiPteToAddress(ValidPteList[i]));
-
//
// Get the PFN entry for the page itself, and then for its page table
//
- PFN_NUMBER PageFrameIndex = PFN_FROM_PTE(&TempPte);
- PMMPFN Pfn1 = MiGetPfnEntry(PageFrameIndex);
- PMMPFN Pfn2 = MiGetPfnEntry(Pfn1->u4.PteFrame);
+ PageFrameIndex = PFN_FROM_PTE(&TempPte);
+ Pfn1 = MiGetPfnEntry(PageFrameIndex);
+ Pfn2 = MiGetPfnEntry(Pfn1->u4.PteFrame);
//
// Decrement the share count on the page table, and then on the page
// itself
//
- KIRQL OldIrql = MiAcquirePfnLock();
MiDecrementShareCount(Pfn2, Pfn1->u4.PteFrame);
MI_SET_PFN_DELETED(Pfn1);
MiDecrementShareCount(Pfn1, PageFrameIndex);
- MiReleasePfnLock(OldIrql);
//
// Make the page decommitted
@@ -2543,6 +2532,7 @@ MiProcessValidPteList(
// and then release the PFN lock
//
KeFlushCurrentTb();
+ MiReleasePfnLock(OldIrql);
}
ULONG
@@ -2557,6 +2547,7 @@ MiDecommitPages(IN PVOID StartingAddress,
ULONG CommitReduction = 0;
PMMPTE ValidPteList[256];
ULONG PteCount = 0;
+ PMMPFN Pfn1;
MMPTE PteContents;
PETHREAD CurrentThread = PsGetCurrentThread();
@@ -2588,10 +2579,10 @@ MiDecommitPages(IN PVOID StartingAddress,
// such, and does not flush the entire TLB all the time, but right
// now we have bigger problems to worry about than TLB flushing.
//
- PointerPde = MiPteToPde(PointerPte);
+ PointerPde = MiAddressToPde(StartingAddress);
if (PteCount)
{
- MiProcessValidPteList(&Process->Vm, ValidPteList, PteCount);
+ MiProcessValidPteList(ValidPteList, PteCount);
PteCount = 0;
}
@@ -2626,13 +2617,21 @@ MiDecommitPages(IN PVOID StartingAddress,
//Process->NumberOfPrivatePages--;
if (PteContents.u.Hard.Valid)
{
+ //
+ // It's valid. At this point make sure that it is not a ROS
+ // PFN. Also, we don't support ProtoPTEs in this code path.
+ //
+ Pfn1 = MiGetPfnEntry(PteContents.u.Hard.PageFrameNumber);
+ ASSERT(MI_IS_ROS_PFN(Pfn1) == FALSE);
+ ASSERT(Pfn1->u3.e1.PrototypePte == FALSE);
+
//
// Flush any pending PTEs that we had not yet flushed, if our
// list has gotten too big, then add this PTE to the flush list.
//
if (PteCount == 256)
{
- MiProcessValidPteList(&Process->Vm, ValidPteList,
PteCount);
+ MiProcessValidPteList(ValidPteList, PteCount);
PteCount = 0;
}
ValidPteList[PteCount++] = PointerPte;
@@ -2662,7 +2661,7 @@ MiDecommitPages(IN PVOID StartingAddress,
// This used to be a zero PTE and it no longer is, so we must add a
// reference to the pagetable.
//
- MiIncrementPageTableReferences(MiPteToAddress(PointerPte));
+ MiIncrementPageTableReferences(StartingAddress);
//
// Next, we account for decommitted PTEs and make the PTE as such
@@ -2672,16 +2671,17 @@ MiDecommitPages(IN PVOID StartingAddress,
}
//
- // Move to the next PTE
+ // Move to the next PTE and the next address
//
PointerPte++;
+ StartingAddress = (PVOID)((ULONG_PTR)StartingAddress + PAGE_SIZE);
}
//
// Flush any dangling PTEs from the loop in the last page table, and then
// release the working set and return the commit reduction accounting.
//
- if (PteCount) MiProcessValidPteList(&Process->Vm, ValidPteList, PteCount);
+ if (PteCount) MiProcessValidPteList(ValidPteList, PteCount);
MiUnlockProcessWorkingSetUnsafe(Process, CurrentThread);
return CommitReduction;
}
diff --git a/ntoskrnl/mm/marea.c b/ntoskrnl/mm/marea.c
index d2ade0eb04b..1942dd76b27 100644
--- a/ntoskrnl/mm/marea.c
+++ b/ntoskrnl/mm/marea.c
@@ -572,14 +572,15 @@ MmDeleteProcessAddressSpace(PEPROCESS Process)
#if (_MI_PAGING_LEVELS == 2)
{
+ KIRQL OldIrql;
PVOID Address;
PMMPDE pointerPde;
- KAPC_STATE ApcState;
/* Attach to Process */
- KeStackAttachProcess(&Process->Pcb, &ApcState);
+ KeAttachProcess(&Process->Pcb);
- MiLockProcessWorkingSet(Process, PsGetCurrentThread());
+ /* Acquire PFN lock */
+ OldIrql = MiAcquirePfnLock();
for (Address = MI_LOWEST_VAD_ADDRESS;
Address < MM_HIGHEST_VAD_ADDRESS;
@@ -603,10 +604,11 @@ MmDeleteProcessAddressSpace(PEPROCESS Process)
ASSERT(pointerPde->u.Hard.Valid == 0);
}
- MiUnlockProcessWorkingSet(Process, PsGetCurrentThread());
+ /* Release lock */
+ MiReleasePfnLock(OldIrql);
/* Detach */
- KeUnstackDetachProcess(&ApcState);
+ KeDetachProcess();
}
#endif