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 374fef2d59.

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"
 

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 */
+        /* The caller expects us to release the PFN lock */
-        if (Process > HYDRA_PROCESS)
+        MiReleasePfnLock(OldIrql);
-        {
-            /* FIXME: Also support session VM scenario */
-            MiInsertInWorkingSetList(&Process->Vm, Address, Protection);
-        }
         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 */
+                MiReleasePfnLock(LockIrql);
-                MiInsertInWorkingSetList(&CurrentProcess->Vm, Address, ProtectionCode);
-
 
                 /* 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);

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 */
+                    KIRQL OldIrql = MiAcquirePfnLock();
-                    MiRemoveFromWorkingSetList(AddressSpace, MiPteToAddress(PointerPte));
 
                     /* 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(
+MiProcessValidPteList(IN PMMPTE *ValidPteList,
-    _Inout_ PMMSUPPORT Vm,
+                      IN ULONG Count)
-    _Inout_ 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);
+        PageFrameIndex = PFN_FROM_PTE(&TempPte);
-        PMMPFN Pfn1 = MiGetPfnEntry(PageFrameIndex);
+        Pfn1 = MiGetPfnEntry(PageFrameIndex);
-        PMMPFN Pfn2 = MiGetPfnEntry(Pfn1->u4.PteFrame);
+        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;
 }

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