Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] Group statistics (`avg_load`, `group_has_spare`) and load balancing 
math are corrupted because load and utilization exclude non-preferred CPUs, 
while group capacity and weight incorrectly include them.
- [High] Periodic and busy load balancing can be completely disabled for a 
scheduling domain if the statically designated `group_balance_cpu` becomes 
non-preferred.
--

commit 2dcc3be382ec42f554b59dc47da29e35996ce2d6
Author: Shrikanth Hegde <[email protected]>
Subject: sched/fair: Load balance only among preferred CPUs

This patch modifies load balancing to only consider preferred CPUs,
aiming to prevent balancing against the push task mechanism. It
updates sched_balance_rq() and sched_balance_newidle() to restrict
operations using cpu_preferred_mask.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b8bd308c2d5b1..4ef1167b8c73f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13473,7 +13473,7 @@ static int sched_balance_rq(int this_cpu, struct rq 
> *this_rq,
>       };
>       bool need_unlock = false;
>  
> -     cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
> +     cpumask_and(cpus, sched_domain_span(sd), cpu_preferred_mask);

[Severity: High]
Does restricting env.cpus to cpu_preferred_mask here corrupt the group
statistics calculation in update_sg_lb_stats()?

Looking at update_sg_lb_stats(), it accumulates load and utilization by
iterating only over env->cpus:

kernel/sched/fair.c:update_sg_lb_stats() {
    ...
    for_each_cpu_and(i, sched_group_span(group), env->cpus) {
        ...
    }
    ...
    sgs->group_capacity = group->sgc->capacity;
    sgs->group_weight = group->group_weight;
    ...
}

Since it assigns the full capacity and weight of the entire group, but only
accumulates load for the preferred CPUs, won't this artificially deflate
avg_load? This could cause the load balancer to perceive mixed groups as
severely under-loaded and aggressively pull tasks to already overloaded
preferred CPUs.

[Severity: High]
Can this also completely disable periodic and busy load balancing for a
scheduling domain if the statically designated balance CPU is non-preferred?

Looking at should_we_balance():

kernel/sched/fair.c:should_we_balance() {
    ...
    if (!cpumask_test_cpu(env->dst_cpu, env->cpus))
        return 0;
    ...
    return group_balance_cpu(sg) == env->dst_cpu;
}

If the statically designated group_balance_cpu(sg) is non-preferred, it
will abort immediately because it is excluded from env.cpus. All other
preferred CPUs in the group will also abort because they fail the
group_balance_cpu(sg) == env->dst_cpu check. 

Does this mean no CPU will perform the domain balance, potentially causing
severe starvation?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=7

Reply via email to