[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <10d2f466-4660-4439-a5fb-dfc4384ca432@intel.com>
Date: Fri, 5 Sep 2025 12:22:42 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Jason Gunthorpe <jgg@...dia.com>, Lu Baolu <baolu.lu@...ux.intel.com>
Cc: Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>, Kevin Tian <kevin.tian@...el.com>,
Jann Horn <jannh@...gle.com>, Vasant Hegde <vasant.hegde@....com>,
Alistair Popple <apopple@...dia.com>, Peter Zijlstra <peterz@...radead.org>,
Uladzislau Rezki <urezki@...il.com>,
Jean-Philippe Brucker <jean-philippe@...aro.org>,
Andy Lutomirski <luto@...nel.org>, Yi Lai <yi1.lai@...el.com>,
iommu@...ts.linux.dev, security@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/8] x86/mm: Use pagetable_free()
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.
Powered by blists - more mailing lists