Re: [PATCH] mm/khugepaged: free unused page table when setting a huge PMD fails

["David Hildenbrand (Arm)" <david@kernel.org>]

On 11/5/26 14:04, Kiryl Shutsemau wrote:
> On Thu, Nov 05, 2026 at 01:02:08PM +0000, Kiryl Shutsemau wrote:
>> On Thu, Nov 05, 2026 at 01:16:05PM +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.
>>>
>>> Free vmf.prealloc_pte on failure, as do_fault() already does. Successful
>>> PMD installation consumes the table and clears the pointer.
>>>
>>> Having to do this cleanup outside of do_set_pmd() is rather nasty, but
>>> other callers depend on it already having been allocated. This needs a
>>> cleanup at some point.
>>
>> I hate this being outside of do_set_pmd().
>>
>> What about making do_set_pmd() freeing ->prealloc_pte if it was
>> allocated inside? Nothing changes for caller.
>  
> Or if we want to keep in set_huge_pmd() then set_huge_pmd() has to
> allocate it too. At least, it makes some sense.

It's all rather nasty.

I thought the callers depend on it, but if we just free in the function
it will behave the same way independent of arch_needs_pgtable_deposit(),
so that makes sense.

From 4e8d0a928f0b21e0583dac9234fb1821f3c973b2 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Wed, 4 Nov 2026 20:56:37 +0100
Subject: [PATCH] mm: free unused page table allocated by do_set_pmd()

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@vger.kernel.org
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 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) {
+		pte_free(vma->vm_mm, vmf->prealloc_pte);
+		vmf->prealloc_pte = NULL;
+	}
 	return ret;
 }
 #else
-- 
2.43.0


Apparently we might be allocating in do_set_pmd() from a RCU critical section ... maybe I'll find the
time to look into that.

-- 
Cheers,

David