Re: [PATCH v2 1/4] mm: fix missing page-table destructor in pagetable_free()

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

> On Oct 13, 2026, at 23:46, Kevin Brodsky <kevin.brodsky@arm.com> wrote:
> 
> On 10/10/2026 13:24, Muchun Song wrote:
>> Since commit 49f599666420 ("mm: call ctor/dtor for kernel PTEs"),
>> pte_alloc_one_kernel() runs the page-table constructor for kernel PTE
>> tables.
>> 
>> HVO (HugeTLB Vmemmap Optimization) uses pte_alloc_one_kernel() when
>> splitting a PMD. Restoring the vmemmap backing pages does not collapse
>> the PTE table, so a later memory hot-remove eventually frees it through
>> pagetable_free() on x86-64. That releases the page without running the
>> matching destructor and leaves NR_PAGETABLE elevated.
>> 
>> Make pagetable_free() detect constructor-backed pages and run the
>> destructor before releasing their memory to fix the problem.
>> 
>> Fixes: 49f599666420 ("mm: call ctor/dtor for kernel PTEs")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> v2:
>> - Move destructor handling from x86 into pagetable_free() (suggested by
>>  David Hildenbrand)
>> - Trim and update the commit message (requested by Dave Hansen)
>> ---
>> include/linux/mm.h | 37 ++++++++++++++++++++-----------------
>> 1 file changed, 20 insertions(+), 17 deletions(-)
>> 
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5cca90fc1df6..771a7d60b0a5 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3969,23 +3969,6 @@ static inline void pagetable_free_kernel(struct ptdesc *pt)
>> __pagetable_free(pt);
>> }
>> #endif
>> -/**
>> - * pagetable_free - Free pagetables
>> - * @pt: The page table descriptor
>> - *
>> - * pagetable_free frees the memory of all page tables described by a page
>> - * table descriptor and the memory for the descriptor itself.
>> - */
>> -static inline void pagetable_free(struct ptdesc *pt)
>> -{
>> - 	if (ptdesc_test_kernel(pt)) {
>> - 		ptdesc_clear_kernel(pt);
>> - 		pagetable_free_kernel(pt);
>> - 	} else {
>> - 		__pagetable_free(pt);
>> - 	}
>> -}
>> -
>> #if defined(CONFIG_SPLIT_PTE_PTLOCKS)
>> #if ALLOC_SPLIT_PTLOCKS
>> void __init ptlock_cache_init(void);
>> @@ -4078,6 +4061,26 @@ static inline void pagetable_dtor(struct ptdesc *ptdesc)
>> 	lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>> }
>> 
>> +/**
>> + * pagetable_free - Free pagetables
>> + * @pt: The page table descriptor
>> + *
>> + * pagetable_free frees the memory of all page tables described by a page
>> + * table descriptor and the memory for the descriptor itself.
>> + */
>> +static inline void pagetable_free(struct ptdesc *pt)
>> +{
>> + 	if (PageTable(ptdesc_page(pt)))
>> + 		pagetable_dtor(pt);
> 
> Took me a while to understand why this is fine. Currently we have:
> 
>     ptdesc = pagetable_alloc();
>     pagetable_*_ctor(mm, ptdesc);
>     [...]
>     pagetable_dtor(ptdesc);
>     pagetable_free(ptdesc);
> 
> All good, nice and balanced.
> 
> Now we're saying that pagetable_free() should call the dtor. But of
> course, in most cases, the dtor has already been called. Yes, this is
> fine because in that case PageTable(page) returns false. Still, the
> casual reader is likely to be confused (like I was).
> 
> The commit message should really explain this. I'm not a big fan of the

Maybe I should mention that pagetable_free() is supposed to free page
table page from now on.

> asymmetry we're creating between explicit alloc+ctor and free implicitly
> calling dtor, but I suppose it's safer this way. (And looking at the
> rest of the series, it does bring some nice cleanups.)

Make sense. That's my plan.

> 
> I understand this is a bugfix series so its scope is limited, but if
> time allows please post a follow-up series removing all those explicit
> dtor calls that are no longer required.
> 
> Now don't get me wrong, I am absolutely in favour of moving as much
> handling as possible out of the ctor/dtor and into
> pagetable_{alloc,free}; a series I posted earlier [1] was a step in that

Excellent work.

> direction. It unfortunately had many issues, including the unmatched
> alloc/free calls [2]. The work you're doing in this series, and your
> plan to use pagetable_alloc() for vmemmap, are exactly what I was
> planning to do next, so thank you for taking care of that :)
> With the commit message improved:
> 
> Reviewed-by: Kevin Brodsky <kevin.brodsky@arm.com>

Thanks.

> 
> - Kevin
> 
> [1]
> https://lore.kernel.org/linux-mm/20260714-remove_pgtable_cdtor-v1-0-44be8a7685d7@arm.com/
> [2]
> https://lore.kernel.org/linux-mm/6d167aaa-e9b0-4687-970c-1c67d0df6344@arm.com/
> 
>> +
>> + if (ptdesc_test_kernel(pt)) {
>> + 	ptdesc_clear_kernel(pt);
>> + 	pagetable_free_kernel(pt);
>> + } else {
>> + 	__pagetable_free(pt);
>> + }
>> +}
>> +
>> static inline void pagetable_dtor_free(struct ptdesc *ptdesc)
>> {
>> 	pagetable_dtor(ptdesc);



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv