[ros-diffs] [jgardou] 55770: [NTOSKRNL/MM] - finally, release user shared data at process address space cleanup. - release PDE pages that might not be freed at process end. - Let the caller handle PDE release ...

Author: jgardou
Date: Tue Feb 21 00:32:24 2012
New Revision: 55770

URL: http://svn.reactos.org/svn/reactos?rev=55770&view=rev
Log:
[NTOSKRNL/MM]
 - finally, release user shared data at process address space cleanup.
 - release PDE pages that might not be freed at process end.
 - Let the caller handle PDE release when deleting a PTE
 - restore Richard's ASSERT : All user PDE pages are now freed!

Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c
    trunk/reactos/ntoskrnl/mm/ARM3/procsup.c
    trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
    trunk/reactos/ntoskrnl/mm/balance.c
    trunk/reactos/ntoskrnl/mm/i386/page.c
    trunk/reactos/ntoskrnl/mm/marea.c
    trunk/reactos/ntoskrnl/mm/section.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -931,8 +931,11 @@
         ASSERT(Pfn1->u3.e2.ReferenceCount != 0);
         if (Pfn1->u3.e2.ReferenceCount == 1)
         {
-            /* In ReactOS, this path should always be hit with a deleted PFN */
-            ASSERT(MI_IS_PFN_DELETED(Pfn1) == TRUE);
+            if(Pfn1->u3.e1.PrototypePte == 0)
+            {
+                /* In ReactOS, this path should always be hit with a deleted PFN */
+                ASSERT(MI_IS_PFN_DELETED(Pfn1) == TRUE);
+            }
 
             /* Clear the last reference */
             Pfn1->u3.e2.ReferenceCount = 0;

Modified: trunk/reactos/ntoskrnl/mm/ARM3/procsup.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/procsup.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/procsup.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/procsup.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -1349,6 +1349,8 @@
         /* Free the VAD memory */
         ExFreePool(Vad);
     }
+    /* Delete the shared user data section */
+    MiDeleteVirtualAddresses(USER_SHARED_DATA, USER_SHARED_DATA, NULL);
 
     /* Release the address space */
     MmUnlockAddressSpace(&Process->Vm);
@@ -1363,14 +1365,6 @@
     PFN_NUMBER PageFrameIndex;
 
     //ASSERT(Process->CommitCharge == 0);
-
-    /* Delete the shared user data section (Should be done in clean, not delete) */
-    ASSERT(MmHighestUserAddress > (PVOID)USER_SHARED_DATA);
-    KeAttachProcess(&Process->Pcb);
-    //DPRINT1("Killing shared user data page no longer works -- has someone changed
ARM3 in a way to make this fail now?\n");
-    //MiDeleteVirtualAddresses(USER_SHARED_DATA, USER_SHARED_DATA, NULL);
-    //DPRINT1("Done\n");
-    KeDetachProcess();
     
     /* Acquire the PFN lock */
     OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock);
@@ -1407,11 +1401,8 @@
         MiDecrementShareCount(Pfn1, PageFrameIndex);
         MiDecrementShareCount(Pfn1, PageFrameIndex);
         
-        /* HACK: In Richard's original patch this ASSERT did work */
-        //DPRINT1("Ref count: %lx %lx\n", Pfn1->u3.e2.ReferenceCount,
Pfn1->u2.ShareCount);
-        //ASSERT((Pfn1->u3.e2.ReferenceCount == 0) ||
(Pfn1->u3.e1.WriteInProgress));
-        if(!((Pfn1->u3.e2.ReferenceCount == 0) || (Pfn1->u3.e1.WriteInProgress)))
-            DPRINT1("Ref count: %lx %lx\n", Pfn1->u3.e2.ReferenceCount,
Pfn1->u2.ShareCount);
+        /* Page table is now dead. Bye bye... */
+        ASSERT((Pfn1->u3.e2.ReferenceCount == 0) ||
(Pfn1->u3.e1.WriteInProgress));
     }
     else
     {

Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -258,9 +258,6 @@
 #if (_MI_PAGING_LEVELS == 2)
         }
 #endif
-        /* Drop the reference on the page table. */
-        MiDecrementShareCount(MiGetPfnEntry(PFN_FROM_PTE(PointerPde)),
PFN_FROM_PTE(PointerPde));
-
         /* Drop the share count */
         MiDecrementShareCount(Pfn1, PageFrameIndex);
 

Modified: trunk/reactos/ntoskrnl/mm/balance.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/balance.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/balance.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/balance.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -250,6 +250,9 @@
     {
         /* Clean up the unused PDEs */
         ULONG_PTR Address;
+        PEPROCESS Process = PsGetCurrentProcess();
+
+        /* Acquire PFN lock */
         KIRQL OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock);
         PMMPDE pointerPde;
         for(Address = (ULONG_PTR)MI_LOWEST_VAD_ADDRESS;
@@ -260,10 +263,11 @@
             {
                 pointerPde = MiAddressToPde(Address);
                 if(pointerPde->u.Hard.Valid)
-                    MiDeletePte(pointerPde, MiPdeToPte(pointerPde),
PsGetCurrentProcess(), NULL);
+                    MiDeletePte(pointerPde, MiPdeToPte(pointerPde), Process, NULL);
                 ASSERT(pointerPde->u.Hard.Valid == 0);
             }
         }
+        /* Release lock */
         KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql);
     }
 #endif

Modified: trunk/reactos/ntoskrnl/mm/i386/page.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/i386/page.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/i386/page.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/i386/page.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -248,6 +248,10 @@
             PMMPDE PdeBase;
             ULONG PdeOffset = MiGetPdeOffset(Address);
             
+            /* Nobody but page fault should ask for creating the PDE,
+             * Which imples that Process is the current one */
+            ASSERT(Create == FALSE);
+            
             PdeBase =
MmCreateHyperspaceMapping(PTE_TO_PFN(Process->Pcb.DirectoryTableBase[0]));
             if (PdeBase == NULL)
             {
@@ -256,24 +260,8 @@
             PointerPde = PdeBase + PdeOffset;
             if (PointerPde->u.Hard.Valid == 0)
             {
-                /* Nobody but page fault should ask for creating the PDE,
-                 * Which imples that Process is the current one */
                 MmDeleteHyperspaceMapping(PdeBase);
-                if(Create == FALSE)
-                    return NULL;
-                else
-                {
-                    KAPC_STATE ApcState;
-                    PULONG ret;
-                    /* Attach to process */
-                    KeStackAttachProcess(&Process->Pcb, &ApcState);
-                    
-                    /* Retry */
-                    ret = MmGetPageTableForProcess(Process, Address, TRUE);
-                    
-                    /* Get Back to original process */
-                    KeUnstackDetachProcess(&ApcState);
-                }
+                return NULL;
             }
             else
             {
@@ -717,9 +705,10 @@
         KeBugCheck(MEMORY_MANAGEMENT);
     }
 
-    Pt = MmGetPageTableForProcess(Process, Address, TRUE);
+    Pt = MmGetPageTableForProcess(Process, Address, FALSE);
     if (Pt == NULL)
     {
+        /* Nobody should page out an address that hasn't even been mapped */
         KeBugCheck(MEMORY_MANAGEMENT);
     }
     Pte = InterlockedExchangePte(Pt, SwapEntry << 1);

Modified: trunk/reactos/ntoskrnl/mm/marea.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/marea.c?rev=55…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/marea.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/marea.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -1066,6 +1066,37 @@
             KeBugCheck(MEMORY_MANAGEMENT);
       }
    }
+   
+#if (_MI_PAGING_LEVELS == 2)
+    {
+        KIRQL OldIrql;
+        PMMPDE pointerPde;
+        /* Attach to Process */
+        KeAttachProcess(&Process->Pcb);
+        
+        /* Acquire PFN lock */
+        OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock);
+        
+        for(Address = MI_LOWEST_VAD_ADDRESS;
+            Address < MM_HIGHEST_VAD_ADDRESS;
+            Address =(PVOID)((ULONG_PTR)Address + (PAGE_SIZE * PTE_COUNT)))
+        {
+            /* At this point all references should be dead */
+            ASSERT(MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)] ==
0);
+            pointerPde = MiAddressToPde(Address);
+            /* Unlike in ARM3, we don't necesarrily free the PDE page as soon as
reference reaches 0,
+             * so we must clean up a bit when process closes */
+            if(pointerPde->u.Hard.Valid)
+                MiDeletePte(pointerPde, MiPdeToPte(pointerPde), Process, NULL);
+            ASSERT(pointerPde->u.Hard.Valid == 0);
+        }
+        /* Release lock */
+        KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql);
+        
+        /* Detach */
+        KeDetachProcess();
+    }
+#endif
 
    MmUnlockAddressSpace(&Process->Vm);
 

Modified: trunk/reactos/ntoskrnl/mm/section.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] Tue Feb 21 00:32:24 2012
@@ -119,6 +119,7 @@
    ULONG Offset;
    BOOLEAN WasDirty;
    BOOLEAN Private;
+   PEPROCESS CallingProcess;
 }
 MM_SECTION_PAGEOUT_CONTEXT;
 
@@ -2052,10 +2053,11 @@
    }
 
 #if (_MI_PAGING_LEVELS == 2)
-    if(Address < MmSystemRangeStart)
+    /* If this is for the calling process, we take care of te reference in the main
function */
+    if((Address < MmSystemRangeStart) && (Process !=
PageOutContext->CallingProcess))
     {
-        if(Process->VmDeleted) DPRINT1("deleted!!!");
        
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+       
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
     }
 #endif
 
@@ -2091,6 +2093,7 @@
     */
    Context.Segment = MemoryArea->Data.SectionData.Segment;
    Context.Section = MemoryArea->Data.SectionData.Section;
+   Context.CallingProcess = Process;
 
    Context.Offset = (ULONG)((ULONG_PTR)Address -
(ULONG_PTR)MemoryArea->StartingAddress
                     + MemoryArea->Data.SectionData.ViewOffset);
@@ -2223,6 +2226,14 @@
       }
       if (!Context.WasDirty && SwapEntry != 0)
       {
+#if (_MI_PAGING_LEVELS == 2)
+        /* We keep the pagefile index global to the segment, not in the PTE */
+        if(Address < MmSystemRangeStart)
+        {
+           
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+           
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
+        }
+#endif
          MmSetSavedSwapEntryPage(Page, 0);
          MmSetPageEntrySectionSegment(Context.Segment, Context.Offset,
MAKE_SWAP_SSE(SwapEntry));
          MmReleasePageMemoryConsumer(MC_USER, Page);
@@ -2241,6 +2252,14 @@
       }
       if (!Context.WasDirty || SwapEntry != 0)
       {
+#if (_MI_PAGING_LEVELS == 2)
+        /* We keep the pagefile index global to the segment, not in the PTE */
+         if(Address < MmSystemRangeStart)
+         {
+           
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+           
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
+         }
+#endif
          MmSetSavedSwapEntryPage(Page, 0);
          if (SwapEntry != 0)
          {
@@ -2254,6 +2273,14 @@
    }
    else if (!Context.Private && DirectMapped)
    {
+#if (_MI_PAGING_LEVELS == 2)
+      /* Read only page, no need for a pagefile entry -> PDE-- */
+      if(Address < MmSystemRangeStart)
+      {
+        
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+        
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
+      }
+#endif
       if (SwapEntry != 0)
       {
          DPRINT1("Found a swapentry for a non private and direct mapped page
(address %x)\n",
@@ -2282,6 +2309,14 @@
                  Address);
          KeBugCheck(MEMORY_MANAGEMENT);
       }
+#if (_MI_PAGING_LEVELS == 2)
+      /* Non dirty, non private, non direct-mapped -> PDE-- */
+      if(Address < MmSystemRangeStart)
+      {
+        
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+        
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
+      }
+#endif
       MmReleasePageMemoryConsumer(MC_USER, Page);
       PageOp->Status = STATUS_SUCCESS;
       MmspCompleteAndReleasePageOp(PageOp);
@@ -2291,13 +2326,6 @@
    {
       MmSetSavedSwapEntryPage(Page, 0);
       MmLockAddressSpace(AddressSpace);
- #if (_MI_PAGING_LEVELS == 2)
-      /* Page table was dereferenced while deleting the RMAP */
-      if(Address < MmSystemRangeStart)
-      {
-        
AddressSpace->VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++;
-      }
-#endif
       Status = MmCreatePageFileMapping(Process,
                                        Address,
                                        SwapEntry);
@@ -2325,13 +2353,6 @@
          /*
           * For private pages restore the old mappings.
           */
- #if (_MI_PAGING_LEVELS == 2)
-        /* Page table was dereferenced while deleting the RMAP */
-        if(Address < MmSystemRangeStart)
-        {
-           
AddressSpace->VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++;
-        }
-#endif
          if (Context.Private)
          {
             Status = MmCreateVirtualMapping(Process,
@@ -2382,13 +2403,6 @@
        * As above: undo our actions.
        * FIXME: Also free the swap page.
        */
-#if (_MI_PAGING_LEVELS == 2)
-        /* Page table was dereferenced while deleting the RMAP */
-        if(Address < MmSystemRangeStart)
-        {
-           
AddressSpace->VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++;
-        }
-#endif
       MmLockAddressSpace(AddressSpace);
       if (Context.Private)
       {
@@ -2440,13 +2454,6 @@
    if (Context.Private)
    {
       MmLockAddressSpace(AddressSpace);
- #if (_MI_PAGING_LEVELS == 2)
-      /* Page table was dereferenced while deleting the RMAP */
-      if(Address < MmSystemRangeStart)
-      {
-         
AddressSpace->VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++;
-      }
-#endif
       Status = MmCreatePageFileMapping(Process,
                                        Address,
                                        SwapEntry);
@@ -2458,6 +2465,14 @@
    }
    else
    {
+#if (_MI_PAGING_LEVELS == 2)
+      /* We keep the pagefile index global to the segment, not in the PTE */
+      if(Address < MmSystemRangeStart)
+      {
+        
Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]--;
+        
ASSERT(Process->Vm.VmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]
< PTE_COUNT);
+      }
+#endif
       Entry = MAKE_SWAP_SSE(SwapEntry);
       MmSetPageEntrySectionSegment(Context.Segment, Context.Offset, Entry);
    }