targeted TLB sync IPIs for lockless page table walkers
Subsystems (14)
Lance Yang
reviewerv4 cover targeted TLB sync IPIs for lockless page table walkers
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast and perf_get_page_size() set/clear the tracker around their walk; tlb_remove_table_sync_mm() uses it and replaces the previous broadcast in the free/unshare paths.
On x86, when the TLB flush path already sends IPIs (native without INVLPGB, or KVM), the extra sync IPI is redundant. We add a property on pv_mmu_ops so each backend can declare whether its flush_tlb_multi sends real IPIs; if so, tlb_remove_table_sync_mm() is a no-op. We also have tlb_flush() pass both freed_tables and unshared_tables so lazy-TLB CPUs get IPIs during hugetlb unshare.
David Hildenbrand did the initial implementation. I built on his work and relied on off-list discussions to push it further - thanks a lot David!
[1] https://lore.kernel.org/linux-mm/[email protected]/
v3 -> v4:
- Rework based on David's two-step direction and per-CPU idea:
1) Targeted IPIs: per-CPU variable when entering/leaving lockless page table walk; tlb_remove_table_sync_mm() IPIs only those CPUs.
2) On x86, pv_mmu_ops property set at init to skip the extra sync when flush_tlb_multi() already sends IPIs.
https://lore.kernel.org/linux-mm/[email protected]/
- https://lore.kernel.org/linux-mm/[email protected]/
v2 -> v3:
- Complete rewrite: use dynamic IPI tracking instead of static checks (per Dave Hansen, thanks!)
- Track IPIs via mmu_gather: native_flush_tlb_multi() sets flag when actually sending IPIs
- Motivation for skipping redundant IPIs explained by David:
https://lore.kernel.org/linux-mm/[email protected]/
- https://lore.kernel.org/linux-mm/[email protected]/
v1 -> v2:
- Fix cover letter encoding to resolve send-email issues. Apologies for any email flood caused by the failed send attempts :(
RFC -> v1:
- Use a callback function in pv_mmu_ops instead of comparing function pointers (per David)
- Embed the check directly in tlb_remove_table_sync_one() instead of requiring every caller to check explicitly (per David)
- Move tlb_table_flush_implies_ipi_broadcast() outside of CONFIG_MMU_GATHER_RCU_TABLE_FREE to fix build error on architectures that don't enable this config.
https://lore.kernel.org/oe-kbuild-all/[email protected]/
- https://lore.kernel.org/linux-mm/[email protected]/
Lance Yang (3):
mm: use targeted IPIs for TLB sync with lockless page table walkers
mm: switch callers to tlb_remove_table_sync_mm() x86/tlb: add architecture-specific TLB IPI optimization support
arch/x86/hyperv/mmu.c | 5 ++ arch/x86/include/asm/paravirt.h | 5 ++ arch/x86/include/asm/paravirt_types.h | 6 +++ arch/x86/include/asm/tlb.h | 20 +++++++- arch/x86/kernel/kvm.c | 6 +++ arch/x86/kernel/paravirt.c | 18 +++++++ arch/x86/kernel/smpboot.c | 1 + arch/x86/xen/mmu_pv.c | 2 + include/asm-generic/tlb.h | 28 +++++++++-- include/linux/mm.h | 34 +++++++++++++ kernel/events/core.c | 2 + mm/gup.c | 2 + mm/khugepaged.c | 2 +- mm/mmu_gather.c | 69 ++++++++++++++++++++++++--- 14 files changed, 187 insertions(+), 13 deletions(-)
Signature · 1 line
2.49.0
Lance Yang
reviewerPATCH v4 1/3 mm: use targeted IPIs for TLB sync with lockless page table walkers
From: Lance Yang <[email protected]>
Currently, tlb_remove_table_sync_one() broadcasts IPIs to all CPUs to wait for any concurrent lockless page table walkers (e.g., GUP-fast). This is inefficient on systems with many CPUs, especially for RT workloads[1].
This patch introduces a per-CPU tracking mechanism to record which CPUs are actively performing lockless page table walks for a specific mm_struct. When freeing/unsharing page tables, we can now send IPIs only to the CPUs that are actually walking that mm, instead of broadcasting to all CPUs.
In preparation for targeted IPIs; a follow-up will switch callers to tlb_remove_table_sync_mm().
Note that the tracking adds ~3% latency to GUP-fast, as measured on a 64-core system.
[1] https://lore.kernel.org/linux-mm/[email protected]/
--- include/asm-generic/tlb.h | 2 ++ include/linux/mm.h | 34 ++++++++++++++++++++++++++ kernel/events/core.c | 2 ++ mm/gup.c | 2 ++ mm/mmu_gather.c | 50 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+)
include/asm-generic/tlb.h +2 -0
@@ -250,6 +250,7 @@ static inline void tlb_remove_table(struct mmu_gather *tlb, void *table)#endif void tlb_remove_table_sync_one (void ); +void tlb_remove_table_sync_mm (struct mm_struct *mm);#else
@@ -258,6 +259,7 @@ void tlb_remove_table_sync_one(void);#endif static inline void tlb_remove_table_sync_one (void ) { } +static inline void tlb_remove_table_sync_mm (struct mm_struct *mm) { }#endif /* CONFIG_MMU_GATHER_RCU_TABLE_FREE */
include/linux/mm.h +34 -0
@@ -2995,6 +2995,40 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, pgoff_t *offset);int folio_add_pins (struct folio *folio,unsigned int pins); +/* +* Track CPUs doing lockless page table walks to avoid broadcast IPIs +* during TLB flushes. +*/ +DECLARE_PER_CPU(struct mm_struct *, active_lockless_pt_walk_mm); + +static inline void pt_walk_lockless_start (struct mm_struct *mm) +{ +lockdep_assert_irqs_disabled (); + +/* +* Tell other CPUs we're doing lockless page table walk. +* +* Full barrier needed to prevent page table reads from being +* reordered before this write. +* +* Pairs with smp_rmb() in tlb_remove_table_sync_mm(). +*/ +this_cpu_write (active_lockless_pt_walk_mm, mm); +smp_mb (); +} + +static inline void pt_walk_lockless_end (void ) +{ +lockdep_assert_irqs_disabled (); + +/* +* Clear the pointer so other CPUs no longer see this CPU as walking +* the mm. Use smp_store_release to ensure page table reads complete +* before the clear is visible to other CPUs. +*/ +smp_store_release (this_cpu_ptr (&active_lockless_pt_walk_mm),NULL ); +} +int get_user_pages_fast (unsigned long start,int nr_pages,unsigned int gup_flags,struct page **pages); int pin_user_pages_fast(unsigned long start, int nr_pages,
kernel/events/core.c +2 -0
@@ -8190,7 +8190,9 @@ static u64 perf_get_page_size(unsigned long addr) mm = &init_mm; } +pt_walk_lockless_start (mm); size =perf_get_pgtable_size (mm, addr); +pt_walk_lockless_end ();local_irq_restore (flags);
mm/gup.c +2 -0
@@ -3154,7 +3154,9 @@ static unsigned long gup_fast(unsigned long start, unsigned long end,* that come from callers of tlb_remove_table_sync_one(). */ local_irq_save (flags); +pt_walk_lockless_start (current->mm);gup_fast_pgd_range (start, end, gup_flags, pages, &nr_pinned); +pt_walk_lockless_end ();local_irq_restore (flags);/*
mm/mmu_gather.c +50 -0
56 lines not on this page · raw message
Signature · 1 line
2.49.0
Peter Zijlstra
maintainerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Quoted 11 lines · Lance Yang
From: Lance Yang <[email protected]>
Currently,
tlb_remove_table_sync_one()broadcasts IPIs to all CPUs to wait for any concurrent lockless page table walkers (e.g., GUP-fast). This is inefficient on systems with many CPUs, especially for RT workloads[1].This patch introduces a per-CPU tracking mechanism to record which CPUs are actively performing lockless page table walks for a specific
mm_struct. When freeing/unsharing page tables, we can now send IPIs only to the CPUs that are actually walking that mm, instead of broadcasting to all CPUs.
Quoted 5 lines
In preparation for targeted IPIs; a follow-up will switch callers to
tlb_remove_table_sync_mm().Note that the tracking adds ~3% latency to GUP-fast, as measured on a 64-core system.
What architecture, and that is acceptable?
Quoted 10 lines
include/linux/mm.h +10
-0+/* +* Track CPUs doing lockless page table walks to avoid broadcast IPIs +* during TLB flushes. +*/ +DECLARE_PER_CPU(struct mm_struct *, active_lockless_pt_walk_mm); + +static inline void pt_walk_lockless_start (struct mm_struct *mm) +{ +lockdep_assert_irqs_disabled (); +
include/linux/mm.h long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, · line 3017
Quoted 10 lines
include/linux/mm.h +10
-0+/* +* Tell other CPUs we're doing lockless page table walk. +* +* Full barrier needed to prevent page table reads from being +* reordered before this write. +* +* Pairs with smp_rmb() in tlb_remove_table_sync_mm(). +*/ +this_cpu_write (active_lockless_pt_walk_mm, mm); +smp_mb ();
One thing to try is something like:
xchg(this_cpu_ptr(&active_lockless_pt_walk_mm), mm);
That *might* be a little better on x86_64, on anything else you really don't want to use this_cpu_() ops when you *know* IRQs are already disabled.
Quoted 23 lines
include/linux/mm.h +14
-0+} + +static inline void pt_walk_lockless_end (void ) +{ +lockdep_assert_irqs_disabled (); + +/* +* Clear the pointer so other CPUs no longer see this CPU as walking +* the mm. Use smp_store_release to ensure page table reads complete +* before the clear is visible to other CPUs. +*/ +smp_store_release (this_cpu_ptr (&active_lockless_pt_walk_mm),NULL ); +} +int get_user_pages_fast (unsigned long start,int nr_pages,unsigned int gup_flags,struct page **pages); int pin_user_pages_fast(unsigned long start, int nr_pages,mm/mmu_gather.c +0
-0
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 289
Quoted 5 lines
mm/mmu_gather.c +2
-0@@ -285,6 +285,56 @@ void tlb_remove_table_sync_one(void)smp_call_function (tlb_remove_table_smp_sync,NULL ,1 ); } +DEFINE_PER_CPU(struct mm_struct *, active_lockless_pt_walk_mm); +EXPORT_PER_CPU_SYMBOL_GPL (active_lockless_pt_walk_mm);
Why the heck is this exported? Both users are firmly core code.
Quoted 19 lines
mm/mmu_gather.c +19
-0+/** +* tlb_remove_table_sync_mm - send IPIs to CPUs doing lockless page table +* walk for @mm +* +* @mm: target mm; only CPUs walking this mm get an IPI. +* +* Like tlb_remove_table_sync_one() but only targets CPUs in +* active_lockless_pt_walk_mm. +*/ +void tlb_remove_table_sync_mm (struct mm_struct *mm) +{ +cpumask_var_t target_cpus; +bool found_any = false; +int cpu; + +if (WARN_ONCE (!mm,"NULL mm in %s\n" , __func__)) { +tlb_remove_table_sync_one (); +return ; + }
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 319
Quoted 10 lines
mm/mmu_gather.c +10
-0+ +/* If we can't, fall back to broadcast. */ +if (!alloc_cpumask_var (&target_cpus,GFP_ATOMIC )) { +tlb_remove_table_sync_one (); +return ; + } + +cpumask_clear (target_cpus); + +/* Pairs with smp_mb() in pt_walk_lockless_start(). */
Pairs how? The start thing does something like:
[W] active_lockless_pt_walk_mm = mm
MB
[L] page-tables
So this is:
[L] page-tables
RMB
[L] active_lockless_pt_walk_mm
?
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 325
Quoted 6 lines
mm/mmu_gather.c +6
-0+smp_rmb (); + +/* Find CPUs doing lockless page table walks for this mm */ +for_each_online_cpu (cpu) { +if (per_cpu (active_lockless_pt_walk_mm, cpu) == mm) { +cpumask_set_cpu (cpu, target_cpus);
You really don't need this to be atomic.
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 333
Quoted 8 lines
mm/mmu_gather.c +8
-0+ found_any = true; + } + } + +/* Only send IPIs to CPUs actually doing lockless walks */ +if (found_any) +smp_call_function_many (target_cpus, tlb_remove_table_smp_sync, +NULL ,1 );
Coding style wants { } here. Also, isn't this what we have smp_call_function_many_cond() for?
Quoted 9 lines
mm/mmu_gather.c +3
-0+free_cpumask_var (target_cpus); +} +static void tlb_remove_table_rcu (struct rcu_head *head) {__tlb_remove_table_free (container_of(head ,struct mmu_table_batch ,rcu ));Signature · 1 line
2.49.0
Lance Yang
reviewerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Hi Peter,
Thanks for taking time to review!
Quoted 12 lines · Peter Zijlstra
Quoted 11 lines · Lance Yang
From: Lance Yang <[email protected]>
Currently,
tlb_remove_table_sync_one()broadcasts IPIs to all CPUs to wait for any concurrent lockless page table walkers (e.g., GUP-fast). This is inefficient on systems with many CPUs, especially for RT workloads[1].This patch introduces a per-CPU tracking mechanism to record which CPUs are actively performing lockless page table walks for a specific
mm_struct. When freeing/unsharing page tables, we can now send IPIs only to the CPUs that are actually walking that mm, instead of broadcasting to all CPUs.
Quoted 7 lines
Quoted 5 lines
In preparation for targeted IPIs; a follow-up will switch callers to
tlb_remove_table_sync_mm().Note that the tracking adds ~3% latency to GUP-fast, as measured on a 64-core system.
What architecture, and that is acceptable?
x86-64.
I ran ./gup_bench which spawns 60 threads, each doing 500k GUP-fast operations (pinning 8 pages per call) via the gup_test ioctl.
Results for pin pages:
- Before: avg 1.489s (10 runs)
- After: avg 1.533s (10 runs)
Given we avoid broadcast IPIs on large systems, I think this is a reasonable trade-off :)
Quoted 24 lines
Quoted 11 lines
include/linux/mm.h +10
-0+/* +* Track CPUs doing lockless page table walks to avoid broadcast IPIs +* during TLB flushes. +*/ +DECLARE_PER_CPU(struct mm_struct *, active_lockless_pt_walk_mm); + +static inline void pt_walk_lockless_start (struct mm_struct *mm) +{ +lockdep_assert_irqs_disabled (); +include/linux/mm.h long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, · line 3017
Quoted 10 lines
include/linux/mm.h +10
-0+/* +* Tell other CPUs we're doing lockless page table walk. +* +* Full barrier needed to prevent page table reads from being +* reordered before this write. +* +* Pairs with smp_rmb() in tlb_remove_table_sync_mm(). +*/ +this_cpu_write (active_lockless_pt_walk_mm, mm); +smp_mb ();One thing to try is something like:
Quoted 5 lines
xchg(this_cpu_ptr(&active_lockless_pt_walk_mm), mm);That *might* be a little better on
x86_64, on anything else you really don't want to usethis_cpu_()ops when you *know* IRQs are already disabled.
Ah, good to know that. Thanks!
IIUC, xchg() provides the full barrier we need ;)
Quoted 27 lines
Quoted 27 lines
include/linux/mm.h +14
-0+} + +static inline void pt_walk_lockless_end (void ) +{ +lockdep_assert_irqs_disabled (); + +/* +* Clear the pointer so other CPUs no longer see this CPU as walking +* the mm. Use smp_store_release to ensure page table reads complete +* before the clear is visible to other CPUs. +*/ +smp_store_release (this_cpu_ptr (&active_lockless_pt_walk_mm),NULL ); +} +int get_user_pages_fast (unsigned long start,int nr_pages,unsigned int gup_flags,struct page **pages); int pin_user_pages_fast(unsigned long start, int nr_pages,mm/mmu_gather.c +0
-0@@ -285,6 +285,56 @@ void tlb_remove_table_sync_one(void)smp_call_function (tlb_remove_table_smp_sync,NULL ,1 ); }
Quoted 4 lines
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 289
Quoted 2 lines
mm/mmu_gather.c +2
-0+DEFINE_PER_CPU(struct mm_struct *, active_lockless_pt_walk_mm); +EXPORT_PER_CPU_SYMBOL_GPL (active_lockless_pt_walk_mm);Why the heck is this exported? Both users are firmly core code.
OK. Will drop this export.
Quoted 37 lines
Quoted 20 lines
mm/mmu_gather.c +19
-0+/** +* tlb_remove_table_sync_mm - send IPIs to CPUs doing lockless page table +* walk for @mm +* +* @mm: target mm; only CPUs walking this mm get an IPI. +* +* Like tlb_remove_table_sync_one() but only targets CPUs in +* active_lockless_pt_walk_mm. +*/ +void tlb_remove_table_sync_mm (struct mm_struct *mm) +{ +cpumask_var_t target_cpus; +bool found_any = false; +int cpu; + +if (WARN_ONCE (!mm,"NULL mm in %s\n" , __func__)) { +tlb_remove_table_sync_one (); +return ; + }mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 319
Quoted 10 lines
mm/mmu_gather.c +10
-0+ +/* If we can't, fall back to broadcast. */ +if (!alloc_cpumask_var (&target_cpus,GFP_ATOMIC )) { +tlb_remove_table_sync_one (); +return ; + } + +cpumask_clear (target_cpus); + +/* Pairs with smp_mb() in pt_walk_lockless_start(). */ Pairs how? The start thing does something like:
[W]
active_lockless_pt_walk_mm= mm
MB
[L] page-tables
Quoted 7 lines
So this is:
[L] page-tables
RMB
[L]active_lockless_pt_walk_mm?
On the walker side (pt_walk_lockless_start):
[W] active_lockless_pt_walk_mm = mm
MB
[L] page-tables (walker reads page tables)
So the walker publishes "I'm walking this mm" before reading page tables.
On the sync side we don't read page-tables. We do:
RMB
[L] active_lockless_pt_walk_mm (we read the per-CPU pointer below)
We need to observe the walker's store of active_lockless_pt_walk_mm before we decide which CPUs to IPI.
So on the sync side we do smp_rmb(), then read active_lockless_pt_walk_mm.
That pairs with the full barrier in pt_walk_lockless_start().
Quoted 10 lines
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 325
Quoted 7 lines
mm/mmu_gather.c +6
-0+smp_rmb (); + +/* Find CPUs doing lockless page table walks for this mm */ +for_each_online_cpu (cpu) { +if (per_cpu (active_lockless_pt_walk_mm, cpu) == mm) { +cpumask_set_cpu (cpu, target_cpus);You really don't need this to be atomic.
Quoted 11 lines
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 333
Quoted 8 lines
mm/mmu_gather.c +8
-0+ found_any = true; + } + } + +/* Only send IPIs to CPUs actually doing lockless walks */ +if (found_any) +smp_call_function_many (target_cpus, tlb_remove_table_smp_sync, +NULL ,1 );Coding style wants { } here. Also, isn't this what we have
smp_call_function_many_cond()for?
Right! That would be better, something like:
static bool tlb_remove_table_sync_mm_cond(int cpu, void *mm)
{
return per_cpu(active_lockless_pt_walk_mm, cpu) == (struct mm_struct *)mm;
}on_each_cpu_cond_mask(tlb_remove_table_sync_mm_cond,
tlb_remove_table_smp_sync, (void *)mm, true, cpu_online_mask);
Quoted 10 lines
Quoted 10 lines
mm/mmu_gather.c +3
-0+free_cpumask_var (target_cpus); +} +static void tlb_remove_table_rcu (struct rcu_head *head) {__tlb_remove_table_free (container_of(head ,struct mmu_table_batch ,rcu ));Signature · 1 line
2.49.0
Thanks,
Lance
Peter Zijlstra
maintainerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Quoted 29 lines · Lance Yang
Quoted 8 lines
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 319
Quoted 1 line
mm/mmu_gather.c +1
-0+/* Pairs with smp_mb() in pt_walk_lockless_start(). */ Pairs how? The start thing does something like:
[W]
active_lockless_pt_walk_mm= mm
MB
[L] page-tablesQuoted 7 lines
So this is:
[L] page-tables
RMB
[L]active_lockless_pt_walk_mm?
On the walker side (
pt_walk_lockless_start):[W]
active_lockless_pt_walk_mm= mm
MB
[L] page-tables (walker reads page tables)So the walker publishes "I'm walking this mm" before reading page tables.
On the sync side we don't read page-tables. We do:
RMB
[L]active_lockless_pt_walk_mm(we read the per-CPU pointer below)
Quoted 6 lines
We need to observe the walker's store of
active_lockless_pt_walk_mmbefore we decide which CPUs to IPI.So on the sync side we do
smp_rmb(), then readactive_lockless_pt_walk_mm.That pairs with the full barrier in
pt_walk_lockless_start().
No it doesn't; this is not how memory barriers work.
Lance Yang
reviewerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Quoted 31 lines · Peter Zijlstra
Quoted 29 lines · Lance Yang
Quoted 8 lines
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 319
Quoted 1 line
mm/mmu_gather.c +1
-0+/* Pairs with smp_mb() in pt_walk_lockless_start(). */ Pairs how? The start thing does something like:
[W]
active_lockless_pt_walk_mm= mm
MB
[L] page-tablesQuoted 7 lines
So this is:
[L] page-tables
RMB
[L]active_lockless_pt_walk_mm?
On the walker side (
pt_walk_lockless_start):[W]
active_lockless_pt_walk_mm= mm
MB
[L] page-tables (walker reads page tables)So the walker publishes "I'm walking this mm" before reading page tables.
On the sync side we don't read page-tables. We do:
RMB
[L]active_lockless_pt_walk_mm(we read the per-CPU pointer below)
Quoted 8 lines
Quoted 6 lines
We need to observe the walker's store of
active_lockless_pt_walk_mmbefore we decide which CPUs to IPI.So on the sync side we do
smp_rmb(), then readactive_lockless_pt_walk_mm.That pairs with the full barrier in
pt_walk_lockless_start().No it doesn't; this is not how memory barriers work.
Hmm... we need MB rather than RMB on the sync side. Is that correct?
Walker:
[W]active_lockless_pt_walk_mm = mm -> MB -> [L]page-tables
Sync:
[W]page-tables -> MB -> [L]active_lockless_pt_walk_mm
Thanks,
Lance
Peter Zijlstra
maintainerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Quoted 8 lines · Lance Yang
Hmm... we need MB rather than RMB on the sync side. Is that correct?
Walker:
[W]active_lockless_pt_walk_mm = mm -> MB -> [L]page-tablesSync:
[W]page-tables -> MB -> [L]active_lockless_pt_walk_mm
This can work -- but only if the walker and sync touch the same page-table address.
Now, typically I would imagine they both share the p4d/pud address at the very least, right?
Lance Yang
reviewerRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
Quoted 11 lines · Peter Zijlstra
Quoted 8 lines · Lance Yang
Hmm... we need MB rather than RMB on the sync side. Is that correct?
Walker:
[W]active_lockless_pt_walk_mm = mm -> MB -> [L]page-tablesSync:
[W]page-tables -> MB -> [L]active_lockless_pt_walk_mm
Quoted 5 lines
This can work -- but only if the walker and sync touch the same page-table address.
Now, typically I would imagine they both share the p4d/pud address at the very least, right?
Thanks. I think I see the confusion ...
To be clear, the goal is not to make the walker see page-table writes through the MB pairing, but to wait for any concurrent lockless page table walkers to finish.
The flow is:
1) Page tables are modified
2) TLB flush is done
3) Read active_lockless_pt_walk_mm (with MB to order page-table writes before this read) to find which CPUs are locklessly walking this mm
4) IPI those CPUs
5) The IPI forces them to sync, so after the IPI returns, any in-flight lockless page table walk has finished (or will restart and see the new page
tables)
The synchronization relies on the IPI to ensure walkers stop before continuing.
I would assume the TLB flush (step 2) should imply some barrier.
Does that clarify?
Re: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
In reply to Lance YangQuoted 10 lines · Lance Yang
Quoted 4 lines
Quoted 2 lines
Note that the tracking adds ~3% latency to GUP-fast, as measured on a 64-core system.
What architecture, and that is acceptable?
x86-64.
I ran ./gup_bench which spawns 60 threads, each doing 500k GUP-fast operations (pinning 8 pages per call) via the
gup_testioctl.
Quoted 6 lines
Results for pin pages:
- Before: avg 1.489s (10 runs)
- After: avg 1.533s (10 runs)Given we avoid broadcast IPIs on large systems, I think this is a reasonable trade-off 🙂
I thought the big databases were really sensitive to GUP-fast latency. They like big systems, too. Won't they howl when this finally hits their testing?
Also, two of the "write" side here are:
* collapse_huge_page() (khugepaged)
* tlb_remove_table() (in an "-ENOMEM" path)
Those are quite slow paths, right? Shouldn't the design here favor keeping gup-fast as fast as possible as opposed to impacting those?
kernel test robot
build botRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
In reply to Lance Yangkernel test robot noticed the following build errors:
Hi Lance,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on tip/x86/core arnd-asm-generic/master linus/master v6.19-rc8 next-20260130] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Lance-Yang/mm-use-targeted-IPIs-for-TLB-sync-with-lockless-page-table-walkers/20260202-154938 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20260202074557.16544-2-lance.yang%40linux.dev patch subject: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20260202/[email protected]/config) compiler: riscv64-linux-gcc (GCC) 15.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260202/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
riscv64-linux-ld: mm/gup.o: in function `get_user_pages_unlocked':
>> gup.c:(.text+0x2900): undefined reference to `active_lockless_pt_walk_mm' >> riscv64-linux-ld: gup.c:(.text+0x2922): undefined reference to `active_lockless_pt_walk_mm'
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers
In reply to Lance Yangkernel test robot noticed the following build errors:
Hi Lance,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on tip/x86/core arnd-asm-generic/master linus/master v6.19-rc8] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Lance-Yang/mm-use-targeted-IPIs-for-TLB-sync-with-lockless-page-table-walkers/20260202-154938 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20260202074557.16544-2-lance.yang%40linux.dev patch subject: [PATCH v4 1/3] mm: use targeted IPIs for TLB sync with lockless page table walkers config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20260204/[email protected]/config) compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260204/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: kernel/events/core.o: in function `pt_walk_lockless_end':
>> core.c:(.text.pt_walk_lockless_end+0x368): undefined reference to `active_lockless_pt_walk_mm' arm-linux-gnueabi-ld: kernel/events/core.o: in function `pt_walk_lockless_start': >> core.c:(.text.pt_walk_lockless_start+0x370): undefined reference to `active_lockless_pt_walk_mm'
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Lance Yang
reviewerPATCH v4 2/3 mm: switch callers to tlb_remove_table_sync_mm()
In reply to Lance YangFrom: Lance Yang <[email protected]>
Now that we have tlb_remove_table_sync_mm(), convert callers from tlb_remove_table_sync_one() to enable targeted IPIs instead of broadcast.
Three callers updated:
1) collapse_huge_page() - after flushing the old PMD, only IPIs CPUs walking this mm instead of all CPUs.
2) tlb_flush_unshared_tables() - when unsharing hugetlb page tables, use tlb->mm for targeted IPIs.
3) __tlb_remove_table_one() - updated to take mmu_gather parameter so it can use tlb->mm when batch allocation fails.
Note that pmdp_get_lockless_sync() (PAE only) also calls tlb_remove_table_sync_one() under PTL to ensure all ongoing PMD split-reads complete between pmdp_get_lockless_{start,end}; the critical section is very short. I'm inclined not to convert it since PAE systems typically don't have many cores.
--- include/asm-generic/tlb.h | 11 ++++++----- mm/khugepaged.c | 2 +- mm/mmu_gather.c | 12 ++++++------ 3 files changed, 13 insertions(+), 12 deletions(-)
include/asm-generic/tlb.h +6 -5
@@ -831,17 +831,18 @@ static inline void tlb_flush_unshared_tables(struct mmu_gather *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. +* elsewhere. So send 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. +* We only perform this when we are the last sharer of a page table. +* Use targeted IPI to CPUs actively walking this mm instead of +* broadcast. * -* Note that on configs where tlb_remove_table_sync_one() is a NOP, +* Note that on configs where tlb_remove_table_sync_mm() 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_remove_table_sync_mm (tlb->mm); tlb->fully_unshared_tables = false; } }
mm/khugepaged.c +1 -1
@@ -1173,7 +1173,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a _pmd =pmdp_collapse_flush (vma, address, pmd);spin_unlock (pmd_ptl);mmu_notifier_invalidate_range_end (&range); -tlb_remove_table_sync_one (); +tlb_remove_table_sync_mm (mm); pte =pte_offset_map_lock (mm, &_pmd, address, &pte_ptl);if (pte) {
mm/mmu_gather.c +6 -6
@@ -378,7 +378,7 @@ static inline void __tlb_remove_table_one_rcu(struct rcu_head *head)__tlb_remove_table (ptdesc); } -static inline void __tlb_remove_table_one (void *table) +static inline void __tlb_remove_table_one (struct mmu_gather *tlb,void *table) {struct ptdesc *ptdesc;
@@ -386,16 +386,16 @@ static inline void __tlb_remove_table_one(void *table) call_rcu(&ptdesc->pt_rcu_head, __tlb_remove_table_one_rcu); }#else -static inline void __tlb_remove_table_one (void *table) +static inline void __tlb_remove_table_one (struct mmu_gather *tlb,void *table) { -tlb_remove_table_sync_one (); +tlb_remove_table_sync_mm (tlb->mm);__tlb_remove_table (table); }#endif /* CONFIG_PT_RECLAIM */ -static void tlb_remove_table_one (void *table) +static void tlb_remove_table_one (struct mmu_gather *tlb,void *table) { -__tlb_remove_table_one (table); +__tlb_remove_table_one (tlb, table); }static void tlb_table_flush (struct mmu_gather *tlb)
@@ -417,7 +417,7 @@ void tlb_remove_table(struct mmu_gather *tlb, void *table) *batch = (struct mmu_table_batch *)__get_free_page (GFP_NOWAIT );if (*batch ==NULL ) {tlb_table_invalidate (tlb); -tlb_remove_table_one (table); +tlb_remove_table_one (tlb, table);return ; } (*batch)->nr =0 ;
Signature · 1 line
2.49.0
Lance Yang
reviewerPATCH v4 3/3 x86/tlb: add architecture-specific TLB IPI optimization support
In reply to Lance YangFrom: Lance Yang <[email protected]>
When the TLB flush path already sends IPIs (e.g. native without INVLPGB, or KVM), tlb_remove_table_sync_mm() does not need to send another round.
Add a property on pv_mmu_ops so each paravirt backend can indicate whether its flush_tlb_multi sends real IPIs; if so, tlb_remove_table_sync_mm() is a no-op.
Native sets it in native_pv_tlb_init() when still using native_flush_tlb_multi() and INVLPGB is disabled. KVM sets it true; Xen and Hyper-V set it false because they use hypercalls.
Also pass both freed_tables and unshared_tables from tlb_flush() into flush_tlb_mm_range() so lazy-TLB CPUs get IPIs during hugetlb unshare.
--- arch/x86/hyperv/mmu.c | 5 +++++ arch/x86/include/asm/paravirt.h | 5 +++++ arch/x86/include/asm/paravirt_types.h | 6 ++++++ arch/x86/include/asm/tlb.h | 20 +++++++++++++++++++- arch/x86/kernel/kvm.c | 6 ++++++ arch/x86/kernel/paravirt.c | 18 ++++++++++++++++++ arch/x86/kernel/smpboot.c | 1 + arch/x86/xen/mmu_pv.c | 2 ++ include/asm-generic/tlb.h | 15 +++++++++++++++ mm/mmu_gather.c | 7 +++++++ 10 files changed, 84 insertions(+), 1 deletion(-)
arch/x86/hyperv/mmu.c +5 -0
@@ -243,4 +243,9 @@ void hyperv_setup_mmu_ops(void)pr_info ("Using hypercall for remote TLB flush\n" ); pv_ops.mmu.flush_tlb_multi = hyperv_flush_tlb_multi; +/* +* Hyper-V uses hypercalls for TLB flush, not real IPIs. +* Keep the property as false. +*/ + pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast = false; }
arch/x86/include/asm/paravirt.h +5 -0
@@ -698,6 +698,7 @@ static __always_inline unsigned long arch_local_irq_save(void)extern void default_banner (void );void native_pv_lock_init (void )__init ; +void native_pv_tlb_init (void )__init ;#else /* __ASSEMBLER__ */
@@ -727,6 +728,10 @@ void native_pv_lock_init(void) __init;static inline void native_pv_lock_init (void ) { } + +static inline void native_pv_tlb_init (void ) +{ +}#endif #endif /* !CONFIG_PARAVIRT */
arch/x86/include/asm/paravirt_types.h +6 -0
@@ -133,6 +133,12 @@ struct pv_mmu_ops {void (*flush_tlb_multi)(const struct cpumask *cpus,const struct flush_tlb_info *info); +/* +* Indicates whether flush_tlb_multi IPIs provide sufficient +* synchronization during TLB flush when freeing or unsharing page tables. +*/ +bool flush_tlb_multi_implies_ipi_broadcast; +/* Hook for intercepting the destruction of an mm_struct. */ void (*exit_mmap)(struct mm_struct *mm);void (*notify_page_enc_status_changed)(unsigned long pfn,int npages,bool enc);
arch/x86/include/asm/tlb.h +19 -1
@@ -5,10 +5,23 @@#define tlb_flush tlb_flushstatic inline void tlb_flush (struct mmu_gather *tlb); +#define tlb_table_flush_implies_ipi_broadcast tlb_table_flush_implies_ipi_broadcast +static inline bool tlb_table_flush_implies_ipi_broadcast (void ); +#include <asm-generic/tlb.h> #include <linux/kernel.h> #include <vdso/bits.h> #include <vdso/page.h> +#include <asm/paravirt.h> + +static inline bool tlb_table_flush_implies_ipi_broadcast (void ) +{ +#ifdef CONFIG_PARAVIRT +return pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast; +#else +return !cpu_feature_enabled (X86_FEATURE_INVLPGB ); +#endif +}static inline void tlb_flush (struct mmu_gather *tlb) {
@@ -20,7 +33,12 @@ static inline void tlb_flush(struct mmu_gather *tlb) end = tlb->end; } -flush_tlb_mm_range (tlb->mm, start, end, stride_shift, tlb->freed_tables); +/* +* During TLB flushes, pass both freed_tables and unshared_tables +* so lazy-TLB CPUs receive IPIs. +*/ +flush_tlb_mm_range (tlb->mm, start, end, stride_shift, + tlb->freed_tables || tlb->unshared_tables); }static inline void invlpg (unsigned long addr)
arch/x86/kernel/kvm.c +6 -0
@@ -856,6 +856,12 @@ static void __init kvm_guest_init(void)#ifdef CONFIG_SMP if (pv_tlb_flush_supported ()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; +/* +* KVM's flush implementation calls native_flush_tlb_multi(), +* which sends real IPIs when INVLPGB is not available. +*/ +if (!cpu_feature_enabled (X86_FEATURE_INVLPGB )) + pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast = true;pr_info ("KVM setup pv remote TLB flush\n" ); }
arch/x86/kernel/paravirt.c +18 -0
@@ -60,6 +60,23 @@ void __init native_pv_lock_init(void)static_branch_enable (&virt_spin_lock_key); } +void __init native_pv_tlb_init (void ) +{ +/* +* Check if we're still using native TLB flush (not overridden by +* a PV backend) and don't have INVLPGB support. +* +* In this case, native IPI-based TLB flush provides sufficient +* synchronization for GUP-fast. +* +* PV backends (KVM, Xen, HyperV) should set this property in their +* own initialization code if their flush implementation sends IPIs. +*/ +if (pv_ops.mmu.flush_tlb_multi == native_flush_tlb_multi && + !cpu_feature_enabled (X86_FEATURE_INVLPGB )) + pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast = true; +} +struct static_key paravirt_steal_enabled;struct static_key paravirt_steal_rq_enabled;
@@ -173,6 +190,7 @@ struct paravirt_patch_template pv_ops = { .mmu.flush_tlb_kernel = native_flush_tlb_global, .mmu.flush_tlb_one_user = native_flush_tlb_one_user, .mmu.flush_tlb_multi = native_flush_tlb_multi, + .mmu.flush_tlb_multi_implies_ipi_broadcast = false, .mmu.exit_mmap = paravirt_nop, .mmu.notify_page_enc_status_changed = paravirt_nop,
arch/x86/kernel/smpboot.c +1 -0
@@ -1167,6 +1167,7 @@ void __init native_smp_prepare_boot_cpu(void)switch_gdt_and_percpu_base (me);native_pv_lock_init (); +native_pv_tlb_init (); }void __init native_smp_cpus_done (unsigned int max_cpus)
arch/x86/xen/mmu_pv.c +2 -0
@@ -2185,6 +2185,8 @@ static const typeof(pv_ops) xen_mmu_ops __initconst = { .flush_tlb_kernel = xen_flush_tlb, .flush_tlb_one_user = xen_flush_tlb_one_user, .flush_tlb_multi = xen_flush_tlb_multi, +/* Xen uses hypercalls for TLB flush, not real IPIs */ + .flush_tlb_multi_implies_ipi_broadcast = false, .pgd_alloc = xen_pgd_alloc, .pgd_free = xen_pgd_free,
include/asm-generic/tlb.h +15 -0
@@ -240,6 +240,21 @@ static inline void tlb_remove_table(struct mmu_gather *tlb, void *table) }#endif /* CONFIG_MMU_GATHER_TABLE_FREE */ +/* +* Architectures can override this to indicate whether TLB flush operations +* send IPIs that are sufficient to synchronize with lockless page table +* walkers (e.g., GUP-fast). If true, tlb_remove_table_sync_mm() becomes +* a no-op as the TLB flush already provided the necessary IPI. +* +* Default is false, meaning we need explicit IPIs via tlb_remove_table_sync_mm(). +*/ +#ifndef tlb_table_flush_implies_ipi_broadcast +static inline bool tlb_table_flush_implies_ipi_broadcast (void ) +{ +return false; +} +#endif +#ifdef CONFIG_MMU_GATHER_RCU_TABLE_FREE /* * This allows an architecture that does not use the linux page-tables for
mm/mmu_gather.c +7 -0
@@ -303,6 +303,13 @@ void tlb_remove_table_sync_mm(struct mm_struct *mm)bool found_any = false;int cpu; +/* +* If the architecture's TLB flush already sent IPIs that are sufficient +* for synchronization, we don't need to send additional IPIs. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +if (WARN_ONCE (!mm,"NULL mm in %s\n" , __func__)) {tlb_remove_table_sync_one ();return ;
Signature · 1 line
2.49.0
Sean Christopherson
reviewerRe: [PATCH v4 3/3] x86/tlb: add architecture-specific TLB IPI optimization support
arch/x86/kernel/kvm.c static void __init kvm_guest_init(void) · line 861
Quoted 11 lines · Lance Yang
arch/x86/kernel/kvm.c +3
-0@@ -856,6 +856,12 @@ static void __init kvm_guest_init(void)#ifdef CONFIG_SMP if (pv_tlb_flush_supported ()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; +/* +* KVM's flush implementation calls native_flush_tlb_multi(), +* which sends real IPIs when INVLPGB is not available.
Not on all (virtual) CPUs. The entire point of KVM's PV TLB flush is to elide the IPIs. If a vCPU was scheduled out by the host, the guest sets a flag and relies on the host to flush the TLB on behalf of the guest prior to the next VM-Enter.
for_each_cpu(cpu, flushmask) {
/*
* The local vCPU is never preempted, so we do not explicitly
* skip check for local vCPU - it will never be cleared from
* flushmask.
*/
src = &per_cpu(steal_time, cpu);
state = READ_ONCE(src->preempted);
if ((state & KVM_VCPU_PREEMPTED)) {
if (try_cmpxchg(&src->preempted, &state,
state | KVM_VCPU_FLUSH_TLB))
__cpumask_clear_cpu(cpu, flushmask); <=== removes CPU from the IPI set
}
}
native_flush_tlb_multi(flushmask, info);Quoted 4 lines
arch/x86/kernel/kvm.c +2
-0+if (!cpu_feature_enabled (X86_FEATURE_INVLPGB )) + pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast = true;pr_info ("KVM setup pv remote TLB flush\n" ); }
Lance Yang
reviewerRe: [PATCH v4 3/3] x86/tlb: add architecture-specific TLB IPI optimization support
Quoted 17 lines · Sean Christopherson
arch/x86/kernel/kvm.c static void __init kvm_guest_init(void) · line 861
Quoted 11 lines · Lance Yang
arch/x86/kernel/kvm.c +3
-0@@ -856,6 +856,12 @@ static void __init kvm_guest_init(void)#ifdef CONFIG_SMP if (pv_tlb_flush_supported ()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; +/* +* KVM's flush implementation calls native_flush_tlb_multi(), +* which sends real IPIs when INVLPGB is not available. Not on all (virtual) CPUs. The entire point of KVM's PV TLB flush is to elide the IPIs. If a vCPU was scheduled out by the host, the guest sets a flag and relies on the host to flush the TLB on behalf of the guest prior to the next VM-Enter.
Ah, I see. Thanks for the correction!
KVM only sends IPIs to running vCPUs; preempted ones are left out of the mask and flushed on VM-Enter. So the old comment was wrong ...
IIUC, we still set the flag to true because only running vCPUs can be in a software/lockless walk, and they all get the IPI, so the flush is enough.
Does that match what you had in mind?
Thanks,
Lance
Quoted 22 lines
for_each_cpu(cpu, flushmask) { /* * The local vCPU is never preempted, so we do not explicitly * skip check for local vCPU - it will never be cleared from * flushmask. */ src = &per_cpu(steal_time, cpu); state = READ_ONCE(src->preempted); if ((state & KVM_VCPU_PREEMPTED)) { if (try_cmpxchg(&src->preempted, &state, state | KVM_VCPU_FLUSH_TLB)) __cpumask_clear_cpu(cpu, flushmask); <=== removes CPU from the IPI set } } native_flush_tlb_multi(flushmask, info);Quoted 4 lines
arch/x86/kernel/kvm.c +2
-0+if (!cpu_feature_enabled (X86_FEATURE_INVLPGB )) + pv_ops.mmu.flush_tlb_multi_implies_ipi_broadcast = true;pr_info ("KVM setup pv remote TLB flush\n" ); }
Sean Christopherson
reviewerRe: [PATCH v4 3/3] x86/tlb: add architecture-specific TLB IPI optimization support
Quoted 21 lines · Lance Yang
Quoted 17 lines · Sean Christopherson
arch/x86/kernel/kvm.c static void __init kvm_guest_init(void) · line 861
Quoted 11 lines · Lance Yang
arch/x86/kernel/kvm.c +3
-0@@ -856,6 +856,12 @@ static void __init kvm_guest_init(void)#ifdef CONFIG_SMP if (pv_tlb_flush_supported ()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; +/* +* KVM's flush implementation calls native_flush_tlb_multi(), +* which sends real IPIs when INVLPGB is not available. Not on all (virtual) CPUs. The entire point of KVM's PV TLB flush is to elide the IPIs. If a vCPU was scheduled out by the host, the guest sets a flag and relies on the host to flush the TLB on behalf of the guest prior to the next VM-Enter.
Ah, I see. Thanks for the correction!
Quoted 7 lines
KVM only sends IPIs to running vCPUs; preempted ones are left out of the mask and flushed on VM-Enter. So the old comment was wrong ...
IIUC, we still set the flag to true because only running vCPUs can be in a software/lockless walk, and they all get the IPI, so the flush is enough.
Does that match what you had in mind?
No, because from the guest kernel's perspective, the vCPU is running. The kernel can't make any assumptions about what code the vCPU was executing when the vCPU was preempted by the host scheduler, i.e. it's entirely possible the vCPU is in a software/lockless walk.
Lance Yang
reviewerRe: [PATCH v4 3/3] x86/tlb: add architecture-specific TLB IPI optimization support
Quoted 28 lines · Sean Christopherson
Quoted 27 lines · Lance Yang
Quoted 17 lines · Sean Christopherson
arch/x86/kernel/kvm.c static void __init kvm_guest_init(void) · line 861
Quoted 11 lines · Lance Yang
arch/x86/kernel/kvm.c +3
-0@@ -856,6 +856,12 @@ static void __init kvm_guest_init(void)#ifdef CONFIG_SMP if (pv_tlb_flush_supported ()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; +/* +* KVM's flush implementation calls native_flush_tlb_multi(), +* which sends real IPIs when INVLPGB is not available. Not on all (virtual) CPUs. The entire point of KVM's PV TLB flush is to elide the IPIs. If a vCPU was scheduled out by the host, the guest sets a flag and relies on the host to flush the TLB on behalf of the guest prior to the next VM-Enter.
Ah, I see. Thanks for the correction!
KVM only sends IPIs to running vCPUs; preempted ones are left out of the mask and flushed on VM-Enter. So the old comment was wrong ...
IIUC, we still set the flag to true because only running vCPUs can be in a software/lockless walk, and they all get the IPI, so the flush is enough.
Quoted 6 lines
Quoted 1 line
Does that match what you had in mind?
No, because from the guest kernel's perspective, the vCPU is running. The kernel can't make any assumptions about what code the vCPU was executing when the vCPU was preempted by the host scheduler, i.e. it's entirely possible the vCPU is in a software/lockless walk.
Thanks a lot for setting me straight!
So any PV that has its own things and doesn't call native_flush_tlb_multi() directly cannot be trusted to provide the IPI guarantees we need.
So we should only set the flag for the native path, which truly calls native_flush_tlb_multi() directly.
Have a great weekend,
Lance
Peter Zijlstra
maintainerQuoted 11 lines · Lance Yang
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU
active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast andperf_get_page_size()set/clear the tracker around their walk;tlb_remove_table_sync_mm()uses it and replaces the previous broadcast in the free/unshare paths.
I'm confused. This only happens when !PT_RECLAIM, because if PT_RECLAIM __tlb_remove_table_one() actually uses RCU.
So why are you making things more expensive for no reason?
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · Lance Yang
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU
active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast andperf_get_page_size()set/clear the tracker around their walk;tlb_remove_table_sync_mm()uses it and replaces the previous broadcast in the free/unshare paths.
Quoted 4 lines
I'm confused. This only happens when !PT_RECLAIM, because if
PT_RECLAIM__tlb_remove_table_one()actually uses RCU.So why are you making things more expensive for no reason?
You're right that when CONFIG_PT_RECLAIM is set, __tlb_remove_table_one() uses call_rcu() and we never call any sync there — this series doesn't touch that path.
In the !PT_RECLAIM table-free path (same __tlb_remove_table_one() branch that calls tlb_remove_table_sync_mm(tlb->mm) before __tlb_remove_table), we're not adding any new sync; we're replacing the existing broadcast IPI (tlb_remove_table_sync_one()) with targeted IPIs (tlb_remove_table_sync_mm()).
One thing I just realized: when CONFIG_MMU_GATHER_RCU_TABLE_FREE is not set, the sync path isn't used at all (tlb_remove_table_sync_one() and friends aren't even compiled), so we don't need the tracker in that config.
Thanks for raising this!
Lance
Peter Zijlstra
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 20 lines · Lance Yang
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · Lance Yang
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU
active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast andperf_get_page_size()set/clear the tracker around their walk;tlb_remove_table_sync_mm()uses it and replaces the previous broadcast in the free/unshare paths.Quoted 4 lines
I'm confused. This only happens when !PT_RECLAIM, because if
PT_RECLAIM__tlb_remove_table_one()actually uses RCU.So why are you making things more expensive for no reason?
Quoted 8 lines
You're right that when
CONFIG_PT_RECLAIMis set,__tlb_remove_table_one()usescall_rcu()and we never call any sync there — this series doesn't touch that path.In the !PT_RECLAIM table-free path (same
__tlb_remove_table_one()branch that callstlb_remove_table_sync_mm(tlb->mm)before__tlb_remove_table), we're not adding any new sync; we're replacing the existing broadcast IPI (tlb_remove_table_sync_one()) with targeted IPIs (tlb_remove_table_sync_mm()).
Right, but if we can use full RCU for PT_RECLAIM, why can't we do so unconditionally and not add overhead?
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 25 lines · Peter Zijlstra
Quoted 24 lines · Lance Yang
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · Lance Yang
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU
active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast andperf_get_page_size()set/clear the tracker around their walk;tlb_remove_table_sync_mm()uses it and replaces the previous broadcast in the free/unshare paths.Quoted 4 lines
I'm confused. This only happens when !PT_RECLAIM, because if
PT_RECLAIM__tlb_remove_table_one()actually uses RCU.So why are you making things more expensive for no reason?
You're right that when
CONFIG_PT_RECLAIMis set,__tlb_remove_table_one()usescall_rcu()and we never call any sync there — this series doesn't touch that path.
Quoted 7 lines
Quoted 4 lines
In the !PT_RECLAIM table-free path (same
__tlb_remove_table_one()branch that callstlb_remove_table_sync_mm(tlb->mm)before__tlb_remove_table), we're not adding any new sync; we're replacing the existing broadcast IPI (tlb_remove_table_sync_one()) with targeted IPIs (tlb_remove_table_sync_mm()).Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?
The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 33 lines · Lance Yang
Quoted 30 lines · Peter Zijlstra
Quoted 28 lines · Lance Yang
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · Lance Yang
When freeing or unsharing page tables we send an IPI to synchronize with concurrent lockless page table walkers (e.g. GUP-fast). Today we broadcast that IPI to all CPUs, which is costly on large machines and hurts RT workloads[1].
This series makes those IPIs targeted. We track which CPUs are currently doing a lockless page table walk for a given mm (per-CPU
active_lockless_pt_walk_mm). When we need to sync, we only IPI those CPUs. GUP-fast andperf_get_page_size()set/clear the tracker around their walk;tlb_remove_table_sync_mm()uses it and replaces the previous broadcast in the free/unshare paths.Quoted 4 lines
I'm confused. This only happens when !PT_RECLAIM, because if
PT_RECLAIM__tlb_remove_table_one()actually uses RCU.So why are you making things more expensive for no reason?
You're right that when
CONFIG_PT_RECLAIMis set,__tlb_remove_table_one()usescall_rcu()and we never call any sync there — this series doesn't touch that path.In the !PT_RECLAIM table-free path (same
__tlb_remove_table_one()branch that callstlb_remove_table_sync_mm(tlb->mm)before__tlb_remove_table), we're not adding any new sync; we're replacing the existing broadcast IPI (tlb_remove_table_sync_one()) with targeted IPIs (tlb_remove_table_sync_mm()).
Quoted 5 lines
Quoted 2 lines
Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
In addition: We need the sync when we modify page tables (e.g. unshare, collapse), not only when we free them. RCU can defer freeing but does not prevent lockless walkers from seeing concurrent in-place modifications, so we need the IPI to synchronize with those walkers first.
Peter Zijlstra
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 11 lines · Lance Yang
Quoted 5 lines
Quoted 2 lines
Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
In addition: We need the sync when we modify page tables (e.g. unshare, collapse), not only when we free them. RCU can defer freeing but does not prevent lockless walkers from seeing concurrent in-place modifications, so we need the IPI to synchronize with those walkers first.
Currently PT_RECLAIM=y has no IPI; are you saying that is broken? If not, then why do we need this at all?
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · Lance Yang
Quoted 5 lines
Quoted 2 lines
Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
Quoted 8 lines
Quoted 5 lines
In addition: We need the sync when we modify page tables (e.g. unshare, collapse), not only when we free them. RCU can defer freeing but does not prevent lockless walkers from seeing concurrent in-place modifications, so we need the IPI to synchronize with those walkers first.
Currently PT_RECLAIM=y has no IPI; are you saying that is broken? If not, then why do we need this at all?
PT_RECLAIM=y does have IPI for unshare/collapse — those paths call tlb_flush_unshared_tables() (for hugetlb unshare) and collapse_huge_page() (in khugepaged collapse), which already send IPIs today (broadcast to all CPUs via tlb_remove_table_sync_one()).
What PT_RECLAIM=y doesn't need IPI for is table freeing ( __tlb_remove_table_one() uses call_rcu() instead). But table modification (unshare, collapse) still needs IPI to synchronize with lockless walkers, regardless of PT_RECLAIM.
So PT_RECLAIM=y is not broken; it already has IPI where needed. This series just makes those IPIs targeted instead of broadcast. Does that clarify?
Thanks,
Lance
Peter Zijlstra
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 25 lines · Lance Yang
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · Lance Yang
Quoted 5 lines
Quoted 2 lines
Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
Quoted 8 lines
Quoted 5 lines
In addition: We need the sync when we modify page tables (e.g. unshare, collapse), not only when we free them. RCU can defer freeing but does not prevent lockless walkers from seeing concurrent in-place modifications, so we need the IPI to synchronize with those walkers first.
Currently PT_RECLAIM=y has no IPI; are you saying that is broken? If not, then why do we need this at all?
PT_RECLAIM=y does have IPI for unshare/collapse — those paths call
tlb_flush_unshared_tables()(for hugetlb unshare) andcollapse_huge_page()(in khugepaged collapse), which already send IPIs today (broadcast to all CPUs viatlb_remove_table_sync_one()).
Quoted 7 lines
What PT_RECLAIM=y doesn't need IPI for is table freeing (
__tlb_remove_table_one()usescall_rcu()instead). But table modification (unshare, collapse) still needs IPI to synchronize with lockless walkers, regardless ofPT_RECLAIM.So PT_RECLAIM=y is not broken; it already has IPI where needed. This series just makes those IPIs targeted instead of broadcast. Does that clarify?
Oh bah, reading is hard. I had missed they had more table_sync_one() calls, rather than remove_table_one().
So you *can* replace table_sync_one() with rcu_sync(), that will provide the same guarantees. Its just a 'little' bit slower on the update side, but does not incur the read side cost.
I really think anything here needs to better explain the various requirements. Because now everybody gets to pay the price for hugetlb shared crud, while 'nobody' will actually use that.
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 34 lines · Peter Zijlstra
Quoted 25 lines · Lance Yang
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · Lance Yang
Quoted 5 lines
Quoted 2 lines
Right, but if we can use full RCU for
PT_RECLAIM, why can't we do so unconditionally and not add overhead?The sync (IPI) is mainly needed for unshare (e.g. hugetlb) and collapse (khugepaged) paths, regardless of whether table free uses RCU, IIUC.
Quoted 8 lines
Quoted 5 lines
In addition: We need the sync when we modify page tables (e.g. unshare, collapse), not only when we free them. RCU can defer freeing but does not prevent lockless walkers from seeing concurrent in-place modifications, so we need the IPI to synchronize with those walkers first.
Currently PT_RECLAIM=y has no IPI; are you saying that is broken? If not, then why do we need this at all?
PT_RECLAIM=y does have IPI for unshare/collapse — those paths call
tlb_flush_unshared_tables()(for hugetlb unshare) andcollapse_huge_page()(in khugepaged collapse), which already send IPIs today (broadcast to all CPUs viatlb_remove_table_sync_one()).Quoted 7 lines
What PT_RECLAIM=y doesn't need IPI for is table freeing (
__tlb_remove_table_one()usescall_rcu()instead). But table modification (unshare, collapse) still needs IPI to synchronize with lockless walkers, regardless ofPT_RECLAIM.So PT_RECLAIM=y is not broken; it already has IPI where needed. This series just makes those IPIs targeted instead of broadcast. Does that clarify?
Quoted 6 lines
Oh bah, reading is hard. I had missed they had more
table_sync_one()calls, rather thanremove_table_one().So you *can* replace
table_sync_one()withrcu_sync(), that will provide the same guarantees. Its just a 'little' bit slower on the update side, but does not incur the read side cost.
Yep, we could replace the IPI with synchronize_rcu() on the sync side:
- Currently: TLB flush → send IPI → wait for walkers to finish
- With synchronize_rcu(): TLB flush → synchronize_rcu() -> waits for grace period
Lockless walkers (e.g. GUP-fast) use local_irq_disable(); synchronize_rcu() also waits for regions with preemption/interrupts disabled, so it should work, IIUC.
And then, the trade-off would be:
- Read side: zero cost (no per-CPU tracking)
- Write side: wait for RCU grace period (potentially slower)
For collapse/unshare, that write-side latency might be acceptable :)
@David, what do you think?
Quoted 4 lines
I really think anything here needs to better explain the various requirements. Because now everybody gets to pay the price for hugetlb shared crud, while 'nobody' will actually use that.
Right. If we go with synchronize_rcu(), the read-side cost goes away ...
Thanks,
Lance
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 36 lines · Lance Yang
Quoted 17 lines · Peter Zijlstra
Quoted 8 lines · Lance Yang
PT_RECLAIM=y does have IPI for unshare/collapse — those paths call
tlb_flush_unshared_tables()(for hugetlb unshare) andcollapse_huge_page()(in khugepaged collapse), which already send IPIs today (broadcast to all CPUs viatlb_remove_table_sync_one()).Quoted 7 lines
What PT_RECLAIM=y doesn't need IPI for is table freeing (
__tlb_remove_table_one()usescall_rcu()instead). But table modification (unshare, collapse) still needs IPI to synchronize with lockless walkers, regardless ofPT_RECLAIM.So PT_RECLAIM=y is not broken; it already has IPI where needed. This series just makes those IPIs targeted instead of broadcast. Does that clarify?
Quoted 6 lines
Oh bah, reading is hard. I had missed they had more
table_sync_one()calls, rather thanremove_table_one().So you *can* replace
table_sync_one()withrcu_sync(), that will provide the same guarantees. Its just a 'little' bit slower on the update side, but does not incur the read side cost.Yep, we could replace the IPI with
synchronize_rcu()on the sync side:- Currently: TLB flush → send IPI → wait for walkers to finish
- Withsynchronize_rcu(): TLB flush →synchronize_rcu()-> waits for grace periodLockless walkers (e.g. GUP-fast) use
local_irq_disable();synchronize_rcu()also waits for regions with preemption/interrupts disabled, so it should work, IIUC.
Quoted 7 lines
And then, the trade-off would be:
- Read side: zero cost (no per-CPU tracking)
- Write side: wait for RCU grace period (potentially slower)For collapse/unshare, that write-side latency might be acceptable :)
@David, what do you think?
Given that we just fixed the write-side latency from breaking Oracle's databases completely, we have to be a bit careful here :)
The thing is: on many x86 configs we don't need *any* TLB flushed or RCU syncs.
So "how much slower" are we talking about, especially on bigger/loaded systems?
Signature · 3 lines
Cheers, David
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 41 lines · David Hildenbrand (Arm)
Quoted 40 lines · Lance Yang
Quoted 17 lines · Peter Zijlstra
Quoted 8 lines · Lance Yang
PT_RECLAIM=y does have IPI for unshare/collapse — those paths call
tlb_flush_unshared_tables()(for hugetlb unshare) andcollapse_huge_page()(in khugepaged collapse), which already send IPIs today (broadcast to all CPUs viatlb_remove_table_sync_one()).Quoted 7 lines
What PT_RECLAIM=y doesn't need IPI for is table freeing (
__tlb_remove_table_one()usescall_rcu()instead). But table modification (unshare, collapse) still needs IPI to synchronize with lockless walkers, regardless ofPT_RECLAIM.So PT_RECLAIM=y is not broken; it already has IPI where needed. This series just makes those IPIs targeted instead of broadcast. Does that clarify?
Quoted 6 lines
Oh bah, reading is hard. I had missed they had more
table_sync_one()calls, rather thanremove_table_one().So you *can* replace
table_sync_one()withrcu_sync(), that will provide the same guarantees. Its just a 'little' bit slower on the update side, but does not incur the read side cost.Yep, we could replace the IPI with
synchronize_rcu()on the sync side:- Currently: TLB flush → send IPI → wait for walkers to finish
- Withsynchronize_rcu(): TLB flush →synchronize_rcu()-> waits for grace periodLockless walkers (e.g. GUP-fast) use
local_irq_disable();synchronize_rcu()also waits for regions with preemption/interrupts disabled, so it should work, IIUC.And then, the trade-off would be:
- Read side: zero cost (no per-CPU tracking)
- Write side: wait for RCU grace period (potentially slower)
Quoted 5 lines
Quoted 3 lines
For collapse/unshare, that write-side latency might be acceptable :)
@David, what do you think?
Given that we just fixed the write-side latency from breaking Oracle's databases completely, we have to be a bit careful here :)
Yep, agreed.
Quoted 2 lines
The thing is: on many x86 configs we don't need *any* TLB flushed or RCU syncs.
Right. Looks like that is low-hanging fruit. I'll send that out separately :)
Quoted 2 lines
So "how much slower" are we talking about, especially on bigger/loaded systems?
Unfortunately the numbers are pretry bad. On an x86-64 64-core system under high load, each synchronize_rcu() is about *22.9* ms on average ...
So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g. synchronize_rcu() is too slow on large systems.
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 34 lines · Lance Yang
Quoted 17 lines · David Hildenbrand (Arm)
Quoted 16 lines · Lance Yang
Yep, we could replace the IPI with
synchronize_rcu()on the sync side:- Currently: TLB flush → send IPI → wait for walkers to finish
- Withsynchronize_rcu(): TLB flush →synchronize_rcu()-> waits for grace periodLockless walkers (e.g. GUP-fast) use
local_irq_disable();synchronize_rcu()also waits for regions with preemption/interrupts disabled, so it should work, IIUC.And then, the trade-off would be:
- Read side: zero cost (no per-CPU tracking)
- Write side: wait for RCU grace period (potentially slower)Quoted 5 lines
Quoted 3 lines
For collapse/unshare, that write-side latency might be acceptable :)
@David, what do you think?
Given that we just fixed the write-side latency from breaking Oracle's databases completely, we have to be a bit careful here :)
Yep, agreed.
Quoted 2 lines
The thing is: on many x86 configs we don't need *any* TLB flushed or RCU syncs.
Right. Looks like that is low-hanging fruit. I'll send that out separately :)
Quoted 1 line
Quoted 8 lines
Quoted 1 line
So "how much slower" are we talking about, especially on bigger/loaded systems?
Unfortunately the numbers are pretry bad. On an x86-64 64-core system under high load, each
synchronize_rcu()is about *22.9* ms on average ...So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.
GUP-fast is 3%, right? Any way we can reduce that to 1% and call it noise? :)
Signature · 3 lines
Cheers, David
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 37 lines · David Hildenbrand (Arm)
Quoted 36 lines · Lance Yang
Quoted 17 lines · David Hildenbrand (Arm)
Quoted 16 lines · Lance Yang
Yep, we could replace the IPI with
synchronize_rcu()on the sync side:- Currently: TLB flush → send IPI → wait for walkers to finish
- Withsynchronize_rcu(): TLB flush →synchronize_rcu()-> waits for grace periodLockless walkers (e.g. GUP-fast) use
local_irq_disable();synchronize_rcu()also waits for regions with preemption/interrupts disabled, so it should work, IIUC.And then, the trade-off would be:
- Read side: zero cost (no per-CPU tracking)
- Write side: wait for RCU grace period (potentially slower)Quoted 5 lines
Quoted 3 lines
For collapse/unshare, that write-side latency might be acceptable :)
@David, what do you think?
Given that we just fixed the write-side latency from breaking Oracle's databases completely, we have to be a bit careful here :)
Yep, agreed.
Quoted 2 lines
The thing is: on many x86 configs we don't need *any* TLB flushed or RCU syncs.
Right. Looks like that is low-hanging fruit. I'll send that out separately :)
Quoted 2 lines
So "how much slower" are we talking about, especially on bigger/ loaded systems?
Quoted 8 lines
Quoted 6 lines
Unfortunately the numbers are pretry bad. On an x86-64 64-core system under high load, each
synchronize_rcu()is about *22.9* ms on average ...So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.GUP-fast is 3%, right? Any way we can reduce that to 1% and call it noise? :)
Yes, GUP-fast is ~3%. I'll keep trying to do that, but first getting the low-hanging fruit done :)
Re: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
In reply to Lance YangQuoted 3 lines · Lance Yang
So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.
Which of the writers truly *need* synchronize_rcu()?
What are they doing with the memory that they can't move forward unless it's quiescent *now*?
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 9 lines · Dave Hansen
Quoted 3 lines · Lance Yang
So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.Which of the writers truly *need*
synchronize_rcu()?What are they doing with the memory that they can't move forward unless it's quiescent *now*?
Without IPIs or synchronize_rcu(), IIUC, we have no way to know if there are ongoing concurrent lockless page-table walks — the walkers just disable IRQs and walk.
Re: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 6 lines · Lance Yang
Quoted 5 lines · Dave Hansen
Quoted 3 lines · Lance Yang
So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.
Quoted 8 lines
Quoted 4 lines
Which of the writers truly *need*
synchronize_rcu()?What are they doing with the memory that they can't move forward unless it's quiescent *now*?
Without IPIs or
synchronize_rcu(), IIUC, we have no way to know if there are ongoing concurrent lockless page-table walks — the walkers just disable IRQs and walk.
Yeah, but one aim of RCU is ensuring that readers see valid data but not necessarily the most up to date data.
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
Don't forget that we also have pesky concurrent lockless page-table walkers called CPUs. They're extra pesky in that they don't even stop for IPIs. ;)
Lance Yang
reviewerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 16 lines · Dave Hansen
Quoted 6 lines · Lance Yang
Quoted 5 lines · Dave Hansen
Quoted 3 lines · Lance Yang
So for now, neither approach looks good: tracking on the read side adss cost to GUP-fast, and syncing on the write side e.g.
synchronize_rcu()is too slow on large systems.Quoted 8 lines
Quoted 4 lines
Which of the writers truly *need*
synchronize_rcu()?What are they doing with the memory that they can't move forward unless it's quiescent *now*?
Without IPIs or
synchronize_rcu(), IIUC, we have no way to know if there are ongoing concurrent lockless page-table walks — the walkers just disable IRQs and walk.
Quoted 6 lines
Yeah, but one aim of RCU is ensuring that readers see valid data but not necessarily the most up to date data.
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
The issue is we're about to free the page table (e.g. pmdp_collapse_flush()).
We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
Quoted 3 lines
Don't forget that we also have pesky concurrent lockless page-table walkers called CPUs. They're extra pesky in that they don't even stop for IPIs. ;)
I assume those walkers that don't disable IRQs only read the PMD and don't walk into the table; otherwise the current sync wouldn't work for them.
Thanks,
Lance
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 12 lines · Lance Yang
Quoted 9 lines · Dave Hansen
Quoted 4 lines · Lance Yang
Without IPIs or
synchronize_rcu(), IIUC, we have no way to know if there are ongoing concurrent lockless page-table walks — the walkers just disable IRQs and walk.Yeah, but one aim of RCU is ensuring that readers see valid data but not necessarily the most up to date data.
Quoted 8 lines
Quoted 3 lines
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
The issue is we're about to free the page table (e.g.
pmdp_collapse_flush()).We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
Right, and walking a page table that is suddenly no longer a page table is the real fun :)
... or trying to lookup the page of something that is not even a page.
Quoted 8 lines
Quoted 4 lines
Don't forget that we also have pesky concurrent lockless page-table walkers called CPUs. They're extra pesky in that they don't even stop for IPIs. ;)
I assume those walkers that don't disable IRQs only read the PMD and don't walk into the table; otherwise the current sync wouldn't work for them.
CPU page table walkers are much easier to control in that regard :)
Signature · 3 lines
Cheers, David
Re: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
In reply to Lance YangQuoted 8 lines · Lance Yang
Quoted 6 lines · Dave Hansen
Yeah, but one aim of RCU is ensuring that readers see valid data but not necessarily the most up to date data.
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
Quoted 5 lines
The issue is we're about to free the page table (e.g.
pmdp_collapse_flush()).We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
But can't we RCU-free the page table? Why do we need to wait for the RCU readers to finish?
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 9 lines · Matthew Wilcox
Quoted 8 lines · Lance Yang
Quoted 6 lines · Dave Hansen
Yeah, but one aim of RCU is ensuring that readers see valid data but not necessarily the most up to date data.
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
Quoted 8 lines
Quoted 5 lines
The issue is we're about to free the page table (e.g.
pmdp_collapse_flush()).We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
But can't we RCU-free the page table? Why do we need to wait for the RCU readers to finish?
For unsharing hugetlb PMD tables the problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user. It's complicated.
For page table freeing, we only do it if we fail to allocate memory -- if we cannot use RCU IIRC.
khugepaged, no idea.
Signature · 3 lines
Cheers, David
Re: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 6 lines · David Hildenbrand (Arm)
Quoted 2 lines
But can't we RCU-free the page table? Why do we need to wait for the RCU readers to finish?
For unsharing hugetlb PMD tables the problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user. It's complicated.
Letting the previously-shared table get released to everything else in the system sounds like a fixable problem. tlb_flush_unshared_tables() talks about this, and it makes sense that once locks get dropped that something else could get mapped in and start using the PMD.
The RCU way of fixing that would be to allocate new page table, replace the old one, and RCU-free the old one. Read, Copy, Update. :)
It does temporarily eat up an extra page, and cost an extra copy. But neither of those seems expensive compared to IPI'ing the world.
Quoted 2 lines
For page table freeing, we only do it if we fail to allocate memory -- if we cannot use RCU IIRC.
But that case is fine to be slow and use synchronize_rcu(). If you're failing to allocate a single page, you're in a way slow path anyway.
Quoted 1 line
khugepaged, no idea.
Sounds like Lance has some homework to do there. :)
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
Quoted 4 lines · Dave Hansen
Quoted 3 lines · David Hildenbrand (Arm)
Quoted 2 lines
But can't we RCU-free the page table? Why do we need to wait for the RCU readers to finish?
Quoted 8 lines
Quoted 3 lines
For unsharing hugetlb PMD tables the problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user. It's complicated.
Letting the previously-shared table get released to everything else in the system sounds like a fixable problem.
tlb_flush_unshared_tables()talks about this, and it makes sense that once locks get dropped that something else could get mapped in and start using the PMD.
Yeah, I tried to document that carefully.
Quoted 6 lines
The RCU way of fixing that would be to allocate new page table, replace the old one, and RCU-free the old one. Read, Copy, Update. :)
It does temporarily eat up an extra page, and cost an extra copy. But neither of those seems expensive compared to IPI'ing the world.
I played with many such ideas, including never reusing a page table again once it was once shared. All turned out rather horrible.
RCU-way: replacing a shared page table involves updating all processes that share the page table :/ . I think another issue I stumbled into while trying to implement was around failing to allocate memory (but being required to make progress). It all turned to quite some complexity and inefficiency, so I had to give up on that. :)
Quoted 6 lines
Quoted 3 lines
For page table freeing, we only do it if we fail to allocate memory -- if we cannot use RCU IIRC.
But that case is fine to be slow and use
synchronize_rcu(). If you're failing to allocate a single page, you're in a way slow path anyway.
That's true. We could likely do that already and avoid the IPI broadcast there that was once reported to be a problem for RT applications.
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerRe: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
In reply to David Hildenbrand (Arm)Quoted 9 lines · David Hildenbrand (Arm)
Quoted 8 lines · Matthew Wilcox
Quoted 6 lines · Lance Yang
The issue is we're about to free the page table (e.g.
pmdp_collapse_flush()).We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
Quoted 8 lines
Quoted 2 lines
But can't we RCU-free the page table? Why do we need to wait for the RCU readers to finish?
For unsharing hugetlb PMD tables the problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user. It's complicated.
For page table freeing, we only do it if we fail to allocate memory -- if we cannot use RCU IIRC.
khugepaged, no idea.
Now that I had dinner my memory comes back: for khugepaged, we have to make sure there is no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Signature · 3 lines
Cheers, David
Re: [PATCH v4 0/3] targeted TLB sync IPIs for lockless page table
In reply to Lance YangOn 2/5/26 08:30, Lance Yang wrote:
...
Quoted 9 lines
Quoted 3 lines
Are there cases where ongoing concurrent lockless page-table walks need to see the writes and they can't tolerate seeing valid but slightly stale data?
The issue is we're about to free the page table (e.g.
pmdp_collapse_flush()).We have to ensure no walker is still doing a lockless page-table walk when the page directories are freed, otherwise we get use-after-free.
But isn't this already solved by the existing RCU freeing approach documented above tlb_remove_table_smp_sync()?
This seems like a rather classic way to use RCU: wait to free until RCU says there can't be a reader any more. You don't have to sit there and wait for it, you just use call_rcu() which will hold off the free until it's safe.