Re: [PATCH v4 5/8] x86/mm: Use pagetable_free()

Dave Hansen
2025-09-05T19:22:45Z

On 9/5/25 11:41, Jason Gunthorpe wrote:
...
>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
>> index 76e33bd7c556..86b4297c1984 100644
>> --- a/arch/x86/mm/init_64.c
>> +++ b/arch/x86/mm/init_64.c
>> @@ -1013,7 +1013,7 @@ static void __meminit free_pagetable(struct page *page, int order)
>> free_reserved_pages(page, nr_pages);
>> #endif
>> } else {
>> - free_pages((unsigned long)page_address(page), order);
>> + pagetable_free(page_ptdesc(page));
>> }
>> }
>
> Er.. So if bootmem happens to be under the table and we happen to free
> it due to memory hotplug we don't go through the SVA fixing path?
>
> Seems wrong??

Yes, it does.

> Bootmem still has a ptdesc and a usable linked list, right?

Yep, I think so.

> So maybe this should be redone into an arch hook in/around
> __pagetable_free() and all the above frees just use the normal ptdesc
> free path, including the SVA work queue?
There are reserved pages and the put_page_bootmem() ones too. The
put_page_bootmem() ones are especially annoying because they can't
always be freed.

put_page_bootmem() only has three call sites. Maybe we should just make
it return if the caller *should* free the page instead of actually
freeing it.

Then we get something like:

free_pagetable()
{
if (PageReserved(page)) {
if (is_bootmem(page) && put_page_bootmem(page)) {
// refcount didn't drop to 0, can't free
return;
}
unreserve_page(page);
}

pagetable_free(page_ptdesc(page));
}

Which doesn't seem too bad.