From aac37d162ed3fede5b9a717736153dd510bc69ea Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Sun, 25 Sep 2011 19:19:50 +0000 Subject: [PATCH] [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 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. svn path=/trunk/; revision=53857 --- reactos/boot/freeldr/freeldr/fs/fat.c | 20 ++++++++++---------- reactos/boot/freeldr/freeldr/linuxboot.c | 6 +++--- reactos/boot/freeldr/freeldr/mm/heap.c | 19 ++++++++++++------- reactos/boot/freeldr/freeldr/mm/meminit.c | 11 ++++++++++- reactos/boot/freeldr/freeldr/mm/mm.c | 9 ++------- reactos/boot/freeldr/freeldr/video/video.c | 2 +- 6 files changed, 38 insertions(+), 29 deletions(-) diff --git a/reactos/boot/freeldr/freeldr/fs/fat.c b/reactos/boot/freeldr/freeldr/fs/fat.c index c4f5aba35d7..643be7b6daa 100644 --- a/reactos/boot/freeldr/freeldr/fs/fat.c +++ b/reactos/boot/freeldr/freeldr/fs/fat.c @@ -402,7 +402,7 @@ PVOID FatBufferDirectory(PFAT_VOLUME_INFO Volume, ULONG DirectoryStartCluster, U // 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 @@ PVOID FatBufferDirectory(PFAT_VOLUME_INFO Volume, ULONG DirectoryStartCluster, U { if (!FatReadVolumeSectors(Volume, Volume->RootDirSectorStart, Volume->RootDirSectors, DirectoryBuffer)) { - MmFreeMemory(DirectoryBuffer); + MmHeapFree(DirectoryBuffer); return NULL; } } @@ -424,7 +424,7 @@ PVOID FatBufferDirectory(PFAT_VOLUME_INFO Volume, ULONG DirectoryStartCluster, U { if (!FatReadClusterChain(Volume, DirectoryStartCluster, 0xFFFFFFFF, DirectoryBuffer)) { - MmFreeMemory(DirectoryBuffer); + MmHeapFree(DirectoryBuffer); return NULL; } } @@ -774,7 +774,7 @@ LONG FatLookupFile(PFAT_VOLUME_INFO Volume, PCSTR FileName, ULONG DeviceId, PFAT { if (!FatXSearchDirectoryBufferForFile(Volume, DirectoryBuffer, DirectorySize, PathPart, &FatFileInfo)) { - MmFreeMemory(DirectoryBuffer); + MmHeapFree(DirectoryBuffer); return ENOENT; } } @@ -782,12 +782,12 @@ LONG FatLookupFile(PFAT_VOLUME_INFO Volume, PCSTR FileName, ULONG DeviceId, PFAT { 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 @@ LONG FatLookupFile(PFAT_VOLUME_INFO Volume, PCSTR FileName, ULONG DeviceId, PFAT // 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 @@ ULONG* FatGetClusterChainArray(PFAT_VOLUME_INFO Volume, ULONG StartCluster) // // Allocate array memory // - ArrayPointer = MmAllocateMemory(ArraySize); + ArrayPointer = MmHeapAlloc(ArraySize); if (ArrayPointer == NULL) { @@ -1044,7 +1044,7 @@ ULONG* FatGetClusterChainArray(PFAT_VOLUME_INFO Volume, ULONG StartCluster) // if (!FatGetFatEntry(Volume, StartCluster, &StartCluster)) { - MmFreeMemory(ArrayPointer); + MmHeapFree(ArrayPointer); return NULL; } } diff --git a/reactos/boot/freeldr/freeldr/linuxboot.c b/reactos/boot/freeldr/freeldr/linuxboot.c index e6015b2ff64..5eacc285686 100644 --- a/reactos/boot/freeldr/freeldr/linuxboot.c +++ b/reactos/boot/freeldr/freeldr/linuxboot.c @@ -18,7 +18,7 @@ */ #ifndef _M_ARM - + #include #include #ifdef __i386__ @@ -283,7 +283,7 @@ BOOLEAN LinuxParseIniSection(PCSTR OperatingSystemName) 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 @@ BOOLEAN LinuxReadSetupSector(PFILE LinuxKernelFile) } // Allocate memory for setup sectors - LinuxSetupSector = (PLINUX_SETUPSECTOR)MmAllocateMemory(SetupSectorSize); + LinuxSetupSector = MmAllocateMemoryWithType(SetupSectorSize, LoaderSystemCode); if (LinuxSetupSector == NULL) { return FALSE; diff --git a/reactos/boot/freeldr/freeldr/mm/heap.c b/reactos/boot/freeldr/freeldr/mm/heap.c index f1d2223b9b2..27f4e06f88a 100644 --- a/reactos/boot/freeldr/freeldr/mm/heap.c +++ b/reactos/boot/freeldr/freeldr/mm/heap.c @@ -20,6 +20,10 @@ #include #include +//#define MM_DBG 1 // needs #define BufStats 1 in bget.c + +ULONG MmMaximumHeapAlloc; + DBG_DEFAULT_CHANNEL(MEMORY); VOID MmInitializeHeap(PVOID PageLookupTable) @@ -62,15 +66,16 @@ PVOID MmHeapAlloc(ULONG MemorySize) { 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; diff --git a/reactos/boot/freeldr/freeldr/mm/meminit.c b/reactos/boot/freeldr/freeldr/mm/meminit.c index 8b9b3afac1f..d818218d970 100644 --- a/reactos/boot/freeldr/freeldr/mm/meminit.c +++ b/reactos/boot/freeldr/freeldr/mm/meminit.c @@ -333,7 +333,7 @@ VOID MmInitPageLookupTable(PVOID PageLookupTable, ULONG TotalPageCount) 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 @@ VOID MmMarkPagesInLookupTable(PVOID PageLookupTable, ULONG StartPage, ULONG Page 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++) { diff --git a/reactos/boot/freeldr/freeldr/mm/mm.c b/reactos/boot/freeldr/freeldr/mm/mm.c index 223225cb5a9..bf1b2063f25 100644 --- a/reactos/boot/freeldr/freeldr/mm/mm.c +++ b/reactos/boot/freeldr/freeldr/mm/mm.c @@ -71,7 +71,8 @@ PVOID MmAllocateMemoryWithType(ULONG MemorySize, TYPE_OF_MEMORY MemoryType) 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 @@ -82,12 +83,6 @@ PVOID MmAllocateMemoryWithType(ULONG MemorySize, TYPE_OF_MEMORY MemoryType) return MemPointer; } -PVOID MmAllocateMemory(ULONG MemorySize) -{ - // Temporary forwarder... - return MmAllocateMemoryWithType(MemorySize, LoaderOsloaderHeap); -} - PVOID MmAllocateMemoryAtAddress(ULONG MemorySize, PVOID DesiredAddress, TYPE_OF_MEMORY MemoryType) { ULONG PagesNeeded; diff --git a/reactos/boot/freeldr/freeldr/video/video.c b/reactos/boot/freeldr/freeldr/video/video.c index 5401d81320e..2fb3759445e 100644 --- a/reactos/boot/freeldr/freeldr/video/video.c +++ b/reactos/boot/freeldr/freeldr/video/video.c @@ -33,7 +33,7 @@ PVOID VideoAllocateOffScreenBuffer(VOID) BufferSize = MachVideoGetBufferSize(); - VideoOffScreenBuffer = MmAllocateMemory(BufferSize); + VideoOffScreenBuffer = MmAllocateMemoryWithType(BufferSize, LoaderFirmwareTemporary); return VideoOffScreenBuffer; }