mm/hugetlb: fixes for PMD table sharing (incl. using mmu_gather)
Subsystems (7)
David Hildenbrand (Red Hat)
maintainerv3 cover mm/hugetlb: fixes for PMD table sharing (incl. using mmu_gather)
One functional fix, one performance regression fix, and two related comment fixes.
I cleaned up my prototype I recently shared [1] for the performance fix, deferring most of the cleanups I had in the prototype to a later point. While doing that I identified the other things.
The goal of this patch set is to be backported to stable trees "fairly" easily. At least patch #1 and #4.
Patch #1 fixes hugetlb_pmd_shared() not detecting any sharing Patch #2 + #3 are simple comment fixes that patch #4 interacts with. Patch #4 is a fix for the reported performance regression due to excessive IPI broadcasts during fork()+exit().
The last patch is all about TLB flushes, IPIs and mmu_gather.
Read: complicated
I added as much comments + description that I possibly could, and I am hoping for review from Jann.
There are plenty of cleanups in the future to be had + one reasonable optimization on x86. But that's all out of scope for this series.
Compile tested on plenty of architectures.
Runtime tested, with a focus on fixing the performance regression using the original reproducer [2] on x86.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
--
v2 -> v3:
* Rebased to 6.19-rc2 and retested on x86
* Changes on last patch:
* Introduce and use tlb_gather_mmu_vma() for properly setting up mmu_gather for hugetlb -- thanks to Harry for pointing me once again at the nasty hugetlb integration in mmu_gather
* Move tlb_remove_huge_tlb_entry() after move_huge_pte()
* For consistency, always call tlb_gather_mmu_vma() after flush_cache_range()
* Don't pass mmu_gather to hugetlb_change_protection(), simply use a local one for now. (avoids messing with tlb_start_vma() / tlb_start_end())
* Dropped Lorenzo's RB due to the changes
v1 -> v2:
* Picked RB's/ACK's, hopefully I didn't miss any
* Added the initialization of fully_unshared_tables in __tlb_gather_mmu() (Thanks Nadav!)
* Refined some comments based on Lorenzo's feedback.
David Hildenbrand (Red Hat) (4):
mm/hugetlb: fix hugetlb_pmd_shared()
mm/hugetlb: fix two comments related to huge_pmd_unshare() mm/rmap: fix two comments related to huge_pmd_unshare() mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables
using mmu_gather
include/asm-generic/tlb.h | 77 +++++++++++++++++++++- include/linux/hugetlb.h | 17 +++-- include/linux/mm_types.h | 1 + mm/hugetlb.c | 131 +++++++++++++++++++++----------------- mm/mmu_gather.c | 33 ++++++++++ mm/rmap.c | 45 ++++++------- 6 files changed, 213 insertions(+), 91 deletions(-)
base-commit: b927546677c876e26eba308550207c2ddf812a43
Signature · 1 line
2.52.0
David Hildenbrand (Red Hat)
maintainerPATCH RESEND v3 1/4 mm/hugetlb: fix hugetlb_pmd_shared()
We switched from (wrongly) using the page count to an independent shared count. Now, shared page tables have a refcount of 1 (excluding speculative references) and instead use ptdesc->pt_share_count to identify sharing.
We didn't convert hugetlb_pmd_shared(), so right now, we would never detect a shared PMD table as such, because sharing/unsharing no longer touches the refcount of a PMD table.
Page migration, like mbind() or migrate_pages() would allow for migrating folios mapped into such shared PMD tables, even though the folios are not exclusive. In smaps we would account them as "private" although they are "shared", and we would be wrongly setting the PM_MMAP_EXCLUSIVE in the pagemap interface.
Fix it by properly using ptdesc_pmd_is_shared() in hugetlb_pmd_shared().
--- include/linux/hugetlb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/hugetlb.h +1 -1
@@ -1326,7 +1326,7 @@ static inline __init void hugetlb_cma_reserve(int order)#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING static inline bool hugetlb_pmd_shared (pte_t *pte) { -return page_count (virt_to_page (pte)) >1 ; +return ptdesc_pmd_is_shared (virt_to_ptdesc (pte)); }#else static inline bool hugetlb_pmd_shared (pte_t *pte)
Signature · 1 line
2.52.0
David Hildenbrand (Red Hat)
maintainerPATCH RESEND v3 2/4 mm/hugetlb: fix two comments related to huge_pmd_unshare()
In reply to David Hildenbrand (Red Hat)Ever since we stopped using the page count to detect shared PMD page tables, these comments are outdated.
The only reason we have to flush the TLB early is because once we drop the i_mmap_rwsem, the previously shared page table could get freed (to then get reallocated and used for other purpose). So we really have to flush the TLB before that could happen.
So let's simplify the comments a bit.
The "If we unshared PMDs, the TLB flush was not recorded in mmu_gather." part introduced as in commit a4a118f2eead ("hugetlbfs: flush TLBs correctly after huge_pmd_unshare") was confusing: sure it is recorded in the mmu_gather, otherwise tlb_flush_mmu_tlbonly() wouldn't do anything. So let's drop that comment while at it as well.
We'll centralize these comments in a single helper as we rework the code next.
--- mm/hugetlb.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)
mm/hugetlb.c +8 -16
@@ -5304,17 +5304,10 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,tlb_end_vma (tlb, vma);/* -* If we unshared PMDs, the TLB flush was not recorded in mmu_gather. We -* could defer the flush until now, since by holding i_mmap_rwsem we -* guaranteed that the last reference would not be dropped. But we must -* do the flushing before we return, as otherwise i_mmap_rwsem will be -* dropped and the last reference to the shared PMDs page might be -* dropped as well. -* -* In theory we could defer the freeing of the PMD pages as well, but -* huge_pmd_unshare() relies on the exact page_count for the PMD page to -* detect sharing, so we cannot defer the release of the page either. -* Instead, do flush now. +* There is nothing protecting a previously-shared page table that we +* unshared through huge_pmd_unshare() from getting freed after we +* release i_mmap_rwsem, so flush the TLB now. If huge_pmd_unshare() +* succeeded, flush the range corresponding to the pud. */ if (force_flush)tlb_flush_mmu_tlbonly (tlb);
@@ -6536,11 +6529,10 @@ long hugetlb_change_protection(struct vm_area_struct *vma,cond_resched (); }/* -* Must flush TLB before releasing i_mmap_rwsem: x86's huge_pmd_unshare -* may have cleared our pud entry and done put_page on the page table: -* once we release i_mmap_rwsem, another task can do the final put_page -* and that page table be reused and filled with junk. If we actually -* did unshare a page of pmds, flush the range corresponding to the pud. +* There is nothing protecting a previously-shared page table that we +* unshared through huge_pmd_unshare() from getting freed after we +* release i_mmap_rwsem, so flush the TLB now. If huge_pmd_unshare() +* succeeded, flush the range corresponding to the pud. */ if (shared_pmd)flush_hugetlb_tlb_range (vma, range.start, range.end);
Signature · 1 line
2.52.0
David Hildenbrand (Red Hat)
maintainerPATCH RESEND v3 3/4 mm/rmap: fix two comments related to huge_pmd_unshare()
In reply to David Hildenbrand (Red Hat)PMD page table unsharing no longer touches the refcount of a PMD page table. Also, it is not about dropping the refcount of a "PMD page" but the "PMD page table".
Let's just simplify by saying that the PMD page table was unmapped, consequently also unmapping the folio that was mapped into this page.
This code should be deduplicated in the future.
--- mm/rmap.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-)
mm/rmap.c +4 -16
@@ -2016,14 +2016,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,flush_tlb_range (vma, range.start, range.end);/* -* The ref count of the PMD page was -* dropped which is part of the way map -* counting is done for shared PMDs. -* Return 'true' here. When there is -* no other sharing, huge_pmd_unshare -* returns false and we will unmap the -* actual page and drop map count -* to zero. +* The PMD table was unmapped, +* consequently unmapping the folio. */ goto walk_done; }
@@ -2416,14 +2410,8 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma, range.start, range.end);/* -* The ref count of the PMD page was -* dropped which is part of the way map -* counting is done for shared PMDs. -* Return 'true' here. When there is -* no other sharing, huge_pmd_unshare -* returns false and we will unmap the -* actual page and drop map count -* to zero. +* The PMD table was unmapped, +* consequently unmapping the folio. */ page_vma_mapped_walk_done (&pvmw);break ;
Signature · 1 line
2.52.0
David Hildenbrand (Red Hat)
maintainerPATCH RESEND v3 4/4 mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather
In reply to David Hildenbrand (Red Hat)As reported, ever since commit 1013af4f585f ("mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race") we can end up in some situations where we perform so many IPI broadcasts when unsharing hugetlb PMD page tables that it severely regresses some workloads.
In particular, when we fork()+exit(), or when we munmap() a large area backed by many shared PMD tables, we perform one IPI broadcast per unshared PMD table.
There are two optimizations to be had:
(1) When we process (unshare) multiple such PMD tables, such as during exit(), it is sufficient to send a single IPI broadcast (as long as we respect locking rules) instead of one per PMD table.
Locking prevents that any of these PMD tables could get reused before we drop the lock.
(2) When we are not the last sharer (> 2 users including us), there is no need to send the IPI broadcast. The shared PMD tables cannot become exclusive (fully unshared) before an IPI will be broadcasted by the last sharer.
Concurrent GUP-fast could walk into a PMD table just before we unshared it. It could then succeed in grabbing a page from the shared page table even after munmap() etc succeeded (and supressed an IPI). But there is not difference compared to GUP-fast just sleeping for a while after grabbing the page and re-enabling IRQs.
Most importantly, GUP-fast will never walk into page tables that are no-longer shared, because the last sharer will issue an IPI broadcast.
(if ever required, checking whether the PUD changed in GUP-fast
after grabbing the page like we do in the PTE case could handle this)
So let's rework PMD sharing TLB flushing + IPI sync to use the mmu_gather infrastructure so we can implement these optimizations and demystify the code at least a bit. Extend the mmu_gather infrastructure to be able to deal with our special hugetlb PMD table sharing implementation.
To make initialization of the mmu_gather easier when working on a single VMA (in particular, when dealing with hugetlb), provide tlb_gather_mmu_vma().
We'll consolidate the handling for (full) unsharing of PMD tables in tlb_unshare_pmd_ptdesc() and tlb_flush_unshared_tables(), and track in "struct mmu_gather" whether we had (full) unsharing of PMD tables.
Because locking is very special (concurrent unsharing+reuse must be prevented), we disallow deferring flushing to tlb_finish_mmu() and instead require an explicit earlier call to tlb_flush_unshared_tables().
From hugetlb code, we call huge_pmd_unshare_flush() where we make sure that the expected lock protecting us from concurrent unsharing+reuse is still held.
Check with a VM_WARN_ON_ONCE() in tlb_finish_mmu() that tlb_flush_unshared_tables() was properly called earlier.
Document it all properly.
Notes about tlb_remove_table_sync_one() interaction with unsharing:
There are two fairly tricky things:
(1) tlb_remove_table_sync_one() is a NOP on architectures without CONFIG_MMU_GATHER_RCU_TABLE_FREE.
Here, the assumption is that the previous TLB flush would send an IPI to all relevant CPUs. Careful: some architectures like x86 only send IPIs to all relevant CPUs when tlb->freed_tables is set.
The relevant architectures should be selecting MMU_GATHER_RCU_TABLE_FREE, but x86 might not do that in stable kernels and it might have been problematic before this patch.
Also, the arch flushing behavior (independent of IPIs) is different when tlb->freed_tables is set. Do we have to enlighten them to also take care of tlb->unshared_tables? So far we didn't care, so hopefully we are fine. Of course, we could be setting tlb->freed_tables as well, but that might then unnecessarily flush too much, because the semantics of tlb->freed_tables are a bit fuzzy.
This patch changes nothing in this regard.
(2) tlb_remove_table_sync_one() is not a NOP on architectures with CONFIG_MMU_GATHER_RCU_TABLE_FREE that actually don't need a sync.
Take x86 as an example: in the common case (!pv, !X86_FEATURE_INVLPGB) we still issue IPIs during TLB flushes and don't actually need the second tlb_remove_table_sync_one().
This optimized can be implemented on top of this, by checking e.g., in tlb_remove_table_sync_one() whether we really need IPIs. But as described in (1), it really must honor tlb->freed_tables then to send IPIs to all relevant CPUs.
Notes on TLB flushing changes:
(1) Flushing for non-shared PMD tables
We're converting from flush_hugetlb_tlb_range() to tlb_remove_huge_tlb_entry(). Given that we properly initialize the MMU gather in tlb_gather_mmu_vma() to be hugetlb aware, similar to __unmap_hugepage_range(), that should be fine.
(2) Flushing for shared PMD tables
We're converting from various things (flush_hugetlb_tlb_range(), tlb_flush_pmd_range(), flush_tlb_range()) to tlb_flush_pmd_range().
tlb_flush_pmd_range() achieves the same that tlb_remove_huge_tlb_entry() would achieve in these scenarios. Note that tlb_remove_huge_tlb_entry() also calls __tlb_remove_tlb_entry(), however that is only implemented on powerpc, which does not support PMD table sharing.
Similar to (1), tlb_gather_mmu_vma() should make sure that TLB flushing keeps on working as expected.
Further, note that the ptdesc_pmd_pts_dec() in huge_pmd_share() is not a concern, as we are holding the i_mmap_lock the whole time, preventing concurrent unsharing. That ptdesc_pmd_pts_dec() usage will be removed separately as a cleanup later.
There are plenty more cleanups to be had, but they have to wait until this is fixed.
--- include/asm-generic/tlb.h | 77 +++++++++++++++++++++++- include/linux/hugetlb.h | 15 +++-- include/linux/mm_types.h | 1 + mm/hugetlb.c | 123 ++++++++++++++++++++++---------------- mm/mmu_gather.c | 33 ++++++++++ mm/rmap.c | 25 +++++--- 6 files changed, 208 insertions(+), 66 deletions(-)
include/asm-generic/tlb.h +75 -2
@@ -46,7 +46,8 @@* * The mmu_gather API consists of: * -* - tlb_gather_mmu() / tlb_gather_mmu_fullmm() / tlb_finish_mmu() +* - tlb_gather_mmu() / tlb_gather_mmu_fullmm() / tlb_gather_mmu_vma() / +* tlb_finish_mmu() * * start and finish a mmu_gather *
@@ -364,6 +365,20 @@ struct mmu_gather { unsigned int vma_huge : 1; unsigned int vma_pfn : 1; +/* +* Did we unshare (unmap) any shared page tables? For now only +* used for hugetlb PMD table sharing. +*/ + unsigned int unshared_tables : 1; + +/* +* Did we unshare any page tables such that they are now exclusive +* and could get reused+modified by the new owner? When setting this +* flag, "unshared_tables" will be set as well. For now only used +* for hugetlb PMD table sharing. +*/ + unsigned int fully_unshared_tables : 1; +unsigned int batch_count;#ifndef CONFIG_MMU_GATHER_NO_GATHER
@@ -400,6 +415,7 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb) tlb->cleared_pmds =0 ; tlb->cleared_puds =0 ; tlb->cleared_p4ds =0 ; + tlb->unshared_tables =0 ;/* * Do not reset mmu_gather::vma_* fields here, we do not * call into tlb_start_vma() again to set them if there is an
@@ -484,7 +500,7 @@ static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)* these bits. */ if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds || - tlb->cleared_puds || tlb->cleared_p4ds)) + tlb->cleared_puds || tlb->cleared_p4ds || tlb->unshared_tables))return ;tlb_flush (tlb);
@@ -773,6 +789,63 @@ static inline bool huge_pmd_needs_flush(pmd_t oldpmd, pmd_t newpmd) }#endif +#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING +static inline void tlb_unshare_pmd_ptdesc (struct mmu_gather *tlb,struct ptdesc *pt, +unsigned long addr) +{ +/* +* The caller must make sure that concurrent unsharing + exclusive +* reuse is impossible until tlb_flush_unshared_tables() was called. +*/ +VM_WARN_ON_ONCE (!ptdesc_pmd_is_shared (pt)); +ptdesc_pmd_pts_dec (pt); + +/* Clearing a PUD pointing at a PMD table with PMD leaves. */ +tlb_flush_pmd_range (tlb, addr &PUD_MASK ,PUD_SIZE ); + +/* +* If the page table is now exclusively owned, we fully unshared +* a page table. +*/ +if (!ptdesc_pmd_is_shared (pt)) + tlb->fully_unshared_tables = true; + tlb->unshared_tables = true; +} + +static inline void tlb_flush_unshared_tables (struct mmu_gather *tlb) +{ +/* +* As soon as the caller drops locks to allow for reuse of +* previously-shared tables, these tables could get modified and +* even reused outside of hugetlb context, so we have to make sure that +* any page table walkers (incl. TLB, GUP-fast) are aware of that +* change. +* +* Even if we are not fully unsharing a PMD table, we must +* flush the TLB for the unsharer now. +*/ +if (tlb->unshared_tables) +tlb_flush_mmu_tlbonly (tlb); + +/* +* Similarly, we must make sure that concurrent GUP-fast will not +* walk previously-shared page tables that are getting modified+reused +* elsewhere. So broadcast an IPI to wait for any concurrent GUP-fast. +* +* We only perform this when we are the last sharer of a page table, +* as the IPI will reach all CPUs: any GUP-fast. +* +* Note that on configs where tlb_remove_table_sync_one() is a NOP, +* the expectation is that the tlb_flush_mmu_tlbonly() would have issued +* required IPIs already for us. +*/ +if (tlb->fully_unshared_tables) { +tlb_remove_table_sync_one (); + tlb->fully_unshared_tables = false; + } +} +#endif /* CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING */ +#endif /* CONFIG_MMU */ #endif /* _ASM_GENERIC__TLB_H */
include/linux/hugetlb.h +10 -5
@@ -240,8 +240,9 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,pte_t *huge_pte_offset (struct mm_struct *mm,unsigned long addr,unsigned long sz);unsigned long hugetlb_mask_last_page (struct hstate *h); -int huge_pmd_unshare (struct mm_struct *mm,struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep); +int huge_pmd_unshare (struct mmu_gather *tlb,struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep); +void huge_pmd_unshare_flush (struct mmu_gather *tlb,struct vm_area_struct *vma);void adjust_range_if_pmd_sharing_possible (struct vm_area_struct *vma,unsigned long *start,unsigned long *end);
@@ -300,13 +301,17 @@ static inline struct address_space *hugetlb_folio_mapping_lock_write(return NULL ; } -static inline int huge_pmd_unshare (struct mm_struct *mm, -struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline int huge_pmd_unshare (struct mmu_gather *tlb, +struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {return 0 ; } +static inline void huge_pmd_unshare_flush (struct mmu_gather *tlb, +struct vm_area_struct *vma) +{ +} +static inline void adjust_range_if_pmd_sharing_possible (struct vm_area_struct *vma,unsigned long *start,unsigned long *end)
include/linux/mm_types.h +1 -0
@@ -1522,6 +1522,7 @@ static inline unsigned int mm_cid_size(void)struct mmu_gather ;extern void tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm);extern void tlb_gather_mmu_fullmm (struct mmu_gather *tlb,struct mm_struct *mm); +void tlb_gather_mmu_vma (struct mmu_gather *tlb,struct vm_area_struct *vma);extern void tlb_finish_mmu (struct mmu_gather *tlb);struct vm_fault ;
mm/hugetlb.c +72 -51
274 lines not on this page · raw message
mm/mmu_gather.c +33 -0
@@ -10,6 +10,7 @@#include <linux/swap.h> #include <linux/rmap.h> #include <linux/pgalloc.h> +#include <linux/hugetlb.h> #include <asm/tlb.h>
@@ -426,6 +427,7 @@ static void __tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,#endif tlb->vma_pfn =0 ; + tlb->fully_unshared_tables =0 ;__tlb_reset_range (tlb);inc_tlb_flush_pending (tlb->mm); }
@@ -459,6 +461,31 @@ void tlb_gather_mmu_fullmm(struct mmu_gather *tlb, struct mm_struct *mm)__tlb_gather_mmu (tlb, mm, true); } +/** +* tlb_gather_mmu - initialize an mmu_gather structure for operating on a single +* VMA +* @tlb: the mmu_gather structure to initialize +* @vma: the vm_area_struct +* +* Called to initialize an (on-stack) mmu_gather structure for operating on +* a single VMA. In contrast to tlb_gather_mmu(), calling this function will +* not require another call to tlb_start_vma(). In contrast to tlb_start_vma(), +* this function will *not* call flush_cache_range(). +* +* For hugetlb VMAs, this function will also initialize the mmu_gather +* page_size accordingly, not requiring a separate call to +* tlb_change_page_size(). +* +*/ +void tlb_gather_mmu_vma (struct mmu_gather *tlb,struct vm_area_struct *vma) +{ +tlb_gather_mmu (tlb, vma->vm_mm); +tlb_update_vma_flags (tlb, vma); +if (is_vm_hugetlb_page (vma)) +/* All entries have the same size. */ +tlb_change_page_size (tlb,huge_page_size (hstate_vma (vma))); +} +/** * tlb_finish_mmu - finish an mmu_gather structure * @tlb: the mmu_gather structure to finish
@@ -468,6 +495,12 @@ void tlb_gather_mmu_fullmm(struct mmu_gather *tlb, struct mm_struct *mm)*/ void tlb_finish_mmu (struct mmu_gather *tlb) { +/* +* We expect an earlier huge_pmd_unshare_flush() call to sort this out, +* due to complicated locking requirements with page table unsharing. +*/ +VM_WARN_ON_ONCE (tlb->fully_unshared_tables); +/* * If there are parallel threads are doing PTE changes on same range * under non-exclusive lock (e.g., mmap_lock read-side) but defer TLB
mm/rmap.c +17 -8
@@ -76,7 +76,7 @@#include <linux/mm_inline.h> #include <linux/oom.h> -#include <asm/tlbflush.h> +#include <asm/tlb.h> #define CREATE_TRACE_POINTS #include <trace/events/migrate.h>
@@ -2008,13 +2008,17 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,* if unsuccessful. */ if (!anon) { +struct mmu_gather tlb; +VM_BUG_ON (!(flags &TTU_RMAP_LOCKED ));if (!hugetlb_vma_trylock_write (vma)) goto walk_abort; -if (huge_pmd_unshare (mm, vma, address, pvmw.pte)) { + +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) {hugetlb_vma_unlock_write (vma); -flush_tlb_range (vma, - range.start, range.end); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb);/* * The PMD table was unmapped, * consequently unmapping the folio.
@@ -2022,6 +2026,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, goto walk_done; }hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb); } pteval =huge_ptep_clear_flush (vma, address, pvmw.pte);if (pte_dirty (pteval))
@@ -2398,17 +2403,20 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,* fail if unsuccessful. */ if (!anon) { +struct mmu_gather tlb; +VM_BUG_ON (!(flags &TTU_RMAP_LOCKED ));if (!hugetlb_vma_trylock_write (vma)) {page_vma_mapped_walk_done (&pvmw); ret = false;break ; } -if (huge_pmd_unshare (mm, vma, address, pvmw.pte)) { -hugetlb_vma_unlock_write (vma); -flush_tlb_range (vma, - range.start, range.end); +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { +hugetlb_vma_unlock_write (vma); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb);/* * The PMD table was unmapped, * consequently unmapping the folio.
@@ -2417,6 +2425,7 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,break ; }hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb); }/* Nuke the hugetlb page table entry */ pteval =huge_ptep_clear_flush (vma, address, pvmw.pte);
Signature · 1 line
2.52.0
David Hildenbrand (Red Hat)
maintainerRe: [PATCH RESEND v3 4/4] mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather
Quoted 131 lines · David Hildenbrand (Red Hat)
As reported, ever since commit
1013af4f585f("mm/hugetlb: fixhuge_pmd_unshare()vs GUP-fast race") we can end up in some situations where we perform so many IPI broadcasts when unsharing hugetlb PMD page tables that it severely regresses some workloads.In particular, when we fork()+exit(), or when we
munmap()a large area backed by many shared PMD tables, we perform one IPI broadcast per unshared PMD table.There are two optimizations to be had:
(1) When we process (unshare) multiple such PMD tables, such as during
exit(), it is sufficient to send a single IPI broadcast (as long as we respect locking rules) instead of one per PMD table.Locking prevents that any of these PMD tables could get reused before we drop the lock.
(2) When we are not the last sharer (> 2 users including us), there is no need to send the IPI broadcast. The shared PMD tables cannot become exclusive (fully unshared) before an IPI will be broadcasted by the last sharer.
Concurrent GUP-fast could walk into a PMD table just before we unshared it. It could then succeed in grabbing a page from the shared page table even after
munmap()etc succeeded (and supressed an IPI). But there is not difference compared to GUP-fast just sleeping for a while after grabbing the page and re-enabling IRQs.Most importantly, GUP-fast will never walk into page tables that are no-longer shared, because the last sharer will issue an IPI broadcast.
(if ever required, checking whether the PUD changed in GUP-fast
after grabbing the page like we do in the PTE case could handle this)So let's rework PMD sharing TLB flushing + IPI sync to use the
mmu_gatherinfrastructure so we can implement these optimizations and demystify the code at least a bit. Extend themmu_gatherinfrastructure to be able to deal with our special hugetlb PMD table sharing implementation.To make initialization of the
mmu_gathereasier when working on a single VMA (in particular, when dealing with hugetlb), providetlb_gather_mmu_vma().We'll consolidate the handling for (full) unsharing of PMD tables in
tlb_unshare_pmd_ptdesc()andtlb_flush_unshared_tables(), and track in "structmmu_gather" whether we had (full) unsharing of PMD tables.Because locking is very special (concurrent unsharing+reuse must be prevented), we disallow deferring flushing to
tlb_finish_mmu()and instead require an explicit earlier call totlb_flush_unshared_tables().From hugetlb code, we call
huge_pmd_unshare_flush()where we make sure
that the expected lock protecting us from concurrent unsharing+reuse is still held.Check with a
VM_WARN_ON_ONCE()intlb_finish_mmu()thattlb_flush_unshared_tables()was properly called earlier.Document it all properly.
Notes about
tlb_remove_table_sync_one()interaction with unsharing:There are two fairly tricky things:
(1)
tlb_remove_table_sync_one()is a NOP on architectures withoutCONFIG_MMU_GATHER_RCU_TABLE_FREE.Here, the assumption is that the previous TLB flush would send an IPI to all relevant CPUs. Careful: some architectures like x86 only send IPIs to all relevant CPUs when
tlb->freed_tablesis set.The relevant architectures should be selecting
MMU_GATHER_RCU_TABLE_FREE, but x86 might not do that in stable kernels and it might have been problematic before this patch.Also, the arch flushing behavior (independent of IPIs) is different when
tlb->freed_tablesis set. Do we have to enlighten them to also take care oftlb->unshared_tables? So far we didn't care, so hopefully we are fine. Of course, we could be settingtlb->freed_tablesas well, but that might then unnecessarily flush too much, because the semantics oftlb->freed_tablesare a bit fuzzy.This patch changes nothing in this regard.
(2)
tlb_remove_table_sync_one()is not a NOP on architectures withCONFIG_MMU_GATHER_RCU_TABLE_FREEthat actually don't need a sync.Take x86 as an example: in the common case (!pv, !X86_FEATURE_INVLPGB) we still issue IPIs during TLB flushes and don't actually need the second
tlb_remove_table_sync_one().This optimized can be implemented on top of this, by checking e.g., in
tlb_remove_table_sync_one()whether we really need IPIs. But as described in (1), it really must honortlb->freed_tablesthen to send IPIs to all relevant CPUs.Notes on TLB flushing changes:
(1) Flushing for non-shared PMD tables
We're converting from
flush_hugetlb_tlb_range()totlb_remove_huge_tlb_entry(). Given that we properly initialize the MMU gather intlb_gather_mmu_vma()to be hugetlb aware, similar to__unmap_hugepage_range(), that should be fine.(2) Flushing for shared PMD tables
We're converting from various things (
flush_hugetlb_tlb_range(),tlb_flush_pmd_range(),flush_tlb_range()) totlb_flush_pmd_range().
tlb_flush_pmd_range()achieves the same thattlb_remove_huge_tlb_entry()would achieve in these scenarios. Note thattlb_remove_huge_tlb_entry()also calls__tlb_remove_tlb_entry(), however that is only implemented on powerpc, which does not support PMD table sharing.Similar to (1),
tlb_gather_mmu_vma()should make sure that TLB flushing keeps on working as expected.Further, note that the
ptdesc_pmd_pts_dec()inhuge_pmd_share()is not a concern, as we are holding thei_mmap_lockthe whole time, preventing concurrent unsharing. Thatptdesc_pmd_pts_dec()usage will be removed separately as a cleanup later.There are plenty more cleanups to be had, but they have to wait until this is fixed.
Quoted 7 lines
Fixes 1013af4f585f ("mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race")Reported-by Uschakow, Stanislav" <[email protected]>Tested-by Laurence Oberman <[email protected]>Cc <[email protected]>Signed-off-by David Hildenbrand (Red Hat) <[email protected]>---
The following doc fixup on top, reported by buildbots on my private branch:
From 3556c4ce6b645f680be8040c8512beadb5f84d38 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Red Hat)" <[email protected]>
Date: Thu, 25 Dec 2025 10:41:55 +0100
Subject: [PATCH] fixup
--- mm/mmu_gather.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/mmu_gather.c +2 -2
@@ -462,8 +462,8 @@ void tlb_gather_mmu_fullmm(struct mmu_gather *tlb, struct mm_struct *mm) }/** -* tlb_gather_mmu - initialize an mmu_gather structure for operating on a single -* VMA +* tlb_gather_mmu_vma - initialize an mmu_gather structure for operating on a +* single VMA * @tlb: the mmu_gather structure to initialize * @vma: the vm_area_struct *
Signature · 7 lines
2.52.0 -- Cheers David
Re: [PATCH RESEND v3 4/4] mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather
In reply to David Hildenbrand (Red Hat)Quoted 131 lines · David Hildenbrand (Red Hat)
As reported, ever since commit
1013af4f585f("mm/hugetlb: fixhuge_pmd_unshare()vs GUP-fast race") we can end up in some situations where we perform so many IPI broadcasts when unsharing hugetlb PMD page tables that it severely regresses some workloads.In particular, when we fork()+exit(), or when we
munmap()a large area backed by many shared PMD tables, we perform one IPI broadcast per unshared PMD table.There are two optimizations to be had:
(1) When we process (unshare) multiple such PMD tables, such as during
exit(), it is sufficient to send a single IPI broadcast (as long as we respect locking rules) instead of one per PMD table.Locking prevents that any of these PMD tables could get reused before we drop the lock.
(2) When we are not the last sharer (> 2 users including us), there is no need to send the IPI broadcast. The shared PMD tables cannot become exclusive (fully unshared) before an IPI will be broadcasted by the last sharer.
Concurrent GUP-fast could walk into a PMD table just before we unshared it. It could then succeed in grabbing a page from the shared page table even after
munmap()etc succeeded (and supressed an IPI). But there is not difference compared to GUP-fast just sleeping for a while after grabbing the page and re-enabling IRQs.Most importantly, GUP-fast will never walk into page tables that are no-longer shared, because the last sharer will issue an IPI broadcast.
(if ever required, checking whether the PUD changed in GUP-fast
after grabbing the page like we do in the PTE case could handle this)So let's rework PMD sharing TLB flushing + IPI sync to use the
mmu_gatherinfrastructure so we can implement these optimizations and demystify the code at least a bit. Extend themmu_gatherinfrastructure to be able to deal with our special hugetlb PMD table sharing implementation.To make initialization of the
mmu_gathereasier when working on a single VMA (in particular, when dealing with hugetlb), providetlb_gather_mmu_vma().We'll consolidate the handling for (full) unsharing of PMD tables in
tlb_unshare_pmd_ptdesc()andtlb_flush_unshared_tables(), and track in "structmmu_gather" whether we had (full) unsharing of PMD tables.Because locking is very special (concurrent unsharing+reuse must be prevented), we disallow deferring flushing to
tlb_finish_mmu()and instead require an explicit earlier call totlb_flush_unshared_tables().From hugetlb code, we call
huge_pmd_unshare_flush()where we make sure that the expected lock protecting us from concurrent unsharing+reuse is still held.Check with a
VM_WARN_ON_ONCE()intlb_finish_mmu()thattlb_flush_unshared_tables()was properly called earlier.Document it all properly.
Notes about
tlb_remove_table_sync_one()interaction with unsharing:There are two fairly tricky things:
(1)
tlb_remove_table_sync_one()is a NOP on architectures withoutCONFIG_MMU_GATHER_RCU_TABLE_FREE.Here, the assumption is that the previous TLB flush would send an IPI to all relevant CPUs. Careful: some architectures like x86 only send IPIs to all relevant CPUs when
tlb->freed_tablesis set.The relevant architectures should be selecting
MMU_GATHER_RCU_TABLE_FREE, but x86 might not do that in stable kernels and it might have been problematic before this patch.Also, the arch flushing behavior (independent of IPIs) is different when
tlb->freed_tablesis set. Do we have to enlighten them to also take care oftlb->unshared_tables? So far we didn't care, so hopefully we are fine. Of course, we could be settingtlb->freed_tablesas well, but that might then unnecessarily flush too much, because the semantics oftlb->freed_tablesare a bit fuzzy.This patch changes nothing in this regard.
(2)
tlb_remove_table_sync_one()is not a NOP on architectures withCONFIG_MMU_GATHER_RCU_TABLE_FREEthat actually don't need a sync.Take x86 as an example: in the common case (!pv, !X86_FEATURE_INVLPGB) we still issue IPIs during TLB flushes and don't actually need the second
tlb_remove_table_sync_one().This optimized can be implemented on top of this, by checking e.g., in
tlb_remove_table_sync_one()whether we really need IPIs. But as described in (1), it really must honortlb->freed_tablesthen to send IPIs to all relevant CPUs.Notes on TLB flushing changes:
(1) Flushing for non-shared PMD tables
We're converting from
flush_hugetlb_tlb_range()totlb_remove_huge_tlb_entry(). Given that we properly initialize the MMU gather intlb_gather_mmu_vma()to be hugetlb aware, similar to__unmap_hugepage_range(), that should be fine.(2) Flushing for shared PMD tables
We're converting from various things (
flush_hugetlb_tlb_range(),tlb_flush_pmd_range(),flush_tlb_range()) totlb_flush_pmd_range().
tlb_flush_pmd_range()achieves the same thattlb_remove_huge_tlb_entry()would achieve in these scenarios. Note thattlb_remove_huge_tlb_entry()also calls__tlb_remove_tlb_entry(), however that is only implemented on powerpc, which does not support PMD table sharing.Similar to (1),
tlb_gather_mmu_vma()should make sure that TLB flushing keeps on working as expected.Further, note that the
ptdesc_pmd_pts_dec()inhuge_pmd_share()is not a concern, as we are holding thei_mmap_lockthe whole time, preventing concurrent unsharing. Thatptdesc_pmd_pts_dec()usage will be removed separately as a cleanup later.There are plenty more cleanups to be had, but they have to wait until this is fixed.
Quoted 7 lines
Fixes 1013af4f585f ("mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race")Reported-by Uschakow, Stanislav" <[email protected]>Tested-by Laurence Oberman <[email protected]>Cc <[email protected]>Signed-off-by David Hildenbrand (Red Hat) <[email protected]>---
Looks correct and nothing jumped out at me during review, so:
Signature · 2 lines
Cheers, Harry / Hyeonggon
David Hildenbrand (Red Hat)
maintainerRe: [PATCH RESEND v3 4/4] mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather
Quoted 9 lines
Quoted 8 lines
Fixes 1013af4f585f ("mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race")Reported-by Uschakow, Stanislav" <[email protected]>Tested-by Laurence Oberman <[email protected]>Cc <[email protected]>Signed-off-by David Hildenbrand (Red Hat) <[email protected]>---
Quoted 3 lines
Looks correct and nothing jumped out at me during review, so:
Acked-by Harry Yoo <[email protected]>
Thanks Harry! And thanks again for making me look again at the hugetlb mmu_gather horrors :)
Signature · 3 lines
Cheers David
Re: [PATCH RESEND v3 4/4] mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather
In reply to David Hildenbrand (Red Hat)Quoted 131 lines · David Hildenbrand (Red Hat)
As reported, ever since commit
1013af4f585f("mm/hugetlb: fixhuge_pmd_unshare()vs GUP-fast race") we can end up in some situations where we perform so many IPI broadcasts when unsharing hugetlb PMD page tables that it severely regresses some workloads.In particular, when we fork()+exit(), or when we
munmap()a large area backed by many shared PMD tables, we perform one IPI broadcast per unshared PMD table.There are two optimizations to be had:
(1) When we process (unshare) multiple such PMD tables, such as during
exit(), it is sufficient to send a single IPI broadcast (as long as we respect locking rules) instead of one per PMD table.Locking prevents that any of these PMD tables could get reused before we drop the lock.
(2) When we are not the last sharer (> 2 users including us), there is no need to send the IPI broadcast. The shared PMD tables cannot become exclusive (fully unshared) before an IPI will be broadcasted by the last sharer.
Concurrent GUP-fast could walk into a PMD table just before we unshared it. It could then succeed in grabbing a page from the shared page table even after
munmap()etc succeeded (and supressed an IPI). But there is not difference compared to GUP-fast just sleeping for a while after grabbing the page and re-enabling IRQs.Most importantly, GUP-fast will never walk into page tables that are no-longer shared, because the last sharer will issue an IPI broadcast.
(if ever required, checking whether the PUD changed in GUP-fast
after grabbing the page like we do in the PTE case could handle this)So let's rework PMD sharing TLB flushing + IPI sync to use the
mmu_gatherinfrastructure so we can implement these optimizations and demystify the code at least a bit. Extend themmu_gatherinfrastructure to be able to deal with our special hugetlb PMD table sharing implementation.To make initialization of the
mmu_gathereasier when working on a single VMA (in particular, when dealing with hugetlb), providetlb_gather_mmu_vma().We'll consolidate the handling for (full) unsharing of PMD tables in
tlb_unshare_pmd_ptdesc()andtlb_flush_unshared_tables(), and track in "structmmu_gather" whether we had (full) unsharing of PMD tables.Because locking is very special (concurrent unsharing+reuse must be prevented), we disallow deferring flushing to
tlb_finish_mmu()and instead require an explicit earlier call totlb_flush_unshared_tables().From hugetlb code, we call
huge_pmd_unshare_flush()where we make sure that the expected lock protecting us from concurrent unsharing+reuse is still held.Check with a
VM_WARN_ON_ONCE()intlb_finish_mmu()thattlb_flush_unshared_tables()was properly called earlier.Document it all properly.
Notes about
tlb_remove_table_sync_one()interaction with unsharing:There are two fairly tricky things:
(1)
tlb_remove_table_sync_one()is a NOP on architectures withoutCONFIG_MMU_GATHER_RCU_TABLE_FREE.Here, the assumption is that the previous TLB flush would send an IPI to all relevant CPUs. Careful: some architectures like x86 only send IPIs to all relevant CPUs when
tlb->freed_tablesis set.The relevant architectures should be selecting
MMU_GATHER_RCU_TABLE_FREE, but x86 might not do that in stable kernels and it might have been problematic before this patch.Also, the arch flushing behavior (independent of IPIs) is different when
tlb->freed_tablesis set. Do we have to enlighten them to also take care oftlb->unshared_tables? So far we didn't care, so hopefully we are fine. Of course, we could be settingtlb->freed_tablesas well, but that might then unnecessarily flush too much, because the semantics oftlb->freed_tablesare a bit fuzzy.This patch changes nothing in this regard.
(2)
tlb_remove_table_sync_one()is not a NOP on architectures withCONFIG_MMU_GATHER_RCU_TABLE_FREEthat actually don't need a sync.Take x86 as an example: in the common case (!pv, !X86_FEATURE_INVLPGB) we still issue IPIs during TLB flushes and don't actually need the second
tlb_remove_table_sync_one().This optimized can be implemented on top of this, by checking e.g., in
tlb_remove_table_sync_one()whether we really need IPIs. But as described in (1), it really must honortlb->freed_tablesthen to send IPIs to all relevant CPUs.Notes on TLB flushing changes:
(1) Flushing for non-shared PMD tables
We're converting from
flush_hugetlb_tlb_range()totlb_remove_huge_tlb_entry(). Given that we properly initialize the MMU gather intlb_gather_mmu_vma()to be hugetlb aware, similar to__unmap_hugepage_range(), that should be fine.(2) Flushing for shared PMD tables
We're converting from various things (
flush_hugetlb_tlb_range(),tlb_flush_pmd_range(),flush_tlb_range()) totlb_flush_pmd_range().
tlb_flush_pmd_range()achieves the same thattlb_remove_huge_tlb_entry()would achieve in these scenarios. Note thattlb_remove_huge_tlb_entry()also calls__tlb_remove_tlb_entry(), however that is only implemented on powerpc, which does not support PMD table sharing.Similar to (1),
tlb_gather_mmu_vma()should make sure that TLB flushing keeps on working as expected.Further, note that the
ptdesc_pmd_pts_dec()inhuge_pmd_share()is not a concern, as we are holding thei_mmap_lockthe whole time, preventing concurrent unsharing. Thatptdesc_pmd_pts_dec()usage will be removed separately as a cleanup later.There are plenty more cleanups to be had, but they have to wait until this is fixed.
Quoted 6 lines
Fixes 1013af4f585f ("mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race")Reported-by Uschakow, Stanislav" <[email protected]>Tested-by Laurence Oberman <[email protected]>Cc <[email protected]>Signed-off-by David Hildenbrand (Red Hat) <[email protected]>
OK with some local testing, ample use of git range-diff, this LGTM, hopefully no horrifying weird arch strangeness in some other corner lurking to bite us :P
So:
Quoted 615 lines
--- include/asm-generic/tlb.h | 77 +++++++++++++++++++++++- include/linux/hugetlb.h | 15 +++-- include/linux/mm_types.h | 1 + mm/hugetlb.c | 123 ++++++++++++++++++++++---------------- mm/mmu_gather.c | 33 ++++++++++ mm/rmap.c | 25 +++++--- 6 files changed, 208 insertions(+), 66 deletions(-)include/asm-generic/tlb.h +75
-2@@ -46,7 +46,8 @@* * The mmu_gather API consists of: * -* - tlb_gather_mmu() / tlb_gather_mmu_fullmm() / tlb_finish_mmu() +* - tlb_gather_mmu() / tlb_gather_mmu_fullmm() / tlb_gather_mmu_vma() / +* tlb_finish_mmu() * * start and finish a mmu_gather * @@ -364,6 +365,20 @@ struct mmu_gather { unsigned int vma_huge : 1; unsigned int vma_pfn : 1; +/* +* Did we unshare (unmap) any shared page tables? For now only +* used for hugetlb PMD table sharing. +*/ + unsigned int unshared_tables : 1; + +/* +* Did we unshare any page tables such that they are now exclusive +* and could get reused+modified by the new owner? When setting this +* flag, "unshared_tables" will be set as well. For now only used +* for hugetlb PMD table sharing. +*/ + unsigned int fully_unshared_tables : 1; +unsigned int batch_count;#ifndef CONFIG_MMU_GATHER_NO_GATHER @@ -400,6 +415,7 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb) tlb->cleared_pmds =0 ; tlb->cleared_puds =0 ; tlb->cleared_p4ds =0 ; + tlb->unshared_tables =0 ;/* * Do not reset mmu_gather::vma_* fields here, we do not * call into tlb_start_vma() again to set them if there is an @@ -484,7 +500,7 @@ static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)* these bits. */ if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds || - tlb->cleared_puds || tlb->cleared_p4ds)) + tlb->cleared_puds || tlb->cleared_p4ds || tlb->unshared_tables))return ;tlb_flush (tlb);@@ -773,6 +789,63 @@ static inline bool huge_pmd_needs_flush(pmd_t oldpmd, pmd_t newpmd) }#endif +#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING +static inline void tlb_unshare_pmd_ptdesc (struct mmu_gather *tlb,struct ptdesc *pt, +unsigned long addr) +{ +/* +* The caller must make sure that concurrent unsharing + exclusive +* reuse is impossible until tlb_flush_unshared_tables() was called. +*/ +VM_WARN_ON_ONCE (!ptdesc_pmd_is_shared (pt)); +ptdesc_pmd_pts_dec (pt); + +/* Clearing a PUD pointing at a PMD table with PMD leaves. */ +tlb_flush_pmd_range (tlb, addr &PUD_MASK ,PUD_SIZE ); + +/* +* If the page table is now exclusively owned, we fully unshared +* a page table. +*/ +if (!ptdesc_pmd_is_shared (pt)) + tlb->fully_unshared_tables = true; + tlb->unshared_tables = true; +} + +static inline void tlb_flush_unshared_tables (struct mmu_gather *tlb) +{ +/* +* As soon as the caller drops locks to allow for reuse of +* previously-shared tables, these tables could get modified and +* even reused outside of hugetlb context, so we have to make sure that +* any page table walkers (incl. TLB, GUP-fast) are aware of that +* change. +* +* Even if we are not fully unsharing a PMD table, we must +* flush the TLB for the unsharer now. +*/ +if (tlb->unshared_tables) +tlb_flush_mmu_tlbonly (tlb); + +/* +* Similarly, we must make sure that concurrent GUP-fast will not +* walk previously-shared page tables that are getting modified+reused +* elsewhere. So broadcast an IPI to wait for any concurrent GUP-fast. +* +* We only perform this when we are the last sharer of a page table, +* as the IPI will reach all CPUs: any GUP-fast. +* +* Note that on configs where tlb_remove_table_sync_one() is a NOP, +* the expectation is that the tlb_flush_mmu_tlbonly() would have issued +* required IPIs already for us. +*/ +if (tlb->fully_unshared_tables) { +tlb_remove_table_sync_one (); + tlb->fully_unshared_tables = false; + } +} +#endif /* CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING */ +#endif /* CONFIG_MMU */ #endif /* _ASM_GENERIC__TLB_H */ include/linux/hugetlb.h +10
-5@@ -240,8 +240,9 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,pte_t *huge_pte_offset (struct mm_struct *mm,unsigned long addr,unsigned long sz);unsigned long hugetlb_mask_last_page (struct hstate *h); -int huge_pmd_unshare (struct mm_struct *mm,struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep); +int huge_pmd_unshare (struct mmu_gather *tlb,struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep); +void huge_pmd_unshare_flush (struct mmu_gather *tlb,struct vm_area_struct *vma);void adjust_range_if_pmd_sharing_possible (struct vm_area_struct *vma,unsigned long *start,unsigned long *end);@@ -300,13 +301,17 @@ static inline struct address_space *hugetlb_folio_mapping_lock_write(return NULL ; } -static inline int huge_pmd_unshare (struct mm_struct *mm, -struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline int huge_pmd_unshare (struct mmu_gather *tlb, +struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {return 0 ; } +static inline void huge_pmd_unshare_flush (struct mmu_gather *tlb, +struct vm_area_struct *vma) +{ +} +static inline void adjust_range_if_pmd_sharing_possible (struct vm_area_struct *vma,unsigned long *start,unsigned long *end)include/linux/mm_types.h +1
-0@@ -1522,6 +1522,7 @@ static inline unsigned int mm_cid_size(void)struct mmu_gather ;extern void tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm);extern void tlb_gather_mmu_fullmm (struct mmu_gather *tlb,struct mm_struct *mm); +void tlb_gather_mmu_vma (struct mmu_gather *tlb,struct vm_area_struct *vma);extern void tlb_finish_mmu (struct mmu_gather *tlb);struct vm_fault ;mm/hugetlb.c +72
-51@@ -5096,7 +5096,7 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,unsigned long last_addr_mask;pte_t *src_pte, *dst_pte;struct mmu_notifier_range range; -bool shared_pmd = false; +struct mmu_gather tlb;mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , mm, old_addr, old_end);@@ -5106,6 +5106,7 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,* range. */ flush_cache_range (vma, range.start, range.end); +tlb_gather_mmu_vma (&tlb, vma);mmu_notifier_invalidate_range_start (&range); last_addr_mask =hugetlb_mask_last_page (h);@@ -5122,8 +5123,7 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,if (huge_pte_none (huge_ptep_get (mm, old_addr, src_pte)))continue ; -if (huge_pmd_unshare (mm, vma, old_addr, src_pte)) { - shared_pmd = true; +if (huge_pmd_unshare (&tlb, vma, old_addr, src_pte)) { old_addr |= last_addr_mask; new_addr |= last_addr_mask;continue ;@@ -5134,15 +5134,16 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,break ;move_huge_pte (vma, old_addr, new_addr, src_pte, dst_pte, sz); +tlb_remove_huge_tlb_entry (h, &tlb, src_pte, old_addr); } -if (shared_pmd) -flush_hugetlb_tlb_range (vma, range.start, range.end); -else -flush_hugetlb_tlb_range (vma, old_end - len, old_end); +tlb_flush_mmu_tlbonly (&tlb); +huge_pmd_unshare_flush (&tlb, vma); +mmu_notifier_invalidate_range_end (&range);i_mmap_unlock_write (mapping);hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb);return len + old_addr - old_end; }@@ -5161,7 +5162,6 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,unsigned long sz =huge_page_size (h);bool adjust_reservation;unsigned long last_addr_mask; -bool force_flush = false;WARN_ON (!is_vm_hugetlb_page (vma));BUG_ON (start & ~huge_page_mask (h));@@ -5184,10 +5184,8 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, } ptl =huge_pte_lock (h, mm, ptep); -if (huge_pmd_unshare (mm, vma, address, ptep)) { +if (huge_pmd_unshare (tlb, vma, address, ptep)) {spin_unlock (ptl); -tlb_flush_pmd_range (tlb, address &PUD_MASK ,PUD_SIZE ); - force_flush = true; address |= last_addr_mask;continue ; }@@ -5303,14 +5301,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, }tlb_end_vma (tlb, vma); -/* -* There is nothing protecting a previously-shared page table that we -* unshared through huge_pmd_unshare() from getting freed after we -* release i_mmap_rwsem, so flush the TLB now. If huge_pmd_unshare() -* succeeded, flush the range corresponding to the pud. -*/ -if (force_flush) -tlb_flush_mmu_tlbonly (tlb); +huge_pmd_unshare_flush (tlb, vma); } void __hugetlb_zap_begin(struct vm_area_struct *vma,@@ -6409,11 +6400,11 @@ long hugetlb_change_protection(struct vm_area_struct *vma,pte_t pte;struct hstate *h =hstate_vma (vma);long pages =0 , psize =huge_page_size (h); -bool shared_pmd = false;struct mmu_notifier_range range;unsigned long last_addr_mask;bool uffd_wp = cp_flags &MM_CP_UFFD_WP ;bool uffd_wp_resolve = cp_flags &MM_CP_UFFD_WP_RESOLVE ; +struct mmu_gather tlb;/* * In the case of shared PMDs, the area to flush could be beyond @@ -6426,6 +6417,7 @@ long hugetlb_change_protection(struct vm_area_struct *vma,BUG_ON (address >= end);flush_cache_range (vma, range.start, range.end); +tlb_gather_mmu_vma (&tlb, vma);mmu_notifier_invalidate_range_start (&range);hugetlb_vma_lock_write (vma);@@ -6452,7 +6444,7 @@ long hugetlb_change_protection(struct vm_area_struct *vma, } } ptl =huge_pte_lock (h, mm, ptep); -if (huge_pmd_unshare (mm, vma, address, ptep)) { +if (huge_pmd_unshare (&tlb, vma, address, ptep)) {/* * When uffd-wp is enabled on the vma, unshare * shouldn't happen at all. Warn about it if it @@ -6461,7 +6453,6 @@ long hugetlb_change_protection(struct vm_area_struct *vma,WARN_ON_ONCE (uffd_wp || uffd_wp_resolve); pages++;spin_unlock (ptl); - shared_pmd = true; address |= last_addr_mask;continue ; }@@ -6522,22 +6513,16 @@ long hugetlb_change_protection(struct vm_area_struct *vma, pte =huge_pte_clear_uffd_wp (pte);huge_ptep_modify_prot_commit (vma, address, ptep, old_pte, pte); pages++; +tlb_remove_huge_tlb_entry (h, &tlb, ptep, address); } next:spin_unlock (ptl);cond_resched (); } -/* -* There is nothing protecting a previously-shared page table that we -* unshared through huge_pmd_unshare() from getting freed after we -* release i_mmap_rwsem, so flush the TLB now. If huge_pmd_unshare() -* succeeded, flush the range corresponding to the pud. -*/ -if (shared_pmd) -flush_hugetlb_tlb_range (vma, range.start, range.end); -else -flush_hugetlb_tlb_range (vma, start, end); + +tlb_flush_mmu_tlbonly (&tlb); +huge_pmd_unshare_flush (&tlb, vma);/* * No need to call mmu_notifier_arch_invalidate_secondary_tlbs() we are * downgrading page table protection not changing it to point to a new @@ -6548,6 +6533,7 @@ long hugetlb_change_protection(struct vm_area_struct *vma,i_mmap_unlock_write (vma->vm_file->f_mapping);hugetlb_vma_unlock_write (vma);mmu_notifier_invalidate_range_end (&range); +tlb_finish_mmu (&tlb);return pages >0 ? (pages << h->order) : pages; }@@ -6904,18 +6890,27 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,return pte; } -/* -* unmap huge page backed by shared pte. +/** +* huge_pmd_unshare - Unmap a pmd table if it is shared by multiple users +* @tlb: the current mmu_gather. +* @vma: the vma covering the pmd table. +* @addr: the address we are trying to unshare. +* @ptep: pointer into the (pmd) page table. +* +* Called with the page table lock held, the i_mmap_rwsem held in write mode +* and the hugetlb vma lock held in write mode. * -* Called with page table lock held. +* Note: The caller must call huge_pmd_unshare_flush() before dropping the +* i_mmap_rwsem. * -* returns: 1 successfully unmapped a shared pte page -* 0 the underlying pte page is not shared, or it is the last user +* Returns: 1 if it was a shared PMD table and it got unmapped, or 0 if it +* was not a shared PMD table. */ -int huge_pmd_unshare (struct mm_struct *mm,struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +int huge_pmd_unshare (struct mmu_gather *tlb,struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) {unsigned long sz =huge_page_size (hstate_vma (vma)); +struct mm_struct *mm = vma->vm_mm;pgd_t *pgd =pgd_offset (mm, addr);p4d_t *p4d =p4d_offset (pgd, addr);pud_t *pud =pud_offset (p4d, addr);@@ -6927,18 +6922,36 @@ int huge_pmd_unshare(struct mm_struct *mm, struct vm_area_struct *vma,i_mmap_assert_write_locked (vma->vm_file->f_mapping);hugetlb_vma_assert_locked (vma);pud_clear (pud); -/* -* Once our caller drops the rmap lock, some other process might be -* using this page table as a normal, non-hugetlb page table. -* Wait for pending gup_fast() in other threads to finish before letting -* that happen. -*/ -tlb_remove_table_sync_one (); -ptdesc_pmd_pts_dec (virt_to_ptdesc (ptep)); + +tlb_unshare_pmd_ptdesc (tlb,virt_to_ptdesc (ptep), addr); +mm_dec_nr_pmds (mm); return 1; } +/* +* huge_pmd_unshare_flush - Complete a sequence of huge_pmd_unshare() calls +* @tlb: the current mmu_gather. +* @vma: the vma covering the pmd table. +* +* Perform necessary TLB flushes or IPI broadcasts to synchronize PMD table +* unsharing with concurrent page table walkers. +* +* This function must be called after a sequence of huge_pmd_unshare() +* calls while still holding the i_mmap_rwsem. +*/ +void huge_pmd_unshare_flush (struct mmu_gather *tlb,struct vm_area_struct *vma) +{ +/* +* We must synchronize page table unsharing such that nobody will +* try reusing a previously-shared page table while it might still +* be in use by previous sharers (TLB, GUP_fast). +*/ +i_mmap_assert_write_locked (vma->vm_file->f_mapping); + +tlb_flush_unshared_tables (tlb); +} +#else /* !CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING */ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,@@ -6947,12 +6960,16 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,return NULL ; } -int huge_pmd_unshare (struct mm_struct *mm,struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +int huge_pmd_unshare (struct mmu_gather *tlb,struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) {return 0 ; } +void huge_pmd_unshare_flush (struct mmu_gather *tlb,struct vm_area_struct *vma) +{ +} +void adjust_range_if_pmd_sharing_possible (struct vm_area_struct *vma,unsigned long *start,unsigned long *end) {@@ -7219,6 +7236,7 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,unsigned long sz =huge_page_size (h);struct mm_struct *mm = vma->vm_mm;struct mmu_notifier_range range; +struct mmu_gather tlb;unsigned long address;spinlock_t *ptl;pte_t *ptep;@@ -7230,6 +7248,8 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,return ;flush_cache_range (vma, start, end); +tlb_gather_mmu_vma (&tlb, vma); +/* * No need to call adjust_range_if_pmd_sharing_possible(), because * we have already done the PUD_SIZE alignment. @@ -7248,10 +7268,10 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,if (!ptep)continue ; ptl =huge_pte_lock (h, mm, ptep); -huge_pmd_unshare (mm, vma, address, ptep); +huge_pmd_unshare (&tlb, vma, address, ptep);spin_unlock (ptl); } -flush_hugetlb_tlb_range (vma, start, end); +huge_pmd_unshare_flush (&tlb, vma);if (take_locks) {i_mmap_unlock_write (vma->vm_file->f_mapping);hugetlb_vma_unlock_write (vma);@@ -7261,6 +7281,7 @@ static void hugetlb_unshare_pmds(struct vm_area_struct *vma,* Documentation/mm/mmu_notifier.rst. */ mmu_notifier_invalidate_range_end (&range); +tlb_finish_mmu (&tlb); }/* mm/mmu_gather.c +33
-0@@ -10,6 +10,7 @@#include <linux/swap.h> #include <linux/rmap.h> #include <linux/pgalloc.h> +#include <linux/hugetlb.h> #include <asm/tlb.h> @@ -426,6 +427,7 @@ static void __tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,#endif tlb->vma_pfn =0 ; + tlb->fully_unshared_tables =0 ;__tlb_reset_range (tlb);inc_tlb_flush_pending (tlb->mm); }@@ -459,6 +461,31 @@ void tlb_gather_mmu_fullmm(struct mmu_gather *tlb, struct mm_struct *mm)__tlb_gather_mmu (tlb, mm, true); } +/** +* tlb_gather_mmu - initialize an mmu_gather structure for operating on a single +* VMA +* @tlb: the mmu_gather structure to initialize +* @vma: the vm_area_struct +* +* Called to initialize an (on-stack) mmu_gather structure for operating on +* a single VMA. In contrast to tlb_gather_mmu(), calling this function will +* not require another call to tlb_start_vma(). In contrast to tlb_start_vma(), +* this function will *not* call flush_cache_range(). +* +* For hugetlb VMAs, this function will also initialize the mmu_gather +* page_size accordingly, not requiring a separate call to +* tlb_change_page_size(). +* +*/ +void tlb_gather_mmu_vma (struct mmu_gather *tlb,struct vm_area_struct *vma) +{ +tlb_gather_mmu (tlb, vma->vm_mm); +tlb_update_vma_flags (tlb, vma); +if (is_vm_hugetlb_page (vma)) +/* All entries have the same size. */ +tlb_change_page_size (tlb,huge_page_size (hstate_vma (vma))); +} +/** * tlb_finish_mmu - finish an mmu_gather structure * @tlb: the mmu_gather structure to finish @@ -468,6 +495,12 @@ void tlb_gather_mmu_fullmm(struct mmu_gather *tlb, struct mm_struct *mm)*/ void tlb_finish_mmu (struct mmu_gather *tlb) { +/* +* We expect an earlier huge_pmd_unshare_flush() call to sort this out, +* due to complicated locking requirements with page table unsharing. +*/ +VM_WARN_ON_ONCE (tlb->fully_unshared_tables); +/* * If there are parallel threads are doing PTE changes on same range * under non-exclusive lock (e.g., mmap_lock read-side) but defer TLB mm/rmap.c +17
-9@@ -76,7 +76,7 @@#include <linux/mm_inline.h> #include <linux/oom.h> -#include <asm/tlbflush.h> +#include <asm/tlb.h> #define CREATE_TRACE_POINTS #include <trace/events/migrate.h> @@ -2008,13 +2008,17 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,* if unsuccessful. */ if (!anon) { +struct mmu_gather tlb; +VM_BUG_ON (!(flags &TTU_RMAP_LOCKED ));if (!hugetlb_vma_trylock_write (vma)) goto walk_abort; -if (huge_pmd_unshare (mm, vma, address, pvmw.pte)) { + +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) {hugetlb_vma_unlock_write (vma); -flush_tlb_range (vma, - range.start, range.end); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb);/* * The PMD table was unmapped, * consequently unmapping the folio. @@ -2022,6 +2026,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, goto walk_done; }hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb); } pteval =huge_ptep_clear_flush (vma, address, pvmw.pte);if (pte_dirty (pteval))@@ -2398,17 +2403,20 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,* fail if unsuccessful. */ if (!anon) { +struct mmu_gather tlb; +VM_BUG_ON (!(flags &TTU_RMAP_LOCKED ));if (!hugetlb_vma_trylock_write (vma)) {page_vma_mapped_walk_done (&pvmw); ret = false;break ; } -if (huge_pmd_unshare (mm, vma, address, pvmw.pte)) { -hugetlb_vma_unlock_write (vma); -flush_tlb_range (vma, - range.start, range.end); +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { +hugetlb_vma_unlock_write (vma); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb);/* * The PMD table was unmapped, * consequently unmapping the folio. @@ -2417,6 +2425,7 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,break ; }hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb); }/* Nuke the hugetlb page table entry */ pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); --2.52.0
Quoted 99 lines · David Hildenbrand (Red Hat)
One functional fix, one performance regression fix, and two related comment fixes.
I cleaned up my prototype I recently shared [1] for the performance fix,
deferring most of the cleanups I had in the prototype to a later point.
While doing that I identified the other things.The goal of this patch set is to be backported to stable trees "fairly"
easily. At least patch #1 and #4.Patch #1 fixes
hugetlb_pmd_shared()not detecting any sharing Patch #2 + #3 are simple comment fixes that patch #4 interacts with. Patch #4 is a fix for the reported performance regression due to excessive
IPI broadcasts during fork()+exit().The last patch is all about TLB flushes, IPIs and
mmu_gather.
Read: complicatedI added as much comments + description that I possibly could, and I am
hoping for review from Jann.There are plenty of cleanups in the future to be had + one reasonable optimization on x86. But that's all out of scope for this series.
Compile tested on plenty of architectures.
Runtime tested, with a focus on fixing the performance regression using
the original reproducer [2] on x86.[1]
https://lore.kernel.org/all/[email protected]/
[2]
https://lore.kernel.org/all/[email protected]/--
v2 -> v3:
* Rebased to 6.19-rc2 and retested on x86
* Changes on last patch:
* Introduce and usetlb_gather_mmu_vma()for properly setting upmmu_gather
for hugetlb -- thanks to Harry for pointing me once again at the nasty
hugetlb integration inmmu_gather
* Movetlb_remove_huge_tlb_entry()aftermove_huge_pte()
* For consistency, always calltlb_gather_mmu_vma()afterflush_cache_range()
* Don't passmmu_gathertohugetlb_change_protection(), simply use a local one for now. (avoids messing withtlb_start_vma()/tlb_start_end())
* Dropped Lorenzo's RB due to the changesv1 -> v2:
* Picked RB's/ACK's, hopefully I didn't miss any
* Added the initialization offully_unshared_tablesin__tlb_gather_mmu()
(Thanks Nadav!)
* Refined some comments based on Lorenzo's feedback.Cc Will Deacon <[email protected]>Cc "Aneesh Kumar K.V" <[email protected]>Cc Andrew Morton <[email protected]>Cc Nick Piggin <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Muchun Song <[email protected]>Cc Oscar Salvador <[email protected]>Cc "Liam R. Howlett" <[email protected]>Cc Lorenzo Stoakes <[email protected]>Cc Vlastimil Babka <[email protected]>Cc Jann Horn <[email protected]>Cc Pedro Falcato <[email protected]>Cc Rik van Riel <[email protected]>Cc Harry Yoo <[email protected]>Cc Uschakow, Stanislav" <[email protected]>Cc Laurence Oberman <[email protected]>Cc Prakash Sangappa <[email protected]>Cc Nadav Amit <[email protected]>David Hildenbrand (Red Hat) (4):
mm/hugetlb: fixhugetlb_pmd_shared()
mm/hugetlb: fix two comments related tohuge_pmd_unshare()mm/rmap: fix two comments related tohuge_pmd_unshare()mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables usingmmu_gatherinclude/asm-generic/tlb.h | 77 +++++++++++++++++++++- include/linux/hugetlb.h | 17 +++-- include/linux/mm_types.h | 1 +
mm/hugetlb.c | 131 +++++++++++++++++++++--------------- --
mm/mmu_gather.c | 33 ++++++++++
mm/rmap.c | 45 ++++++-------
Quoted 4 lines
6 files changed, 213 insertions(+), 91 deletions(-)base-commit:
b927546677c876e26eba308550207c2ddf812a43
Hello David
For the V3 series, I re-ran the tests and the original reproducer and its clean. I see the same almost 6x improvement for the original reproducer
# uname -r 6.19.0-rc2-hugetlbv3+
Un-patched Result of reproducer Iteration completed in 3436 ms V3 Patched Result of reproducer Iteration completed in 639 ms
I also ran a test to map every hugepage I could access (460GB of them) then fill and validate and had no issues.
David Hildenbrand (Red Hat)
maintainerQuoted 113 lines · Laurence Oberman
Quoted 102 lines · David Hildenbrand (Red Hat)
One functional fix, one performance regression fix, and two related comment fixes.
I cleaned up my prototype I recently shared [1] for the performance fix,
deferring most of the cleanups I had in the prototype to a later point.
While doing that I identified the other things.The goal of this patch set is to be backported to stable trees "fairly"
easily. At least patch #1 and #4.Patch #1 fixes
hugetlb_pmd_shared()not detecting any sharing Patch #2 + #3 are simple comment fixes that patch #4 interacts with. Patch #4 is a fix for the reported performance regression due to excessive
IPI broadcasts during fork()+exit().The last patch is all about TLB flushes, IPIs and
mmu_gather.
Read: complicatedI added as much comments + description that I possibly could, and I am
hoping for review from Jann.There are plenty of cleanups in the future to be had + one reasonable optimization on x86. But that's all out of scope for this series.
Compile tested on plenty of architectures.
Runtime tested, with a focus on fixing the performance regression using
the original reproducer [2] on x86.[1]
https://lore.kernel.org/all/[email protected]/
[2]
https://lore.kernel.org/all/[email protected]/--
v2 -> v3:
* Rebased to 6.19-rc2 and retested on x86
* Changes on last patch:
* Introduce and usetlb_gather_mmu_vma()for properly setting upmmu_gather
for hugetlb -- thanks to Harry for pointing me once again at the
nasty
hugetlb integration inmmu_gather
* Movetlb_remove_huge_tlb_entry()aftermove_huge_pte()
* For consistency, always calltlb_gather_mmu_vma()afterflush_cache_range()
* Don't passmmu_gathertohugetlb_change_protection(), simply use a local one for now. (avoids messing withtlb_start_vma()/tlb_start_end())
* Dropped Lorenzo's RB due to the changesv1 -> v2:
* Picked RB's/ACK's, hopefully I didn't miss any
* Added the initialization offully_unshared_tablesin__tlb_gather_mmu()
(Thanks Nadav!)
* Refined some comments based on Lorenzo's feedback.Cc Will Deacon <[email protected]>Cc "Aneesh Kumar K.V" <[email protected]>Cc Andrew Morton <[email protected]>Cc Nick Piggin <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Muchun Song <[email protected]>Cc Oscar Salvador <[email protected]>Cc "Liam R. Howlett" <[email protected]>Cc Lorenzo Stoakes <[email protected]>Cc Vlastimil Babka <[email protected]>Cc Jann Horn <[email protected]>Cc Pedro Falcato <[email protected]>Cc Rik van Riel <[email protected]>Cc Harry Yoo <[email protected]>Cc Uschakow, Stanislav" <[email protected]>Cc Laurence Oberman <[email protected]>Cc Prakash Sangappa <[email protected]>Cc Nadav Amit <[email protected]>David Hildenbrand (Red Hat) (4):
mm/hugetlb: fixhugetlb_pmd_shared()
mm/hugetlb: fix two comments related tohuge_pmd_unshare()mm/rmap: fix two comments related tohuge_pmd_unshare()mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables usingmmu_gatherinclude/asm-generic/tlb.h | 77 +++++++++++++++++++++- include/linux/hugetlb.h | 17 +++-- include/linux/mm_types.h | 1 + mm/hugetlb.c | 131 +++++++++++++++++++++--------------- -- mm/mmu_gather.c | 33 ++++++++++ mm/rmap.c | 45 ++++++------- 6 files changed, 213 insertions(+), 91 deletions(-)Quoted 1 line
base-commit:
b927546677c876e26eba308550207c2ddf812a43Hello David
For the V3 series, I re-ran the tests and the original reproducer and its clean. I see the same almost 6x improvement for the original reproducer
# uname -r 6.19.0-rc2-hugetlbv3+
Quoted 7 lines
Un-patched Result of reproducer Iteration completed in 3436 ms V3 Patched Result of reproducer Iteration completed in 639 ms
I also ran a test to map every hugepage I could access (460GB of them) then fill and validate and had no issues.
Tested-by Laurence Oberman <[email protected]>
Thanks a lot for the quick retest Laurence! I'd love to get some generic hugetlb testing on arm64 and powerpc, that do hugetlb TLB flushing stuff a bit more special.
I'll try doing some arm64 testing early in the new year myself.
Signature · 3 lines
Cheers David
Hi,
Any update on this series? It's a hotfix series and I don't see it queued up anywhere in either mm-hotfixes-unstable or mm-hotfixes-stable, this issue is causing ongoing problems for a lot of people, is there any reason it's being delayed?
It's received extensive approval and testing, so should be GTG right?
Andrew, David?
Thanks, Lorenzo
Quoted 94 lines · David Hildenbrand (Red Hat)
One functional fix, one performance regression fix, and two related comment fixes.
I cleaned up my prototype I recently shared [1] for the performance fix, deferring most of the cleanups I had in the prototype to a later point. While doing that I identified the other things.
The goal of this patch set is to be backported to stable trees "fairly" easily. At least patch #1 and #4.
Patch #1 fixes
hugetlb_pmd_shared()not detecting any sharing Patch #2 + #3 are simple comment fixes that patch #4 interacts with. Patch #4 is a fix for the reported performance regression due to excessive IPI broadcasts during fork()+exit().The last patch is all about TLB flushes, IPIs and
mmu_gather.
Read: complicatedI added as much comments + description that I possibly could, and I am hoping for review from Jann.
There are plenty of cleanups in the future to be had + one reasonable optimization on x86. But that's all out of scope for this series.
Compile tested on plenty of architectures.
Runtime tested, with a focus on fixing the performance regression using the original reproducer [2] on x86.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/--
v2 -> v3:
* Rebased to 6.19-rc2 and retested on x86
* Changes on last patch:
* Introduce and usetlb_gather_mmu_vma()for properly setting upmmu_gatherfor hugetlb -- thanks to Harry for pointing me once again at the nasty hugetlb integration inmmu_gather
* Movetlb_remove_huge_tlb_entry()aftermove_huge_pte()
* For consistency, always calltlb_gather_mmu_vma()afterflush_cache_range()
* Don't passmmu_gathertohugetlb_change_protection(), simply use a local one for now. (avoids messing withtlb_start_vma()/tlb_start_end())
* Dropped Lorenzo's RB due to the changesv1 -> v2:
* Picked RB's/ACK's, hopefully I didn't miss any
* Added the initialization offully_unshared_tablesin__tlb_gather_mmu()(Thanks Nadav!)
* Refined some comments based on Lorenzo's feedback.Cc Will Deacon <[email protected]>Cc "Aneesh Kumar K.V" <[email protected]>Cc Andrew Morton <[email protected]>Cc Nick Piggin <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Muchun Song <[email protected]>Cc Oscar Salvador <[email protected]>Cc "Liam R. Howlett" <[email protected]>Cc Lorenzo Stoakes <[email protected]>Cc Vlastimil Babka <[email protected]>Cc Jann Horn <[email protected]>Cc Pedro Falcato <[email protected]>Cc Rik van Riel <[email protected]>Cc Harry Yoo <[email protected]>Cc Uschakow, Stanislav" <[email protected]>Cc Laurence Oberman <[email protected]>Cc Prakash Sangappa <[email protected]>Cc Nadav Amit <[email protected]>David Hildenbrand (Red Hat) (4):
mm/hugetlb: fixhugetlb_pmd_shared()
mm/hugetlb: fix two comments related tohuge_pmd_unshare()mm/rmap: fix two comments related tohuge_pmd_unshare()mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables
usingmmu_gatherinclude/asm-generic/tlb.h | 77 +++++++++++++++++++++- include/linux/hugetlb.h | 17 +++-- include/linux/mm_types.h | 1 + mm/hugetlb.c | 131 +++++++++++++++++++++----------------- mm/mmu_gather.c | 33 ++++++++++ mm/rmap.c | 45 ++++++------- 6 files changed, 213 insertions(+), 91 deletions(-)base-commit:
b927546677c876e26eba308550207c2ddf812a43--
2.52.0
Andrew Morton
maintainerQuoted 6 lines · On Thu, 15 Jan 2026 17:22:30 +0000 Lorenzo Stoakes…
Any update on this series? It's a hotfix series and I don't see it queued up anywhere in either mm-hotfixes-unstable or mm-hotfixes-stable, this issue is causing ongoing problems for a lot of people, is there any reason it's being delayed?
It's received extensive approval and testing, so should be GTG right?
This has been in mm-unstable for a long time. As the series had a mixture of cc:stable and not-cc:stable patches, I figured we'd merge them all into next merge window and let the -stable maintainers figure it all out.
As this is more urgent than I believed, we need to figure out what to do.
a) Pluck out the two cc:stable patches, merge just those into 6.19-rc.
b) Merge all of them into 6.19-rc, let -stable maintainers figure it out
c) Stick with my original plan.
<checks>
The cc:stable "mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using mmu_gather" has dependencies on the preceding two non-cc:stable patches.
So I'm thinking I add cc:stable to everything and send the whole series into 6.19-rcX. wdyt?
Also, David, where do we stand with
: I'd love to get some generic hugetlb testing on arm64 and powerpc, : that do hugetlb TLB flushing stuff a bit more special. :
: I'll try doing some arm64 testing early in the new year myself.
?
David Hildenbrand (Red Hat)
maintainerQuoted 14 lines · Andrew Morton
Quoted 6 lines · On Thu, 15 Jan 2026 17:22:30 +0000 Lorenzo Stoakes…
Any update on this series? It's a hotfix series and I don't see it queued up anywhere in either mm-hotfixes-unstable or mm-hotfixes-stable, this issue is causing ongoing problems for a lot of people, is there any reason it's being delayed?
It's received extensive approval and testing, so should be GTG right?
This has been in mm-unstable for a long time. As the series had a mixture of cc:stable and not-cc:stable patches, I figured we'd merge them all into next merge window and let the -stable maintainers figure it all out.
Quoted 6 lines
As this is more urgent than I believed, we need to figure out what to do.
a) Pluck out the two cc:stable patches, merge just those into 6.19-rc.
b) Merge all of them into 6.19-rc, let -stable maintainers figure it out
Right. We can just CC: stable on comment fixes #2 and #3 to make back-porting easier.
Quoted 8 lines
c) Stick with my original plan.
<checks>
The cc:stable "mm/hugetlb: fix excessive IPI broadcasts when unsharing PMD tables using
mmu_gather" has dependencies on the preceding two non-cc:stable patches.
At least one of them (doc update), yes.
Quoted 3 lines
So I'm thinking I add cc:stable to everything and send the whole series into 6.19-rcX. wdyt?
Works for me.
Quoted 10 lines
Also, David, where do we stand with
: I'd love to get some generic hugetlb testing on arm64 and powerpc, : that do hugetlb TLB flushing stuff a bit more special. :
: I'll try doing some arm64 testing early in the new year myself.?
Not done yet, unfortunately. (I don't (yet) have easy access to decent arm64 hardware ;) )
I still hope that Jann could quickly have a look, but it's been a while already since I posted v1.
Signature · 3 lines
Cheers David
Andrew Morton
maintainerQuoted 18 lines · On Thu, 15 Jan 2026 20:40:13 +0100 "David Hildenbrand (Red Hat)"…
Quoted 17 lines · Andrew Morton
Quoted 6 lines · On Thu, 15 Jan 2026 17:22:30 +0000 Lorenzo Stoakes…
Any update on this series? It's a hotfix series and I don't see it queued up anywhere in either mm-hotfixes-unstable or mm-hotfixes-stable, this issue is causing ongoing problems for a lot of people, is there any reason it's being delayed?
It's received extensive approval and testing, so should be GTG right?
This has been in mm-unstable for a long time. As the series had a mixture of cc:stable and not-cc:stable patches, I figured we'd merge them all into next merge window and let the -stable maintainers figure it all out.
As this is more urgent than I believed, we need to figure out what to do.
Quoted 6 lines
Quoted 3 lines
a) Pluck out the two cc:stable patches, merge just those into 6.19-rc.
b) Merge all of them into 6.19-rc, let -stable maintainers figure it out
Right. We can just CC: stable on comment fixes #2 and #3 to make back-porting easier.
Yep. Seems lame to be backporting comment fixes because real fixes were textually dependent. Also seems lame to retain known-wrong comments in stable kernels!
Quoted 9 lines
Quoted 9 lines
Also, David, where do we stand with
: I'd love to get some generic hugetlb testing on arm64 and powerpc, : that do hugetlb TLB flushing stuff a bit more special. :
: I'll try doing some arm64 testing early in the new year myself.
Quoted 7 lines
Quoted 1 line
?
Not done yet, unfortunately. (I don't (yet) have easy access to decent arm64 hardware ;) )
I still hope that Jann could quickly have a look, but it's been a while already since I posted v1.
OK. It may well have had the hoped-for testing in linux-next, only we didn't hear about it.