[PATCH 0/2] Fix imbalance issue when balancing fork

[PATCH 0/2] Fix imbalance issue when balancing fork

[Adam Li]

Load imbalance is observed when the workload frequently forks new threads.
Due to CPU affinity, the workload can run on CPU 0-7 in the first
group, and only on CPU 8-11 in the second group. CPU 12-15 are always idle.

{ 0 1 2 3 4 5 6 7 } {8 9 10 11 12 13 14 15}
  * * * * * * * *    * * *  *

When looking for dst group for newly forked threads, in many times
update_sg_wakeup_stats() reports the second group has more idle CPUs
than the first group. The scheduler thinks the second group is less
busy. Then it selects least busy CPUs among CPU 8-11. So CPU 8-11 can be
crowded with newly forked threads, at the same time CPU 0-7 can be idle.

The first patch 'Only update stats of allowed CPUs when looking for dst
group' *alone* can fix this imbalance issue.

And I think the second patch also makes sense in this scenario. If group
weight includes CPUs a task cannot use, group classification can be
incorrect. Please comment.

Adam Li (2):
  sched/fair: Only update stats of allowed CPUs when looking for dst
    group
  sched/fair: Only count group weight for allowed CPUs when looking for
    dst group

 kernel/sched/fair.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/2] sched/fair: Only update stats for allowed CPUs when looking for dst group

[Adam Li]

A task may not use all the CPUs in a schedule group due to CPU affinity.
Only update schedule group statistics for allowed CPUs.

Signed-off-by: Adam Li <adamli@os.amperecomputing.com>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7a14da5396fb..78a3d9b78e07 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10693,7 +10693,7 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
 	if (sd->flags & SD_ASYM_CPUCAPACITY)
 		sgs->group_misfit_task_load = 1;
 
-	for_each_cpu(i, sched_group_span(group)) {
+	for_each_cpu_and(i, sched_group_span(group), p->cpus_ptr) {
 		struct rq *rq = cpu_rq(i);
 		unsigned int local;
 
-- 
2.34.1

[PATCH 2/2] sched/fair: Only count group weight for allowed CPUs when looking for dst group

[Adam Li]

A task may not use all the CPUs in a schedule group due to CPU affinity.
If group weight includes CPUs not allowed to run the task,
group classification may be incorrect.

Signed-off-by: Adam Li <adamli@os.amperecomputing.com>
---
 kernel/sched/fair.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 78a3d9b78e07..452e2df961b9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10722,7 +10722,9 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
 
 	sgs->group_capacity = group->sgc->capacity;
 
-	sgs->group_weight = group->group_weight;
+	/* Only count group_weight if p can run on these cpus */
+	sgs->group_weight = cpumask_weight_and(sched_group_span(group),
+				p->cpus_ptr);
 
 	sgs->group_type = group_classify(sd->imbalance_pct, group, sgs);
 
-- 
2.34.1

Re: [PATCH 0/2] Fix imbalance issue when balancing fork

[Peter Zijlstra]

On Tue, Jul 01, 2025 at 02:45:47AM +0000, Adam Li wrote:
> Load imbalance is observed when the workload frequently forks new threads.
> Due to CPU affinity, the workload can run on CPU 0-7 in the first
> group, and only on CPU 8-11 in the second group. CPU 12-15 are always idle.
> 
> { 0 1 2 3 4 5 6 7 } {8 9 10 11 12 13 14 15}
>   * * * * * * * *    * * *  *
> 
> When looking for dst group for newly forked threads, in many times
> update_sg_wakeup_stats() reports the second group has more idle CPUs
> than the first group. The scheduler thinks the second group is less
> busy. Then it selects least busy CPUs among CPU 8-11. So CPU 8-11 can be
> crowded with newly forked threads, at the same time CPU 0-7 can be idle.
> 
> The first patch 'Only update stats of allowed CPUs when looking for dst
> group' *alone* can fix this imbalance issue.
> 
> And I think the second patch also makes sense in this scenario. If group
> weight includes CPUs a task cannot use, group classification can be
> incorrect. Please comment.
> 
> Adam Li (2):
>   sched/fair: Only update stats of allowed CPUs when looking for dst
>     group
>   sched/fair: Only count group weight for allowed CPUs when looking for
>     dst group
> 
>  kernel/sched/fair.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Hurm... so the thing I noticed is that update_sg_wakeup_stats() and
update_sg_lb_stats() are *very* similar.

Specifically, the first patch does something to wakeup_stats that
lb_stats already does. While the second patch seems to do something that
might also apply to lb_stats.

Is there no way we can unify this?

Re: [PATCH 0/2] Fix imbalance issue when balancing fork

[Adam Li]

On 7/4/2025 5:17 PM, Peter Zijlstra wrote:
> On Tue, Jul 01, 2025 at 02:45:47AM +0000, Adam Li wrote:
>> Load imbalance is observed when the workload frequently forks new threads.
>> Due to CPU affinity, the workload can run on CPU 0-7 in the first
>> group, and only on CPU 8-11 in the second group. CPU 12-15 are always idle.
>>
>> { 0 1 2 3 4 5 6 7 } {8 9 10 11 12 13 14 15}
>>   * * * * * * * *    * * *  *
>>
>> When looking for dst group for newly forked threads, in many times
>> update_sg_wakeup_stats() reports the second group has more idle CPUs
>> than the first group. The scheduler thinks the second group is less
>> busy. Then it selects least busy CPUs among CPU 8-11. So CPU 8-11 can be
>> crowded with newly forked threads, at the same time CPU 0-7 can be idle.
>>
>> The first patch 'Only update stats of allowed CPUs when looking for dst
>> group' *alone* can fix this imbalance issue.
>>
>> And I think the second patch also makes sense in this scenario. If group
>> weight includes CPUs a task cannot use, group classification can be
>> incorrect. Please comment.
>>
>> Adam Li (2):
>>   sched/fair: Only update stats of allowed CPUs when looking for dst
>>     group
>>   sched/fair: Only count group weight for allowed CPUs when looking for
>>     dst group
>>
>>  kernel/sched/fair.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>

Hi Peter,
> 
> Hurm... so the thing I noticed is that update_sg_wakeup_stats() and
> update_sg_lb_stats() are *very* similar.
> 
> Specifically, the first patch does something to wakeup_stats that
> lb_stats already does. While the second patch seems to do something that
> might also apply to lb_stats.
> 
Thanks for the idea. I am testing this.

> Is there no way we can unify this?
I will try to unify the common logic in update_sg_wakeup_stats() and
update_sg_lb_stats() into a single function. It seems not easy :).

Thanks,
-adam

Re: [PATCH 0/2] Fix imbalance issue when balancing fork

[Shubhang Kaushik Prasanna Kumar]

This patch-set tackles a significant task placement imbalance encountered under restricted CPU affinity, particularly on Ampere Altra systems. Our performance tests reveal that the first patch demonstrably improves task placement by 20-30% in scenarios where the number of threads increasingly outnumbers the available cores. However, the second patch unexpectedly introduces regressions when tested against higher thread count scenarios, requiring further investigation.

In light of these findings and the potential regression, the following questions arise:
 Q. Has the regression been investigated to determine if it is specific to two-level scheduling domains or manifests more broadly?
 Q. Were function-level latencies profiled to pinpoint the exact source of the regression within the code path introduced by the second patch?
 Q. How do these patches influence performance and task placement on systems with linear topologies, and do they exhibit more consistent behavior in those configurations?

- Shubhang