Re: [PATCH] x86/mm: fix freeing of PMD-sized vmemmap pages

[David Hildenbrand (Arm)]

On 4/29/26 04:12, Lance Yang wrote:
> 
> On Tue, Apr 28, 2026 at 12:29:36PM +0200, David Hildenbrand (Arm) wrote:
>> In commit bf9e4e30f353 ("x86/mm: use pagetable_free()"), we switched
>>from freeing non-boot page tables through __free_pages() to
>> pagetable_free().
>>
>> However, the function is also called to free vmemmap pages.
>>
>> Given that vmemmap pages are not page tables, already the page_ptdesc(page)
>> is wrong. But worse, pagetable_free() calls
>>
>> 	__free_pages(page, compound_order(page));
>>
>> As vmemmap pages are not compound pages (see vmemmap_alloc_block()) --
>> except for HVO, which doesn't apply here -- we will only free the first
>> page when freeing a PMD-sized vmemmap page, leaking the other ones.
>>
>> Fix it by properly decoupling pagetable and vmemmap freeing.
>> free_pagetable() no longer has to mess with SECTION_INFO, as only the
>> vmemmap is marked like that in register_page_bootmem_memmap().
>>
>> While at it, just wire up the altmap parameter for remove_pte_table().
>> Also, the indentation in remove_pmd_table() is messed up, let's fix that
>> while touching it.
> 
> One thing I'm not sure about is passing altmap down into
> remove_pte_table().
> 
> Do we actually know that a non-NULL altmap means that the vmemmap
> backing page came from that altmap?

I thought with an altmap we'd never get into the situation of allocating outside
the altmap.

But you're right that sub-section hotplug might actually trigger this.


> 
> On x86 we still have in vmemmap_populate():
> 
> 	if (end - start < PAGES_PER_SECTION * sizeof(struct page))
> 		err = vmemmap_populate_basepages(start, end, node, NULL);
> 
> So for smaller-than-section vmemmap ranges, even if the caller has an
> altmap, the backing pages are allocated from normal memory. But with
> this fix the PTE removal path would now call vmem_altmap_free() just
> because altmap is non-NULL, and would not free the actual backing page,
> IIUC :)
> 
> Maybe free_vmemmap_pages() should first check that the backing page is
> really inside the altmap range before using vmem_altmap_free()?

Probably I'll just leave it as is for now, and simply pass "NULL" for the PTE
case like we effectively did before.

Thanks!


-- 
Cheers,

David