Re: [PATCH] arm64: mm: call pagetable dtor when freeing hot-removed page tables

[Vishal Moola <vishal.moola@gmail.com>]

On Wed, May 27, 2026 at 09:34:19AM +0200, Kevin Brodsky wrote:
> On 26/05/2026 14:31, David Hildenbrand (Arm) wrote:
> > On 5/26/26 13:54, Kevin Brodsky wrote:
> >> On 22/05/2026 11:36, Vishal Moola wrote:
> >>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >>>> index 4c8959153ac4..9d42cbddce27 100644
> >>>> --- a/arch/arm64/mm/mmu.c
> >>>> +++ b/arch/arm64/mm/mmu.c
> >>>> @@ -1441,6 +1441,9 @@ static void free_hotplug_page_range(struct page *page, size_t size,
> >>>>  
> >>>>  static void free_hotplug_pgtable_page(struct page *page)
> >>>>  {
> >>>> +	if (folio_test_pgtable(page_folio(page)))
> >>> This should work.
> >>>
> >>>> +		pagetable_dtor(page_ptdesc(page));
> >>>> +
> >>>>  	free_hotplug_page_range(page, PAGE_SIZE, NULL);
> >>> In the case we presumably have a page table page (ptdesc) at this
> >>> point, we should really be freeing it with pagetable_free() as well.
> >> Agreed, I think this is the right thing to do, something like:
> >>
> >> if (folio_test_pgtable(page_folio(page)))
> >> pagetable_dtor_free(page_ptdesc(page)); else
> >> free_hotplug_page_range(page, PAGE_SIZE, NULL);
> > That code pattern is wrong.
> >
> > folio_test_pgtable() shouldn't exist.
> >
> > In the future, something is either a pgtable or a folio, not both.
> >
> > So check the type against the page, not the folio.
> 
> In other words use PageTable(page) instead? Interestingly I can see a
> few calls to folio_test_pgtable() across the kernel but none to
> PageTable(), maybe just an antipattern then? The ctor/dtor also use
> __folio_{set,clear}_pgtable().

If we know for sure we have the head page (which we probably do here), we
should use PageTable(page).

I included the folio APIs as a defensive way to ensure large order page
table users don't break. We only set the type on the head page, and using
a folio guarantees we're accessing that head page. This can go away as soon
as someone looks at the architectures that allocate large order ptdescs.

Also, most (if not all) 'pgtables' don't use ptdescs yet, but they
probably should... Anyway, in this particular case, it looks like pgtable
is just a name symbolizing 'page table' and not our type 'pgtable_t' anyway
so I'm just rambling.