[jgardou] 63822: [NTOS/MM] - Call the right delete routine for ARM3 section object - Properly free the pages of pagefile-backed segments when those are deleted - Put the right assert at the right p...

Author: jgardou
Date: Wed Aug  6 21:53:57 2014
New Revision: 63822

URL: http://svn.reactos.org/svn/reactos?rev=63822&view=rev
Log:
[NTOS/MM]
 - Call the right delete routine for ARM3 section object
 - Properly free the pages of pagefile-backed segments when those are deleted
 - Put the right assert at the right place in MmUnlinkPageFromList
 - Remove DPRINT from previous commit which is interesting but noisy

Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
    trunk/reactos/ntoskrnl/mm/ARM3/pfnlist.c
    trunk/reactos/ntoskrnl/mm/ARM3/section.c
    trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
    trunk/reactos/ntoskrnl/mm/balance.c
    trunk/reactos/ntoskrnl/mm/section.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?r…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h	[iso-8859-1] Wed Aug  6 21:53:57 2014
@@ -2274,6 +2274,12 @@
     IN ULONG_PTR Vpn
 );
 
+VOID
+NTAPI
+MiDeleteARM3Section(
+    PVOID ObjectBody
+);
+
 NTSTATUS
 NTAPI
 MiQueryMemorySectionName(

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] Wed Aug  6 21:53:57 2014
@@ -338,10 +338,11 @@
 
     /* We are not on a list anymore */
     Pfn->u1.Flink = Pfn->u2.Blink = 0;
-    ASSERT_LIST_INVARIANT(ListHead);
 
     /* Remove one entry from the list */
     ListHead->Total--;
+
+    ASSERT_LIST_INVARIANT(ListHead);
 }
 
 PFN_NUMBER
@@ -1234,6 +1235,9 @@
                      0);
     }
 
+    /* Page should at least have one reference */
+    ASSERT(Pfn1->u3.e2.ReferenceCount != 0);
+
     /* Check if the share count is now 0 */
     ASSERT(Pfn1->u2.ShareCount < 0xF000000);
     if (!--Pfn1->u2.ShareCount)
@@ -1257,7 +1261,7 @@
             TempPte.u.Soft.Prototype = 0;
             TempPte.u.Soft.Protection = Pfn1->OriginalPte.u.Soft.Protection;
             MI_WRITE_INVALID_PTE(PointerPte, TempPte);
-            DPRINT("Marking PTE: %p as transition (%p - %lx)\n", PointerPte,
Pfn1, MiGetPfnEntryIndex(Pfn1));
+            DPRINT1("Marking PTE: %p as transition (%p - %lx)\n", PointerPte,
Pfn1, MiGetPfnEntryIndex(Pfn1));
         }
 
         /* Put the page in transition */
@@ -1266,8 +1270,6 @@
         /* PFN lock must be held */
         ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
 
-        /* Page should at least have one reference */
-        ASSERT(Pfn1->u3.e2.ReferenceCount != 0);
         if (Pfn1->u3.e2.ReferenceCount == 1)
         {
             /* Is there still a PFN for this page? */

Modified: trunk/reactos/ntoskrnl/mm/ARM3/section.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/section.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/section.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/section.c	[iso-8859-1] Wed Aug  6 21:53:57 2014
@@ -608,6 +608,8 @@
     SEGMENT_FLAGS SegmentFlags;
     PSUBSECTION Subsection;
     PMMPTE PointerPte, LastPte, PteForProto;
+    PMMPFN Pfn1;
+    PFN_NUMBER PageFrameIndex;
     MMPTE TempPte;
     KIRQL OldIrql;
 
@@ -621,7 +623,7 @@
 
     /* These things are not supported yet */
     ASSERT(ControlArea->DereferenceList.Flink == NULL);
-    ASSERT(!(ControlArea->u.Flags.Image) & !(ControlArea->u.Flags.File));
+    ASSERT(!(ControlArea->u.Flags.Image) && !(ControlArea->u.Flags.File));
     ASSERT(ControlArea->u.Flags.GlobalOnlyPerSession == 0);
     ASSERT(ControlArea->u.Flags.Rom == 0);
 
@@ -661,7 +663,52 @@
         TempPte = *PointerPte;
         ASSERT(SegmentFlags.LargePages == 0);
         ASSERT(TempPte.u.Hard.Valid == 0);
-        ASSERT(TempPte.u.Soft.Prototype == 1);
+
+        /* See if we should clean things up */
+        if (!(ControlArea->u.Flags.Image) && !(ControlArea->u.Flags.File))
+        {
+            /*
+             * This is a section backed by the pagefile. Now that it doesn't exist
anymore,
+             * we can give everything back to the system.
+             */
+            ASSERT(TempPte.u.Soft.Prototype == 0);
+
+            if (TempPte.u.Soft.Transition == 1)
+            {
+                /* We can give the page back for other use */
+                DPRINT1("Releasing page for transition PTE %p\n", PointerPte);
+                PageFrameIndex = PFN_FROM_PTE(&TempPte);
+                Pfn1 = MI_PFN_ELEMENT(PageFrameIndex);
+
+                /* As this is a paged-backed section, nobody should reference it anymore
(no cache or whatever) */
+                ASSERT(Pfn1->u3.ReferenceCount == 0);
+
+                /* And it should be in standby or modified list */
+                ASSERT((Pfn1->u3.e1.PageLocation == ModifiedPageList) ||
(Pfn1->u3.e1.PageLocation == StandbyPageList));
+
+                /* Unlink it and put it back in free list */
+                MiUnlinkPageFromList(Pfn1);
+
+                /* Temporarily mark this as active and make it free again */
+                Pfn1->u3.e1.PageLocation = ActiveAndValid;
+                MI_SET_PFN_DELETED(Pfn1);
+
+                MiInsertPageInFreeList(PageFrameIndex);
+            }
+            else if (TempPte.u.Soft.PageFileHigh != 0)
+            {
+                /* Should not happen for now */
+                ASSERT(FALSE);
+            }
+        }
+        else
+        {
+            /* unsupported for now */
+            ASSERT(FALSE);
+
+            /* File-backed section must have prototype PTEs */
+            ASSERT(TempPte.u.Soft.Prototype == 1);
+        }
 
         /* Zero the PTE and keep going */
         PointerPte->u.Long = 0;
@@ -3050,6 +3097,34 @@
     return STATUS_SUCCESS;
 }
 
+VOID
+NTAPI
+MiDeleteARM3Section(PVOID ObjectBody)
+{
+    PSECTION SectionObject;
+    PCONTROL_AREA ControlArea;
+    KIRQL OldIrql;
+
+    SectionObject = (PSECTION)ObjectBody;
+
+    /* Lock the PFN database */
+    OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock);
+
+    ASSERT(SectionObject->Segment);
+    ASSERT(SectionObject->Segment->ControlArea);
+
+    ControlArea = SectionObject->Segment->ControlArea;
+
+    /* Dereference */
+    ControlArea->NumberOfSectionReferences--;
+    ControlArea->NumberOfUserReferences--;
+
+    ASSERT(ControlArea->u.Flags.BeingDeleted == 0);
+
+    /* Check it. It will delete it if there is no more reference to it */
+    MiCheckControlArea(ControlArea, OldIrql);
+}
+
 /* SYSTEM CALLS ***************************************************************/
 
 NTSTATUS

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] Wed Aug  6 21:53:57 2014
@@ -423,10 +423,18 @@
             /* Drop the reference on the page table. */
             MiDecrementShareCount(MiGetPfnEntry(Pfn1->u4.PteFrame),
Pfn1->u4.PteFrame);
 
+            ASSERT(Pfn1->u3.e1.PrototypePte == 0);
+
+            /* Make the page free. For prototypes, it will be made free when deleting the
section object */
             if (Pfn1->u2.ShareCount == 0)
             {
                 NT_ASSERT(Pfn1->u3.e2.ReferenceCount == 0);
-                /* Mark the page temporarily as valid, we're going to make it free
soon */
+
+                /* And it should be in standby or modified list */
+                ASSERT((Pfn1->u3.e1.PageLocation == ModifiedPageList) ||
(Pfn1->u3.e1.PageLocation == StandbyPageList));
+
+                /* Unlink it and put it back in free list */
+                MiUnlinkPageFromList(Pfn1);
                 Pfn1->u3.e1.PageLocation = ActiveAndValid;
 
                 /* Bring it back into the free list */

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] Wed Aug  6 21:53:57 2014
@@ -355,8 +355,6 @@
    return(STATUS_SUCCESS);
 }
 
-
-extern MMPFNLIST MmModifiedPageListByColor[];
 
 VOID NTAPI
 MiBalancerThread(PVOID Unused)
@@ -427,9 +425,6 @@
                   KeBugCheck(NO_PAGES_AVAILABLE);
               }
           } while (InitialTarget != 0);
-
-          if (MmModifiedPageListByColor[0].Total != 0)
-              DPRINT1("There are %u pages ready to be paged out in the modified
list.\n", MmModifiedPageListByColor[0].Total);
       }
       else
       {

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] Wed Aug  6 21:53:57 2014
@@ -2623,6 +2623,13 @@
 MmpDeleteSection(PVOID ObjectBody)
 {
    PROS_SECTION_OBJECT Section = (PROS_SECTION_OBJECT)ObjectBody;
+
+    /* Check if it's an ARM3, or ReactOS section */
+    if (!MiIsRosSectionObject(Section))
+    {
+        MiDeleteARM3Section(ObjectBody);
+        return;
+    }
 
    DPRINT("MmpDeleteSection(ObjectBody %p)\n", ObjectBody);
    if (Section->AllocationAttributes & SEC_IMAGE)