Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings

[David Hildenbrand (Arm)]

On 10/14/26 16:51, Lorenzo Stoakes (ARM) wrote:
> On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote:
>> As uffd_lock_vma() is part of the same lookup machinery and
>> also contains a misleading VM_SHARED check, change that one as well
>> directly for consistency.
> 
> OK, I think that one isn't _strictly necessary_, but while we're there,
> worth doing too, clearly.
> 
> We should also go through and convert the remaining !VM_SHARED checks ->
> vma_is_cow_mapping() too if possible, but that's a follow up, obviously!
> 
>>
>> A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma)
>> lock in copy_hugetlb_page_range() entirely. Further, the usage of
> 
> Hmm really? Is it possible to do that without the hugetlb lock?
> 

Yes, we don't have to sync against PMD sharing if PMD sharing is impossible.
This is what I have laying around:

commit ad6a5c65bd5fa31d93a2de2d2e5e473b26ad6a3d
Author: David Hildenbrand (Arm) <david@xxxxxxxxxx>
Date:   Sat Oct 10 13:00:13 2026 +0200

    mm/hugetlb: stop syncing against PMD table sharing in copy_hugetlb_page_range()
    
    We must never end up in copy_hugetlb_page_range() when PMD sharing
    applies, yet we still have to perform the hugetlb_vma_lock_read() so
    hugetlb_walk() is happy.
    
    Let's introduce a new vma_supports_pmd_sharing() and use that instead
    in hugetlb_walk(). Also convert want_pmd_share(), which now looks a lot
    cleaner, and document why the uffd checks cannot go into
    __vma_shareable_lock().
    
    We can now relocate our safety checks in copy_hugetlb_page_range() to
    the beginning of the function.
    
    No functional change intended.
    
    Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 5029c72418631..b2da5d5816944 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -1389,6 +1389,20 @@ static inline bool __vma_shareable_lock(struct vm_area_struct *vma)
 
 bool __vma_private_lock(struct vm_area_struct *vma);
 
+static inline bool vma_supports_pmd_sharing(struct vm_area_struct *vma)
+{
+#ifdef CONFIG_USERFAULTFD
+	/*
+	 * Keep the UFFD checks out of __vma_shareable_lock(): UFFD flags can
+	 * change at runtime while the hugetlb VMA lock is held. They must not
+	 * cause unlocking or freeing an existing lock to be skipped.
+	 */
+	if (uffd_disable_huge_pmd_share(vma))
+		return false;
+#endif
+	return __vma_shareable_lock(vma);
+}
+
 /*
  * Safe version of huge_pte_offset() to check the locks.  See comments
  * above huge_pte_offset().
@@ -1406,7 +1420,7 @@ hugetlb_walk(struct vm_area_struct *vma, unsigned long addr, unsigned long sz)
 	 *
 	 * NOTE: lockdep_is_held() is only defined with CONFIG_LOCKDEP.
 	 */
-	if (__vma_shareable_lock(vma))
+	if (vma_supports_pmd_sharing(vma))
 		WARN_ON_ONCE(!lockdep_is_held(&vma_lock->rw_sema) &&
 			     !lockdep_is_held(
 				 &vma->vm_file->f_mapping->i_mmap_rwsem));
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 60f9b209c8db1..606e93e32eefb 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4987,22 +4987,16 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 		 * Shared mappings only need copying when we could have
 		 * information stored in page tables (e.g., userfaultfd PTE
 		 * bits or markers), which is incompatible with PMD sharing.
-		 * Still take the hugetlb VMA lock for now to satisfy
-		 * hugetlb_walk()'s locking requirements.
 		 */
-		hugetlb_vma_lock_read(src_vma);
+		if (WARN_ON_ONCE(vma_supports_pmd_sharing(src_vma) ||
+				 vma_supports_pmd_sharing(dst_vma)))
+			return -EINVAL;
 	}
 
 	last_addr_mask = hugetlb_mask_last_page(h);
 	for (addr = src_vma->vm_start; addr < src_vma->vm_end; addr += sz) {
 		spinlock_t *src_ptl, *dst_ptl;
 
-		if (WARN_ON_ONCE(want_pmd_share(src_vma, addr) ||
-				 want_pmd_share(dst_vma, addr))) {
-			ret = -EINVAL;
-			break;
-		}
-
 		src_pte = hugetlb_walk(src_vma, addr, sz);
 		if (!src_pte) {
 			addr |= last_addr_mask;
@@ -5147,8 +5141,6 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 	if (cow) {
 		raw_write_seqcount_end(&src->write_protect_seq);
 		mmu_notifier_invalidate_range_end(&range);
-	} else {
-		hugetlb_vma_unlock_read(src_vma);
 	}
 
 	return ret;
@@ -7005,17 +6997,9 @@ bool want_pmd_share(struct vm_area_struct *vma, unsigned long addr)
 	unsigned long start = addr & PUD_MASK;
 	unsigned long end = start + PUD_SIZE;
 
-#ifdef CONFIG_USERFAULTFD
-	if (uffd_disable_huge_pmd_share(vma))
-		return false;
-#endif
-	/*
-	 * check on proper vm_flags and page table alignment
-	 */
-	if (!(vma->vm_flags & VM_MAYSHARE))
-		return false;
-	if (!vma->vm_private_data)	/* vma lock required for sharing */
+	if (!vma_supports_pmd_sharing(vma))
 		return false;
+
 	if (!range_in_vma(vma, start, end))
 		return false;
 	return true;


Will send that out for the next cycle.

> It'd be good in general if we could get rid of that lock as a whole :) but maybe
> dreaming.

Yeah :)

Thanks!

-- 
Cheers,

David