[RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Tim Chen]

Repost comments:

There have been past discussions about avoiding serialization in load
balancing, but no objections were raised to this patch itself during
its last posting:
https://lore.kernel.org/lkml/20250416035823.1846307-1-tim.c.chen@linux.intel.com/

Vincent and Chen Yu have already provided their Reviewed-by tags.

We recently encountered this issue again on a 2-socket, 240-core
Clearwater Forest server running SPECjbb. In this case, 14% of CPU
cycles were wasted on unnecessary acquisitions of
sched_balance_running. This reinforces the need for the change, and we
hope it can be merged.

Tim

---

During load balancing, balancing at the LLC level and above must be
serialized. The scheduler currently checks the atomic
`sched_balance_running` flag before verifying whether a balance is
actually due. This causes high contention, as multiple CPUs may attempt
to acquire the flag concurrently.

On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
operations for `sched_balance_running`. In most cases, the attempt
aborts immediately after acquisition because the load balance time is
not yet due.

Fix this by checking whether a balance is due *before* trying to
acquire `sched_balance_running`. This avoids many wasted acquisitions
and reduces the cmpxchg overhead in `sched_balance_domain()` from 7.6%
to 0.05%. As a result, OLTP throughput improves by 11%.

Reviewed-by: Chen Yu <yu.c.chen@intel.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 kernel/sched/fair.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8ce56a8d507f..bedd785c4a39 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12126,13 +12126,13 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 
 		interval = get_sd_balance_interval(sd, busy);
 
-		need_serialize = sd->flags & SD_SERIALIZE;
-		if (need_serialize) {
-			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
-				goto out;
-		}
-
 		if (time_after_eq(jiffies, sd->last_balance + interval)) {
+			need_serialize = sd->flags & SD_SERIALIZE;
+			if (need_serialize) {
+				if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
+					goto out;
+			}
+
 			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
 				/*
 				 * The LBF_DST_PINNED logic could have changed
@@ -12144,9 +12144,9 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 			}
 			sd->last_balance = jiffies;
 			interval = get_sd_balance_interval(sd, busy);
+			if (need_serialize)
+				atomic_set_release(&sched_balance_running, 0);
 		}
-		if (need_serialize)
-			atomic_set_release(&sched_balance_running, 0);
 out:
 		if (time_after(next_balance, sd->last_balance + interval)) {
 			next_balance = sd->last_balance + interval;
-- 
2.32.0

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/3/25 4:30 AM, Tim Chen wrote:
> Repost comments:
> 
> There have been past discussions about avoiding serialization in load
> balancing, but no objections were raised to this patch itself during
> its last posting:
> https://lore.kernel.org/lkml/20250416035823.1846307-1-tim.c.chen@linux.intel.com/
> 
> Vincent and Chen Yu have already provided their Reviewed-by tags.
> 
> We recently encountered this issue again on a 2-socket, 240-core
> Clearwater Forest server running SPECjbb. In this case, 14% of CPU
> cycles were wasted on unnecessary acquisitions of
> sched_balance_running. This reinforces the need for the change, and we
> hope it can be merged.
> 
> Tim
> 
> ---
> 
> During load balancing, balancing at the LLC level and above must be
> serialized. The scheduler currently checks the atomic
> `sched_balance_running` flag before verifying whether a balance is
> actually due. This causes high contention, as multiple CPUs may attempt
> to acquire the flag concurrently.
> 
> On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
> and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
> operations for `sched_balance_running`. In most cases, the attempt
> aborts immediately after acquisition because the load balance time is
> not yet due.
> 
> Fix this by checking whether a balance is due *before* trying to
> acquire `sched_balance_running`. This avoids many wasted acquisitions
> and reduces the cmpxchg overhead in `sched_balance_domain()` from 7.6%
> to 0.05%. As a result, OLTP throughput improves by 11%.
> 
> Reviewed-by: Chen Yu <yu.c.chen@intel.com>
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> ---

Hi Tim.

Fine by me. unnecessary atomic operations do hurt on large systems.
The further optimization that i pointed out can come in later i guess.
That would help only further. this should be good to begin with.

With that.
Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Tim Chen]

On Fri, 2025-10-03 at 10:53 +0530, Shrikanth Hegde wrote:
> 
> On 10/3/25 4:30 AM, Tim Chen wrote:
> > Repost comments:
> > 
> > There have been past discussions about avoiding serialization in load
> > balancing, but no objections were raised to this patch itself during
> > its last posting:
> > https://lore.kernel.org/lkml/20250416035823.1846307-1-tim.c.chen@linux.intel.com/
> > 
> > Vincent and Chen Yu have already provided their Reviewed-by tags.
> > 
> > We recently encountered this issue again on a 2-socket, 240-core
> > Clearwater Forest server running SPECjbb. In this case, 14% of CPU
> > cycles were wasted on unnecessary acquisitions of
> > sched_balance_running. This reinforces the need for the change, and we
> > hope it can be merged.
> > 
> > Tim
> > 
> > ---
> > 
> > During load balancing, balancing at the LLC level and above must be
> > serialized. The scheduler currently checks the atomic
> > `sched_balance_running` flag before verifying whether a balance is
> > actually due. This causes high contention, as multiple CPUs may attempt
> > to acquire the flag concurrently.
> > 
> > On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
> > and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
> > operations for `sched_balance_running`. In most cases, the attempt
> > aborts immediately after acquisition because the load balance time is
> > not yet due.
> > 
> > Fix this by checking whether a balance is due *before* trying to
> > acquire `sched_balance_running`. This avoids many wasted acquisitions
> > and reduces the cmpxchg overhead in `sched_balance_domain()` from 7.6%
> > to 0.05%. As a result, OLTP throughput improves by 11%.
> > 
> > Reviewed-by: Chen Yu <yu.c.chen@intel.com>
> > Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> > Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> > ---
> 
> Hi Tim.
> 
> Fine by me. unnecessary atomic operations do hurt on large systems.
> The further optimization that i pointed out can come in later i guess.
> That would help only further. this should be good to begin with.

Thanks for your review and your past comments. We'll look into further
optimization if we find that this became a hot path again.
For now this change seemed to be good enough.


Tim

> 
> With that.
> Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> 

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Peter Zijlstra]

On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:

> During load balancing, balancing at the LLC level and above must be
> serialized. 

I would argue the wording here, there is no *must*, they *are*. Per the
current rules SD_NUMA and up get SD_SERIALIZE.

This is a *very* old thing, done by Christoph Lameter back when he was
at SGI. I'm not sure this default is still valid or not. Someone would
have to investigate. An alternative would be moving it into
node_reclaim_distance or somesuch.

> The scheduler currently checks the atomic
> `sched_balance_running` flag before verifying whether a balance is
> actually due. This causes high contention, as multiple CPUs may attempt
> to acquire the flag concurrently.

Right.

> On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
> and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
> operations for `sched_balance_running`. In most cases, the attempt
> aborts immediately after acquisition because the load balance time is
> not yet due.

So I'm not sure I understand the situation, @continue_balancing should
limit this concurrency to however many groups are on this domain -- your
granite thing with SNC on would have something like 6 groups?

> Fix this by checking whether a balance is due *before* trying to
> acquire `sched_balance_running`. This avoids many wasted acquisitions
> and reduces the cmpxchg overhead in `sched_balance_domain()` from 7.6%
> to 0.05%. As a result, OLTP throughput improves by 11%.

Yeah, I see no harm flipping this, but the Changelog needs help.

> Reviewed-by: Chen Yu <yu.c.chen@intel.com>
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> ---
>  kernel/sched/fair.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8ce56a8d507f..bedd785c4a39 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12126,13 +12126,13 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  
>  		interval = get_sd_balance_interval(sd, busy);
>  
> -		need_serialize = sd->flags & SD_SERIALIZE;
> -		if (need_serialize) {
> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> -				goto out;
> -		}
> -
>  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
> +			need_serialize = sd->flags & SD_SERIALIZE;
> +			if (need_serialize) {
> +				if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> +					goto out;
> +			}
> +
>  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>  				/*
>  				 * The LBF_DST_PINNED logic could have changed
> @@ -12144,9 +12144,9 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  			}
>  			sd->last_balance = jiffies;
>  			interval = get_sd_balance_interval(sd, busy);
> +			if (need_serialize)
> +				atomic_set_release(&sched_balance_running, 0);
>  		}
> -		if (need_serialize)
> -			atomic_set_release(&sched_balance_running, 0);
>  out:
>  		if (time_after(next_balance, sd->last_balance + interval)) {
>  			next_balance = sd->last_balance + interval;

Instead of making the indenting worse, could we make it better?

---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e743d9d0576c..6318834ff42a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12215,6 +12215,8 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 		}
 
 		interval = get_sd_balance_interval(sd, busy);
+		if (time_before(jiffies, sd->last_balance + interval))
+			goto out;
 
 		need_serialize = sd->flags & SD_SERIALIZE;
 		if (need_serialize) {
@@ -12222,19 +12224,18 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 				goto out;
 		}
 
-		if (time_after_eq(jiffies, sd->last_balance + interval)) {
-			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
-				/*
-				 * The LBF_DST_PINNED logic could have changed
-				 * env->dst_cpu, so we can't know our idle
-				 * state even if we migrated tasks. Update it.
-				 */
-				idle = idle_cpu(cpu);
-				busy = !idle && !sched_idle_cpu(cpu);
-			}
-			sd->last_balance = jiffies;
-			interval = get_sd_balance_interval(sd, busy);
+		if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
+			/*
+			 * The LBF_DST_PINNED logic could have changed
+			 * env->dst_cpu, so we can't know our idle
+			 * state even if we migrated tasks. Update it.
+			 */
+			idle = idle_cpu(cpu);
+			busy = !idle && !sched_idle_cpu(cpu);
 		}
+		sd->last_balance = jiffies;
+		interval = get_sd_balance_interval(sd, busy);
+
 		if (need_serialize)
 			atomic_set_release(&sched_balance_running, 0);
 out:

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Chen, Yu C]

On 10/13/2025 10:26 PM, Peter Zijlstra wrote:
> On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:
> 
>> During load balancing, balancing at the LLC level and above must be
>> serialized.
> 
> I would argue the wording here, there is no *must*, they *are*. Per the
> current rules SD_NUMA and up get SD_SERIALIZE.
> 
> This is a *very* old thing, done by Christoph Lameter back when he was
> at SGI. I'm not sure this default is still valid or not. Someone would
> have to investigate. An alternative would be moving it into
> node_reclaim_distance or somesuch.
> 

Do you mean the following:

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 444bdfdab731..436c899d8da2 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1697,11 +1697,16 @@ sd_init(struct sched_domain_topology_level *tl,
                 sd->cache_nice_tries = 2;

                 sd->flags &= ~SD_PREFER_SIBLING;
-               sd->flags |= SD_SERIALIZE;
                 if (sched_domains_numa_distance[tl->numa_level] > 
node_reclaim_distance) {
                         sd->flags &= ~(SD_BALANCE_EXEC |
                                        SD_BALANCE_FORK |
                                        SD_WAKE_AFFINE);
+                       /*
+                        * Nodes that are far away need to be serialized to
+                        * reduce the overhead of long-distance task 
migration
+                        * caused by load balancing.
+                        */
+                       sd->flags |= SD_SERIALIZE;
                 }

We can launch some tests to see if removing SD_SERIALIZE would
bring any impact.

>> On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
>> and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
>> operations for `sched_balance_running`. In most cases, the attempt
>> aborts immediately after acquisition because the load balance time is
>> not yet due.
> 
> So I'm not sure I understand the situation, @continue_balancing should
> limit this concurrency to however many groups are on this domain -- your
> granite thing with SNC on would have something like 6 groups?
> 

My understanding is that, continue_balancing is set to false after
atomic_cmpxhg(sched_balance_running), so if sched_balance_domains()
is invoked by many CPUs in parallel, the atomic operation still compete?

thanks,
Chenyu

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/13/25 10:02 PM, Chen, Yu C wrote:
> On 10/13/2025 10:26 PM, Peter Zijlstra wrote:
>> On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:
>>
>>> During load balancing, balancing at the LLC level and above must be
>>> serialized.
>>
>> I would argue the wording here, there is no *must*, they *are*. Per the
>> current rules SD_NUMA and up get SD_SERIALIZE.
>>
>> This is a *very* old thing, done by Christoph Lameter back when he was
>> at SGI. I'm not sure this default is still valid or not. Someone would
>> have to investigate. An alternative would be moving it into
>> node_reclaim_distance or somesuch.
>>
> 
> Do you mean the following:
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 444bdfdab731..436c899d8da2 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1697,11 +1697,16 @@ sd_init(struct sched_domain_topology_level *tl,
>                  sd->cache_nice_tries = 2;
> 
>                  sd->flags &= ~SD_PREFER_SIBLING;
> -               sd->flags |= SD_SERIALIZE;
>                  if (sched_domains_numa_distance[tl->numa_level] > 
> node_reclaim_distance) {
>                          sd->flags &= ~(SD_BALANCE_EXEC |
>                                         SD_BALANCE_FORK |
>                                         SD_WAKE_AFFINE);
> +                       /*
> +                        * Nodes that are far away need to be serialized to
> +                        * reduce the overhead of long-distance task 
> migration
> +                        * caused by load balancing.
> +                        */
> +                       sd->flags |= SD_SERIALIZE;
>                  }
> 
> We can launch some tests to see if removing SD_SERIALIZE would
> bring any impact.
> 
>>> On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
>>> and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
>>> operations for `sched_balance_running`. In most cases, the attempt
>>> aborts immediately after acquisition because the load balance time is
>>> not yet due.
>>
>> So I'm not sure I understand the situation, @continue_balancing should
>> limit this concurrency to however many groups are on this domain -- your
>> granite thing with SNC on would have something like 6 groups?
>>
> 
> My understanding is that, continue_balancing is set to false after
> atomic_cmpxhg(sched_balance_running), so if sched_balance_domains()
> is invoked by many CPUs in parallel, the atomic operation still compete?
> 

 From what i could remember,

This mostly always happens at SMT after which continue_balancing = 0.
Since multiple CPUs end up calling it (specially on busy system)
it causes a lot cacheline bouncing. and ends up showing in perf profile.

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Chen, Yu C]

On 10/14/2025 12:41 AM, Shrikanth Hegde wrote:
> 
> 
> On 10/13/25 10:02 PM, Chen, Yu C wrote:
>> On 10/13/2025 10:26 PM, Peter Zijlstra wrote:
>>> On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:
>>>
>>>> During load balancing, balancing at the LLC level and above must be
>>>> serialized.
>>>
>>> I would argue the wording here, there is no *must*, they *are*. Per the
>>> current rules SD_NUMA and up get SD_SERIALIZE.
>>>
>>> This is a *very* old thing, done by Christoph Lameter back when he was
>>> at SGI. I'm not sure this default is still valid or not. Someone would
>>> have to investigate. An alternative would be moving it into
>>> node_reclaim_distance or somesuch.
>>>
>>
>> Do you mean the following:
>>
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index 444bdfdab731..436c899d8da2 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -1697,11 +1697,16 @@ sd_init(struct sched_domain_topology_level *tl,
>>                  sd->cache_nice_tries = 2;
>>
>>                  sd->flags &= ~SD_PREFER_SIBLING;
>> -               sd->flags |= SD_SERIALIZE;
>>                  if (sched_domains_numa_distance[tl->numa_level] > 
>> node_reclaim_distance) {
>>                          sd->flags &= ~(SD_BALANCE_EXEC |
>>                                         SD_BALANCE_FORK |
>>                                         SD_WAKE_AFFINE);
>> +                       /*
>> +                        * Nodes that are far away need to be 
>> serialized to
>> +                        * reduce the overhead of long-distance task 
>> migration
>> +                        * caused by load balancing.
>> +                        */
>> +                       sd->flags |= SD_SERIALIZE;
>>                  }
>>
>> We can launch some tests to see if removing SD_SERIALIZE would
>> bring any impact.
>>
>>>> On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
>>>> and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
>>>> operations for `sched_balance_running`. In most cases, the attempt
>>>> aborts immediately after acquisition because the load balance time is
>>>> not yet due.
>>>
>>> So I'm not sure I understand the situation, @continue_balancing should
>>> limit this concurrency to however many groups are on this domain -- your
>>> granite thing with SNC on would have something like 6 groups?
>>>
>>
>> My understanding is that, continue_balancing is set to false after
>> atomic_cmpxhg(sched_balance_running), so if sched_balance_domains()
>> is invoked by many CPUs in parallel, the atomic operation still compete?
>>
> 
>  From what i could remember,
> 
> This mostly always happens at SMT after which continue_balancing = 0.
> Since multiple CPUs end up calling it (specially on busy system)
> it causes a lot cacheline bouncing. and ends up showing in perf profile.
> 

I see, when reaching NUMA domain, the continue_balancing  should already
be set to 0. Thanks for pointing it out.

thanks,
Chenyu

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Peter Zijlstra]

On Tue, Oct 14, 2025 at 12:32:57AM +0800, Chen, Yu C wrote:
> On 10/13/2025 10:26 PM, Peter Zijlstra wrote:
> > On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:
> > 
> > > During load balancing, balancing at the LLC level and above must be
> > > serialized.
> > 
> > I would argue the wording here, there is no *must*, they *are*. Per the
> > current rules SD_NUMA and up get SD_SERIALIZE.
> > 
> > This is a *very* old thing, done by Christoph Lameter back when he was
> > at SGI. I'm not sure this default is still valid or not. Someone would
> > have to investigate. An alternative would be moving it into
> > node_reclaim_distance or somesuch.
> > 
> 
> Do you mean the following:
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 444bdfdab731..436c899d8da2 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1697,11 +1697,16 @@ sd_init(struct sched_domain_topology_level *tl,
>                 sd->cache_nice_tries = 2;
> 
>                 sd->flags &= ~SD_PREFER_SIBLING;
> -               sd->flags |= SD_SERIALIZE;
>                 if (sched_domains_numa_distance[tl->numa_level] >
> node_reclaim_distance) {
>                         sd->flags &= ~(SD_BALANCE_EXEC |
>                                        SD_BALANCE_FORK |
>                                        SD_WAKE_AFFINE);
> +                       /*
> +                        * Nodes that are far away need to be serialized to
> +                        * reduce the overhead of long-distance task
> migration
> +                        * caused by load balancing.
> +                        */
> +                       sd->flags |= SD_SERIALIZE;
>                 }
> 
> We can launch some tests to see if removing SD_SERIALIZE would
> bring any impact.

Yeah, something like that. But lets first get this other thing sorted. I
agree that the SD_SERIALIZE thing is in the wrong place.

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Tim Chen]

On Mon, 2025-10-13 at 16:26 +0200, Peter Zijlstra wrote:
> On Thu, Oct 02, 2025 at 04:00:12PM -0700, Tim Chen wrote:
> 
> > During load balancing, balancing at the LLC level and above must be
> > serialized. 
> 
> I would argue the wording here, there is no *must*, they *are*. Per the
> current rules SD_NUMA and up get SD_SERIALIZE.
> 
> This is a *very* old thing, done by Christoph Lameter back when he was
> at SGI. I'm not sure this default is still valid or not. Someone would
> have to investigate. An alternative would be moving it into
> node_reclaim_distance or somesuch.
> 
> > The scheduler currently checks the atomic
> > `sched_balance_running` flag before verifying whether a balance is
> > actually due. This causes high contention, as multiple CPUs may attempt
> > to acquire the flag concurrently.
> 
> Right.
> 
> > On a 2-socket Granite Rapids system with sub-NUMA clustering enabled
> > and running OLTP workloads, 7.6% of CPU cycles were spent on cmpxchg
> > operations for `sched_balance_running`. In most cases, the attempt
> > aborts immediately after acquisition because the load balance time is
> > not yet due.
> 
> So I'm not sure I understand the situation, @continue_balancing should
> limit this concurrency to however many groups are on this domain -- your
> granite thing with SNC on would have something like 6 groups?

That's a good point.  But I think the contention is worse than
6 CPUs.

The hierarchy would be

SMT
NUMA-level1
NUMA-level2
NUMA-level3
NUMA-level4

There would be multiple CPUs in that are first in the SMT group
with continue_balancing=1 going up in the hierachy and
attempting the cmpxchg in the first NUMA domain level,
before calling should_we_balance() and finding that they are
not the first in the NUMA domain and set continue_balancing=0
and abort. Those CPUS are in same L3.
But at the same time, there could be CPUs in other sockets
cmpxchg on sched_balance_running.

In our experiment, we have not applied
NUMA hierarchy fix [1]. The system can have up to 4 NUMA domains,
with 3 NUMA levels spanning different sockets.
That means there can be up to three concurrent cmpxchg attempts
per level from CPUs in separate sockets. So that would make
things worse.

[1] https://lore.kernel.org/lkml/cover.1759515405.git.tim.c.chen@linux.intel.com/

> 
> > Fix this by checking whether a balance is due *before* trying to
> > acquire `sched_balance_running`. This avoids many wasted acquisitions
> > and reduces the cmpxchg overhead in `sched_balance_domain()` from 7.6%
> > to 0.05%. As a result, OLTP throughput improves by 11%.
> 
> Yeah, I see no harm flipping this, but the Changelog needs help.
> 
> > Reviewed-by: Chen Yu <yu.c.chen@intel.com>
> > Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> > Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> > ---
> >  kernel/sched/fair.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 8ce56a8d507f..bedd785c4a39 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -12126,13 +12126,13 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >  
> >  		interval = get_sd_balance_interval(sd, busy);
> >  
> > -		need_serialize = sd->flags & SD_SERIALIZE;
> > -		if (need_serialize) {
> > -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> > -				goto out;
> > -		}
> > -
> >  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
> > +			need_serialize = sd->flags & SD_SERIALIZE;
> > +			if (need_serialize) {
> > +				if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> > +					goto out;
> > +			}
> > +
> >  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
> >  				/*
> >  				 * The LBF_DST_PINNED logic could have changed
> > @@ -12144,9 +12144,9 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >  			}
> >  			sd->last_balance = jiffies;
> >  			interval = get_sd_balance_interval(sd, busy);
> > +			if (need_serialize)
> > +				atomic_set_release(&sched_balance_running, 0);
> >  		}
> > -		if (need_serialize)
> > -			atomic_set_release(&sched_balance_running, 0);
> >  out:
> >  		if (time_after(next_balance, sd->last_balance + interval)) {
> >  			next_balance = sd->last_balance + interval;
> 
> Instead of making the indenting worse, could we make it better?

Yes, this is much better.  Thanks.

Tim

> 
> ---
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e743d9d0576c..6318834ff42a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12215,6 +12215,8 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  		}
>  
>  		interval = get_sd_balance_interval(sd, busy);
> +		if (time_before(jiffies, sd->last_balance + interval))
> +			goto out;
>  
>  		need_serialize = sd->flags & SD_SERIALIZE;
>  		if (need_serialize) {
> @@ -12222,19 +12224,18 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  				goto out;
>  		}
>  
> -		if (time_after_eq(jiffies, sd->last_balance + interval)) {
> -			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
> -				/*
> -				 * The LBF_DST_PINNED logic could have changed
> -				 * env->dst_cpu, so we can't know our idle
> -				 * state even if we migrated tasks. Update it.
> -				 */
> -				idle = idle_cpu(cpu);
> -				busy = !idle && !sched_idle_cpu(cpu);
> -			}
> -			sd->last_balance = jiffies;
> -			interval = get_sd_balance_interval(sd, busy);
> +		if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
> +			/*
> +			 * The LBF_DST_PINNED logic could have changed
> +			 * env->dst_cpu, so we can't know our idle
> +			 * state even if we migrated tasks. Update it.
> +			 */
> +			idle = idle_cpu(cpu);
> +			busy = !idle && !sched_idle_cpu(cpu);
>  		}
> +		sd->last_balance = jiffies;
> +		interval = get_sd_balance_interval(sd, busy);
> +
>  		if (need_serialize)
>  			atomic_set_release(&sched_balance_running, 0);
>  out:

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Peter Zijlstra]

On Mon, Oct 13, 2025 at 02:54:19PM -0700, Tim Chen wrote:

> > So I'm not sure I understand the situation, @continue_balancing should
> > limit this concurrency to however many groups are on this domain -- your
> > granite thing with SNC on would have something like 6 groups?
> 
> That's a good point.  But I think the contention is worse than
> 6 CPUs.
> 
> The hierarchy would be
> 
> SMT
> NUMA-level1
> NUMA-level2
> NUMA-level3
> NUMA-level4

Aren't you missing the LLC/NODE domain here? We should have at least one
!SD_NUMA domain above SMT.

> There would be multiple CPUs in that are first in the SMT group
> with continue_balancing=1 going up in the hierachy and
> attempting the cmpxchg in the first NUMA domain level,
> before calling should_we_balance() and finding that they are
> not the first in the NUMA domain and set continue_balancing=0
> and abort. Those CPUS are in same L3.
> But at the same time, there could be CPUs in other sockets
> cmpxchg on sched_balance_running.

Right, Yu Chen said something like that as well, should_we_balance() is
too late.

Should we instead move the whole serialize thing inside
sched_balance_rq() like so:

---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bc0b7ce8a65d..e9f719ba17e1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11722,6 +11722,22 @@ static void update_lb_imbalance_stat(struct lb_env *env, struct sched_domain *sd
 	}
 }
 
+
+/*
+ * This flag serializes load-balancing passes over large domains
+ * (above the NODE topology level) - only one load-balancing instance
+ * may run at a time, to reduce overhead on very large systems with
+ * lots of CPUs and large NUMA distances.
+ *
+ * - Note that load-balancing passes triggered while another one
+ *   is executing are skipped and not re-tried.
+ *
+ * - Also note that this does not serialize rebalance_domains()
+ *   execution, as non-SD_SERIALIZE domains will still be
+ *   load-balanced in parallel.
+ */
+static atomic_t sched_balance_running = ATOMIC_INIT(0);
+
 /*
  * Check this_cpu to ensure it is balanced within domain. Attempt to move
  * tasks if there is an imbalance.
@@ -11747,6 +11763,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
 		.fbq_type	= all,
 		.tasks		= LIST_HEAD_INIT(env.tasks),
 	};
+	int need_unlock = false;
 
 	cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
 
@@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
 		goto out_balanced;
 	}
 
+	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
+		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
+			goto out_balanced;
+		need_unlock = true;
+	}
+
 	group = sched_balance_find_src_group(&env);
 	if (!group) {
 		schedstat_inc(sd->lb_nobusyg[idle]);
@@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
 	    sd->balance_interval < sd->max_interval)
 		sd->balance_interval *= 2;
 out:
+	if (need_unlock)
+		atomic_set_release(&sched_balance_running, 0);
+
 	return ld_moved;
 }
 
@@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
 	return 0;
 }
 
-/*
- * This flag serializes load-balancing passes over large domains
- * (above the NODE topology level) - only one load-balancing instance
- * may run at a time, to reduce overhead on very large systems with
- * lots of CPUs and large NUMA distances.
- *
- * - Note that load-balancing passes triggered while another one
- *   is executing are skipped and not re-tried.
- *
- * - Also note that this does not serialize rebalance_domains()
- *   execution, as non-SD_SERIALIZE domains will still be
- *   load-balanced in parallel.
- */
-static atomic_t sched_balance_running = ATOMIC_INIT(0);
-
 /*
  * Scale the max sched_balance_rq interval with the number of CPUs in the system.
  * This trades load-balance latency on larger machines for less cross talk.
@@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 	/* Earliest time when we have to do rebalance again */
 	unsigned long next_balance = jiffies + 60*HZ;
 	int update_next_balance = 0;
-	int need_serialize, need_decay = 0;
+	int need_decay = 0;
 	u64 max_cost = 0;
 
 	rcu_read_lock();
@@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 		}
 
 		interval = get_sd_balance_interval(sd, busy);
-
-		need_serialize = sd->flags & SD_SERIALIZE;
-		if (need_serialize) {
-			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
-				goto out;
-		}
-
 		if (time_after_eq(jiffies, sd->last_balance + interval)) {
 			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
 				/*
@@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
 			sd->last_balance = jiffies;
 			interval = get_sd_balance_interval(sd, busy);
 		}
-		if (need_serialize)
-			atomic_set_release(&sched_balance_running, 0);
-out:
+
 		if (time_after(next_balance, sd->last_balance + interval)) {
 			next_balance = sd->last_balance + interval;
 			update_next_balance = 1;

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/14/25 2:54 PM, Peter Zijlstra wrote:
> On Mon, Oct 13, 2025 at 02:54:19PM -0700, Tim Chen wrote:
> 
>>> So I'm not sure I understand the situation, @continue_balancing should
>>> limit this concurrency to however many groups are on this domain -- your
>>> granite thing with SNC on would have something like 6 groups?
>>
>> That's a good point.  But I think the contention is worse than
>> 6 CPUs.
>>
>> The hierarchy would be
>>
>> SMT
>> NUMA-level1
>> NUMA-level2
>> NUMA-level3
>> NUMA-level4
> 
> Aren't you missing the LLC/NODE domain here? We should have at least one
> !SD_NUMA domain above SMT.
> 
>> There would be multiple CPUs in that are first in the SMT group
>> with continue_balancing=1 going up in the hierachy and
>> attempting the cmpxchg in the first NUMA domain level,
>> before calling should_we_balance() and finding that they are
>> not the first in the NUMA domain and set continue_balancing=0
>> and abort. Those CPUS are in same L3.
>> But at the same time, there could be CPUs in other sockets
>> cmpxchg on sched_balance_running.
> 
> Right, Yu Chen said something like that as well, should_we_balance() is
> too late.
> 
> Should we instead move the whole serialize thing inside
> sched_balance_rq() like so:
> 
> ---
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bc0b7ce8a65d..e9f719ba17e1 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -11722,6 +11722,22 @@ static void update_lb_imbalance_stat(struct lb_env *env, struct sched_domain *sd
>   	}
>   }
>   
> +
> +/*
> + * This flag serializes load-balancing passes over large domains
> + * (above the NODE topology level) - only one load-balancing instance
> + * may run at a time, to reduce overhead on very large systems with
> + * lots of CPUs and large NUMA distances.
> + *
> + * - Note that load-balancing passes triggered while another one
> + *   is executing are skipped and not re-tried.
> + *
> + * - Also note that this does not serialize rebalance_domains()
> + *   execution, as non-SD_SERIALIZE domains will still be
> + *   load-balanced in parallel.
> + */
> +static atomic_t sched_balance_running = ATOMIC_INIT(0);
> +
>   /*
>    * Check this_cpu to ensure it is balanced within domain. Attempt to move
>    * tasks if there is an imbalance.
> @@ -11747,6 +11763,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   		.fbq_type	= all,
>   		.tasks		= LIST_HEAD_INIT(env.tasks),
>   	};
> +	int need_unlock = false;
>   
>   	cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
>   
> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   		goto out_balanced;
>   	}
>   
> +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
> +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> +			goto out_balanced;

Maybe goto out instead of out_balanced ?

> +		need_unlock = true;
> +	}
> +
>   	group = sched_balance_find_src_group(&env);
>   	if (!group) {
>   		schedstat_inc(sd->lb_nobusyg[idle]);
> @@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   	    sd->balance_interval < sd->max_interval)
>   		sd->balance_interval *= 2;
>   out:
> +	if (need_unlock)
> +		atomic_set_release(&sched_balance_running, 0);
> +
>   	return ld_moved;
>   }
>   
> @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
>   	return 0;
>   }
>   
> -/*
> - * This flag serializes load-balancing passes over large domains
> - * (above the NODE topology level) - only one load-balancing instance
> - * may run at a time, to reduce overhead on very large systems with
> - * lots of CPUs and large NUMA distances.
> - *
> - * - Note that load-balancing passes triggered while another one
> - *   is executing are skipped and not re-tried.
> - *
> - * - Also note that this does not serialize rebalance_domains()
> - *   execution, as non-SD_SERIALIZE domains will still be
> - *   load-balanced in parallel.
> - */
> -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> -
>   /*
>    * Scale the max sched_balance_rq interval with the number of CPUs in the system.
>    * This trades load-balance latency on larger machines for less cross talk.
> @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   	/* Earliest time when we have to do rebalance again */
>   	unsigned long next_balance = jiffies + 60*HZ;
>   	int update_next_balance = 0;
> -	int need_serialize, need_decay = 0;
> +	int need_decay = 0;
>   	u64 max_cost = 0;
>   
>   	rcu_read_lock();
> @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   		}
>   
>   		interval = get_sd_balance_interval(sd, busy);
> -
> -		need_serialize = sd->flags & SD_SERIALIZE;
> -		if (need_serialize) {
> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> -				goto out;
> -		}
> -
>   		if (time_after_eq(jiffies, sd->last_balance + interval)) {
>   			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>   				/*
> @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   			sd->last_balance = jiffies;
>   			interval = get_sd_balance_interval(sd, busy);
>   		}
> -		if (need_serialize)
> -			atomic_set_release(&sched_balance_running, 0);
> -out:
> +
>   		if (time_after(next_balance, sd->last_balance + interval)) {
>   			next_balance = sd->last_balance + interval;
>   			update_next_balance = 1;

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Peter Zijlstra]

On Tue, Oct 14, 2025 at 03:03:41PM +0530, Shrikanth Hegde wrote:

> > @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
> >   		goto out_balanced;
> >   	}
> > +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
> > +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> > +			goto out_balanced;
> 
> Maybe goto out instead of out_balanced ?

That would be inconsistent with the !should_we_balance() goto
out_balanced right above this, no?

> > +		need_unlock = true;
> > +	}
> > +
> >   	group = sched_balance_find_src_group(&env);
> >   	if (!group) {
> >   		schedstat_inc(sd->lb_nobusyg[idle]);
> > @@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
> >   	    sd->balance_interval < sd->max_interval)
> >   		sd->balance_interval *= 2;
> >   out:
> > +	if (need_unlock)
> > +		atomic_set_release(&sched_balance_running, 0);
> > +
> >   	return ld_moved;
> >   }
> > @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
> >   	return 0;
> >   }
> > -/*
> > - * This flag serializes load-balancing passes over large domains
> > - * (above the NODE topology level) - only one load-balancing instance
> > - * may run at a time, to reduce overhead on very large systems with
> > - * lots of CPUs and large NUMA distances.
> > - *
> > - * - Note that load-balancing passes triggered while another one
> > - *   is executing are skipped and not re-tried.
> > - *
> > - * - Also note that this does not serialize rebalance_domains()
> > - *   execution, as non-SD_SERIALIZE domains will still be
> > - *   load-balanced in parallel.
> > - */
> > -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> > -
> >   /*
> >    * Scale the max sched_balance_rq interval with the number of CPUs in the system.
> >    * This trades load-balance latency on larger machines for less cross talk.
> > @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >   	/* Earliest time when we have to do rebalance again */
> >   	unsigned long next_balance = jiffies + 60*HZ;
> >   	int update_next_balance = 0;
> > -	int need_serialize, need_decay = 0;
> > +	int need_decay = 0;
> >   	u64 max_cost = 0;
> >   	rcu_read_lock();
> > @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >   		}
> >   		interval = get_sd_balance_interval(sd, busy);
> > -
> > -		need_serialize = sd->flags & SD_SERIALIZE;
> > -		if (need_serialize) {
> > -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> > -				goto out;
> > -		}
> > -
> >   		if (time_after_eq(jiffies, sd->last_balance + interval)) {
> >   			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
> >   				/*
> > @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >   			sd->last_balance = jiffies;
> >   			interval = get_sd_balance_interval(sd, busy);
> >   		}
> > -		if (need_serialize)
> > -			atomic_set_release(&sched_balance_running, 0);
> > -out:
> > +
> >   		if (time_after(next_balance, sd->last_balance + interval)) {
> >   			next_balance = sd->last_balance + interval;
> >   			update_next_balance = 1;
> 

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/14/25 3:12 PM, Peter Zijlstra wrote:
> On Tue, Oct 14, 2025 at 03:03:41PM +0530, Shrikanth Hegde wrote:
> 
>>> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>>>    		goto out_balanced;
>>>    	}
>>> +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
>>> +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
>>> +			goto out_balanced;
>>
>> Maybe goto out instead of out_balanced ?
> 
> That would be inconsistent with the !should_we_balance() goto
> out_balanced right above this, no?

Yes. But whats the reason for saying out_balanced for !should_we_balance?

Load balance wasn't even attempted there right? Could this be updating it wrongly?
At-least comments around out_all_pinned doesn't make sense if we came here via !swb

         schedstat_inc(sd->lb_balanced[idle]);
         sd->nr_balance_failed = 0;


> 
>>> +		need_unlock = true;
>>> +	}
>>> +
>>>    	group = sched_balance_find_src_group(&env);
>>>    	if (!group) {
>>>    		schedstat_inc(sd->lb_nobusyg[idle]);
>>> @@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>>>    	    sd->balance_interval < sd->max_interval)
>>>    		sd->balance_interval *= 2;
>>>    out:
>>> +	if (need_unlock)
>>> +		atomic_set_release(&sched_balance_running, 0);
>>> +
>>>    	return ld_moved;
>>>    }
>>> @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
>>>    	return 0;
>>>    }
>>> -/*
>>> - * This flag serializes load-balancing passes over large domains
>>> - * (above the NODE topology level) - only one load-balancing instance
>>> - * may run at a time, to reduce overhead on very large systems with
>>> - * lots of CPUs and large NUMA distances.
>>> - *
>>> - * - Note that load-balancing passes triggered while another one
>>> - *   is executing are skipped and not re-tried.
>>> - *
>>> - * - Also note that this does not serialize rebalance_domains()
>>> - *   execution, as non-SD_SERIALIZE domains will still be
>>> - *   load-balanced in parallel.
>>> - */
>>> -static atomic_t sched_balance_running = ATOMIC_INIT(0);
>>> -
>>>    /*
>>>     * Scale the max sched_balance_rq interval with the number of CPUs in the system.
>>>     * This trades load-balance latency on larger machines for less cross talk.
>>> @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>>>    	/* Earliest time when we have to do rebalance again */
>>>    	unsigned long next_balance = jiffies + 60*HZ;
>>>    	int update_next_balance = 0;
>>> -	int need_serialize, need_decay = 0;
>>> +	int need_decay = 0;
>>>    	u64 max_cost = 0;
>>>    	rcu_read_lock();
>>> @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>>>    		}
>>>    		interval = get_sd_balance_interval(sd, busy);
>>> -
>>> -		need_serialize = sd->flags & SD_SERIALIZE;
>>> -		if (need_serialize) {
>>> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
>>> -				goto out;
>>> -		}
>>> -
>>>    		if (time_after_eq(jiffies, sd->last_balance + interval)) {
>>>    			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>>>    				/*
>>> @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>>>    			sd->last_balance = jiffies;
>>>    			interval = get_sd_balance_interval(sd, busy);
>>>    		}
>>> -		if (need_serialize)
>>> -			atomic_set_release(&sched_balance_running, 0);
>>> -out:
>>> +
>>>    		if (time_after(next_balance, sd->last_balance + interval)) {
>>>    			next_balance = sd->last_balance + interval;
>>>    			update_next_balance = 1;
>>

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/14/25 3:12 PM, Peter Zijlstra wrote:
> On Tue, Oct 14, 2025 at 03:03:41PM +0530, Shrikanth Hegde wrote:
> 
>>> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>>>    		goto out_balanced;
>>>    	}
>>> +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
>>> +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
>>> +			goto out_balanced;
>>
>> Maybe goto out instead of out_balanced ?
> 
> That would be inconsistent with the !should_we_balance() goto
> out_balanced right above this, no?
> 
Hi Peter.

Did similar probe points numbers compared to this. Even the patch is quite similar to what
was suggested there a while ago.
https://lore.kernel.org/all/41e11090-a100-48a7-a0dd-c989772822d7@linux.ibm.com/

480 CPUs system with 6 NUMA nodes. (different system than last time)

tl;dr

- Number of time sched_balance_running is taken is way less after the swb check. (which is great)
- Number of time it fails to set is very low after swb. (So out_balanced vs out may not make a
significant difference.)
- Patch is at the end. It is this patch + redo stuff + (ref_variable_stuff(ignore))


--- detailed log----

++++++++++++ probe points +++++++++++++++
(added a ref("crap") so i could put a probe where i want )

       0  static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
...
      20                 max_cost += sd->max_newidle_lb_cost;
          
                         /*
                          * Stop the load balance at this level. There is another
                          * CPU in our sched group which is doing load balancing more
                          * actively.
                          */
                         if (!continue_balancing) {
                                 if (need_decay)
                                         continue;
                                 break;
                         }
      33                 if (sd->flags & SD_SERIALIZE)
      34                         ref = ref + 5;



<sched_balance_rq@/home/shrikanth/sched_tip/kernel/sched/fair.c:0>
       0  static int sched_balance_rq(int this_cpu, struct rq *this_rq,
                                 struct sched_domain *sd, enum cpu_idle_type idle,
                                 int *continue_balancing)
...
                 int need_unlock = false;
          
                 cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
          
      25         schedstat_inc(sd->lb_count[idle]);
...
      34         if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
      35                 if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1)) {
      36                         ref = ref+1;
      37                         goto out_balanced;
                         }
      39                 ref = ref + 2;
      40                 need_unlock = true;
...
                                         env.loop_break = SCHED_NR_MIGRATE_BREAK;
     167                                 if (need_unlock) {
     168                                         ref = ref+3;
     169                                         atomic_set_release(&sched_balance_running, 0);
                                         }
                                         goto redo;
...
          out:
     287         if (need_unlock) {
     288                 ref = ref +4;
     289                 atomic_set_release(&sched_balance_running, 0);
                 }
          
                 return ld_moved;

   probe:sched_balance_domains_L34 (on sched_balance_domains:34@kernel/sched/fair.c)
   probe:sched_balance_rq_L168 (on sched_balance_rq:168@kernel/sched/fair.c)
   probe:sched_balance_rq_L21 (on sched_balance_rq+312@kernel/sched/fair.c)
   probe:sched_balance_rq_L288 (on sched_balance_rq+312@kernel/sched/fair.c)
   probe:sched_balance_rq_L35 (on sched_balance_rq+312@kernel/sched/fair.c)
   probe:sched_balance_rq_L36 (on sched_balance_rq+312@kernel/sched/fair.c)
   probe:sched_balance_rq_L39 (on sched_balance_rq+312@kernel/sched/fair.c)

+++++++++++ Data on various load points ++++++++++++++++++++++++
--- idle ---
perf stat -a -e probe:* sleep 10

              6,123      probe:sched_balance_domains_L34
             10,378      probe:sched_balance_rq_L21
                 79      probe:sched_balance_rq_L35
                 17      probe:sched_balance_rq_L36
                 62      probe:sched_balance_rq_L39
                  0      probe:sched_balance_rq_L168
                 62      probe:sched_balance_rq_L288

--- 25% load ---
perf stat -a -e probe:* stress-ng --cpu=480 -l 25 -t 10

            510,551      probe:sched_balance_domains_L34
            303,892      probe:sched_balance_rq_L21
                442      probe:sched_balance_rq_L35
                  3      probe:sched_balance_rq_L36
                439      probe:sched_balance_rq_L39
                  0      probe:sched_balance_rq_L168
                439      probe:sched_balance_rq_L288

--- 50% load ---

            248,969      probe:sched_balance_domains_L34
            187,864      probe:sched_balance_rq_L21
                926      probe:sched_balance_rq_L35
                  6      probe:sched_balance_rq_L36
                920      probe:sched_balance_rq_L39
                  0      probe:sched_balance_rq_L168
                920      probe:sched_balance_rq_L288

--- 75% load ---

            110,294      probe:sched_balance_domains_L34
             71,568      probe:sched_balance_rq_L21
                861      probe:sched_balance_rq_L35
                  6      probe:sched_balance_rq_L36
                855      probe:sched_balance_rq_L39
                  0      probe:sched_balance_rq_L168
                855      probe:sched_balance_rq_L288

--- 100% load ---

             85,960      probe:sched_balance_domains_L34
             48,169      probe:sched_balance_rq_L21
                 71      probe:sched_balance_rq_L35
                  4      probe:sched_balance_rq_L36
                 67      probe:sched_balance_rq_L39
                  0      probe:sched_balance_rq_L168
                 67      probe:sched_balance_rq_L288

++++++++++++++++++ patch ++++++++++++++++++++++++++++++++++++
(ignore ref crap)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index cee1793e8277..832104705500 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11722,10 +11722,29 @@ static void update_lb_imbalance_stat(struct lb_env *env, struct sched_domain *sd
  	}
  }
  
+
+/*
+ * This flag serializes load-balancing passes over large domains
+ * (above the NODE topology level) - only one load-balancing instance
+ * may run at a time, to reduce overhead on very large systems with
+ * lots of CPUs and large NUMA distances.
+ *
+ * - Note that load-balancing passes triggered while another one
+ *   is executing are skipped and not re-tried.
+ *
+ * - Also note that this does not serialize rebalance_domains()
+ *   execution, as non-SD_SERIALIZE domains will still be
+ *   load-balanced in parallel.
+ */
+static atomic_t sched_balance_running = ATOMIC_INIT(0);
+
  /*
   * Check this_cpu to ensure it is balanced within domain. Attempt to move
   * tasks if there is an imbalance.
   */
+
+int ref = 0;
+
  static int sched_balance_rq(int this_cpu, struct rq *this_rq,
  			struct sched_domain *sd, enum cpu_idle_type idle,
  			int *continue_balancing)
@@ -11747,10 +11766,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
  		.fbq_type	= all,
  		.tasks		= LIST_HEAD_INIT(env.tasks),
  	};
+	int need_unlock = false;
  
  	cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
  
  	schedstat_inc(sd->lb_count[idle]);
+	ref = 1;
  
  redo:
  	if (!should_we_balance(&env)) {
@@ -11758,6 +11779,15 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
  		goto out_balanced;
  	}
  
+	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
+		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1)) {
+			ref = ref+1;
+			goto out_balanced;
+		}
+		ref = ref + 2;
+		need_unlock = true;
+	}
+
  	group = sched_balance_find_src_group(&env);
  	if (!group) {
  		schedstat_inc(sd->lb_nobusyg[idle]);
@@ -11882,6 +11912,10 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
  			if (!cpumask_subset(cpus, env.dst_grpmask)) {
  				env.loop = 0;
  				env.loop_break = SCHED_NR_MIGRATE_BREAK;
+				if (need_unlock) {
+					ref = ref+3;
+					atomic_set_release(&sched_balance_running, 0);
+				}
  				goto redo;
  			}
  			goto out_all_pinned;
@@ -11998,6 +12032,11 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
  	    sd->balance_interval < sd->max_interval)
  		sd->balance_interval *= 2;
  out:
+	if (need_unlock) {
+		ref = ref +4;
+		atomic_set_release(&sched_balance_running, 0);
+	}
+
  	return ld_moved;
  }
  
@@ -12122,21 +12161,6 @@ static int active_load_balance_cpu_stop(void *data)
  	return 0;
  }
  
-/*
- * This flag serializes load-balancing passes over large domains
- * (above the NODE topology level) - only one load-balancing instance
- * may run at a time, to reduce overhead on very large systems with
- * lots of CPUs and large NUMA distances.
- *
- * - Note that load-balancing passes triggered while another one
- *   is executing are skipped and not re-tried.
- *
- * - Also note that this does not serialize rebalance_domains()
- *   execution, as non-SD_SERIALIZE domains will still be
- *   load-balanced in parallel.
- */
-static atomic_t sched_balance_running = ATOMIC_INIT(0);
-
  /*
   * Scale the max sched_balance_rq interval with the number of CPUs in the system.
   * This trades load-balance latency on larger machines for less cross talk.
@@ -12192,7 +12216,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
  	/* Earliest time when we have to do rebalance again */
  	unsigned long next_balance = jiffies + 60*HZ;
  	int update_next_balance = 0;
-	int need_serialize, need_decay = 0;
+	int need_decay = 0;
  	u64 max_cost = 0;
  
  	rcu_read_lock();
@@ -12215,14 +12239,10 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
  			break;
  		}
  
-		interval = get_sd_balance_interval(sd, busy);
-
-		need_serialize = sd->flags & SD_SERIALIZE;
-		if (need_serialize) {
-			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
-				goto out;
-		}
+		if (sd->flags & SD_SERIALIZE)
+			ref = ref + 5;
  
+		interval = get_sd_balance_interval(sd, busy);
  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
  				/*
@@ -12236,9 +12256,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
  			sd->last_balance = jiffies;
  			interval = get_sd_balance_interval(sd, busy);
  		}
-		if (need_serialize)
-			atomic_set_release(&sched_balance_running, 0);
-out:
+
  		if (time_after(next_balance, sd->last_balance + interval)) {
  			next_balance = sd->last_balance + interval;
  			update_next_balance = 1;

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/16/25 7:33 PM, Shrikanth Hegde wrote:
> 
> 
> On 10/14/25 3:12 PM, Peter Zijlstra wrote:
>> On Tue, Oct 14, 2025 at 03:03:41PM +0530, Shrikanth Hegde wrote:
>>
>>>> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, 
>>>> struct rq *this_rq,
>>>>            goto out_balanced;
>>>>        }
>>>> +    if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
>>>> +        if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
>>>> +            goto out_balanced;
>>>
>>> Maybe goto out instead of out_balanced ?
>>
>> That would be inconsistent with the !should_we_balance() goto
>> out_balanced right above this, no?
>>
> Hi Peter.
> 
> Did similar probe points numbers compared to this. Even the patch is 
> quite similar to what
> was suggested there a while ago.
> https://lore.kernel.org/all/41e11090-a100-48a7-a0dd- 
> c989772822d7@linux.ibm.com/
> 
> 480 CPUs system with 6 NUMA nodes. (different system than last time)
> 

Hi Peter, Tim.

How are we proceeding here? Should i send the below patch with changelog or
is it being worked out by either of you.

It is really beneficial for large systems to avoid un-neccessary cache bouncing.

> tl;dr
> 
> - Number of time sched_balance_running is taken is way less after the 
> swb check. (which is great)
> - Number of time it fails to set is very low after swb. (So out_balanced 
> vs out may not make a
> significant difference.)
> - Patch is at the end. It is this patch + redo stuff + 
> (ref_variable_stuff(ignore))
> 
>

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Srikar Dronamraju]

* Peter Zijlstra <peterz@infradead.org> [2025-10-14 11:24:36]:

> On Mon, Oct 13, 2025 at 02:54:19PM -0700, Tim Chen wrote:
> 
> 
> Right, Yu Chen said something like that as well, should_we_balance() is
> too late.
> 
> Should we instead move the whole serialize thing inside
> sched_balance_rq() like so:
> 
> @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
>  	return 0;
>  }
>  
> -/*
> - * This flag serializes load-balancing passes over large domains
> - * (above the NODE topology level) - only one load-balancing instance
> - * may run at a time, to reduce overhead on very large systems with
> - * lots of CPUs and large NUMA distances.
> - *
> - * - Note that load-balancing passes triggered while another one
> - *   is executing are skipped and not re-tried.
> - *
> - * - Also note that this does not serialize rebalance_domains()
> - *   execution, as non-SD_SERIALIZE domains will still be
> - *   load-balanced in parallel.
> - */
> -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> -
>  /*
>   * Scale the max sched_balance_rq interval with the number of CPUs in the system.
>   * This trades load-balance latency on larger machines for less cross talk.
> @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  	/* Earliest time when we have to do rebalance again */
>  	unsigned long next_balance = jiffies + 60*HZ;
>  	int update_next_balance = 0;
> -	int need_serialize, need_decay = 0;
> +	int need_decay = 0;
>  	u64 max_cost = 0;
>  
>  	rcu_read_lock();
> @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  		}
>  
>  		interval = get_sd_balance_interval(sd, busy);
> -
> -		need_serialize = sd->flags & SD_SERIALIZE;
> -		if (need_serialize) {
> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> -				goto out;
> -		}
> -
>  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
>  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>  				/*
> @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  			sd->last_balance = jiffies;
>  			interval = get_sd_balance_interval(sd, busy);
>  		}
> -		if (need_serialize)
> -			atomic_set_release(&sched_balance_running, 0);
> -out:
> +
>  		if (time_after(next_balance, sd->last_balance + interval)) {
>  			next_balance = sd->last_balance + interval;
>  			update_next_balance = 1;

I think this is better since previously the one CPU which was not suppose to
do the balancing may increment the atomic variable. If the CPU, that was
suppose to do the balance now tries it may fail since the variable was not
yet decremented.

-- 
Thanks and Regards
Srikar Dronamraju

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Peter Zijlstra]

On Tue, Oct 14, 2025 at 07:20:35PM +0530, Srikar Dronamraju wrote:
> * Peter Zijlstra <peterz@infradead.org> [2025-10-14 11:24:36]:
> 
> > On Mon, Oct 13, 2025 at 02:54:19PM -0700, Tim Chen wrote:
> > 
> > 
> > Right, Yu Chen said something like that as well, should_we_balance() is
> > too late.
> > 
> > Should we instead move the whole serialize thing inside
> > sched_balance_rq() like so:
> > 
> > @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
> >  	return 0;
> >  }
> >  
> > -/*
> > - * This flag serializes load-balancing passes over large domains
> > - * (above the NODE topology level) - only one load-balancing instance
> > - * may run at a time, to reduce overhead on very large systems with
> > - * lots of CPUs and large NUMA distances.
> > - *
> > - * - Note that load-balancing passes triggered while another one
> > - *   is executing are skipped and not re-tried.
> > - *
> > - * - Also note that this does not serialize rebalance_domains()
> > - *   execution, as non-SD_SERIALIZE domains will still be
> > - *   load-balanced in parallel.
> > - */
> > -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> > -
> >  /*
> >   * Scale the max sched_balance_rq interval with the number of CPUs in the system.
> >   * This trades load-balance latency on larger machines for less cross talk.
> > @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >  	/* Earliest time when we have to do rebalance again */
> >  	unsigned long next_balance = jiffies + 60*HZ;
> >  	int update_next_balance = 0;
> > -	int need_serialize, need_decay = 0;
> > +	int need_decay = 0;
> >  	u64 max_cost = 0;
> >  
> >  	rcu_read_lock();
> > @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >  		}
> >  
> >  		interval = get_sd_balance_interval(sd, busy);
> > -
> > -		need_serialize = sd->flags & SD_SERIALIZE;
> > -		if (need_serialize) {
> > -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> > -				goto out;
> > -		}
> > -
> >  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
> >  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
> >  				/*
> > @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
> >  			sd->last_balance = jiffies;
> >  			interval = get_sd_balance_interval(sd, busy);
> >  		}
> > -		if (need_serialize)
> > -			atomic_set_release(&sched_balance_running, 0);
> > -out:
> > +
> >  		if (time_after(next_balance, sd->last_balance + interval)) {
> >  			next_balance = sd->last_balance + interval;
> >  			update_next_balance = 1;
> 
> I think this is better since previously the one CPU which was not suppose to
> do the balancing may increment the atomic variable. If the CPU, that was
> suppose to do the balance now tries it may fail since the variable was not
> yet decremented.

Right, it would do that acquire and then still have at least 2 ways to
not actually balance, which is a waste.

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Shrikanth Hegde]

On 10/14/25 2:54 PM, Peter Zijlstra wrote:

> 
> Should we instead move the whole serialize thing inside
> sched_balance_rq() like so:
> 
> ---
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bc0b7ce8a65d..e9f719ba17e1 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -11722,6 +11722,22 @@ static void update_lb_imbalance_stat(struct lb_env *env, struct sched_domain *sd
>   	}
>   }
>   
> +
> +/*
> + * This flag serializes load-balancing passes over large domains
> + * (above the NODE topology level) - only one load-balancing instance
> + * may run at a time, to reduce overhead on very large systems with
> + * lots of CPUs and large NUMA distances.
> + *
> + * - Note that load-balancing passes triggered while another one
> + *   is executing are skipped and not re-tried.
> + *
> + * - Also note that this does not serialize rebalance_domains()
> + *   execution, as non-SD_SERIALIZE domains will still be
> + *   load-balanced in parallel.
> + */
> +static atomic_t sched_balance_running = ATOMIC_INIT(0);
> +
>   /*
>    * Check this_cpu to ensure it is balanced within domain. Attempt to move
>    * tasks if there is an imbalance.
> @@ -11747,6 +11763,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   		.fbq_type	= all,
>   		.tasks		= LIST_HEAD_INIT(env.tasks),
>   	};
> +	int need_unlock = false;
>   
>   	cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
>   
> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   		goto out_balanced;
>   	}
>   
> +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
> +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> +			goto out_balanced;
> +		need_unlock = true;
> +	}
> +
>   	group = sched_balance_find_src_group(&env);
>   	if (!group) {
>   		schedstat_inc(sd->lb_nobusyg[idle]);
> @@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>   	    sd->balance_interval < sd->max_interval)
>   		sd->balance_interval *= 2;
>   out:
> +	if (need_unlock)
> +		atomic_set_release(&sched_balance_running, 0);
> +
>   	return ld_moved;
>   }
>   
> @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
>   	return 0;
>   }
>   
> -/*
> - * This flag serializes load-balancing passes over large domains
> - * (above the NODE topology level) - only one load-balancing instance
> - * may run at a time, to reduce overhead on very large systems with
> - * lots of CPUs and large NUMA distances.
> - *
> - * - Note that load-balancing passes triggered while another one
> - *   is executing are skipped and not re-tried.
> - *
> - * - Also note that this does not serialize rebalance_domains()
> - *   execution, as non-SD_SERIALIZE domains will still be
> - *   load-balanced in parallel.
> - */
> -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> -
>   /*
>    * Scale the max sched_balance_rq interval with the number of CPUs in the system.
>    * This trades load-balance latency on larger machines for less cross talk.
> @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   	/* Earliest time when we have to do rebalance again */
>   	unsigned long next_balance = jiffies + 60*HZ;
>   	int update_next_balance = 0;
> -	int need_serialize, need_decay = 0;
> +	int need_decay = 0;
>   	u64 max_cost = 0;
>   
>   	rcu_read_lock();
> @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   		}
>   
>   		interval = get_sd_balance_interval(sd, busy);
> -
> -		need_serialize = sd->flags & SD_SERIALIZE;
> -		if (need_serialize) {
> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> -				goto out;
> -		}
> -
>   		if (time_after_eq(jiffies, sd->last_balance + interval)) {
>   			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>   				/*
> @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>   			sd->last_balance = jiffies;
>   			interval = get_sd_balance_interval(sd, busy);
>   		}
> -		if (need_serialize)
> -			atomic_set_release(&sched_balance_running, 0);
> -out:
> +
>   		if (time_after(next_balance, sd->last_balance + interval)) {
>   			next_balance = sd->last_balance + interval;
>   			update_next_balance = 1;

One thing is missing, need to reset to 0 for redo logic to work.

@@ -11882,6 +11905,8 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
                         if (!cpumask_subset(cpus, env.dst_grpmask)) {
                                 env.loop = 0;
                                 env.loop_break = SCHED_NR_MIGRATE_BREAK;
+                               if (need_unlock)
+                                       atomic_set_release(&sched_balance_running, 0);
                                 goto redo;

Re: [RESEND PATCH] sched/fair: Skip sched_balance_running cmpxchg when balance is not due

[Tim Chen]

On Tue, 2025-10-14 at 11:24 +0200, Peter Zijlstra wrote:
> On Mon, Oct 13, 2025 at 02:54:19PM -0700, Tim Chen wrote:
> 
> > > So I'm not sure I understand the situation, @continue_balancing should
> > > limit this concurrency to however many groups are on this domain -- your
> > > granite thing with SNC on would have something like 6 groups?
> > 
> > That's a good point.  But I think the contention is worse than
> > 6 CPUs.
> > 
> > The hierarchy would be
> > 
> > SMT
> > NUMA-level1
> > NUMA-level2
> > NUMA-level3
> > NUMA-level4
> 
> Aren't you missing the LLC/NODE domain here? We should have at least one
> !SD_NUMA domain above SMT.

Yeah, I should have said the MC level which contains SMT groups

SMT
MC
NUMA-level1
...

Actual dmesg log:

[    7.977893] CPU0 attaching sched-domain(s):
[    7.977897]  domain-0: span=0,192 level=SMT
[    7.977902]   groups: 0:{ span=0 cap=972 }, 192:{ span=192 cap=1022 }[    7.977907]   domain-1: span=0-31,192-223 level=MC[    7.977909]    groups: 0:{ span=0,192 cap=1994 }, 1:{ span=1,193
cap=2048 }, 2:{ span=2,194 cap=2047 }, 3:{ span=3,195 cap=2047 }, 4:{ span=4,196 cap=2048 }, 5:{ span=5,197 cap=2046 }, 6:{ span=6,198 cap=2047 }, 7:{ span=7,199 cap=2048 }, 8:{ span=8,200 cap=2047 },
9:{ span=9,201 cap=2046 }, 10:{ span=10,202 cap=2046 }, 11:{ span=11,203 cap=2046 }, 12:{ span=12,204 cap=2044 }, 13:{ span=13,205 cap=2048 }, 14:{ span=14,206 cap=2046 }, 15:{ span=15,207 cap=2043 },
16:{ span=16,208 cap=2046 }, 17:{ span=17,209 cap=2048 }, 18:{ span=18,210 cap=2046 }, 19:{ span=19,211 cap=2045 }, 20:{ span=20,212 cap=2046 }, 21:{ span=21,213 cap=2044 }, 22:{ span=22,214 cap=2045
}, 23:{ span=23,215 cap=2046 }, 24:{ span=24,216 cap=2045 }, 25:{ span=25,217 cap=2044 }, 26:{ span=26,218 cap=2046 }, 27:{ span=27,219 cap=2045 }, 28:{ span=28,220 cap=2045 }, 29:{ span=29,221
cap=2046 }, 30:{ span=30,222 cap=2045 }, 31:{ span=31,223 cap=2045 }
[    7.977956]    domain-2: span=0-63,192-255 level=NUMA
[    7.977958]     groups: 0:{ span=0-31,192-223 cap=65418 }, 32:{ span=32-63,224-255 cap=65453 }
[    7.977962]     domain-3: span=0-95,192-287 level=NUMA
[    7.977963]      groups: 0:{ span=0-63,192-255 mask=0-31,192-223 cap=130871 }, 64:{ span=32-95,224-287 mask=64-95,256-287 cap=130929 }
[    7.977968]      domain-4: span=0-127,192-319 level=NUMA
[    7.977970]       groups: 0:{ span=0-95,192-287 cap=196314 }, 96:{ span=96-127,288-319 cap=65439 }
[    7.977974]       domain-5: span=0-127,160-319,352-383 level=NUMA
[    7.977975]        groups: 0:{ span=0-127,192-319 mask=0-31,192-223 cap=261753 }, 160:{ span=160-191,352-383 cap=65430 }
[    7.977980]        domain-6: span=0-383 level=NUMA 

> 
> > There would be multiple CPUs in that are first in the SMT group
> > with continue_balancing=1 going up in the hierachy and
> > attempting the cmpxchg in the first NUMA domain level,
> > before calling should_we_balance() and finding that they are
> > not the first in the NUMA domain and set continue_balancing=0
> > and abort. Those CPUS are in same L3.
> > But at the same time, there could be CPUs in other sockets
> > cmpxchg on sched_balance_running.
> 
> Right, Yu Chen said something like that as well, should_we_balance() is
> too late.
> 
> Should we instead move the whole serialize thing inside
> sched_balance_rq() like so:

I think that makes sense. Probably a separate patch.

Tim

> 
> ---
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bc0b7ce8a65d..e9f719ba17e1 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -11722,6 +11722,22 @@ static void update_lb_imbalance_stat(struct lb_env *env, struct sched_domain *sd
>  	}
>  }
>  
> +
> +/*
> + * This flag serializes load-balancing passes over large domains
> + * (above the NODE topology level) - only one load-balancing instance
> + * may run at a time, to reduce overhead on very large systems with
> + * lots of CPUs and large NUMA distances.
> + *
> + * - Note that load-balancing passes triggered while another one
> + *   is executing are skipped and not re-tried.
> + *
> + * - Also note that this does not serialize rebalance_domains()
> + *   execution, as non-SD_SERIALIZE domains will still be
> + *   load-balanced in parallel.
> + */
> +static atomic_t sched_balance_running = ATOMIC_INIT(0);
> +
>  /*
>   * Check this_cpu to ensure it is balanced within domain. Attempt to move
>   * tasks if there is an imbalance.
> @@ -11747,6 +11763,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>  		.fbq_type	= all,
>  		.tasks		= LIST_HEAD_INIT(env.tasks),
>  	};
> +	int need_unlock = false;
>  
>  	cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
>  
> @@ -11758,6 +11775,12 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>  		goto out_balanced;
>  	}
>  
> +	if (idle != CPU_NEWLY_IDLE && (sd->flags & SD_SERIALIZE)) {
> +		if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> +			goto out_balanced;
> +		need_unlock = true;
> +	}
> +
>  	group = sched_balance_find_src_group(&env);
>  	if (!group) {
>  		schedstat_inc(sd->lb_nobusyg[idle]);
> @@ -11998,6 +12021,9 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
>  	    sd->balance_interval < sd->max_interval)
>  		sd->balance_interval *= 2;
>  out:
> +	if (need_unlock)
> +		atomic_set_release(&sched_balance_running, 0);
> +
>  	return ld_moved;
>  }
>  
> @@ -12122,21 +12148,6 @@ static int active_load_balance_cpu_stop(void *data)
>  	return 0;
>  }
>  
> -/*
> - * This flag serializes load-balancing passes over large domains
> - * (above the NODE topology level) - only one load-balancing instance
> - * may run at a time, to reduce overhead on very large systems with
> - * lots of CPUs and large NUMA distances.
> - *
> - * - Note that load-balancing passes triggered while another one
> - *   is executing are skipped and not re-tried.
> - *
> - * - Also note that this does not serialize rebalance_domains()
> - *   execution, as non-SD_SERIALIZE domains will still be
> - *   load-balanced in parallel.
> - */
> -static atomic_t sched_balance_running = ATOMIC_INIT(0);
> -
>  /*
>   * Scale the max sched_balance_rq interval with the number of CPUs in the system.
>   * This trades load-balance latency on larger machines for less cross talk.
> @@ -12192,7 +12203,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  	/* Earliest time when we have to do rebalance again */
>  	unsigned long next_balance = jiffies + 60*HZ;
>  	int update_next_balance = 0;
> -	int need_serialize, need_decay = 0;
> +	int need_decay = 0;
>  	u64 max_cost = 0;
>  
>  	rcu_read_lock();
> @@ -12216,13 +12227,6 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  		}
>  
>  		interval = get_sd_balance_interval(sd, busy);
> -
> -		need_serialize = sd->flags & SD_SERIALIZE;
> -		if (need_serialize) {
> -			if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
> -				goto out;
> -		}
> -
>  		if (time_after_eq(jiffies, sd->last_balance + interval)) {
>  			if (sched_balance_rq(cpu, rq, sd, idle, &continue_balancing)) {
>  				/*
> @@ -12236,9 +12240,7 @@ static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle)
>  			sd->last_balance = jiffies;
>  			interval = get_sd_balance_interval(sd, busy);
>  		}
> -		if (need_serialize)
> -			atomic_set_release(&sched_balance_running, 0);
> -out:
> +
>  		if (time_after(next_balance, sd->last_balance + interval)) {
>  			next_balance = sd->last_balance + interval;
>  			update_next_balance = 1;