Re: [PATCH] sched/topology: Disable sched_asym_cpucapacity on domain destruction

[Dietmar Eggemann]

On 15/10/2019 13:07, Quentin Perret wrote:
> On Tuesday 15 Oct 2019 at 11:22:12 (+0200), Dietmar Eggemann wrote:
>> I still don't understand the benefit of the counter approach here.
>> sched_smt_present counts the number of cores with SMT. So in case you
>> have 2 SMT cores with 2 HW threads and you CPU hp out one CPU, you still
>> have sched_smt_present, although 1 CPU doesn't have a SMT thread sibling
>> anymore.
> 
> Right, and we want something similar for the asym static key I think.
> That is, it should be enabled if _at least one_ RD is asymmetric.
> 
>> Valentin's patch makes sure that sched_asym_cpucapacity is correctly set
>> when the sd hierarchy is rebuild due to CPU hp.
> 
> As mentioned in a previous email, I think this patch is broken in case
> you have multiple asymmetric RDs, but counting should fix it, though it
> might not be that easy to implement.

Correct, now I see your point! Can be easily recreated on JUNO setting
up two exclusive cpusets, each with one big CPU and then hp'ing one of them.

And partition_sched_domains_locked()/build_perf_domains() handles this
by distinguishing existing vs. new PDs and or'ing has_eas.

>> Including the unlikely
>> scenario that an asymmetric CPU capacity system (based on DT's
>> capacity-dmips-mhz values) turns normal SMT because of the max frequency
>> values of the CPUs involved.
> 
> I'm not sure what you meant here ?

The rebuild of the sd hierarchy after we know the max frequency values
of the CPUs from CpuFreq (2. SD hierarchy build on asym CPU capacity
systems with CPUfreq).

E.g on Juno when you set capacity-dmips-mhz_{big;little}=791;1024 with
max_CPU_freq_{big;little}=1,100,000;850,000.

It's a theoretical case and might not even work due to rounding errors.

>> Systems with a mix of asymmetric and symmetric CPU capacity rd's have to
>> live with the fact that wake_cap and misfit handling is enabled for
>> them. This should be the case already today.
>>
>> There should be no SD_ASYM_CPUCAPACITY flag on the sd's of the CPUs of
>> the symmetric CPU capacity rd's. I.e. update_top_cache_domain() should
>> set sd_asym_cpucapacity=NULL for those CPUs.
>>
>> So as a rule we could say even if a static key enables a code path, a
>> derefenced sd still has to be checked against NULL.
> 
> Right, that's already true today, and I don't see a possible alternative
> to that. Same thing for the EAS static key and the pd list btw.

Agreed.