[ros-diffs] [ros-arm-bringup] 32358: Why divide up the page array into chunks of 128 pages? Why have a nested loop to initialize system memory in chunks of 128 pages? Why zero the array entries in chunks of 128? The page array is now initialized by simply iterating every page on the system, and filling out its entry in the array. Moved out the division calculations even one more step out of the loop -- now they're really calculated once, instead of 1000 times (an improvement over the 400000 times they were calculated previously).
ros-arm-bringup at svn.reactos.org
ros-arm-bringup at svn.reactos.org
Thu Feb 14 17:59:15 CET 2008
- Previous message: [ros-diffs] [ros-arm-bringup] 32357: Don't loop the page array list THREE times to set it up, ONCE is plenty enough! Remove the incomprehensible PFN allocation being done for the pages holding the page list array. We now: 1) Find the highest usable RAM page 2) Allocate the PTEs to hold the array from that point on and lower. Don't do expensive divisions for every single page on the system being looped! Precompute the values ONCE. Don't set the reference count for the KPCR and KUSER_SHARED_DATA to 0, these are LIVE pages! Removed the hack which pre-initializes the balancer -- this isn't needed anymore since the initial PTEs are allocated always from RAM now. Add some comments about the assumptions being made in this code regarding all PCs having the same kind of memory maps.
- Next message: [ros-diffs] [ros-arm-bringup] 32359: We were looping the memory descriptors in order to find the number of pages that are available to the system, that is to say, your RAM, minus pages that the BIOS said belong to it. This part is good. Next up, we were creating the page array for these pages, up to the highest entry, which we called, the number of pages on the system. This is the problem. Suppose we had 1000 pages somewhere in low memory that were used by the BIOS, we'd now call the total pages RAM - 1000 (correct). However, we'd also set the highest page array entry to RAM - 1000, which is wrong, because esssentially this eats up 10MB of memory, since the top 10MB (which are FREE, usable memory) are never entered into the database. So really, what we want to do is differentiate the TOTAL amount of usable RAM, versus the HIGHEST page that is usable (which is actually what should be the highest entry in the page array). This will reclaim the lost RAM ReactOS has been eating up all these days. But it gets better: eventually, someone noticed ReactOS was eating memory, and added 1MB more to the "total", making the highest entry "1mb higher". This ...kind of... fixes the problem above by giving you one more MB, but what if ReactOS was only eating up 150KB, as was more the case? Then ReactOS would believe that the other 850KB of memory are "Free physical memory", when actually, they're pages that don't even exist. Wow! Fixed these bugs.
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
Author: ros-arm-bringup
Date: Thu Feb 14 19:59:14 2008
New Revision: 32358
URL: http://svn.reactos.org/svn/reactos?rev=32358&view=rev
Log:
Why divide up the page array into chunks of 128 pages? Why have a nested loop to initialize system memory in chunks of 128 pages? Why zero the array entries in chunks of 128? The page array is now initialized by simply iterating every page on the system, and filling out its entry in the array.
Moved out the division calculations even one more step out of the loop -- now they're really calculated once, instead of 1000 times (an improvement over the 400000 times they were calculated previously).
Modified:
trunk/reactos/ntoskrnl/mm/freelist.c
Modified: trunk/reactos/ntoskrnl/mm/freelist.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/freelist.c?rev=32358&r1=32357&r2=32358&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/mm/freelist.c (original)
+++ trunk/reactos/ntoskrnl/mm/freelist.c Thu Feb 14 19:59:14 2008
@@ -292,7 +292,10 @@
NTSTATUS Status;
PFN_TYPE LastPage, Pfn;
ULONG PdeStart = PsGetCurrentProcess()->Pcb.DirectoryTableBase.LowPart;
-
+ ULONG PdePageStart, PdePageEnd;
+ ULONG VideoPageStart, VideoPageEnd;
+ ULONG KernelPageStart, KernelPageEnd;
+
/* Initialize the page lists */
KeInitializeSpinLock(&PageListLock);
InitializeListHead(&UserPageListHead);
@@ -341,133 +344,119 @@
}
}
- /* Now loop every PFN database page again */
- for (i = 0; i < Reserved; i++)
- {
- PVOID Address = (char*)MmPageArray + (i * PAGE_SIZE);
- ULONG j, start, end;
- ULONG PdePageStart, PdePageEnd;
- ULONG VideoPageStart, VideoPageEnd;
- ULONG KernelPageStart, KernelPageEnd;
-
- /* Clear the page array entry */
- memset(Address, 0, PAGE_SIZE);
-
- /* Do the next page'ss worth of entries */
- start = ((ULONG_PTR)Address - (ULONG_PTR)MmPageArray) / sizeof(PHYSICAL_PAGE);
- end = ((ULONG_PTR)Address - (ULONG_PTR)MmPageArray + PAGE_SIZE) / sizeof(PHYSICAL_PAGE);
-
- /* We'll be applying a bunch of hacks -- precompute some static values */
- PdePageStart = PdeStart / PAGE_SIZE;
- PdePageEnd = MmFreeLdrPageDirectoryEnd / PAGE_SIZE;
- VideoPageStart = 0xA0000 / PAGE_SIZE;
- VideoPageEnd = 0x100000 / PAGE_SIZE;
- KernelPageStart = FirstPhysKernelAddress / PAGE_SIZE;
- KernelPageEnd = LastPhysKernelAddress / PAGE_SIZE;
-
- /* Loop each page in this chunk */
- for (j = start; j < end; j++)
+ /* Clear the PFN database */
+ RtlZeroMemory(MmPageArray, MmPageArraySize * sizeof(PHYSICAL_PAGE));
+
+ /* We'll be applying a bunch of hacks -- precompute some static values */
+ PdePageStart = PdeStart / PAGE_SIZE;
+ PdePageEnd = MmFreeLdrPageDirectoryEnd / PAGE_SIZE;
+ VideoPageStart = 0xA0000 / PAGE_SIZE;
+ VideoPageEnd = 0x100000 / PAGE_SIZE;
+ KernelPageStart = FirstPhysKernelAddress / PAGE_SIZE;
+ KernelPageEnd = LastPhysKernelAddress / PAGE_SIZE;
+
+ /* Loop every page on the system */
+ for (i = 0; i < MmPageArraySize; i++)
+ {
+ /* Check if it's part of RAM */
+ if (MiIsPfnRam(BIOSMemoryMap, AddressRangeCount, i))
{
- /* Check if it's part of RAM */
- if (MiIsPfnRam(BIOSMemoryMap, AddressRangeCount, j))
- {
- /* Apply assumptions that all computers are built the same way */
- if (j == 0)
- {
- /* Page 0 is reserved for the IVT */
- MmPageArray[0].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[0].Flags.Consumer = MC_NPPOOL;
- MmPageArray[0].Flags.Zero = 0;
- MmPageArray[0].ReferenceCount = 0;
- MmStats.NrReservedPages++;
- }
- else if (j == 1)
- {
- /* Page 1 is reserved for the PCR */
- MmPageArray[1].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[1].Flags.Consumer = MC_NPPOOL;
- MmPageArray[1].Flags.Zero = 0;
- MmPageArray[1].ReferenceCount = 1;
- MmStats.NrReservedPages++;
- }
- else if (j == 2)
- {
- /* Page 2 is reserved for the KUSER_SHARED_DATA */
- MmPageArray[2].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[2].Flags.Consumer = MC_NPPOOL;
- MmPageArray[2].Flags.Zero = 0;
- MmPageArray[2].ReferenceCount = 1;
- MmStats.NrReservedPages++;
- }
- else if ((j >= PdePageStart) && (j < PdePageEnd))
- {
- /* These pages contain the initial FreeLDR PDEs */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].Flags.Consumer = MC_NPPOOL;
- MmPageArray[j].ReferenceCount = 1;
- MmStats.NrReservedPages++;
- }
- else if ((j >= VideoPageStart) && (j < VideoPageEnd))
- {
- /*
- * These pages are usually for the Video ROM BIOS.
- * Supposedly anyway. We'll simply ignore the fact that
- * many systems have this area somewhere else entirely
- * (which we'll assume to be "free" a couple of lines below)
- */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].Flags.Consumer = MC_NPPOOL;
- MmPageArray[j].ReferenceCount = 1;
- MmStats.NrReservedPages++;
- }
- else if ((j >= KernelPageStart) && (j < KernelPageEnd))
- {
- /* These are pages beloning to the kernel */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_USED;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].Flags.Consumer = MC_NPPOOL;
- MmPageArray[j].ReferenceCount = 2;
- MmPageArray[j].MapCount = 1;
- MmStats.NrSystemPages++;
- }
- else if (j > LastPage)
- {
- /* These are pages we allocated above to hold the PFN DB */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_USED;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].Flags.Consumer = MC_NPPOOL;
- MmPageArray[j].ReferenceCount = 2;
- MmPageArray[j].MapCount = 1;
- MmStats.NrSystemPages++;
- }
- else
- {
- /*
- * These are supposedly free pages.
- * By the way, not all of them are, some contain vital
- * FreeLDR data, but since we choose to ignore the Memory
- * Descriptor List, why bother, right?
- */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_FREE;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].ReferenceCount = 0;
- InsertTailList(&FreeUnzeroedPageListHead,
- &MmPageArray[j].ListEntry);
- UnzeroedPageCount++;
- MmStats.NrFreePages++;
- }
+ /* Apply assumptions that all computers are built the same way */
+ if (i == 0)
+ {
+ /* Page 0 is reserved for the IVT */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].ReferenceCount = 0;
+ MmStats.NrReservedPages++;
+ }
+ else if (i == 1)
+ {
+ /* Page 1 is reserved for the PCR */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].ReferenceCount = 1;
+ MmStats.NrReservedPages++;
+ }
+ else if (i == 2)
+ {
+ /* Page 2 is reserved for the KUSER_SHARED_DATA */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].ReferenceCount = 1;
+ MmStats.NrReservedPages++;
+ }
+ else if ((i >= PdePageStart) && (i < PdePageEnd))
+ {
+ /* These pages contain the initial FreeLDR PDEs */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].ReferenceCount = 1;
+ MmStats.NrReservedPages++;
+ }
+ else if ((i >= VideoPageStart) && (i < VideoPageEnd))
+ {
+ /*
+ * These pages are usually for the Video ROM BIOS.
+ * Supposedly anyway. We'll simply ignore the fact that
+ * many systems have this area somewhere else entirely
+ * (which we'll assume to be "free" a couple of lines below)
+ */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].ReferenceCount = 1;
+ MmStats.NrReservedPages++;
+ }
+ else if ((i >= KernelPageStart) && (i < KernelPageEnd))
+ {
+ /* These are pages beloning to the kernel */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_USED;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].ReferenceCount = 2;
+ MmPageArray[i].MapCount = 1;
+ MmStats.NrSystemPages++;
+ }
+ else if (i > LastPage)
+ {
+ /* These are pages we allocated above to hold the PFN DB */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_USED;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].ReferenceCount = 2;
+ MmPageArray[i].MapCount = 1;
+ MmStats.NrSystemPages++;
}
else
{
- /* These are pages reserved by the BIOS/ROMs */
- MmPageArray[j].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
- MmPageArray[j].Flags.Consumer = MC_NPPOOL;
- MmPageArray[j].Flags.Zero = 0;
- MmPageArray[j].ReferenceCount = 0;
- MmStats.NrReservedPages++;
- }
+ /*
+ * These are supposedly free pages.
+ * By the way, not all of them are, some contain vital
+ * FreeLDR data, but since we choose to ignore the Memory
+ * Descriptor List, why bother, right?
+ */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_FREE;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].ReferenceCount = 0;
+ InsertTailList(&FreeUnzeroedPageListHead,
+ &MmPageArray[i].ListEntry);
+ UnzeroedPageCount++;
+ MmStats.NrFreePages++;
+ }
+ }
+ else
+ {
+ /* These are pages reserved by the BIOS/ROMs */
+ MmPageArray[i].Flags.Type = MM_PHYSICAL_PAGE_BIOS;
+ MmPageArray[i].Flags.Consumer = MC_NPPOOL;
+ MmPageArray[i].Flags.Zero = 0;
+ MmPageArray[i].ReferenceCount = 0;
+ MmStats.NrReservedPages++;
}
}
- Previous message: [ros-diffs] [ros-arm-bringup] 32357: Don't loop the page array list THREE times to set it up, ONCE is plenty enough! Remove the incomprehensible PFN allocation being done for the pages holding the page list array. We now: 1) Find the highest usable RAM page 2) Allocate the PTEs to hold the array from that point on and lower. Don't do expensive divisions for every single page on the system being looped! Precompute the values ONCE. Don't set the reference count for the KPCR and KUSER_SHARED_DATA to 0, these are LIVE pages! Removed the hack which pre-initializes the balancer -- this isn't needed anymore since the initial PTEs are allocated always from RAM now. Add some comments about the assumptions being made in this code regarding all PCs having the same kind of memory maps.
- Next message: [ros-diffs] [ros-arm-bringup] 32359: We were looping the memory descriptors in order to find the number of pages that are available to the system, that is to say, your RAM, minus pages that the BIOS said belong to it. This part is good. Next up, we were creating the page array for these pages, up to the highest entry, which we called, the number of pages on the system. This is the problem. Suppose we had 1000 pages somewhere in low memory that were used by the BIOS, we'd now call the total pages RAM - 1000 (correct). However, we'd also set the highest page array entry to RAM - 1000, which is wrong, because esssentially this eats up 10MB of memory, since the top 10MB (which are FREE, usable memory) are never entered into the database. So really, what we want to do is differentiate the TOTAL amount of usable RAM, versus the HIGHEST page that is usable (which is actually what should be the highest entry in the page array). This will reclaim the lost RAM ReactOS has been eating up all these days. But it gets better: eventually, someone noticed ReactOS was eating memory, and added 1MB more to the "total", making the highest entry "1mb higher". This ...kind of... fixes the problem above by giving you one more MB, but what if ReactOS was only eating up 150KB, as was more the case? Then ReactOS would believe that the other 850KB of memory are "Free physical memory", when actually, they're pages that don't even exist. Wow! Fixed these bugs.
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
More information about the Ros-diffs
mailing list