linux-kernel - Re: [PATCH] sched/fair: Avoid false sharing in nohz struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8445130b-7672-45a7-9c9e-512aa6029a56@intel.com>
Date: Tue, 23 Dec 2025 16:03:23 +0800
From: "Guo, Wangyang" <wangyang.guo@...el.com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, Benjamin Lei <benjamin.lei@...el.com>,
 Tim Chen <tim.c.chen@...ux.intel.com>, Tianyou Li <tianyou.li@...el.com>,
 Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
 Juri Lelli <juri.lelli@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>
Subject: Re: [PATCH] sched/fair: Avoid false sharing in nohz struct

On 12/23/2025 3:27 PM, Shrikanth Hegde wrote:
>>>
>>> Likely in your case, nr_cpus updates are the costly ones.
>>> Try below and see if it helps to fix your issue too.
>>> https://lore.kernel.org/all/20251201183146.74443-1- 
>>> sshegde@...ux.ibm.com/
>>> I Should send out new version soon.
>>>
>>>> 2. Data followed by nohz still share the same cacheline and has
>>>>     potential false sharing issue.
>>>>
>>>
>>> How does your patch handle this?
>>> I don't see any other struct apart from nohz being changed.
>>
>> The data follow by nohz is implicit and determined by compiler.
>> For example, this is the layout from /proc/kallsyms in my machine:
>> ffffffff88600d40 b nohz
>> ffffffff88600d68 B arch_needs_tick_broadcast
>> ffffffff88600d6c b __key.264
>> ffffffff88600d6c b __key.265
>> ffffffff88600d70 b dl_generation
>> ffffffff88600d78 b sched_clock_irqtime
>>
>> What we can do is placing read-mostly `idle_cpus_mask` pointer in a 
>> new cacheline, so data followed by nohz would not be affected by nr_cpus.
>>
> 
> That's a concern. If it is compiler dependent, then sometime it helps, 
> sometime it wont.

This patch wants to make it compiler independent. Sometimes it maybe 
sched_clock_irqtime, sometimes, it maybe baba_vars. Changing on nohz 
makes sure it would not affect others, no matter whatever data followed.

> It should done other way around rather than changing the nohz.
> If there is structure which has a lot of read/updates, it should go into 
> its
> own cacheline rather.
> 
> i.e in your case sched_clock_irqtime should go into its own cacheline.

If that is preferred in kernel, I can resubmit the patch which only 
requires alignment on sched_clock_irqtime.

> ---
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 4f97896887ec..29f9438f9f03 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -25,7 +25,7 @@
>    */
>   DEFINE_PER_CPU(struct irqtime, cpu_irqtime);
> 
> -int sched_clock_irqtime;
> +int sched_clock_irqtime __cacheline_aligned;
> 
>   void enable_sched_clock_irqtime(void)
>   {
> 
> 

BR
Wangyang