All of lore.kernel.org
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Li Zhe <lizhe.67@bytedance.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	aiqi.i7@bytedance.com, akpm@linux-foundation.org,
	david@kernel.org, osalvador@suse.de
Subject: Re: [PATCH v3] mm/hugetlb: fix overbroad MMU notifiers for unshared PMDs
Date: Thu, 24 Sep 2026 17:51:26 +0800	[thread overview]
Message-ID: <76ee3d7a-cee9-4ff3-9d4c-068a46883e30@linux.dev> (raw)
In-Reply-To: <20260924082004.82450-1-lizhe.67@bytedance.com>



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;



  reply	other threads:[~2026-09-24  9:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-09-24 11:20   ` Li Zhe
2026-09-27  4:36     ` Muchun Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76ee3d7a-cee9-4ff3-9d4c-068a46883e30@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=aiqi.i7@bytedance.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizhe.67@bytedance.com \
    --cc=osalvador@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.