Re: [PATCH v3 5/6] sched/fair: Consider SMT in ASYM_PACKING load balance

[Joel Fernandes <joel@joelfernandes.org>]

On Mon, May 17, 2021 at 6:28 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Thu, May 13, 2021 at 08:49:08AM -0700, Ricardo Neri wrote:
> > When deciding to pull tasks in ASYM_PACKING, it is necessary not only to
> > check for the idle state of the destination CPU, dst_cpu, but also of
> > its SMT siblings.
> >
[...]
> > +     if (!local_is_smt) {
> > +             /*
> > +              * If we are here, @dst_cpu is idle and does not have SMT
> > +              * siblings. Pull tasks if candidate group has two or more
> > +              * busy CPUs.
> > +              */
> > +             if (sg_is_smt && sg_busy_cpus >= 2)
> > +                     return true;
> > +
> > +             /*
> > +              * @dst_cpu does not have SMT siblings. @sg may have SMT
> > +              * siblings and only one is busy. In such case, @dst_cpu
> > +              * can help if it has higher priority and is idle.
> > +              */
> > +             return !sds->local_stat.group_util &&
> > +                    sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
> > +     }
> > +
> > +     /* @dst_cpu has SMT siblings. */
> > +
> > +     local_busy_cpus = sds->local->group_weight - sds->local_stat.idle_cpus;
> > +
> > +     if (sg_is_smt) {
> > +             int busy_cpus_delta = sg_busy_cpus - local_busy_cpus;
> > +
> > +             /* Local can always help to even the number busy CPUs. */
> > +             if (busy_cpus_delta >= 2)
> > +                     return true;
> > +
> > +             if (busy_cpus_delta == 1)
> > +                     return sched_asym_prefer(dst_cpu,
> > +                                              sg->asym_prefer_cpu);
> > +
> > +             return false;
> > +     }
> > +
> > +     /*
> > +      * @sg does not have SMT siblings. Ensure that @sds::local does not end
> > +      * up with more than one busy SMT sibling and only pull tasks if there
> > +      * are not busy CPUs. As CPUs move in and out of idle state frequently,
> > +      * also check the group utilization to smoother the decision.
>
> nit: s/smoother/smoothen/
>
> > +      */
> > +     if (!local_busy_cpus && !sds->local_stat.group_util)
> > +             return sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
>
> Hmm, I am not sure but is it possible that there some local_busy_cpus yet
> group_util is 0? If not just check for !group_util ?

Sorry - I meant here, "yet group_util is not 0..."