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

[Kiryl Shutsemau]

On Thu, Nov 05, 2026 at 05:40:39PM +0100, David Hildenbrand (Arm) wrote:
> 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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov