On 3/8/21 2:25 PM, Mike Rapoport wrote: > Hi Anshuman, > > On Mon, Mar 08, 2021 at 08:57:53AM +0530, Anshuman Khandual wrote: >> Platforms like arm and arm64 have redefined pfn_valid() because their early >> memory sections might have contained memmap holes caused by memblock areas >> tagged with MEMBLOCK_NOMAP, which should be skipped while validating a pfn >> for struct page backing. This scenario could be captured with a new option >> CONFIG_HAVE_EARLY_SECTION_MEMMAP_HOLES and then generic pfn_valid() can be >> improved to accommodate such platforms. This reduces overall code footprint >> and also improves maintainability. > > I wonder whether arm64 would still need to free parts of its memmap after free_unused_memmap() is applicable when CONFIG_SPARSEMEM_VMEMMAP is not enabled. I am not sure whether there still might be some platforms or boards which would benefit from this. Hence lets just keep this unchanged for now. > the section size was reduced. Maybe the pain of arm64::pfn_valid() is not > worth the memory savings anymore? arm64 pfn_valid() special case was primarily because of MEMBLOCK_NOMAP tagged memory areas, which are reserved by the firmware. > >> Commit 4f5b0c178996 ("arm, arm64: move free_unused_memmap() to generic mm") >> had used CONFIG_HAVE_ARCH_PFN_VALID to gate free_unused_memmap(), which in >> turn had expanded its scope to new platforms like arc and m68k. Rather lets >> restrict back the scope for free_unused_memmap() to arm and arm64 platforms >> using this new config option i.e CONFIG_HAVE_EARLY_SECTION_MEMMAP. > > The whole point of 4f5b0c178996 was to let arc and m68k to free unused > memory map with FLATMEM so they won't need DISCONTIGMEM or SPARSEMEM. So > whatever implementation there will be for arm/arm64, please keep arc and > m68k functionally intact. Okay. Will protect free_unused_memmap() on HAVE_EARLY_SECTION_MEMMAP_HOLES config as well. diff --git a/mm/memblock.c b/mm/memblock.c index d9fa2e62ab7a..11b624e94127 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -1927,8 +1927,11 @@ static void __init free_unused_memmap(void) unsigned long start, end, prev_end = 0; int i; - if (!IS_ENABLED(CONFIG_HAVE_EARLY_SECTION_MEMMAP_HOLES) || - IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP)) + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP)) + return; + + if (!IS_ENABLED(CONFIG_HAVE_EARLY_SECTION_MEMMAP_HOLES) && + !IS_ENABLED(CONFIG_HAVE_ARCH_PFN_VALID)) return;