Re: [PATCH 2/4] riscv/mm: fix hotplug page-table destructor handling

[Muchun Song]

> On Oct 9, 2026, at 14:09, Björn Töpel <bjorn@xxxxxxxxxx> wrote:
> 
> Muchun Song <songmuchun@xxxxxxxxxxxxx> writes:
> 
>> RISC-V uses the same memory-hotplug teardown code for the linear map and
>> vmemmap, although their page-table pages are not always allocated in the
>> same way. Late linear-map allocations run page-table constructors, while
>> vmemmap and early allocations may provide constructor-free pages.
>> 
>> The PTE path unconditionally runs the destructor, which is wrong for
>> constructor-free vmemmap tables. The PMD path avoids that problem by
>> using is_vmemmap as a proxy for constructor state, but that assumption
>> will no longer hold once runtime vmemmap allocations use the normal
>> pgalloc helpers.
>> 
>> Page-table constructors record their state in PG_table. Centralize
>> page-table freeing and use PageTable() to decide whether the destructor
>> is required. Keep reserved and constructor-free pages on their existing
>> freeing paths.
>> 
>> Fixes: c75a74f4ba19 ("riscv: mm: Add memory hotplugging support")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Assisted-by: LLM
>> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
>> ---
>> arch/riscv/mm/init.c | 29 ++++++++++++++---------------
>> 1 file changed, 14 insertions(+), 15 deletions(-)
>> 
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 857f9a55039c..429a0b015ec1 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -1486,10 +1486,19 @@ struct execmem_info __init *execmem_arch_setup(void)
>> #endif /* CONFIG_EXECMEM */
>> 
>> #ifdef CONFIG_MEMORY_HOTPLUG
>> +static void __meminit free_pagetable(struct page *page)
>> +{
>> + 	if (PageReserved(page))
>> + 		free_reserved_page(page);
>> + 	else if (PageTable(page))
>> + 		pagetable_dtor_free(page_ptdesc(page));
>> + 	else
>> + 		pagetable_free(page_ptdesc(page));
>> +}
> 
> Is PageReserved() enought to determine if there's no dtor to run? On

Yes.

> rv64 your code is correct, but maybe for robustness?
> 
>  | if (PageTable(page))
>  |         pagetable_dtor(page_ptdesc(page));
>  | 
>  | if (PageReserved(page))
>  |         free_reserved_page(page);
>  | else
>  |         pagetable_free(page_ptdesc(page));

his looks good to me as well. I'll change it in the next version. Thanks!

Muchun

> 
> 
> Björn