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

[David Hildenbrand (Arm)]

On 9/8/26 09:09, Li Zhe wrote:
> On 9/7/26 11:29 PM, David Hildenbrand (Arm) wrote:
>> On 9/1/26 02:32, Andrew Morton wrote:
>>>
>>> Thanks.
>>>
>>> So a modest performance improvement?
>> Is that worth the complexity, though?
> 
> 
> That is a fair concern.

In you setup, are the page tables ever being shared? I suspect you just run a VM
with no other processes actually sharing the memory?

One idea would be to just remember whether any sharing ever happened for a
hugetlb file.

> 
> 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));

I really don't like this piece of code to optimize something that is already
questionable in practice: overcommiting hugetlb folios for VMs.

Can you share some more details which mechanism ends up zapping hugetlb folios
for the VM?

If it's virtio-balloon's free-page-reporting, you should likely disable that for
the VM.

-- 
Cheers,

David