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

[Mike Rapoport <rppt@kernel.org>]

(added KMSAN folks)

On Wed, Feb 04, 2026 at 08:02:13PM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2026-02-04 at 09:39 +0200, Mike Rapoport wrote:
> > > I might be missing something but I don't see what would restrict
> > > this
> > > to the early pre-initialized struct pages other than that
> > > early_page_initialised() test, so we can't rely on anything in
> > > struct
> > > page inside memblock_free_pages().
> > 
> > Right, we can't rely on PG_Reserved being cleared for uninitialized
> > pages :/
> > 
> > But I overlooked an easier and actually reliable way: use
> > free_reserved_area() instead of memblock_free_late().
> 
> You mean replace all callers of memblock_free_late() and kill it ?

That would be great, but with all the subtle differences you note below
it's for the future :)

> Or make memblock_free_late() use free_reserved_area() instead of
> memblock_free_pages() ? :-)

Yes, I think either calling free_reserved_page() in the loop in
memblock_free_late() or replacing the entire loop with free_reserved_area().
 
> The former misses:
>  - totalram_pages_inc() and kmemleak_free_part_phys() in
> memblock_free_late()
> 
> They also both miss as far as I can tell:
> 
> 	if (!kmsan_memblock_free_pages(page, order)) {
> 		/* KMSAN will take care of these pages. */
> 		return;
> 	}
> 
> But I don't know if that matters, I don't know anything about kmsan :-)

AFAIU, here kmsan allocates metadata for each page freed to buddy, but it
handles reserved memory differently anyway, so it shouldn't be a problem.
 
> There are other subtle differences between the two implementations
> which probably boil down to the same thing but it's been a while and I
> don't have time today to dig into the gory details :-)
> 
> ie, one does
> 
> 	clear_page_tag_ref(page);
> 	__free_pages_core(page, order, MEMINIT_EARLY);
> 
> ie, clear_page_tag_ref() is done once for the whole "order" (though in
> the memblock_free_late() order is always 0), then __free_pages_core()
> which kind-of hard resets count to 0 etc...
> 
> The other one ends up setting the count to 1 then __free_page() which 
> does a LOT more "stuff" that is new to me since last I looked (such as
> the pcp stuff), ie a lot more convoluted code path, but I don't know if
> it differs practically for that use case :-)

It does not :)
with __free_page() the pages may be freed to PCP lists rather than on
global free lists, but it does not really matter, the pages are free and
buddy can use them.
 
> I assume that the right approach here is to make memblock_free_late()
> call free_reserved_area() instead of memblock_free_pages() so we
> preserve totalram_pages_inc() and kmemleak_free_part_phys() but I might
> be missing something (and I don't know about KMSAN).

free_reserved_page() adjusts totalram internally, so we only need to
preserve kmemleak part.

So it essentially becomes "oneliner" :)

diff --git a/mm/memblock.c b/mm/memblock.c
index e76255e4ff36..6e984bcdf6cd 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1770,10 +1770,8 @@ void __init memblock_free_late(phys_addr_t base, phys_addr_t size)
 	cursor = PFN_UP(base);
 	end = PFN_DOWN(base + size);
 
-	for (; cursor < end; cursor++) {
-		memblock_free_pages(pfn_to_page(cursor), cursor, 0);
-		totalram_pages_inc();
-	}
+	for (; cursor < end; cursor++)
+		free_reserved_page(pfn_to_page(cursor));
 }
 
 /*
 
> Cheers,
> Ben.

-- 
Sincerely yours,
Mike.