sched/fair: Skip sched_balance_running cmpxchg when balance is not due

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/[email protected]/

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 <[email protected]>

Reviewed-by Vincent Guittot <[email protected]>

Signed-off-by Tim Chen <[email protected]>

---

Quoted 36 lines · 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/[email protected]/

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%.

Quoted 4 lines

Reviewed-by Chen Yu <[email protected]>

Reviewed-by Vincent Guittot <[email protected]>

Signed-off-by Tim Chen <[email protected]>

---

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 <[email protected]>

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 <[email protected]>

Reviewed-by Vincent Guittot <[email protected]>

Signed-off-by Tim Chen <[email protected]>

---
 kernel/sched/fair.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

kernel/sched/fair.c +6 -6

index 8ce56a8d507f..bedd785c4a39 100644
@@ -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)
 			}

Quoted 2 lines · Tim Chen

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.

Quoted 5 lines

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?

Quoted 5 lines · Peter Zijlstra

Quoted 2 lines · Tim Chen

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

Quoted 8 lines

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:

kernel/sched/topology.c +3 -1

index 444bdfdab731..436c899d8da2 100644
@@ -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.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 49 lines · Yu C

Quoted 5 lines · Peter Zijlstra

Quoted 2 lines · Tim Chen

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

Quoted 8 lines

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:

kernel/sched/topology.c +0 -1

index 444bdfdab731..436c899d8da2 100644
@@ -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.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 9 lines

Quoted 4 lines

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.

Quoted 5 lines · Peter Zijlstra

Quoted 2 lines · Tim Chen

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

Quoted 8 lines

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:

kernel/sched/topology.c +3 -1

index 444bdfdab731..436c899d8da2 100644
@@ -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

kernel/sched/topology.c +3 -0

+                        * caused by load balancing.
+                        */
+                       sd->flags |= SD_SERIALIZE;
                }

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

Quoted 2 lines · Tim Chen

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.

Quoted 4 lines

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.

Quoted 5 lines

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?

Quoted 5 lines

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.

Quoted 37 lines

Reviewed-by Chen Yu <[email protected]>

Reviewed-by Vincent Guittot <[email protected]>

Signed-off-by Tim Chen <[email protected]>

---
 kernel/sched/fair.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

kernel/sched/fair.c +8 -6

index 8ce56a8d507f..bedd785c4a39 100644
@@ -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);

kernel/sched/fair.c static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle) · line 12152

Quoted 6 lines

kernel/sched/fair.c +0 -2

 		}
-		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?

---

kernel/sched/fair.c +13 -12

index e743d9d0576c..6318834ff42a 100644
@@ -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:

Quoted 3 lines

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.

Quoted 7 lines · Tim Chen

Quoted 3 lines

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.

Quoted 7 lines

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.

Quoted 8 lines

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:

---

kernel/sched/fair.c +17 -0

index bc0b7ce8a65d..e9f719ba17e1 100644
@@ -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);
 

kernel/sched/fair.c +8 -26

+		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;

kernel/sched/fair.c static int sched_balance_rq(int this_cpu, struct rq *this_rq, · line 11779

Quoted 6 lines

kernel/sched/fair.c +3 -0

@@ -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 ?

Quoted 68 lines

kernel/sched/fair.c +8 -26

+		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;

Quoted 8 lines · Shrikanth Hegde

kernel/sched/fair.c static int sched_balance_rq(int this_cpu, struct rq *this_rq, · line 11779

Quoted 6 lines

kernel/sched/fair.c +3 -0

@@ -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?

Quoted 70 lines

Quoted 69 lines

kernel/sched/fair.c +8 -26

+		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;

Quoted 8 lines · Shrikanth Hegde

Review static int sched_balance_rq(int this_cpu, struct rq *this_rq, · line 11779

Quoted 6 lines

(quoted diff) +3 -0

@@ -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?

Quoted 12 lines · Peter Zijlstra

Quoted 9 lines · Shrikanth Hegde

kernel/sched/fair.c static int sched_balance_rq(int this_cpu, · line 11780

Quoted 7 lines

kernel/sched/fair.c +3 -0

@@ -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 ?

Quoted 3 lines

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- [email protected]/

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))

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:

kernel/sched/fair.c +1 -23

@@ -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)) {
 				/*

Quoted 62 lines · Peter Zijlstra

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:

kernel/sched/fair.c +2 -26

@@ -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:
+

kernel/sched/fair.c static void sched_balance_domains(struct rq *rq, enum cpu_idle_type idle) · line 12246

Quoted 3 lines

kernel/sched/fair.c +0 -0

 		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.

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

---

kernel/sched/fair.c +27 -23

index bc0b7ce8a65d..e9f719ba17e1 100644
@@ -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)) {
  				/*

Quoted 9 lines · Tim Chen

Quoted 3 lines

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

Quoted 5 lines

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.

Quoted 9 lines

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:

---

kernel/sched/fair.c +28 -26

index bc0b7ce8a65d..e9f719ba17e1 100644
@@ -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;