[tkreuzer] 53857: [FREELDR] - Get rid of MmAllocateMemory, freeing about 1MB of low physical memory for the kernel - verify that MmMarkPagesInLookupTable is not called with invalid page regions - a...

Author: tkreuzer
Date: Sun Sep 25 19:19:50 2011
New Revision: 53857

URL: http://svn.reactos.org/svn/reactos?rev=53857&view=rev
Log:
[FREELDR]
- Get rid of MmAllocateMemory, freeing about 1MB of low physical memory for the kernel
- verify that MmMarkPagesInLookupTable is not called with invalid page regions
- add maximum allocation to heap statistics

<long explanation>
There was a function called MmAllocateMemory, what was commented as "// Temporary
forwarder..." since January 2008. This function allocated one page of memory and
marked it as LoaderOsLoaderHeap. This function was used by the fat filesystem code (and
linuxboot.c) which allocated and freed memory in small chunks all the time. Since
MmFreeMemory() is not implemented at all (obviously someone removed it) we were allocating
one full page for allocations as small as 8 bytes and never free them. This accumulated to
a total of 240 pages, almost 1MB, split into into 14 chunks. This memory was never freed
by the kernel (the kernel keeps the loader heap memory for some reason) and fragmented the
low memory region.
Remove MmAllocateMemory completely and replace references in the fat code with
MmHeapAlloc. The maximum heap usage after this was 184 KB, heap size is 4MB. 
</long explanation>

Modified:
    trunk/reactos/boot/freeldr/freeldr/fs/fat.c
    trunk/reactos/boot/freeldr/freeldr/linuxboot.c
    trunk/reactos/boot/freeldr/freeldr/mm/heap.c
    trunk/reactos/boot/freeldr/freeldr/mm/meminit.c
    trunk/reactos/boot/freeldr/freeldr/mm/mm.c
    trunk/reactos/boot/freeldr/freeldr/video/video.c

Modified: trunk/reactos/boot/freeldr/freeldr/fs/fat.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/fs/fa…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/fs/fat.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/fs/fat.c [iso-8859-1] Sun Sep 25 19:19:50 2011
@@ -402,7 +402,7 @@
 	// Attempt to allocate memory for directory buffer
 	//
 	TRACE("Trying to allocate (DirectorySize) %d bytes.\n", *DirectorySize);
-	DirectoryBuffer = MmAllocateMemory(*DirectorySize);
+	DirectoryBuffer = MmHeapAlloc(*DirectorySize);
 
 	if (DirectoryBuffer == NULL)
 	{
@@ -416,7 +416,7 @@
 	{
 		if (!FatReadVolumeSectors(Volume, Volume->RootDirSectorStart,
Volume->RootDirSectors, DirectoryBuffer))
 		{
-			MmFreeMemory(DirectoryBuffer);
+			MmHeapFree(DirectoryBuffer);
 			return NULL;
 		}
 	}
@@ -424,7 +424,7 @@
 	{
 		if (!FatReadClusterChain(Volume, DirectoryStartCluster, 0xFFFFFFFF, DirectoryBuffer))
 		{
-			MmFreeMemory(DirectoryBuffer);
+			MmHeapFree(DirectoryBuffer);
 			return NULL;
 		}
 	}
@@ -774,7 +774,7 @@
 		{
 			if (!FatXSearchDirectoryBufferForFile(Volume, DirectoryBuffer, DirectorySize,
PathPart, &FatFileInfo))
 			{
-				MmFreeMemory(DirectoryBuffer);
+				MmHeapFree(DirectoryBuffer);
 				return ENOENT;
 			}
 		}
@@ -782,12 +782,12 @@
 		{
 			if (!FatSearchDirectoryBufferForFile(Volume, DirectoryBuffer, DirectorySize, PathPart,
&FatFileInfo))
 			{
-				MmFreeMemory(DirectoryBuffer);
+				MmHeapFree(DirectoryBuffer);
 				return ENOENT;
 			}
 		}
 
-		MmFreeMemory(DirectoryBuffer);
+		MmHeapFree(DirectoryBuffer);
 
 		//
 		// If we have another sub-directory to go then
@@ -800,12 +800,12 @@
 			//
 			if (!(FatFileInfo.Attributes & ATTR_DIRECTORY))
 			{
-				MmFreeMemory(FatFileInfo.FileFatChain);
+				MmHeapFree(FatFileInfo.FileFatChain);
 				return ENOTDIR;
 			}
 			DirectoryStartCluster = FatFileInfo.FileFatChain[0];
 		}
-		MmFreeMemory(FatFileInfo.FileFatChain);
+		MmHeapFree(FatFileInfo.FileFatChain);
 	}
 
 	memcpy(FatFileInfoPointer, &FatFileInfo, sizeof(FAT_FILE_INFO));
@@ -1011,7 +1011,7 @@
 	//
 	// Allocate array memory
 	//
-	ArrayPointer = MmAllocateMemory(ArraySize);
+	ArrayPointer = MmHeapAlloc(ArraySize);
 
 	if (ArrayPointer == NULL)
 	{
@@ -1044,7 +1044,7 @@
 		//
 		if (!FatGetFatEntry(Volume, StartCluster, &StartCluster))
 		{
-			MmFreeMemory(ArrayPointer);
+			MmHeapFree(ArrayPointer);
 			return NULL;
 		}
 	}

Modified: trunk/reactos/boot/freeldr/freeldr/linuxboot.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/linux…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/linuxboot.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/linuxboot.c [iso-8859-1] Sun Sep 25 19:19:50 2011
@@ -18,7 +18,7 @@
  */
 
 #ifndef _M_ARM
- 
+
 #include <freeldr.h>
 #include <debug.h>
 #ifdef __i386__
@@ -283,7 +283,7 @@
 BOOLEAN LinuxReadBootSector(PFILE LinuxKernelFile)
 {
 	// Allocate memory for boot sector
-	LinuxBootSector = (PLINUX_BOOTSECTOR)MmAllocateMemory(512);
+	LinuxBootSector = MmAllocateMemoryWithType(512, LoaderSystemCode);
 	if (LinuxBootSector == NULL)
 	{
 		return FALSE;
@@ -346,7 +346,7 @@
 	}
 
 	// Allocate memory for setup sectors
-	LinuxSetupSector = (PLINUX_SETUPSECTOR)MmAllocateMemory(SetupSectorSize);
+	LinuxSetupSector = MmAllocateMemoryWithType(SetupSectorSize, LoaderSystemCode);
 	if (LinuxSetupSector == NULL)
 	{
 		return FALSE;

Modified: trunk/reactos/boot/freeldr/freeldr/mm/heap.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/mm/he…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/mm/heap.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/mm/heap.c [iso-8859-1] Sun Sep 25 19:19:50 2011
@@ -19,6 +19,10 @@
 
 #include <freeldr.h>
 #include <debug.h>
+
+//#define MM_DBG 1 // needs #define BufStats 1 in bget.c
+
+ULONG MmMaximumHeapAlloc;
 
 DBG_DEFAULT_CHANNEL(MEMORY);
 
@@ -62,15 +66,16 @@
 	{
 		ERR("Heap allocation for %d bytes failed\n", MemorySize);
 	}
-#if MM_DBG
-    {
-    	LONG CurAlloc, TotalFree, MaxFree, NumberOfGets, NumberOfRels;
+#ifdef MM_DBG
+	{
+		LONG CurAlloc, TotalFree, MaxFree, NumberOfGets, NumberOfRels;
 
-	    // Gather some stats
-	    bstats(&CurAlloc, &TotalFree, &MaxFree, &NumberOfGets,
&NumberOfRels);
+		// Gather some stats
+		bstats(&CurAlloc, &TotalFree, &MaxFree, &NumberOfGets,
&NumberOfRels);
+		if (CurAlloc > MmMaximumHeapAlloc) MmMaximumHeapAlloc = CurAlloc;
 
-	    TRACE("Current alloced %d bytes, free %d bytes, allocs %d, frees %d\n",
-		    CurAlloc, TotalFree, NumberOfGets, NumberOfRels);
+		TRACE("Current alloc %d, free %d, max alloc %lx, allocs %d, frees %d\n",
+		    CurAlloc, TotalFree, MmMaximumHeapAlloc, NumberOfGets, NumberOfRels);
 	}
 #endif
 	return Result;

Modified: trunk/reactos/boot/freeldr/freeldr/mm/meminit.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/mm/me…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/mm/meminit.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/mm/meminit.c [iso-8859-1] Sun Sep 25 19:19:50 2011
@@ -333,7 +333,7 @@
     while ((MemoryDescriptor = ArcGetMemoryDescriptor(MemoryDescriptor)) != NULL)
     {
         // Mark used pages in the lookup table
-        
+
         if (MemoryDescriptor->BasePage + MemoryDescriptor->PageCount <=
TotalPageCount)
         {
             TRACE("Marking pages 0x%lx-0x%lx as type %s\n",
@@ -365,6 +365,15 @@
 	ULONG							Index;
 	TRACE("MmMarkPagesInLookupTable()\n");
 
+    /* Validate the range */
+    if ((StartPage < MmLowestPhysicalPage) ||
+        ((StartPage + PageCount - 1) > MmHighestPhysicalPage))
+    {
+        ERR("Memory (0x%lx:0x%lx) outside of lookup table! Valid range:
0x%lx-0x%lx.\n",
+            StartPage, PageCount, MmLowestPhysicalPage, MmHighestPhysicalPage);
+        return;
+    }
+
     StartPage -= MmLowestPhysicalPage;
 	for (Index=StartPage; Index<(StartPage+PageCount); Index++)
 	{

Modified: trunk/reactos/boot/freeldr/freeldr/mm/mm.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/mm/mm…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/mm/mm.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/mm/mm.c [iso-8859-1] Sun Sep 25 19:19:50 2011
@@ -71,7 +71,8 @@
 	FreePagesInLookupTable -= PagesNeeded;
 	MemPointer = (PVOID)((ULONG_PTR)FirstFreePageFromEnd * MM_PAGE_SIZE);
 
-	TRACE("Allocated %d bytes (%d pages) of memory starting at page %d.\n",
MemorySize, PagesNeeded, FirstFreePageFromEnd);
+	TRACE("Allocated %d bytes (%d pages) of memory (type %ld) starting at page
0x%lx.\n",
+          MemorySize, PagesNeeded, MemoryType, FirstFreePageFromEnd);
 	TRACE("Memory allocation pointer: 0x%x\n", MemPointer);
 
 	// Update LoaderPagesSpanned count
@@ -80,12 +81,6 @@
 
 	// Now return the pointer
 	return MemPointer;
-}
-
-PVOID MmAllocateMemory(ULONG MemorySize)
-{
-	// Temporary forwarder...
-	return MmAllocateMemoryWithType(MemorySize, LoaderOsloaderHeap);
 }
 
 PVOID MmAllocateMemoryAtAddress(ULONG MemorySize, PVOID DesiredAddress, TYPE_OF_MEMORY
MemoryType)

Modified: trunk/reactos/boot/freeldr/freeldr/video/video.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/video…
==============================================================================
--- trunk/reactos/boot/freeldr/freeldr/video/video.c [iso-8859-1] (original)
+++ trunk/reactos/boot/freeldr/freeldr/video/video.c [iso-8859-1] Sun Sep 25 19:19:50
2011
@@ -33,7 +33,7 @@
 
 	BufferSize = MachVideoGetBufferSize();
 
-	VideoOffScreenBuffer = MmAllocateMemory(BufferSize);
+	VideoOffScreenBuffer = MmAllocateMemoryWithType(BufferSize, LoaderFirmwareTemporary);
 
 	return VideoOffScreenBuffer;
 }