Re: [PATCH 4/4] arm64/mm: fix destructor for unconstructed hotplug page tables

[Muchun Song <muchun.song@linux.dev>]

> On Oct 10, 2026, at 04:43, David Hildenbrand (Arm) <david@kernel.org> wrote:
> 
> On 10/8/26 09:30, Muchun Song wrote:
>> Commit c594b83457cc ("arm64: mm: call pagetable dtor when freeing
>> hot-removed page tables") made free_hotplug_pgtable_page()
>> unconditionally run the page-table destructor. This matches page tables
>> allocated by the arm64 mapping code, which runs the corresponding
>> constructors.
>> 
>> However, arm64 also uses the generic sparse-vmemmap population code.
>> Runtime intermediate page tables allocated by that code do not run a
>> page-table constructor.
> 
> Why do we have that inconsistency? It seems to cause pain :)

Ha, yeah, it's a bit painful :)

I think it's because the arch folks didn't realize that vmemmap population
does not run a page-table constructor.

But the inconsistency is temporary — I deliberately kept it so the bug fixes
are easier to backport. After those fixes land, I'll follow up with the
unified series, which will make vmemmap population run a page-table
constructor.

> 
>> Freeing one during memory hot-remove therefore
>> runs a destructor without a matching constructor and corrupts
>> NR_PAGETABLE accounting.
>> 
>> Use PageTable() to run the destructor only for page-table pages whose
>> constructor initialized them. This keeps the arm64-created page-table
>> lifecycle balanced while safely freeing constructor-free vmemmap tables.
>> 
>> Fixes: c594b83457cc ("arm64: mm: call pagetable dtor when freeing hot-removed page tables")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> arch/arm64/mm/mmu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 7343ac9294f8..688b33095651 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1495,7 +1495,8 @@ static void free_hotplug_page_range(struct page *page, size_t size,
>> 
>> static void free_hotplug_pgtable_page(struct page *page)
>> {
>> - 	pagetable_dtor(page_ptdesc(page));
>> + 	if (PageTable(page))
>> + 		pagetable_dtor(page_ptdesc(page));
>> 	free_hotplug_page_range(page, PAGE_SIZE, NULL);
> 
> That results in a __free_pages() for  ones allocated by sparse-vmemmap
> population code. Are we sure that's the right thing to do?

Good point. free_hotplug_pgtable_page() is used to free intermediate
page-table pages, so pagetable_free() is the more appropriate interface
here.

> 
> This is all so inconsistent and confusing :(

Yeah, so I'm working on removing these inconsistencies.

Thanks,
Muchun

> 
> -- 
> Cheers,
> 
> David