Re: [PATCH v3 10/12] x86/mm: Move flush_tlb_info back to the stack

[Nadav Amit <nadav.amit@gmail.com>]

> On 18 Mar 2026, at 19:21, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> +Nadav, org post https://lore.kernel.org/all/20260318045638.1572777-11-zhouchuyi@bytedance.com/
> 
> On 2026-03-18 12:56:36 [+0800], Chuyi Zhou wrote:
>> Commit 3db6d5a5ecaf ("x86/mm/tlb: Remove 'struct flush_tlb_info' from the
>> stack") converted flush_tlb_info from stack variable to per-CPU variable.
>> This brought about a performance improvement of around 3% in extreme test.
>> However, it also required that all flush_tlb* operations keep preemption
>> disabled entirely to prevent concurrent modifications of flush_tlb_info.
>> flush_tlb* needs to send IPIs to remote CPUs and synchronously wait for
>> all remote CPUs to complete their local TLB flushes. The process could
>> take tens of milliseconds when interrupts are disabled or with a large
>> number of remote CPUs.
> …
> PeterZ wasn't too happy to reverse this.
> The snippet below results in the following assembly:
> 
> | 0000000000001ab0 <flush_tlb_kernel_range>:
> …
> |     1ac9:       48 89 e5                mov    %rsp,%rbp
> |     1acc:       48 83 e4 c0             and    $0xffffffffffffffc0,%rsp
> |     1ad0:       48 83 ec 40             sub    $0x40,%rsp
> 
> so it would align it properly which should result in the same cache-line
> movement. I'm not sure about the virtual-to-physical translation of the
> variables as in TLB misses since here we have a virtual mapped stack and
> there we have virtual mapped per-CPU memory.
> 
> Here the below is my quick hack. Does this work, or still a now? I have
> no numbers so…
> 
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 5a3cdc439e38d..4a7f40c7f939a 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -227,7 +227,7 @@ struct flush_tlb_info {
> 	u8			stride_shift;
> 	u8			freed_tables;
> 	u8			trim_cpumask;
> -};
> +} __aligned(SMP_CACHE_BYTES);
> 

This would work, but you are likely to encounter the same problem PeterZ hit
when I did something similar: in some configurations SMP_CACHE_BYTES is very
large.

See https://lore.kernel.org/all/tip-780e0106d468a2962b16b52fdf42898f2639e0a0@git.kernel.org/

Maybe cap the alignment somehow? something like:

#if SMP_CACHE_BYTES > 64
#define FLUSH_TLB_INFO_ALIGN 64
#else
#define FLUSH_TLB_INFO_ALIGN SMP_CACHE_BYTES
#endif

And then use __aligned(FLUSH_TLB_INFO_ALIGN) ?