Re: [PATCH] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs

[Li Zhe]

On 9/7/26 11:29 PM, David Hildenbrand (Arm) wrote:
> On 9/1/26 02:32, Andrew Morton wrote:
>> On Mon, 31 Aug 2026 17:10:23 +0800 "Li Zhe" <lizhe.67@xxxxxxxxxxxxx> wrote:
>>
>>> Hugetlb currently expands MMU notifier ranges to PUD boundaries whenever
>>> PMD sharing is possible. That is only needed when huge_pmd_unshare()
>>> actually detaches a shared PMD page table, because clearing the PUD
>>> invalidates the whole PUD-sized virtual address range.
>>>
>>> For hugetlbfs hole punch and MADV_DONTNEED, a shared mapping can pass
>>> the "PMD sharing is possible" range test in function
>>> adjust_range_if_pmd_sharing_possible() even when the PMD table covering
>>> the target 2M page is not shared. KVM then receives a 1G invalidation for
>>> a 2M operation and zaps unrelated secondary mappings, so the guest has to
>>> fault them back in.
>>>
>>> Fix this by using the existing cheap "sharing possible" test only as a
>>> gate, then inspect the candidate PMD tables under the locks held by the
>>> hugetlb unmap paths. The notifier is expanded only for PUDs whose PMD
>>> table is actually shared, while the other callers keep the existing
>>> conservative expansion.
>> Thanks.
>>
>>> On a Redis-in-VM workload that punches cold 2M hugetlb pages, this
>>> patch improves P99 QPS stability while punching pages, reducing the QPS
>>> degradation ratio from 7.09% to 1.45%.
>> So a modest performance improvement?
> Is that worth the complexity, though?


That is a fair concern.

I tried to simplify the approach. Instead of computing the exact PUD
sub-ranges that contain shared PMD tables, this version keeps the
existing adjust_range_if_pmd_sharing_possible() logic unchanged and only
adds an actual shared-PMD check as a gate before it.

So the behavior becomes:

   - if no shared PMD table is found, keep the notifier range unchanged;
   - if any shared PMD table is found, fall back to the existing
     conservative PUD-sized expansion.

This should still avoid the unnecessary 1G KVM invalidation for the
common unshared-PMD case, while keeping the range expansion policy
unchanged when a shared PMD is present.

The resulting diff would look like this:

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 816dde7..7a61fc3 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5355,10 +5357,12 @@ void __hugetlb_zap_begin(struct vm_area_struct *vma,
      if (!vma->vm_file)    /* hugetlbfs_file_mmap error */
          return;

-    adjust_range_if_pmd_sharing_possible(vma, start, end);
      hugetlb_vma_lock_write(vma);
-    if (vma->vm_file)
+    if (vma->vm_file) {
          i_mmap_lock_write(vma->vm_file->f_mapping);
+        if (range_has_shared_pmd(vma, *start, *end))
+            adjust_range_if_pmd_sharing_possible(vma, start, end);
+    }
  }

  void __hugetlb_zap_end(struct vm_area_struct *vma,
@@ -5397,7 +5401,9 @@ void unmap_hugepage_range(struct vm_area_struct 
*vma, unsigned long start,

      mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
                  start, end);
-    adjust_range_if_pmd_sharing_possible(vma, &range.start, &range.end);
+    if (range_has_shared_pmd(vma, range.start, range.end))
+        adjust_range_if_pmd_sharing_possible(vma, &range.start,
+                             &range.end);
      mmu_notifier_invalidate_range_start(&range);
      tlb_gather_mmu(&tlb, vma->vm_mm);

@@ -6958,6 +6964,52 @@ void adjust_range_if_pmd_sharing_possible(struct 
vm_area_struct *vma,
          *end = ALIGN(*end, PUD_SIZE);
  }

+static bool range_has_shared_pmd(struct vm_area_struct *vma,
+                 unsigned long start, unsigned long end)
+{
+    unsigned long v_start = ALIGN(vma->vm_start, PUD_SIZE);
+    unsigned long v_end = ALIGN_DOWN(vma->vm_end, PUD_SIZE);
+    struct hstate *h = hstate_vma(vma);
+    struct mm_struct *mm = vma->vm_mm;
+    unsigned long address;
+
+    if (huge_page_size(h) != PMD_SIZE)
+        return false;
+
+    /*
+     * First apply the same cheap test as
+     * adjust_range_if_pmd_sharing_possible(). Only the PUD-aligned
+     * intersection can contain shared PMD tables.
+     */
+    if (!(vma->vm_flags & VM_MAYSHARE) || !(v_end > v_start) ||
+        end <= v_start || start >= v_end)
+        return false;
+
+    start = max(ALIGN_DOWN(start, PUD_SIZE), v_start);
+    end = min(ALIGN(end, PUD_SIZE), v_end);
+
+    hugetlb_vma_assert_locked(vma);
+    i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+
+    for (address = start; address < end; address += PUD_SIZE) {
+        pte_t *ptep;
+        bool shared;
+
+        ptep = hugetlb_walk(vma, address, PMD_SIZE);
+        if (!ptep)
+            continue;
+
+        spin_lock(huge_pte_lockptr(h, mm, ptep));
+        shared = ptdesc_pmd_is_shared(virt_to_ptdesc(ptep));
+        spin_unlock(huge_pte_lockptr(h, mm, ptep));
+
+        if (shared)
+            return true;
+    }
+
+    return false;
+}
+
  /*
   * Search for a shareable pmd page for hugetlb. In any case calls 
pmd_alloc()
   * and returns the corresponding pte. While this is not necessary for the
-- 


Does this look like a more reasonable tradeoff?

Thanks,
Zhe

>