[ros-arm-bringup] 34865: It seems unclear as to what the point of the Page Reference Count Table in the React Addres Space structure really was. It seems an over-engineered approach that actually causes more problems then it attempts to solve. The idea is to be able to unmap PDEs when they are not required anymore (a noble idea), which saves you 4KB of non-paged pool whenever a process frees a chunk of 4MB memory (oversimplification). The problem is that to keep track of this, an extremly expens

Author: ros-arm-bringup
Date: Sun Jul 27 17:39:54 2008
New Revision: 34865

URL: http://svn.reactos.org/svn/reactos?rev=34865&view=rev
Log:
It seems unclear as to what the point of the Page Reference Count Table in the React
Addres Space structure really was. It seems an over-engineered approach that actually
causes more problems
then it attempts to solve. The idea is to be able to unmap PDEs when they are not required
anymore (a noble idea), which saves you 4KB of non-paged pool whenever a process frees a
chunk
of 4MB memory (oversimplification). The problem is that to keep track of this, an extremly
expensive series of branches, comparisons, masks and shifts is applied every single time
that
a page is mapped or unmapped. It also adds 8KB of non-paged pool to keep track of the
references, which in some cases can be more wasteful than keeping the page tables around.
Finally, if the process quickly allocates and de-allocates memory in the same PDE range,
we will effectively map and unmap the PDE continously, fragmenting hyperspace and slowing
down perf.
This patch removes this functionality from the system and re-uses the code that was
already present in Mmi386ReleaseMmInfo (with some optimizations and changes) to do this
unmapping when the
process exists. This should make things faster, with a very small amount of increased
memory footprint (we're talking about less than 100kb of non paged pool, in worse-case
scenarios). 


Modified:
    trunk/reactos/ntoskrnl/include/internal/mm.h
    trunk/reactos/ntoskrnl/mm/aspace.c
    trunk/reactos/ntoskrnl/mm/i386/page.c

Modified: trunk/reactos/ntoskrnl/include/internal/mm.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] Sun Jul 27 17:39:54 2008
@@ -255,7 +255,6 @@
     PMEMORY_AREA MemoryAreaRoot;
     PVOID LowestAddress;
     PEPROCESS Process;
-    PUSHORT PageTableRefCountTable;
     PEX_PUSH_LOCK Lock;
 } MADDRESS_SPACE, *PMADDRESS_SPACE;
 

Modified: trunk/reactos/ntoskrnl/mm/aspace.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/aspace.c?rev=3…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/aspace.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/aspace.c [iso-8859-1] Sun Jul 27 17:39:54 2008
@@ -39,8 +39,6 @@
 MmInitializeAddressSpace(PEPROCESS Process,
                          PMADDRESS_SPACE AddressSpace)
 {
-    ULONG Count;
-    
     AddressSpace->MemoryAreaRoot = NULL;
 
     if (Process != NULL)
@@ -48,19 +46,12 @@
         AddressSpace->LowestAddress = MM_LOWEST_USER_ADDRESS;
         AddressSpace->Process = Process;
         AddressSpace->Lock = (PEX_PUSH_LOCK)&Process->AddressCreationLock;
-        ExInitializePushLock((PULONG_PTR)AddressSpace->Lock);
-        Count = MiGetUserPageDirectoryCount();
-        AddressSpace->PageTableRefCountTable = ExAllocatePoolWithTag(NonPagedPool,
-                                                                     Count *
sizeof(USHORT),
-                                                                     TAG_PTRC);
-        RtlZeroMemory(AddressSpace->PageTableRefCountTable, Count * sizeof(USHORT));
-        
+        ExInitializePushLock((PULONG_PTR)AddressSpace->Lock);        
     }
     else
     {
         AddressSpace->LowestAddress = MmSystemRangeStart;
         AddressSpace->Process = NULL;
-        AddressSpace->PageTableRefCountTable = NULL;
         AddressSpace->Lock =
(PEX_PUSH_LOCK)&PsGetCurrentProcess()->AddressCreationLock;
         ExInitializePushLock((PULONG_PTR)AddressSpace->Lock);
     }
@@ -72,11 +63,6 @@
 NTAPI
 MmDestroyAddressSpace(PMADDRESS_SPACE AddressSpace)
 {
-    if (AddressSpace->PageTableRefCountTable)
-    {
-        ExFreePool(AddressSpace->PageTableRefCountTable);
-    }
-    
     return STATUS_SUCCESS;
 }
 

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] Sun Jul 27 17:39:54 2008
@@ -145,9 +145,8 @@
 {
    PUSHORT LdtDescriptor;
    ULONG LdtBase;
-   PULONG Pde;
    PULONG PageDir;
-   ULONG i, j;
+   ULONG i;
 
    DPRINT("Mmi386ReleaseMmInfo(Process %x)\n",Process);
 
@@ -168,28 +167,7 @@
     {
         if (PageDir[i] != 0)
         {
-            DPRINT1("Pde for %08x - %08x is not freed, RefCount %d\n",
-                    i * 4 * 1024 * 1024, (i + 1) * 4 * 1024 * 1024 - 1,
-                   
((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable[i]);
-            Pde = MmCreateHyperspaceMapping(PTE_TO_PFN(PageDir[i]));
-            for (j = 0; j < 1024; j++)
-            {
-                if(Pde[j] != 0)
-                {
-                    if (Pde[j] & PA_PRESENT)
-                    {
-                        DPRINT1("Page at %08x is not freed\n",
-                                i * 4 * 1024 * 1024 + j * PAGE_SIZE);
-                    }
-                    else
-                    {
-                        DPRINT1("Swapentry %x at %x is not freed\n",
-                                Pde[j], i * 4 * 1024 * 1024 + j * PAGE_SIZE);
-                        
-                    }
-                }
-            }
-            MmDeleteHyperspaceMapping(Pde);
+            MiZeroPage(PTE_TO_PFN(PageDir[i]));
             MmReleasePageMemoryConsumer(MC_NPPOOL, PTE_TO_PFN(PageDir[i]));
         }
     }
@@ -604,25 +582,6 @@
       {
          *Page = Pfn;
       }
-   /*
-    * Decrement the reference count for this page table.
-    */
-   if (Process != NULL && WasValid &&
-       ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable != NULL
&&
-       Address < MmSystemRangeStart)
-   {
-      PUSHORT Ptrc;
-      ULONG Idx;
-
-      Ptrc = ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable;
-      Idx = ADDR_TO_PAGE_TABLE(Address);
-
-      Ptrc[Idx]--;
-      if (Ptrc[Idx] == 0)
-      {
-         MmFreePageTable(Process, Address);
-      }
-   }
 }
 
 VOID
@@ -650,25 +609,6 @@
       Pte = InterlockedExchangeUL(Pt, 0);
 
       MiFlushTlb(Pt, Address);
-
-      /*
-       * Decrement the reference count for this page table.
-       */
-      if (Process != NULL && Pte &&
-          ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable != NULL
&&
-          Address < MmSystemRangeStart)
-      {
-         PUSHORT Ptrc;
-
-         Ptrc = ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable;
-
-         Ptrc[ADDR_TO_PAGE_TABLE(Address)]--;
-         if (Ptrc[ADDR_TO_PAGE_TABLE(Address)] == 0)
-         {
-            MmFreePageTable(Process, Address);
-         }
-      }
-
 
       /*
        * Return some information to the caller
@@ -971,17 +911,6 @@
          MmUnmapPageTable(Pt);
       }
 
-   if (Process != NULL &&
-       ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable != NULL
&&
-       Address < MmSystemRangeStart)
-   {
-     PUSHORT Ptrc;
-     ULONG Idx;
-
-     Ptrc = ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable;
-     Idx = ADDR_TO_PAGE_TABLE(Address);
-     Ptrc[Idx]++;
-   }
    return(STATUS_SUCCESS);
 }
 
@@ -1092,16 +1021,6 @@
             MmMarkPageUnmapped(PTE_TO_PFN((Pte)));
          }
 	 (void)InterlockedExchangeUL(Pt, PFN_TO_PTE(Pages[i]) | Attributes);
-         if (Address < MmSystemRangeStart &&
-	     ((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable != NULL
&&
-             Attributes & PA_PRESENT)
-         {
-            PUSHORT Ptrc;
-
-            Ptrc =
((PMADDRESS_SPACE)&Process->VadRoot)->PageTableRefCountTable;
-
-            Ptrc[ADDR_TO_PAGE_TABLE(Addr)]++;
-         }
          if (Pte != 0)
          {
             if (Address > MmSystemRangeStart ||
@@ -1411,13 +1330,6 @@
       }
 }
 
-ULONG
-NTAPI
-MiGetUserPageDirectoryCount(VOID)
-{
-   return ADDR_TO_PDE_OFFSET(MmSystemRangeStart);
-}
-
 VOID
 INIT_FUNCTION
 NTAPI