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

["Guo, Wangyang" <wangyang.guo@intel.com>]

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@linux.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