Re: [PATCH v2 1/3] sched/fair: Add asymmetric CPU capacity wakeup scan

[Valentin Schneider <valentin.schneider@arm.com>]

On 24/01/2020 15:11, Quentin Perret wrote:
>> I think what we were trying to go with here is to not entirely hijack
>> select_idle_sibling(). If we go with the above alternative, topologies
>> with sched_asym_cpucapacity enabled would only ever see
>> select_idle_capacity() and not the rest of select_idle_sibling(). Not sure
>> if it's a bad thing or not, but it's something to ponder over.
> 
> Right, I would think your suggestion above is a pretty sensible policy
> for asymmetric systems, and I don't think the rest of
> select_idle_sibling() will do a much better job on such systems at
> finding an idle CPU than select_idle_capacity() would do, but I see
> your point.
> 
> Now, not having to re-iterate over the CPUs again might keep the wakeup
> latency a bit lower -- perhaps something noticeable with hackbench ?
> Worth a try.
> 

Agreed. Removing SD_BALANCE_WAKE causes most of the perf delta here,
I'll try to get some statistics on the same versions but with and without
the extra select_idle_capacity() policy and see how noticeable it is.

I might still do it if the delta is lost in the noise, just gotta put my
brain cells to work. One thing I'm thinking is maybe we're not guaranteed
to have sd_asym_cpucapacity being a superset of sd_llc, so we would need
to go through select_idle_sibling() to cover for the rest, although I think
this falls in the category of topologies we highly recommend *not* to build
(or expose, in case of people playing games with cpu-map).

> In any case, no strong opinion. With that missing call to
> sync_entity_load_avg() fixed, the patch looks pretty decent to me.
> 

Thanks for having a look!

> Thanks,
> Quentin
>