Re: [PATCH v2] mm: Fix memblock_free_late() when using deferred struct page

[Mike Rapoport <rppt@kernel.org>]

On Tue, Feb 17, 2026 at 07:28:12PM +1100, Benjamin Herrenschmidt wrote:
> We have two issues:
> 
> - One is we don't check for pfn_valid(). If this is called for
> a page corresponding to a big enough memory hole that we don't have
> allocated a corresponding sparsemem section for it, it will crash.
> 
> - Then, when using deferred struct page init, we can end up not
> freeing the pages at all. This happens routinely with some of the
> UEFI Boot Services memory, as soon as they fall above the threshold
> of pages whose initialization is deferred.
> 
> We can very easily hit the !early_page_initialised() test in
> memblock_free_pages() since the deferred initializer hasn't even
> started yet. As a result we drop the pages on the floor.
> 
> Now, memblock_free_late() should only ever be called for pages that
> are reserved, and thus for which the struct page has already been
> initialized by memmap_init_reserved_pages().... as long as we check
> for pfn_valid() as a big enough hole might cause entire sections of
> the mem_map to not be allocated at all.
> 
> So it should be safe to just free them normally and ignore the deferred
> initializer, which will skip over them as it skips over anything still
> in the memblock reserved list.
> 
> This helps recover something like 140MB of RAM on EC2 t3a.nano instances
> who only have 512MB to begin with (as to why UEFI uses that much, that's
> a question for another day).
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> v2. Reworked a bit to add the pfn_valid() check, remove the bogus memblock
> access in debug mode, and add a test of PageReserved() for sanity.
> 
> We could separately do a patch forcing UEFI Boot Services into
> memblock.memory but so far I haven't hit a case where that is necessary.
> 
>  mm/memblock.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 905d06b16348a..71eb25b68851e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1770,9 +1770,14 @@ void __init memblock_free_late(phys_addr_t base, phys_addr_t size)
>  	cursor = PFN_UP(base);
>  	end = PFN_DOWN(base + size);
>  
> +	/* Only free pages that were reserved */
>  	for (; cursor < end; cursor++) {
> -		memblock_free_pages(pfn_to_page(cursor), cursor, 0);
> -		totalram_pages_inc();
> +		struct page *p;
> +		if (!pfn_valid(cursor))
> +			continue;
> +		p = pfn_to_page(cursor);
> +		if (!WARN_ON(!PageReserved(p)))

Took me a second with the double negation. I like

	if (WARN_ON(!PageReserved(p)))
		continue;

more.

> +			free_reserved_page(pfn_to_page(cursor));

We already have page here, no need to pfn_to_page() again  :)

I can fix those up when applying.

>  	}
>  }
>  
> -- 
> 2.43.0
> 
> 

-- 
Sincerely yours,
Mike.