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

[Dave Hansen <dave.hansen@intel.com>]

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.