Re: [PATCH v2 1/2] mm: free unused page table allocated by do_set_pmd()

[David Hildenbrand (Arm)]

On 11/8/26 10:14, Lance Yang wrote:
> 
> On Fri, Nov 06, 2026 at 01:36:12PM +0100, David Hildenbrand (Arm) wrote:
>> do_set_pmd() can allocate vmf->prealloc_pte on architectures that need a
>> deposited page table, such as PowerPC with the hash MMU. If the PMD is
>> no longer empty when it takes the PMD lock, it returns VM_FAULT_FALLBACK
>> without consuming that table.
>>
>> Since collapse_pte_mapped_thp() uses mmap_read_lock(), a concurrent
>> userfaultfd operation can install a PTE table before taking the folio
>> lock. If that races with set_huge_pmd(), the unused preallocated table
>> is lost when its local vm_fault goes out of scope.
>>
>> Track whether do_set_pmd() allocated the table and free it after dropping
>> the PMD lock if it was not consumed. Preserve any preallocated table
>> supplied by the caller for PTE fallback. Successful PMD installation
>> consumes the table and clears the pointer.
>>
>> Returning without a preallocated table matches the existing behavior on
>> architectures that do not need a deposited table. All callers
>> already handle that case.
>>
>> Fixes: 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>> ---
>> mm/memory.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index becf53a2a0944..421064aef3cf0 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -5687,6 +5687,7 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
>> 	struct vm_area_struct *vma = vmf->vma;
>> 	bool write = vmf->flags & FAULT_FLAG_WRITE;
>> 	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
>> +	bool pgtable_allocated = false;
>> 	pmd_t entry;
>> 	vm_fault_t ret = VM_FAULT_FALLBACK;
>>
>> @@ -5725,6 +5726,7 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
>> 		vmf->prealloc_pte = pte_alloc_one(vma->vm_mm);
>> 		if (!vmf->prealloc_pte)
>> 			return VM_FAULT_OOM;
>> +		pgtable_allocated = true;
>> 	}
>>
>> 	vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
>> @@ -5755,6 +5757,10 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
>> 	count_vm_event(THP_FILE_MAPPED);
>> out:
>> 	spin_unlock(vmf->ptl);
>> +	if (pgtable_allocated && vmf->prealloc_pte) {
> 
> Took me a sec ... to see why we can't just check vmf->prealloc_pte here
> ...
> 
> Emm ... as it turns out, the caller might have supplied it for fallback :D
> 

Yeah, it's all ... complicated :)

-- 
Cheers,

David