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

[Björn Töpel]

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
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));


Björn