* [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs @ 2026-09-24 8:20 Li Zhe 2026-09-24 9:51 ` Muchun Song 0 siblings, 1 reply; 4+ messages in thread From: Li Zhe @ 2026-09-24 8:20 UTC (permalink / raw) To: akpm, david, muchun.song, osalvador Cc: linux-kernel, linux-mm, aiqi.i7, lizhe.67 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; +} +#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; -- 2.20.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs 2026-09-24 8:20 [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs Li Zhe @ 2026-09-24 9:51 ` Muchun Song 2026-09-24 11:20 ` Li Zhe 0 siblings, 1 reply; 4+ messages in thread From: Muchun Song @ 2026-09-24 9:51 UTC (permalink / raw) To: Li Zhe; +Cc: linux-kernel, linux-mm, aiqi.i7, akpm, david, osalvador 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 > +} > +#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; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs 2026-09-24 9:51 ` Muchun Song @ 2026-09-24 11:20 ` Li Zhe 2026-09-27 4:36 ` Muchun Song 0 siblings, 1 reply; 4+ messages in thread From: Li Zhe @ 2026-09-24 11:20 UTC (permalink / raw) To: Muchun Song; +Cc: linux-kernel, linux-mm, aiqi.i7, akpm, david, osalvador 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. 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; > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs 2026-09-24 11:20 ` Li Zhe @ 2026-09-27 4:36 ` Muchun Song 0 siblings, 0 replies; 4+ messages in thread From: Muchun Song @ 2026-09-27 4:36 UTC (permalink / raw) To: Li Zhe; +Cc: linux-kernel, linux-mm, aiqi.i7, akpm, david, osalvador > 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; ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-27 4:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-24 8:20 [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs Li Zhe 2026-09-24 9:51 ` Muchun Song 2026-09-24 11:20 ` Li Zhe 2026-09-27 4:36 ` Muchun Song
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®