skip redundant sync IPIs when TLB flush sent them
Subsystems (8)
v10 cover skip redundant sync IPIs when TLB flush sent them
Hi all,
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (tlb->freed_tables or tlb->unshared_tables).
On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
This series introduces tlb_table_flush_implies_ipi_broadcast() to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.
A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Two-step plan as David suggested[4]:
Step 1 (this series): Skip redundant sync when we're 100% certain the TLB flush sent IPIs. INVLPGB is excluded because when supported, we cannot guarantee IPIs were sent, keeping it clean and simple.
Step 2 (future work): Send targeted IPIs only to CPUs actually doing software/lockless page table walks, benefiting all architectures.
Regarding Step 2, it obviously only applies to setups where Step 1 does not
apply: like x86 with INVLPGB or arm64. Step 2 work is ongoing; early attempts showed ~3% GUP-fast overhead. Reducing the overhead requires more work and tuning; it will be submitted separately once ready.
On a 64-core Intel x86 server, the CAL interrupt count in /proc/interrupts dropped from 646,316 to 785 when collapsing a 20 GiB range with this series applied.
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]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
[3] https://lore.kernel.org/linux-mm/[email protected]/
[4] https://lore.kernel.org/linux-mm/[email protected]/
v9 -> v10:
- Rename the x86 flush_tlb_info bit from freed_tables to wake_lazy_cpus, to make the lazy-TLB behavior explicit (per Dave, thanks!)
- https://lore.kernel.org/linux-mm/[email protected]/
v8 -> v9:
- Rebase on mm-new; re-tested, no code changes.
- https://lore.kernel.org/linux-mm/[email protected]/
v7 -> v8:
- Pick up Acked-by tags from David, thanks!
- Add CAL interrupt numbers to the cover letter (per Andrew, thanks!)
- Rewrite the [2/2] changelog and reword the comment (per David, thanks!)
- https://lore.kernel.org/linux-mm/[email protected]/
v6 -> v7:
- Simplify init logic and eliminate duplicated X86_FEATURE_INVLPGB checks (per Dave, thanks!)
- Remove flush_tlb_multi_implies_ipi_broadcast property because no PV backend sets it today.
- https://lore.kernel.org/linux-mm/[email protected]/
v5 -> v6:
- Use static_branch to eliminate the branch overhead (per Peter, thanks!)
- https://lore.kernel.org/linux-mm/[email protected]/
v4 -> v5:
- Drop per-CPU tracking (active_lockless_pt_walk_mm) from this series; defer to Step 2 as it adds ~3% GUP-fast overhead
- Keep pv_ops property false for PV backends like KVM: preempted vCPUs cannot be assumed safe (per Sean, thanks!)
https://lore.kernel.org/linux-mm/[email protected]/
- 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 (2):
mm/mmu_gather: prepare to skip redundant sync IPIs x86/tlb: skip redundant sync IPIs for native TLB flush
arch/x86/hyperv/mmu.c | 4 ++-- arch/x86/include/asm/tlb.h | 19 +++++++++++++++- arch/x86/include/asm/tlbflush.h | 6 +++-- arch/x86/kernel/smpboot.c | 1 + arch/x86/mm/tlb.c | 39 +++++++++++++++++++++++---------- include/asm-generic/tlb.h | 17 ++++++++++++++ mm/mmu_gather.c | 15 +++++++++++++ 7 files changed, 84 insertions(+), 17 deletions(-)
Signature · 1 line
2.49.0
PATCH v10 1/2 mm/mmu_gather: prepare to skip redundant sync IPIs
From: Lance Yang <[email protected]>
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (tlb->freed_tables or tlb->unshared_tables).
On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
Introduce tlb_table_flush_implies_ipi_broadcast() to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.
A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Currently always returns false (no behavior change). The follow-up patch will enable the optimization for x86.
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
[3] https://lore.kernel.org/linux-mm/[email protected]/
--- include/asm-generic/tlb.h | 17 +++++++++++++++++ mm/mmu_gather.c | 15 +++++++++++++++ 2 files changed, 32 insertions(+)
include/asm-generic/tlb.h +17 -0
@@ -240,6 +240,23 @@ static inline void tlb_remove_table(struct mmu_gather *tlb, void *table) }#endif /* CONFIG_MMU_GATHER_TABLE_FREE */ +/** +* tlb_table_flush_implies_ipi_broadcast - does TLB flush imply IPI sync +* +* When page table operations require synchronization with software/lockless +* walkers, they flush the TLB (tlb->freed_tables or tlb->unshared_tables) +* then call tlb_remove_table_sync_{one,rcu}(). If the flush already sent +* IPIs to all CPUs, the sync call is redundant. +* +* Returns false by default. Architectures can override by defining this. +*/ +#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 +15 -0
@@ -283,6 +283,14 @@ void tlb_remove_table_sync_one(void)* It is however sufficient for software page-table walkers that rely on * IRQ disabling. */ + +/* +* Skip IPI if the preceding TLB flush already synchronized with +* all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +smp_call_function (tlb_remove_table_smp_sync,NULL ,1 ); }
@@ -312,6 +320,13 @@ static void tlb_remove_table_free(struct mmu_table_batch *batch)*/ void tlb_remove_table_sync_rcu (void ) { +/* +* Skip RCU wait if the preceding TLB flush already synchronized +* with all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +synchronize_rcu (); }
Signature · 1 line
2.49.0
Peter Zijlstra
maintainerRe: [PATCH 7.2 v10 1/2] mm/mmu_gather: prepare to skip redundant sync IPIs
Quoted 22 lines · Lance Yang
mm/mmu_gather.c +8
-0@@ -283,6 +283,14 @@ void tlb_remove_table_sync_one(void)* It is however sufficient for software page-table walkers that rely on * IRQ disabling. */ + +/* +* Skip IPI if the preceding TLB flush already synchronized with +* all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +smp_call_function (tlb_remove_table_smp_sync,NULL ,1 ); }@@ -312,6 +320,13 @@ static void tlb_remove_table_free(struct mmu_table_batch *batch)*/ void tlb_remove_table_sync_rcu (void )
mm/mmu_gather.c static void tlb_remove_table_free(struct mmu_table_batch *batch) · line 331
Quoted 10 lines
mm/mmu_gather.c +7
-0{ +/* +* Skip RCU wait if the preceding TLB flush already synchronized +* with all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +synchronize_rcu (); }
So I don't like this at all.... The comment says there is a preceding TLB flush, but there is nothing that guarantees there is. One would have to go audit all users and ensure this is always true.
This thing is incredibly fragile.
Also, the comment in gup_fast() is nonsense, the local_irq_disable() isn't about tlb_remove_table_sync_one(), it is primarily about TLBI IPIs.
Re: [PATCH 7.2 v10 1/2] mm/mmu_gather: prepare to skip redundant sync IPIs
Quoted 5 lines · Peter Zijlstra
So I don't like this at all.... The comment says there is a preceding TLB flush, but there is nothing that guarantees there is. One would have to go audit all users and ensure this is always true.
This thing is incredibly fragile.
Yeah, this seems like an attempt to apply a code solution to a data structure problem.
I think I talked about this in earlier iterations. But, ideally, what happens here is that the things doing the table freeing or collapsing or whatever would note in a data structure what they did.
Then the actual flushing code can look at the data structure and figure out what kind of flush it needs. Things like "do I need to flush on lazy CPUs?" Or, "have I done an IPI since the last page table free?"
But, if I remember from earlier in this thread, some of the callers of this stuff didn't have a nice data structure (like an mmu_gather) passed in to the places where it would be needed to exfiltrate the information.
I think Lance gave up on that because it looked too invasive to him.
But, I think this boils down to the code being too fragile as-is to support what Lance is trying to do. It actually needs some refactoring love before it can support the desired optimization. I'm not sure there's an easy way out here.
Andrew Morton
maintainerRe: [PATCH 7.2 v10 1/2] mm/mmu_gather: prepare to skip redundant sync IPIs
Quoted 22 lines · On Fri, 24 Apr 2026 08:52:16 -0700 Dave Hansen <[email protected]> wrote:
Quoted 5 lines · Peter Zijlstra
So I don't like this at all.... The comment says there is a preceding TLB flush, but there is nothing that guarantees there is. One would have to go audit all users and ensure this is always true.
This thing is incredibly fragile.
Yeah, this seems like an attempt to apply a code solution to a data structure problem.
I think I talked about this in earlier iterations. But, ideally, what happens here is that the things doing the table freeing or collapsing or whatever would note in a data structure what they did.
Then the actual flushing code can look at the data structure and figure out what kind of flush it needs. Things like "do I need to flush on lazy CPUs?" Or, "have I done an IPI since the last page table free?"
But, if I remember from earlier in this thread, some of the callers of this stuff didn't have a nice data structure (like an
mmu_gather) passed in to the places where it would be needed to exfiltrate the information.
Quoted 6 lines
I think Lance gave up on that because it looked too invasive to him.
But, I think this boils down to the code being too fragile as-is to support what Lance is trying to do. It actually needs some refactoring love before it can support the desired optimization. I'm not sure there's an easy way out here.
Well this is a bummer. Looky:
Quoted 3 lines
On a 64-core Intel x86 server, the CAL interrupt count in
/proc/interruptsdropped from 646,316 to 785 when collapsing a 20 GiB range with this series applied.
It would have been nice.
Oh well, it's been a month with no progress so this patchset appears to be before its time. Thanks all, I'll drop this from the 7.1 queue.
(otoh, "a month with no progress" == "decently tested")!
Re: [PATCH 7.2 v10 1/2] mm/mmu_gather: prepare to skip redundant sync IPIs
In reply to Lance YangQuoted 72 lines · Lance Yang
From: Lance Yang <[email protected]>
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (
tlb->freed_tablesortlb->unshared_tables).On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
Introduce
tlb_table_flush_implies_ipi_broadcast()to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Currently always returns false (no behavior change). The follow-up patch will enable the optimization for x86.
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
[3] https://lore.kernel.org/linux-mm/[email protected]/Suggested-by David Hildenbrand (Arm) <[email protected]>Signed-off-by Lance Yang <[email protected]>---
include/asm-generic/tlb.h| 17 +++++++++++++++++mm/mmu_gather.c| 15 +++++++++++++++2 files changed, 32 insertions(+)include/asm-generic/tlb.h +17
-0@@ -240,6 +240,23 @@ static inline void tlb_remove_table(struct mmu_gather *tlb, void *table) }#endif /* CONFIG_MMU_GATHER_TABLE_FREE */ +/** +* tlb_table_flush_implies_ipi_broadcast - does TLB flush imply IPI sync +* +* When page table operations require synchronization with software/lockless +* walkers, they flush the TLB (tlb->freed_tables or tlb->unshared_tables) +* then call tlb_remove_table_sync_{one,rcu}(). If the flush already sent +* IPIs to all CPUs, the sync call is redundant. +* +* Returns false by default. Architectures can override by defining this. +*/ +#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 +0
-0
mm/mmu_gather.c void tlb_remove_table_sync_one(void) · line 292
Quoted 10 lines
mm/mmu_gather.c +7
-0@@ -283,6 +283,14 @@ void tlb_remove_table_sync_one(void)* It is however sufficient for software page-table walkers that rely on * IRQ disabling. */ + +/* +* Skip IPI if the preceding TLB flush already synchronized with +* all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ;
Sashiko told me[1]:
"
Could skipping the global IPI fail to synchronize with lockless walkers running outside the mm_cpumask?
tlb_remove_table_sync_one() is used (e.g., by khugepaged during THP collapse) to wait for lockless page table walkers to finish. On 32-bit architectures like x86 PAE, pmdp_get_lockless() disables interrupts to prevent torn reads of 64-bit PMDs.
While the preceding TLB flush sends IPIs to CPUs in the target mm's mm_cpumask, lockless walkers such as pte_offset_map() are frequently executed by background threads unrelated to the target mm (e.g., kswapd via page_vma_mapped_walk()). These threads run on CPUs outside of mm_cpumask and would not receive the TLB flush IPI.
If the global smp_call_function(..., 1) IPI is skipped, the modifying thread might not wait for kswapd. Could this allow it to overwrite the PMD while the out-of-context reader is reading it, resulting in a torn PMD?
"
Afraid not.
When CONFIG_MMU_GATHER_RCU_TABLE_FREE=n, tlb_remove_table_sync_one() is just a NOP.
So if lockless walkers outside mm_cpumask really required a separate global IPI here, systems running with CONFIG_MMU_GATHER_RCU_TABLE_FREE=n would already be broken today, because there is no such IPI there to begin with :)
Quoted 5 lines
mm/mmu_gather.c +0
-0smp_call_function (tlb_remove_table_smp_sync,NULL ,1 ); }
mm/mmu_gather.c static void tlb_remove_table_free(struct mmu_table_batch *batch) · line 329
Quoted 10 lines
mm/mmu_gather.c +7
-0@@ -312,6 +320,13 @@ static void tlb_remove_table_free(struct mmu_table_batch *batch)*/ void tlb_remove_table_sync_rcu (void ) { +/* +* Skip RCU wait if the preceding TLB flush already synchronized +* with all CPUs that could be doing software/lockless page table walks. +*/ +if (tlb_table_flush_implies_ipi_broadcast ()) +return ; +
And Sashiko also pointed out[2]:
"
Does skipping synchronize_rcu() here violate the RCU lifetime guarantee of page tables?
Generic software page table walkers, such as pte_offset_map(), rely strictly on rcu_read_lock() to protect page table pages from being freed concurrently. Crucially, they execute with hardware interrupts enabled.
Under CONFIG_PREEMPT_RCU, an IPI broadcast does not wait for rcu_read_lock() critical sections to complete. The IPI simply interrupts the reader, executes the flush, and returns immediately.
Could this allow the page table to be freed while the reader is still actively accessing it, leading to a use-after-free for concurrent pte_offset_map() readers?
"
Nop.
tlb_remove_table_sync_rcu() still has a single caller: the !CONFIG_PT_RECLAIM __tlb_remove_table_one() fallback. It was introduced for that slow batch-allocation-failure path in 1fb3d8c20bfa ("mm/mmu_gather: replace IPI with synchronize_rcu() when batch allocation fails"), replacing the previous tlb_remove_table_sync_one() there.
So if pte_offset_map() readers really required a full RCU grace period in that fallback path, that concern would already have existed before 1fb3d8c20bfa.
So we're safe here :)
Quoted 7 lines
mm/mmu_gather.c +0
-0synchronize_rcu (); }Signature · 1 line
2.49.0
PATCH v10 2/2 x86/tlb: skip redundant sync IPIs for native TLB flush
In reply to Lance YangFrom: Lance Yang <[email protected]>
Some page table operations need to synchronize with software/lockless walkers after a TLB flush by calling tlb_remove_table_sync_{one,rcu}(). On x86, that extra synchronization is redundant when the preceding TLB flush already broadcast IPIs to all relevant CPUs.
native_pv_tlb_init() checks whether native_flush_tlb_multi() is in use. On CONFIG_PARAVIRT systems, it checks pv_ops; on non-PARAVIRT, native flush is always in use.
It decides once at boot whether to enable the optimization: if using native TLB flush and INVLPGB is not supported, we know IPIs were sent and can skip the redundant sync. The decision is fixed via a static key as Peter suggested[1].
PV backends (KVM, Xen, Hyper-V) typically have their own implementations and don't call native_flush_tlb_multi() directly, so they cannot be trusted to provide the IPI guarantees we need.
Also rename the x86 flush_tlb_info bit from freed_tables to wake_lazy_cpus, as Dave suggested[2], to match the behavior it controls: whether the remote flush may skip CPUs in lazy TLB mode. Both freed_tables and unshared_tables set it, because lazy-TLB CPUs must receive IPIs before page tables can be freed or reused. With that guarantee in place, tlb_table_flush_implies_ipi_broadcast() can safely skip the later sync IPI.
Two-step plan as David suggested[3]:
Step 1 (this patch): Skip redundant sync when we're 100% certain the TLB flush sent IPIs. INVLPGB is excluded because when supported, we cannot guarantee IPIs were sent, keeping it clean and simple.
Step 2 (future work): Send targeted IPIs only to CPUs actually doing software/lockless page table walks, benefiting all architectures.
Regarding Step 2, it obviously only applies to setups where Step 1 does not apply: like x86 with INVLPGB or arm64.
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
[3] https://lore.kernel.org/linux-mm/[email protected]/
--- arch/x86/hyperv/mmu.c | 4 ++-- arch/x86/include/asm/tlb.h | 19 +++++++++++++++- arch/x86/include/asm/tlbflush.h | 6 +++-- arch/x86/kernel/smpboot.c | 1 + arch/x86/mm/tlb.c | 39 +++++++++++++++++++++++---------- 5 files changed, 52 insertions(+), 17 deletions(-)
arch/x86/hyperv/mmu.c +2 -2
@@ -63,7 +63,7 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,struct hv_tlb_flush *flush;u64 status;unsigned long flags; -bool do_lazy = !info->freed_tables; +bool do_lazy = !info->wake_lazy_cpus;trace_hyperv_mmu_flush_tlb_multi (cpus, info);
@@ -198,7 +198,7 @@ static u64 hyperv_flush_tlb_others_ex(const struct cpumask *cpus, flush->hv_vp_set.format =HV_GENERIC_SET_SPARSE_4K ; nr_bank =cpumask_to_vpset_skip (&flush->hv_vp_set, cpus, - info->freed_tables ?NULL : cpu_is_lazy); + info->wake_lazy_cpus ?NULL : cpu_is_lazy);if (nr_bank <0 )return HV_STATUS_INVALID_PARAMETER ;
arch/x86/include/asm/tlb.h +18 -1
@@ -5,22 +5,39 @@#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> +DECLARE_STATIC_KEY_FALSE (tlb_ipi_broadcast_key); + +static inline bool tlb_table_flush_implies_ipi_broadcast (void ) +{ +return static_branch_likely (&tlb_ipi_broadcast_key); +} +static inline void tlb_flush (struct mmu_gather *tlb) {unsigned long start =0UL , end =TLB_FLUSH_ALL ;unsigned int stride_shift =tlb_get_unmap_shift (tlb); +/* +* Both freed_tables and unshared_tables must wake lazy-TLB CPUs, so +* they receive IPIs before reusing or freeing page tables, allowing +* us to safely implement tlb_table_flush_implies_ipi_broadcast(). +*/ +bool wake_lazy_cpus = tlb->freed_tables || tlb->unshared_tables; +if (!tlb->fullmm && !tlb->need_flush_all) { start = tlb->start; end = tlb->end; } -flush_tlb_mm_range (tlb->mm, start, end, stride_shift, tlb->freed_tables); +flush_tlb_mm_range (tlb->mm, start, end, stride_shift, wake_lazy_cpus); }static inline void invlpg (unsigned long addr)
arch/x86/include/asm/tlbflush.h +4 -2
@@ -18,6 +18,8 @@DECLARE_PER_CPU (u64, tlbstate_untag_mask); +void __init native_pv_tlb_init (void ); +void __flush_tlb_all (void );#define TLB_FLUSH_ALL -1UL
@@ -225,7 +227,7 @@ struct flush_tlb_info {u64 new_tlb_gen;unsigned int initiating_cpu;u8 stride_shift; -u8 freed_tables; +u8 wake_lazy_cpus;u8 trim_cpumask; };
@@ -315,7 +317,7 @@ static inline bool mm_in_asid_transition(struct mm_struct *mm) { return false; }extern void flush_tlb_all (void );extern void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables); +bool wake_lazy_cpus);extern void flush_tlb_kernel_range (unsigned long start,unsigned long end);static inline void flush_tlb_page (struct vm_area_struct *vma,unsigned long a)
arch/x86/kernel/smpboot.c +1 -0
@@ -1256,6 +1256,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/mm/tlb.c +27 -12
@@ -26,6 +26,8 @@#include "mm_internal.h" +DEFINE_STATIC_KEY_FALSE (tlb_ipi_broadcast_key); +#ifdef CONFIG_PARAVIRT # define STATIC_NOPV #else
@@ -1360,16 +1362,16 @@ STATIC_NOPV void native_flush_tlb_multi(const struct cpumask *cpumask, (info->end - info->start) >> PAGE_SHIFT);/* -* If no page tables were freed, we can skip sending IPIs to -* CPUs in lazy TLB mode. They will flush the CPU themselves -* at the next context switch. +* If lazy-TLB CPUs do not need to be woken, we can skip sending +* IPIs to them. They will flush themselves at the next context +* switch. * -* However, if page tables are getting freed, we need to send the -* IPI everywhere, to prevent CPUs in lazy TLB mode from tripping -* up on the new contents of what used to be page tables, while -* doing a speculative memory access. +* However, if page tables are getting freed or unshared, we need +* to send the IPI everywhere, to prevent CPUs in lazy TLB mode +* from tripping up on the new contents of what used to be page +* tables, while doing a speculative memory access. */ -if (info->freed_tables ||mm_in_asid_transition (info->mm)) +if (info->wake_lazy_cpus ||mm_in_asid_transition (info->mm))on_each_cpu_mask (cpumask, flush_tlb_func, (void *)info, true);else on_each_cpu_cond_mask(should_flush_tlb, flush_tlb_func,
@@ -1402,7 +1404,7 @@ static DEFINE_PER_CPU(unsigned int, flush_tlb_info_idx);static struct flush_tlb_info *get_flush_tlb_info (struct mm_struct *mm,unsigned long start,unsigned long end, -unsigned int stride_shift,bool freed_tables, +unsigned int stride_shift,bool wake_lazy_cpus,u64 new_tlb_gen) {struct flush_tlb_info *info =this_cpu_ptr (&flush_tlb_info);
@@ -1429,7 +1431,7 @@ static struct flush_tlb_info *get_flush_tlb_info(struct mm_struct *mm, info->end = end; info->mm = mm; info->stride_shift = stride_shift; - info->freed_tables = freed_tables; + info->wake_lazy_cpus = wake_lazy_cpus; info->new_tlb_gen = new_tlb_gen; info->initiating_cpu =smp_processor_id (); info->trim_cpumask =0 ;
@@ -1448,7 +1450,7 @@ static void put_flush_tlb_info(void)void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables) +bool wake_lazy_cpus) {struct flush_tlb_info *info;int cpu =get_cpu ();
@@ -1457,7 +1459,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,/* This is also a barrier that synchronizes with switch_mm(). */ new_tlb_gen =inc_mm_tlb_gen (mm); - info =get_flush_tlb_info (mm, start, end, stride_shift, freed_tables, + info =get_flush_tlb_info (mm, start, end, stride_shift, wake_lazy_cpus, new_tlb_gen);/*
@@ -1834,3 +1836,16 @@ static int __init create_tlb_single_page_flush_ceiling(void)return 0 ; }late_initcall (create_tlb_single_page_flush_ceiling); + +void __init native_pv_tlb_init (void ) +{ +#ifdef CONFIG_PARAVIRT +if (pv_ops.mmu.flush_tlb_multi != native_flush_tlb_multi) +return ; +#endif + +if (cpu_feature_enabled (X86_FEATURE_INVLPGB )) +return ; + +static_branch_enable (&tlb_ipi_broadcast_key); +}
Signature · 1 line
2.49.0
Peter Zijlstra
maintainerRe: [PATCH 7.2 v10 2/2] x86/tlb: skip redundant sync IPIs for native TLB flush
Quoted 129 lines · Lance Yang
arch/x86/hyperv/mmu.c +2
-2@@ -63,7 +63,7 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,struct hv_tlb_flush *flush;u64 status;unsigned long flags; -bool do_lazy = !info->freed_tables; +bool do_lazy = !info->wake_lazy_cpus;trace_hyperv_mmu_flush_tlb_multi (cpus, info);@@ -198,7 +198,7 @@ static u64 hyperv_flush_tlb_others_ex(const struct cpumask *cpus, flush->hv_vp_set.format =HV_GENERIC_SET_SPARSE_4K ; nr_bank =cpumask_to_vpset_skip (&flush->hv_vp_set, cpus, - info->freed_tables ?NULL : cpu_is_lazy); + info->wake_lazy_cpus ?NULL : cpu_is_lazy);if (nr_bank <0 )return HV_STATUS_INVALID_PARAMETER ;arch/x86/include/asm/tlb.h +8
-1static inline void tlb_flush (struct mmu_gather *tlb) {unsigned long start =0UL , end =TLB_FLUSH_ALL ;unsigned int stride_shift =tlb_get_unmap_shift (tlb); +/* +* Both freed_tables and unshared_tables must wake lazy-TLB CPUs, so +* they receive IPIs before reusing or freeing page tables, allowing +* us to safely implement tlb_table_flush_implies_ipi_broadcast(). +*/ +bool wake_lazy_cpus = tlb->freed_tables || tlb->unshared_tables; +if (!tlb->fullmm && !tlb->need_flush_all) { start = tlb->start; end = tlb->end; } -flush_tlb_mm_range (tlb->mm, start, end, stride_shift, tlb->freed_tables); +flush_tlb_mm_range (tlb->mm, start, end, stride_shift, wake_lazy_cpus); }static inline void invlpg (unsigned long addr)arch/x86/include/asm/tlbflush.h +2
-2@@ -225,7 +227,7 @@ struct flush_tlb_info {u64 new_tlb_gen;unsigned int initiating_cpu;u8 stride_shift; -u8 freed_tables; +u8 wake_lazy_cpus;u8 trim_cpumask; };@@ -315,7 +317,7 @@ static inline bool mm_in_asid_transition(struct mm_struct *mm) { return false; }extern void flush_tlb_all (void );extern void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables); +bool wake_lazy_cpus);extern void flush_tlb_kernel_range (unsigned long start,unsigned long end);static inline void flush_tlb_page (struct vm_area_struct *vma,unsigned long a)arch/x86/mm/tlb.c +11
-11@@ -1360,16 +1362,16 @@ STATIC_NOPV void native_flush_tlb_multi(const struct cpumask *cpumask, (info->end - info->start) >> PAGE_SHIFT);/* -* If no page tables were freed, we can skip sending IPIs to -* CPUs in lazy TLB mode. They will flush the CPU themselves -* at the next context switch. +* If lazy-TLB CPUs do not need to be woken, we can skip sending +* IPIs to them. They will flush themselves at the next context +* switch. * -* However, if page tables are getting freed, we need to send the -* IPI everywhere, to prevent CPUs in lazy TLB mode from tripping -* up on the new contents of what used to be page tables, while -* doing a speculative memory access. +* However, if page tables are getting freed or unshared, we need +* to send the IPI everywhere, to prevent CPUs in lazy TLB mode +* from tripping up on the new contents of what used to be page +* tables, while doing a speculative memory access. */ -if (info->freed_tables ||mm_in_asid_transition (info->mm)) +if (info->wake_lazy_cpus ||mm_in_asid_transition (info->mm))on_each_cpu_mask (cpumask, flush_tlb_func, (void *)info, true);else on_each_cpu_cond_mask(should_flush_tlb, flush_tlb_func,@@ -1402,7 +1404,7 @@ static DEFINE_PER_CPU(unsigned int, flush_tlb_info_idx);static struct flush_tlb_info *get_flush_tlb_info (struct mm_struct *mm,unsigned long start,unsigned long end, -unsigned int stride_shift,bool freed_tables, +unsigned int stride_shift,bool wake_lazy_cpus,u64 new_tlb_gen) {struct flush_tlb_info *info =this_cpu_ptr (&flush_tlb_info);@@ -1429,7 +1431,7 @@ static struct flush_tlb_info *get_flush_tlb_info(struct mm_struct *mm, info->end = end; info->mm = mm; info->stride_shift = stride_shift; - info->freed_tables = freed_tables; + info->wake_lazy_cpus = wake_lazy_cpus; info->new_tlb_gen = new_tlb_gen; info->initiating_cpu =smp_processor_id (); info->trim_cpumask =0 ;@@ -1448,7 +1450,7 @@ static void put_flush_tlb_info(void)void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables) +bool wake_lazy_cpus) {struct flush_tlb_info *info;int cpu =get_cpu ();
arch/x86/mm/tlb.c void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start, · line 1465
Quoted 8 lines
arch/x86/mm/tlb.c +1
-1@@ -1457,7 +1459,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,/* This is also a barrier that synchronizes with switch_mm(). */ new_tlb_gen =inc_mm_tlb_gen (mm); - info =get_flush_tlb_info (mm, start, end, stride_shift, freed_tables, + info =get_flush_tlb_info (mm, start, end, stride_shift, wake_lazy_cpus, new_tlb_gen);/*
This whole s/freed_tables/wake_lazy_cpus/ rename should probably be its own patch, as should that include unshare_tables thing be.
That seems like unrelated changes.
Re: [PATCH 7.2 v10 2/2] x86/tlb: skip redundant sync IPIs for native TLB flush
Quoted 137 lines · Peter Zijlstra
Quoted 136 lines · Lance Yang
arch/x86/hyperv/mmu.c +2
-2@@ -63,7 +63,7 @@ static void hyperv_flush_tlb_multi(const struct cpumask *cpus,struct hv_tlb_flush *flush;u64 status;unsigned long flags; -bool do_lazy = !info->freed_tables; +bool do_lazy = !info->wake_lazy_cpus;trace_hyperv_mmu_flush_tlb_multi (cpus, info);@@ -198,7 +198,7 @@ static u64 hyperv_flush_tlb_others_ex(const struct cpumask *cpus, flush->hv_vp_set.format =HV_GENERIC_SET_SPARSE_4K ; nr_bank =cpumask_to_vpset_skip (&flush->hv_vp_set, cpus, - info->freed_tables ?NULL : cpu_is_lazy); + info->wake_lazy_cpus ?NULL : cpu_is_lazy);if (nr_bank <0 )return HV_STATUS_INVALID_PARAMETER ;arch/x86/include/asm/tlb.h +8
-1static inline void tlb_flush (struct mmu_gather *tlb) {unsigned long start =0UL , end =TLB_FLUSH_ALL ;unsigned int stride_shift =tlb_get_unmap_shift (tlb); +/* +* Both freed_tables and unshared_tables must wake lazy-TLB CPUs, so +* they receive IPIs before reusing or freeing page tables, allowing +* us to safely implement tlb_table_flush_implies_ipi_broadcast(). +*/ +bool wake_lazy_cpus = tlb->freed_tables || tlb->unshared_tables; +if (!tlb->fullmm && !tlb->need_flush_all) { start = tlb->start; end = tlb->end; } -flush_tlb_mm_range (tlb->mm, start, end, stride_shift, tlb->freed_tables); +flush_tlb_mm_range (tlb->mm, start, end, stride_shift, wake_lazy_cpus); }static inline void invlpg (unsigned long addr)arch/x86/include/asm/tlbflush.h +2
-2@@ -225,7 +227,7 @@ struct flush_tlb_info {u64 new_tlb_gen;unsigned int initiating_cpu;u8 stride_shift; -u8 freed_tables; +u8 wake_lazy_cpus;u8 trim_cpumask; };@@ -315,7 +317,7 @@ static inline bool mm_in_asid_transition(struct mm_struct *mm) { return false; }extern void flush_tlb_all (void );extern void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables); +bool wake_lazy_cpus);extern void flush_tlb_kernel_range (unsigned long start,unsigned long end);static inline void flush_tlb_page (struct vm_area_struct *vma,unsigned long a)arch/x86/mm/tlb.c +12
-12@@ -1360,16 +1362,16 @@ STATIC_NOPV void native_flush_tlb_multi(const struct cpumask *cpumask, (info->end - info->start) >> PAGE_SHIFT);/* -* If no page tables were freed, we can skip sending IPIs to -* CPUs in lazy TLB mode. They will flush the CPU themselves -* at the next context switch. +* If lazy-TLB CPUs do not need to be woken, we can skip sending +* IPIs to them. They will flush themselves at the next context +* switch. * -* However, if page tables are getting freed, we need to send the -* IPI everywhere, to prevent CPUs in lazy TLB mode from tripping -* up on the new contents of what used to be page tables, while -* doing a speculative memory access. +* However, if page tables are getting freed or unshared, we need +* to send the IPI everywhere, to prevent CPUs in lazy TLB mode +* from tripping up on the new contents of what used to be page +* tables, while doing a speculative memory access. */ -if (info->freed_tables ||mm_in_asid_transition (info->mm)) +if (info->wake_lazy_cpus ||mm_in_asid_transition (info->mm))on_each_cpu_mask (cpumask, flush_tlb_func, (void *)info, true);else on_each_cpu_cond_mask(should_flush_tlb, flush_tlb_func,@@ -1402,7 +1404,7 @@ static DEFINE_PER_CPU(unsigned int, flush_tlb_info_idx);static struct flush_tlb_info *get_flush_tlb_info (struct mm_struct *mm,unsigned long start,unsigned long end, -unsigned int stride_shift,bool freed_tables, +unsigned int stride_shift,bool wake_lazy_cpus,u64 new_tlb_gen) {struct flush_tlb_info *info =this_cpu_ptr (&flush_tlb_info);@@ -1429,7 +1431,7 @@ static struct flush_tlb_info *get_flush_tlb_info(struct mm_struct *mm, info->end = end; info->mm = mm; info->stride_shift = stride_shift; - info->freed_tables = freed_tables; + info->wake_lazy_cpus = wake_lazy_cpus; info->new_tlb_gen = new_tlb_gen; info->initiating_cpu =smp_processor_id (); info->trim_cpumask =0 ;@@ -1448,7 +1450,7 @@ static void put_flush_tlb_info(void)void flush_tlb_mm_range (struct mm_struct *mm,unsigned long start,unsigned long end,unsigned int stride_shift, -bool freed_tables) +bool wake_lazy_cpus) {struct flush_tlb_info *info;int cpu =get_cpu ();@@ -1457,7 +1459,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,/* This is also a barrier that synchronizes with switch_mm(). */ new_tlb_gen =inc_mm_tlb_gen (mm); - info =get_flush_tlb_info (mm, start, end, stride_shift, freed_tables, + info =get_flush_tlb_info (mm, start, end, stride_shift, wake_lazy_cpus, new_tlb_gen);
Quoted 6 lines
arch/x86/mm/tlb.c void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start, · line 1465
Quoted 1 line
arch/x86/mm/tlb.c +0
-0/* This whole s/freed_tables/wake_lazy_cpus/ rename should probably be its own patch, as should that include
unshare_tablesthing be.That seems like unrelated changes.
Thanks, makes sense! Will split the pure s/freed_tables/wake_lazy_cpus/ rename out.
For the tlb->unshared_tables part, I would keep it with this patch, since lazy-TLB CPUs still have to be woken before reusing unshared page tables.
@Dave what do you think?
Thanks,
Lance
Andrew Morton
maintainerQuoted 13 lines · On Fri, 24 Apr 2026 14:25:26 +0800 Lance Yang <[email protected]> wrote:
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (
tlb->freed_tablesortlb->unshared_tables).On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
This series introduces
tlb_table_flush_implies_ipi_broadcast()to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.
Quoted 8 lines
A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected]
(I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 24 lines · Andrew Morton
Quoted 13 lines · On Fri, 24 Apr 2026 14:25:26 +0800 Lance Yang <[email protected]> wrote:
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (
tlb->freed_tablesortlb->unshared_tables).On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
This series introduces
tlb_table_flush_implies_ipi_broadcast()to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.Quoted 8 lines
A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Quoted 6 lines
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
+Cc: Roman.
Quoted 2 lines
Andrew Morton
maintainerQuoted 32 lines · On Fri, 24 Apr 2026 13:37:04 +0000 Pasha Tatashin <[email protected]>…
Quoted 24 lines · Andrew Morton
Quoted 13 lines · On Fri, 24 Apr 2026 14:25:26 +0800 Lance Yang <[email protected]> wrote:
When page table operations require synchronization with software/lockless walkers, they call tlb_remove_table_sync_{one,rcu}() after flushing the TLB (
tlb->freed_tablesortlb->unshared_tables).On architectures where the TLB flush already sends IPIs to all target CPUs, the subsequent sync IPI broadcast is redundant. This is not only costly on large systems where it disrupts all CPUs even for single-process page table operations, but has also been reported to hurt RT workloads[1].
This series introduces
tlb_table_flush_implies_ipi_broadcast()to check if the prior TLB flush already provided the necessary synchronization. When true, the sync calls can early-return.Quoted 8 lines
A few cases rely on this synchronization:
1) hugetlb PMD unshare[2]: The problem is not the freeing but the reuse of the PMD table for other purposes in the last remaining user after unsharing.
2) khugepaged collapse[3]: Ensure no concurrent GUP-fast before collapsing and (possibly) freeing the page table / re-depositing it.
Quoted 6 lines
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 7 lines
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Preferences?
(I'd suggest automated-reply-to-all, see who complains, then figure out why they can't figure out mail filtering ;))
Quoted 20 lines · Andrew Morton
Quoted 10 lines · On Fri, 24 Apr 2026 13:37:04 +0000 Pasha Tatashin <[email protected]>…
Quoted 8 lines · Andrew Morton
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 7 lines
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
Quoted 8 lines
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Preferences?
Reply-to-author would likely be better as a first step.
Signature · 3 lines
Cheers, David
Andrew Morton
maintainerQuoted 24 lines · On Fri, 24 Apr 2026 16:20:55 +0200 "David Hildenbrand (Arm)"…
Quoted 23 lines · Andrew Morton
Quoted 10 lines · On Fri, 24 Apr 2026 13:37:04 +0000 Pasha Tatashin <[email protected]>…
Quoted 8 lines · Andrew Morton
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 7 lines
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
Quoted 7 lines
Quoted 5 lines
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Preferences?
Reply-to-author would likely be better as a first step.
Why do you think so?
Pasha, is it too early to determine how reply-to-all is working out for kexec?
Quoted 30 lines · Andrew Morton
Quoted 28 lines · On Fri, 24 Apr 2026 16:20:55 +0200 "David Hildenbrand (Arm)"…
Quoted 27 lines · Andrew Morton
Quoted 10 lines · On Fri, 24 Apr 2026 13:37:04 +0000 Pasha Tatashin <[email protected]>…
Quoted 8 lines · Andrew Morton
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 7 lines
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Preferences?
Reply-to-author would likely be better as a first step.
Why do you think so?
Pasha, is it too early to determine how reply-to-all is working out for kexec?
It is too early, we have opted-in only at the end of last week.
Pasha
Quoted 12 lines · Andrew Morton
Quoted 10 lines · On Fri, 24 Apr 2026 16:20:55 +0200 "David Hildenbrand (Arm)"…
Quoted 9 lines · Andrew Morton
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Preferences?
Reply-to-author would likely be better as a first step.
Why do you think so?
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
Signature · 3 lines
Cheers, David
Quoted 16 lines · David Hildenbrand (Arm)
Quoted 14 lines · Andrew Morton
Quoted 12 lines · On Fri, 24 Apr 2026 16:20:55 +0200 "David Hildenbrand (Arm)"…
Quoted 10 lines · Andrew Morton
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Preferences?
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Reply-to-author would likely be better as a first step.
Why do you think so?
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website). It's possible that Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help. If Sashiko is wrong, it's still useful for more people to see it and point it out, instead of Sashiko privately misguiding the author, especially that reviewers are probably more likely to tell if Sashiko is right or wrong.
I understand the fear of too much noise, but I think it's easy to ignore Sashiko if we want to. It's also a good exercise to spell out why Sashiko is wrong (e.g. improve changelogs/comments to document assumptions more obviously). We can always tune it down later if we think it's too much, right?
Peter Zijlstra
maintainerQuoted 2 lines · Yosry Ahmed
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
That's why I have a script; if I press 'S' in mutt on an 0/n email, said script goes and does webrequest to sashiko and inserts the review comments into my local maildir as properly threaded replies to the series at hand.
No looking at website needed.
Quoted 11 lines · Peter Zijlstra
Quoted 2 lines · Yosry Ahmed
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
That's why I have a script; if I press 'S' in mutt on an 0/n email, said script goes and does webrequest to sashiko and inserts the review comments into my local maildir as properly threaded replies to the series at hand.
No looking at website needed.
Do you mind sharing that script? It looks like a great work flow. Thanks.
Best Regards,
Yan, Zi
Quoted 8 lines · Zi Yan
Quoted 5 lines · Peter Zijlstra
Quoted 2 lines · Yosry Ahmed
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
Quoted 8 lines
Quoted 6 lines
That's why I have a script; if I press 'S' in mutt on an 0/n email, said script goes and does webrequest to sashiko and inserts the review comments into my local maildir as properly threaded replies to the series at hand.
No looking at website needed.
Do you mind sharing that script? It looks like a great work flow. Thanks.
Yeah that sounds great :)
Quoted 14 lines · On Fri, 24 Apr 2026 12:15:30 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 13 lines · Zi Yan
Quoted 10 lines · Peter Zijlstra
Quoted 2 lines · Yosry Ahmed
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
That's why I have a script; if I press 'S' in mutt on an 0/n email, said script goes and does webrequest to sashiko and inserts the review comments into my local maildir as properly threaded replies to the series at hand.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
No looking at website needed.
Do you mind sharing that script? It looks like a great work flow. Thanks.
Yeah that sounds great :)
FWIW my workflow was also similar to PeterZ [1], except I use hkml's Sashiko integration.
The hkml integration requires two key strokes to get the Sashiko review, so I think Peter's script is superior. It provides a draft [2] for sharing full Sashiko review that I can further writeup my comment and directly send, though. It is helpful for asking the author their opinion about Sashiko's review, with a fine pointer to the question.
So I initially started opening web browser to check Sashiko review. After that I used hkml to read the review on terminal and copy-pasted the output to ask question to authors with quote of Sashiko review. Finally I was able to just forward Sashiko review together with my questions to authors. Each step of the changes made my life much easier.
DAMON is opted in to the Sashiko's mail reply service from the beginning, and recently Sashiko started the mail delivery. Now I don't need to use the hkml integration, and I feel my life has been even much easier than before.
[1] https://github.com/sjp38/hackermail/blob/master/USAGE.md#sashikodev
[2] https://github.com/sjp38/hackermail/blob/master/USAGE.md#forwarding-sashikodev-statuscomments-to-mailing-list
Thanks,
SJ
Peter Zijlstra
maintainerQuoted 7 lines · Zi Yan
Quoted 5 lines · Peter Zijlstra
Quoted 2 lines · Yosry Ahmed
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
Quoted 8 lines
Quoted 6 lines
That's why I have a script; if I press 'S' in mutt on an 0/n email, said script goes and does webrequest to sashiko and inserts the review comments into my local maildir as properly threaded replies to the series at hand.
No looking at website needed.
Do you mind sharing that script? It looks like a great work flow. Thanks.
Sure, it is in a 'works-for-me' state and probably should get a once over before you run it locally to make sure it fits your environment (at the very least you should probably change my email address :-)
This is cobbled toghether from a script I got from Thomas and staring at the JSON with generous hints from Gemini (because I can't write Python at all).
.muttrc has:
macro index S "<pipe-entry>grep -i \"\^Message-ID:\" | cut -d: -f2- | tr -d \" <>\" | ~/bin/sashiko.py<enter>"
sashiko.py:
---
#!/usr/bin/env python3
import sys
import mailbox
import requests
from argparse import ArgumentParser
from email.message import EmailMessage
from email.utils import formatdate
from datetime import datetime
def getsash(url, msgid):
surl = f'{url.rstrip("/")}/api/patch'
try:
session = requests.Session() session.headers.update({'User-Agent': 'sash2txt_0.2'}) resp = session.get(surl, params={'id': msgid}, timeout=30)
if resp.status_code == 404:
print(f"Error: Patch ID {msgid} not found (404).", file=sys.stderr) return
resp.raise_for_status()
data = resp.json()
except requests.RequestException as ex:
print(f"Request failed: {ex}", file=sys.stderr) return
patches = data.get('patches', [])
if not patches:
print(f"No patches found for (msgid).", file=sys.stderr) return
msgids = {}
subjects = {}
for p in patches:
msgids[p['id']] = p['message_id']
subjects[p['id']] = p['subject']
reviews = data.get('reviews', [])
if not reviews:
print(f"No reviews found for {msgid}.", file=sys.stderr) return
mdir = mailbox.Maildir('~/Maildir/');
for i, r in enumerate(reviews):
inline = r.get('inline_review', '') or ''
if not inline:
continue
author_name = r.get('author_name', 'Sashiko Reviewer') author_email = r.get('author_email', '[email protected]')
msgid = msgids[r['patch_id']]
subject = subjects[r['patch_id']]
# Create the Email object
msg = EmailMessage()
msg['Subject'] = f"Re: {subject}" msg['From'] = f"{author_name} <{author_email}>" msg['To'] = "[email protected]" msg['Date'] = formatdate(localtime=True)
# The critical threading headers msg['Message-ID'] = f"<review-{i}-{msgid}>" msg['References'] = f"<{msgid}>" msg['In-Reply-To'] = f"<{msgid}>"
msg.set_content(inline)
mdir.add(msg)
mdir.flush()
if __name__ == '__main__':
url = 'https://sashiko.dev/'
# Check if there's data in stdin (piped input)
if not sys.stdin.isatty():
for line in sys.stdin:
msgid = line.strip()
if msgid:
getsash(url, msgid)
else:
# Fallback to arguments if no pipe is detected
parser = ArgumentParser(description='Sashiko retriever to mbox') parser.add_argument('msgid', nargs='?', help='Message id of the patch series') args = parser.parse_args()
if args.msgid:
getsash(url, args.msgid)
else:
print("Usage: echo <msgid> | ./sashiko.py OR ./sashiko.py <msgid>", file=sys.stderr)
Peter Zijlstra
maintainerQuoted 25 lines · Peter Zijlstra
mdir =
mailbox.Maildir('~/Maildir/');for i, r in enumerate(reviews):
inline =r.get('inline_review', '')or ''
if not inline:
continue
author_name=r.get('author_name', 'Sashiko Reviewer')author_email=r.get('author_email', '[email protected]')msgid = msgids[r['patch_id']]
subject = subjects[r['patch_id']]# Create the Email object
msg =EmailMessage()
msg['Subject'] = f"Re: {subject}" msg['From'] = f"{author_name} <{author_email}>" msg['To'] = "[email protected]" msg['Date'] = formatdate(localtime=True)# The critical threading headers msg['Message-ID'] = f"<review-{i}-{msgid}>" msg['References'] = f"<{msgid}>" msg['In-Reply-To'] = f"<{msgid}>"
Quoted 5 lines
msg.set_content(inline)
mdir.add(msg)
mdir.flush()
So Ideally I would have 'Reply-to' header set to the original sender and added 'Cc' like the original email. However, AFAICT the JSON does not contain this information, and while I could use the mdir object to find the original message in my Inbox, this is incredibly slow.
I have a TODO to use python-notmuch to do this, but haven't gotten around to doing this yet.
With that 'fixed' I could actually reply to these messages and it would all 'just' work. For now I copy/paste when needed.
Quoted 38 lines · Peter Zijlstra
Quoted 25 lines · Peter Zijlstra
mdir =
mailbox.Maildir('~/Maildir/');for i, r in enumerate(reviews):
inline =r.get('inline_review', '')or ''
if not inline:
continue
author_name=r.get('author_name', 'Sashiko Reviewer')author_email=r.get('author_email', '[email protected]')msgid = msgids[r['patch_id']]
subject = subjects[r['patch_id']]# Create the Email object
msg =EmailMessage()
msg['Subject'] = f"Re: {subject}" msg['From'] = f"{author_name} <{author_email}>" msg['To'] = "[email protected]" msg['Date'] = formatdate(localtime=True)# The critical threading headers msg['Message-ID'] = f"<review-{i}-{msgid}>" msg['References'] = f"<{msgid}>" msg['In-Reply-To'] = f"<{msgid}>"
Quoted 5 lines
msg.set_content(inline)
mdir.add(msg)
mdir.flush()So Ideally I would have 'Reply-to' header set to the original sender and added 'Cc' like the original email. However, AFAICT the JSON does not contain this information, and while I could use the mdir object to find the original message in my Inbox, this is incredibly slow.
Quoted 5 lines
I have a TODO to use python-notmuch to do this, but haven't gotten around to doing this yet.
With that 'fixed' I could actually reply to these messages and it would all 'just' work. For now I copy/paste when needed.
I can add an api to return a json with the review and all meta-information by the original msgid, will it be useful?
Peter Zijlstra
maintainerQuoted 40 lines · Roman Gushchin
Quoted 38 lines · Peter Zijlstra
Quoted 25 lines · Peter Zijlstra
mdir =
mailbox.Maildir('~/Maildir/');for i, r in enumerate(reviews):
inline =r.get('inline_review', '')or ''
if not inline:
continue
author_name=r.get('author_name', 'Sashiko Reviewer')author_email=r.get('author_email', '[email protected]')msgid = msgids[r['patch_id']]
subject = subjects[r['patch_id']]# Create the Email object
msg =EmailMessage()
msg['Subject'] = f"Re: {subject}" msg['From'] = f"{author_name} <{author_email}>" msg['To'] = "[email protected]" msg['Date'] = formatdate(localtime=True)# The critical threading headers msg['Message-ID'] = f"<review-{i}-{msgid}>" msg['References'] = f"<{msgid}>" msg['In-Reply-To'] = f"<{msgid}>"
Quoted 5 lines
msg.set_content(inline)
mdir.add(msg)
mdir.flush()So Ideally I would have 'Reply-to' header set to the original sender and added 'Cc' like the original email. However, AFAICT the JSON does not contain this information, and while I could use the mdir object to find the original message in my Inbox, this is incredibly slow.
Quoted 8 lines
Quoted 5 lines
I have a TODO to use python-notmuch to do this, but haven't gotten around to doing this yet.
With that 'fixed' I could actually reply to these messages and it would all 'just' work. For now I copy/paste when needed.
I can add an api to return a json with the review and all meta-information by the original msgid, will it be useful?
Oh yes, that would be more convenient. Thanks!
Peter Zijlstra
maintainerQuoted 44 lines · Peter Zijlstra
Quoted 43 lines · Roman Gushchin
Quoted 41 lines · Peter Zijlstra
Quoted 25 lines · Peter Zijlstra
mdir =
mailbox.Maildir('~/Maildir/');for i, r in enumerate(reviews):
inline =r.get('inline_review', '')or ''
if not inline:
continue
author_name=r.get('author_name', 'Sashiko Reviewer')author_email=r.get('author_email', '[email protected]')msgid = msgids[r['patch_id']]
subject = subjects[r['patch_id']]# Create the Email object
msg =EmailMessage()
msg['Subject'] = f"Re: {subject}" msg['From'] = f"{author_name} <{author_email}>" msg['To'] = "[email protected]" msg['Date'] = formatdate(localtime=True)# The critical threading headers msg['Message-ID'] = f"<review-{i}-{msgid}>" msg['References'] = f"<{msgid}>" msg['In-Reply-To'] = f"<{msgid}>"
Quoted 5 lines
msg.set_content(inline)
mdir.add(msg)
mdir.flush()So Ideally I would have 'Reply-to' header set to the original sender and added 'Cc' like the original email. However, AFAICT the JSON does not contain this information, and while I could use the mdir object to find the original message in my Inbox, this is incredibly slow.
I have a TODO to use python-notmuch to do this, but haven't gotten around to doing this yet.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
With that 'fixed' I could actually reply to these messages and it would all 'just' work. For now I copy/paste when needed.
I can add an api to return a json with the review and all meta-information by the original msgid, will it be useful?
Oh yes, that would be more convenient. Thanks!
Roman, did you find time to do this? If not, I'll patiently wait :-)
Andrew Morton
maintainerQuoted 11 lines · On Fri, 24 Apr 2026 11:50:03 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 10 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Preferences?
Reply-to-author would likely be better as a first step.
Why do you think so?
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
Quoted 7 lines
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website). It's possible that Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help. If Sashiko is wrong, it's still useful for more people to see it and point it out, instead of Sashiko privately misguiding the author, especially that reviewers are probably more likely to tell if Sashiko is right or wrong.
Also, Sashiko does like to find unrelated bugs in surrounding code. I saw two of these today.
But whatever, one step at a time. My main interest at present is to stop having to send lame emails linking to the Sashiko reports!
Quoted 8 lines · Yosry Ahmed
Quoted 6 lines · David Hildenbrand (Arm)
Quoted 3 lines · Andrew Morton
Why do you think so?
Quoted 7 lines
Quoted 4 lines
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
I read a lot of them, yes. Maintainers know how to find it.
I even think maintainers should briefly go over it before applying to spot if anything in there is still left unanswered. But that doesn't need a mailing list posting.
I enjoy if contributors are aware of the reports and use that input in a reasonable, like Zi just did [1]. And if they are unsure, they usually ask to double-check.
[1] https://lore.kernel.org/r/[email protected]
Quoted 3 lines
It's possible that
Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help.
Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
Signature · 3 lines
Cheers, David
Peter Zijlstra
maintainerQuoted 5 lines · David Hildenbrand (Arm)
Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
Yes, so I understand that you can run it locally, but then you get to pay for the tokens (and sashiko burns tokens like dry grass). I also understand why Google/LF doesn't open this up, its a money pit no doubt.
OTOH, I see this causing more reposts of series than we would have had previously -- and we all love more versions of series.
I'm not sure I see a solution, other than to read less email -- its not like I can even begin to read all email I get anyway.
Quoted 11 lines · Peter Zijlstra
Quoted 5 lines · David Hildenbrand (Arm)
Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
Yes, so I understand that you can run it locally, but then you get to pay for the tokens (and sashiko burns tokens like dry grass). I also understand why Google/LF doesn't open this up, its a money pit no doubt.
Indeed.
Quoted 3 lines
OTOH, I see this causing more reposts of series than we would have had previously -- and we all love more versions of series.
Fortunately, reposts are easy to review, at least for me, when tags are properly incorporated and the version diff properly describes the changes.
Quoted 3 lines
I'm not sure I see a solution, other than to read less email -- its not like I can even begin to read all email I get anyway.
Sad but true.
I consider the last AI review before something gets merged the most important one.
Signature · 3 lines
Cheers, David
Quoted 34 lines · David Hildenbrand (Arm)
Quoted 8 lines · Yosry Ahmed
Quoted 6 lines · David Hildenbrand (Arm)
Quoted 3 lines · Andrew Morton
Why do you think so?
Quoted 7 lines
Quoted 4 lines
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
I read a lot of them, yes. Maintainers know how to find it.
I even think maintainers should briefly go over it before applying to spot if anything in there is still left unanswered. But that doesn't need a mailing list posting.
I enjoy if contributors are aware of the reports and use that input in a reasonable, like Zi just did [1]. And if they are unsure, they usually ask to double-check.
[1] https://lore.kernel.org/r/[email protected]
Quoted 3 lines
It's possible that
Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help.
Quoted 5 lines
Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
The usefulness of the mailing list posting is that it makes it easier to respond and discuss the review. Yes, what Zi did is great, but it would be nice if contributors/reviewers didn't need to manually quote Sashiko.
That being said, I understand the concerns and the pressure to respond to everything as you mention below. Maybe at some point this will become an easier decision to make as reviews become more refined.
Quoted 39 lines · On Fri, 24 Apr 2026 12:18:49 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 37 lines · David Hildenbrand (Arm)
Quoted 8 lines · Yosry Ahmed
Quoted 6 lines · David Hildenbrand (Arm)
Quoted 3 lines · Andrew Morton
Why do you think so?
Quoted 7 lines
Quoted 4 lines
The most important part for me is that authors are aware of the reports.
Sending them as a mail forces people to publicly reply to the feedback, and at this point in time, I am not convinced that that is the right approach.
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
I read a lot of them, yes. Maintainers know how to find it.
I even think maintainers should briefly go over it before applying to spot if anything in there is still left unanswered. But that doesn't need a mailing list posting.
I enjoy if contributors are aware of the reports and use that input in a reasonable, like Zi just did [1]. And if they are unsure, they usually ask to double-check.
[1] https://lore.kernel.org/r/[email protected]
Quoted 3 lines
It's possible that
Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help.Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
Quoted 7 lines
Quoted 2 lines
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
The usefulness of the mailing list posting is that it makes it easier to respond and discuss the review. Yes, what Zi did is great, but it would be nice if contributors/reviewers didn't need to manually quote Sashiko.
I agree. I had to implement hkml-Sashiko integration [1] to avoid the manual works. People could use such existign tools or develop their own. But I can say Sashiko's direct mailing service has improved my dasy much more than the hkml feature.
Quoted 4 lines
That being said, I understand the concerns and the pressure to respond to everything as you mention below. Maybe at some point this will become an easier decision to make as reviews become more refined.
I agree here, too. Apparently [2] my tooling-motivated Sashiko reply forwarding didn't convince all. My honest feeling was that it is not only unconvincing but might making someone slightly annoyed.
I personally feel no problem at Sashiko review is publicly replied to my patches, but I understand my personality is not necessarily same to others. If some change can make someone happy while also making someone sad, I'm up to reducing sadness.
[1] https://github.com/sjp38/hackermail/blob/master/USAGE.md#forwarding-sashikodev-statuscomments-to-mailing-list
[2] https://lore.kernel.org/[email protected]/
Thanks,
SJ
Quoted 29 lines · Yosry Ahmed
Quoted 27 lines · David Hildenbrand (Arm)
Quoted 5 lines · Yosry Ahmed
On Fri, Apr 24, 2026 at 11:36 AM David Hildenbrand (Arm) <[email protected]> wrote:
But I imagine it's useful for reviewers to see Sashiko's feedback as well (without having to go look on the website).
I read a lot of them, yes. Maintainers know how to find it.
I even think maintainers should briefly go over it before applying to spot if anything in there is still left unanswered. But that doesn't need a mailing list posting.
I enjoy if contributors are aware of the reports and use that input in a reasonable, like Zi just did [1]. And if they are unsure, they usually ask to double-check.
[1] https://lore.kernel.org/r/[email protected]
Quoted 3 lines
It's possible that
Sashiko is right but the author isn't convinced, so getting more eyes on the feedback would help.Forcing contributors to reply to everything. I don't like that, in particular not as long as there is no way for contributors to run it early in private.
Quoted 7 lines
Quoted 2 lines
In most cases, contributors just do the reasonable thing: incorporate the feedback in a new version.
The usefulness of the mailing list posting is that it makes it easier to respond and discuss the review. Yes, what Zi did is great, but it would be nice if contributors/reviewers didn't need to manually quote Sashiko.
I think the most important part is the contributor side. A private posting is sufficient for that.
So if possible, I'd start with that and see how it goes.
Quoted 4 lines
That being said, I understand the concerns and the pressure to respond to everything as you mention below. Maybe at some point this will become an easier decision to make as reviews become more refined.
There is another thing to consider here: impact on other reviewers, in particular, new contributors, when they spot excessive review already posted to the mailing list. And the first thing being them reading that.
Signature · 3 lines
Cheers, David
Andrew Morton
maintainerQuoted 7 lines · On Sat, 25 Apr 2026 07:17:18 +0200 "David Hildenbrand (Arm)"…
Quoted 4 lines
The usefulness of the mailing list posting is that it makes it easier to respond and discuss the review. Yes, what Zi did is great, but it would be nice if contributors/reviewers didn't need to manually quote Sashiko.
I think the most important part is the contributor side. A private posting is sufficient for that.
Thinking through how is this going to work out.
- contributor sends patchset
- akpm looks at the Sashiko scan and thinks "huh, we might be seeing a new version soon".
- Now what? Maybe contributor doesn't like AI, maybe contributor thinks it was all nonsense. But I don't know this!
So I send the email "I see that Sashiko said stuff - can we expect a new version?". Which is no improvement over what's happening now.
What I would like to have is some reasonably reliable and prompt means by which we all learn contributor's views on the Sashiko scan.
One way to bring this about might be to set suitable reply-to headers in the Sashiko->contributor email, along with a few words asking contributor to let everyone know the status.
But whatever - let's not overthink this. To start somewhere, let's send that private email, spend a few weeks evaluating then perhaps make adjustments.
Quoted 12 lines · Andrew Morton
Quoted 7 lines · On Sat, 25 Apr 2026 07:17:18 +0200 "David Hildenbrand (Arm)"…
Quoted 4 lines
The usefulness of the mailing list posting is that it makes it easier to respond and discuss the review. Yes, what Zi did is great, but it would be nice if contributors/reviewers didn't need to manually quote Sashiko.
I think the most important part is the contributor side. A private posting is sufficient for that.
Thinking through how is this going to work out.
Quoted 7 lines
- contributor sends patchset
- akpm looks at the Sashiko scan and thinks "huh, we might be seeing a new version soon".
- Now what? Maybe contributor doesn't like AI, maybe contributor thinks it was all nonsense. But I don't know this!
Right. In the common case, people will just read the output and process it in a reasonable way, like some people do today even without any notification.
But there will be cases where people ignore it, I am sure.
Quoted 3 lines
So I send the email "I see that Sashiko said stuff - can we expect a new version?". Which is no improvement over what's happening now.
That only happens in the scenarios where there is no further feedback from other reviewers and there won't be a new revision.
That's why I am saying that the AI review on the last upstream posting before merging is the most important one from a maintainer perspective.
I agree that when we are about to merge something and AI review was not addressed (no feedback from contributor), that it's a problem.
Quoted 3 lines
What I would like to have is some reasonably reliable and prompt means by which we all learn contributor's views on the Sashiko scan.
Right. At the same time I don't want full AI review posted to the mailing list immediately to each and every revision of a patch set.
As first step, I would be fine with having AI review send a review status in case it went into a problem to everyone as reply to each patch set, just stating whether it failed to apply, or if something was found etc. Paired with a link like you would send.
Quoted 4 lines
One way to bring this about might be to set suitable reply-to headers in the
Sashiko->contributoremail, along with a few words asking contributor to let everyone know the status.
Yes, that's an alternative if we were to send the full AI review to contributors. Which might make sense in addition to the AI review status (above).
Quoted 4 lines
But whatever - let's not overthink this. To start somewhere, let's send that private email, spend a few weeks evaluating then perhaps make adjustments.
Right, let's do this step by step.
Signature · 3 lines
Cheers, David
Quoted 30 lines · On Fri, 24 Apr 2026 07:31:45 -0700 Andrew Morton <[email protected]>…
Quoted 28 lines · On Fri, 24 Apr 2026 16:20:55 +0200 "David Hildenbrand (Arm)"…
Quoted 27 lines · Andrew Morton
Quoted 10 lines · On Fri, 24 Apr 2026 13:37:04 +0000 Pasha Tatashin <[email protected]>…
Quoted 8 lines · Andrew Morton
Sashiko questions:
https://sashiko.dev/#/patchset/[email protected](I never know if my Sashiko emails are welcome/useful. Maybe Sashiko said the same stuff about v9 and it's all wrong. But better safe than sorry!)
Quoted 7 lines
These emails are helpful; but, I do not believe you should have to manually follow up with a link to every new patch series.
Perhaps Sashiko could automatically send a summary email in response to the cover letter, or provide a link once the reviews are complete. For the kexec ML, we opted-in with Roman to receive automated emails from sashiko.
Yep. I'd be OK with an automatic reply-to-all. Maybe some won't like that.
An alternative I've discussed with Roman is an automated reply-to-author with a cc to a dedicated list (we could use mm-commits for now).
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Preferences?
Reply-to-author would likely be better as a first step.
Why do you think so?
Pasha, is it too early to determine how reply-to-all is working out for kexec?
DAMON is also opted in to Sashiko's email service. It is replied to only the author and [email protected]. I understand this is the default form of Sashiko's email service, and can flexibly tuned.
So I cannot say how reply-to-all will work for mm. But I can say the default setup email service was a huge improvement for my life. I made tools for making my Sashiko handling easier and was quite convinced with that. But the Sashiko's email service made my life much better. Of course, it may differntly applied to others. My personality is not necessarily same to others.
Thanks,
SJ