[ros-diffs] [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...

tkreuzer at svn.reactos.org tkreuzer at svn.reactos.org
Sun Sep 25 19:19:52 UTC 2011


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/fat.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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/linuxboot.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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/heap.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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/meminit.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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/video.c?rev=53857&r1=53856&r2=53857&view=diff
==============================================================================
--- 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;
 }




More information about the Ros-diffs mailing list