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

[Muchun Song <muchun.song@linux.dev>]

> On Sep 24, 2026, at 19:20, Li Zhe <lizhe.67@bytedance.com> wrote:
> 
> On 9/24/26 5:51 PM, Muchun Song wrote:
>> 
>> 
>> On 2026/9/24 16:20, Li Zhe 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 similarly for other hugetlb unmap paths,
>>> a shared mapping can pass the "PMD sharing is possible" range test in
>>> adjust_range_if_pmd_sharing_possible() even when the hugetlbfs file does
>>> not currently have any shared PMD page tables. KVM then receives a 1G
>>> invalidation for a 2M operation and zaps unrelated secondary mappings,
>>> so the guest has to fault them back in.
>>> 
>>> Avoid this by tracking active PMD-sharing attachments per hugetlbfs
>>> inode. The count is incremented only after huge_pmd_share() successfully
>>> installs a shared PMD table, and decremented when __huge_pmd_unshare()
>>> actually detaches one. Since huge_pmd_share() can run concurrently under
>>> i_mmap_lock_read(), use atomic operations for the count. A zero count is
>>> used to skip the conservative notifier range expansion only after
>>> excluding concurrent PMD sharing with the mapping write lock.
>>> 
>>> 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%.
>>> 
>>> Reported-by: aiqi.i7 <aiqi.i7@bytedance.com>
>>> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
>>> ---
>>> v2: 
>>> https://lore.kernel.org/all/20260922090749.24905-1-lizhe.67@bytedance.com/
>>> v1: 
>>> https://lore.kernel.org/all/20260831091023.66581-1-lizhe.67@bytedance.com/
>>> 
>>> ChangeLogs:
>>> v2->v3:
>>> - Rework the sticky state based on Andrew's feedback: use a per-inode
>>>    counter of active PMD-sharing attachments, so files can return to the
>>>    no-active-sharing state after PMD sharing ends.
>>> 
>>> v1->v2:
>>> - Rework the implementation based on David's suggestion: remember
>>>    whether PMD sharing ever happened for a hugetlbfs file, and skip the
>>>    conservative notifier range expansion while it has not. This avoids
>>>    the per-unmap page-table walk.
>>> 
>>>   fs/hugetlbfs/inode.c    |  1 +
>>>   include/linux/hugetlb.h | 43 +++++++++++++++++++++++++++++++++++++++++
>>>   mm/hugetlb.c            | 13 ++++++++++---
>>>   3 files changed, 54 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>>> index 7611a8470ea26..78e27ce0a6f63 100644
>>> --- a/fs/hugetlbfs/inode.c
>>> +++ b/fs/hugetlbfs/inode.c
>>> @@ -921,6 +921,7 @@ static struct inode *hugetlbfs_get_inode(struct 
>>> super_block *sb,
>>>           simple_inode_init_ts(inode);
>>>           info->resv_map = resv_map;
>>>           info->seals = F_SEAL_SEAL;
>>> +        hugetlbfs_pmd_sharing_init(inode);
>>>           switch (mode & S_IFMT) {
>>>           default:
>>>               init_special_inode(inode, mode, dev);
>>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>>> index 16c4c4caa126c..6b4f92b7f7ae4 100644
>>> --- a/include/linux/hugetlb.h
>>> +++ b/include/linux/hugetlb.h
>>> @@ -12,6 +12,7 @@
>>>   #include <linux/page_ref.h>
>>>   #include <linux/list.h>
>>>   #include <linux/kref.h>
>>> +#include <linux/atomic.h>
>>>   #include <linux/pgtable.h>
>>>   #include <linux/gfp.h>
>>>   #include <linux/userfaultfd_k.h>
>>> @@ -509,6 +510,9 @@ struct hugetlbfs_inode_info {
>>>       struct inode vfs_inode;
>>>       struct resv_map *resv_map;
>>>       unsigned int seals;
>>> +#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING
>>> +    atomic_t pmd_sharing_count;
>>> +#endif
>>>   };
>>>     static inline struct hugetlbfs_inode_info *HUGETLBFS_I(struct 
>>> inode *inode)
>>> @@ -516,6 +520,45 @@ static inline struct hugetlbfs_inode_info 
>>> *HUGETLBFS_I(struct inode *inode)
>>>       return container_of(inode, struct hugetlbfs_inode_info, 
>>> vfs_inode);
>>>   }
>>>   +#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING
>>> +static inline void hugetlbfs_pmd_sharing_init(struct inode *inode)
>>> +{
>>> +    atomic_set(&HUGETLBFS_I(inode)->pmd_sharing_count, 0);
>>> +}
>>> +
>>> +static inline void hugetlbfs_pmd_sharing_inc(struct inode *inode)
>>> +{
>>> +    atomic_inc(&HUGETLBFS_I(inode)->pmd_sharing_count);
>>> +}
>>> +
>>> +static inline void hugetlbfs_pmd_sharing_dec(struct inode *inode)
>>> +{
>>> +    atomic_dec(&HUGETLBFS_I(inode)->pmd_sharing_count);
>>> +}
>>> +
>>> +/*
>>> + * A 32-bit counter can theoretically wrap, but doing so would require
>>> + * billions of active PMD-sharing attachments to the same inode and 
>>> is not
>>> + * expected in practice. Treat any non-zero value as active so a 
>>> wrapped
>>> + * negative value still takes the conservative notifier range.
>>> + */
>>> +static inline bool hugetlbfs_pmd_sharing_active(struct inode *inode)
>>> +{
>>> +    return atomic_read(&HUGETLBFS_I(inode)->pmd_sharing_count) != 0;
>> 
>> Testing for non zero covers the negative half of the cycle,
>> but the 2^32nd increment changes the value back to zero.
>> 
>> The inode-wide total is not bounded by PID_MAX_LIMIT because
>> one mm can have an attachment in every PUD-sized part of a
>> large mapping.  For example, a 4-level x86 mm has about
>> 131,000 user PUD slots.  Roughly 32,769 child mms can therefore
>> create more than 2^32 attachments by faulting one address in
>> each PUD of a single large inherited MAP_SHARED hugetlb VMA.
>> This also does not require one backing huge page per
>> attachment: huge_pte_alloc() installs or shares the PMD table
>> before hugetlb_no_page() attempts to obtain the huge page.
>> 
>> At the zero value, an unmap can take i_mmap_rwsem for write,
>> observe no active sharing, and issue only the original narrow
>> notifier.  Its subsequent __huge_pmd_unshare() can still
>> clear a PUD and decrement the counter from zero to -1, leaving
>> the rest of the PUD-sized invalidation unreported.
>> 
>> Could this use a 64-bit or saturating counter so an active
>> count can never be mistaken for zero?
>> 
>> Therefore, I recommend using atomic64_t.
>> 
>> Thanks,
>> Muchun
> 
> 
> Thanks for the detailed explanation.
> 
> I did consider that such an extreme case could theoretically cause a
> problem, but I expected it to be practically unreachable. But since we
> can cover this corner case simply by using atomic64_t, that is definitely
> better.
> 
> I will switch the counter to atomic64_t in v4 and drop the 32-bit wrap
> comment.

Another option would be use refcount infrastructure since it already
consider the 32-bit wrap issue itself.

> 
> Thanks,
> Zhe
> 
>> 
>>> +}
>>> +#else
>>> +static inline void hugetlbfs_pmd_sharing_init(struct inode *inode) {}
>>> +
>>> +static inline void hugetlbfs_pmd_sharing_inc(struct inode *inode) {}
>>> +
>>> +static inline void hugetlbfs_pmd_sharing_dec(struct inode *inode) {}
>>> +
>>> +static inline bool hugetlbfs_pmd_sharing_active(struct inode *inode)
>>> +{
>>> +    return false;
>>> +}
>>> +#endif
>>> +
>>>   extern const struct vm_operations_struct hugetlb_vm_ops;
>>>   struct file *hugetlb_file_setup(const char *name, size_t size, 
>>> vma_flags_t acct,
>>>                   int creat_flags, int page_size_log);
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index 4f6f58bf3db6c..cb27c06d1f7a7 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -5361,10 +5361,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 (hugetlbfs_pmd_sharing_active(file_inode(vma->vm_file)))
>>> +            adjust_range_if_pmd_sharing_possible(vma, start, end);
>>> +    }
>>>   }
>>>     void __hugetlb_zap_end(struct vm_area_struct *vma,
>>> @@ -5403,7 +5405,10 @@ 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);
>>> +    i_mmap_assert_write_locked(vma->vm_file->f_mapping);
>>> +    if (hugetlbfs_pmd_sharing_active(file_inode(vma->vm_file)))
>>> +        adjust_range_if_pmd_sharing_possible(vma, &range.start,
>>> +                             &range.end);
>>>       mmu_notifier_invalidate_range_start(&range);
>>>       tlb_gather_mmu(&tlb, vma->vm_mm);
>>>   @@ -7006,6 +7011,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, 
>>> struct vm_area_struct *vma,
>>>       if (pud_none(*pud)) {
>>>           pud_populate(mm, pud,
>>>                   (pmd_t *)((unsigned long)spte & PAGE_MASK));
>>> +        hugetlbfs_pmd_sharing_inc(file_inode(vma->vm_file));
>>>           mm_inc_nr_pmds(mm);
>>>       } else {
>>>           ptdesc_pmd_pts_dec(virt_to_ptdesc(spte));
>>> @@ -7037,6 +7043,7 @@ static int __huge_pmd_unshare(struct mmu_gather 
>>> *tlb,
>>>       pud_clear(pud);
>>>         tlb_unshare_pmd_ptdesc(tlb, virt_to_ptdesc(ptep), addr);
>>> +    hugetlbfs_pmd_sharing_dec(file_inode(vma->vm_file));
>>>         mm_dec_nr_pmds(mm);
>>>       return 1;