On 27/05/2021 09:03, Peter Zijlstra wrote: > On Wed, May 26, 2021 at 11:52:25AM +0200, Dietmar Eggemann wrote: > >> For me asym_cpu_capacity_classify() is pretty hard to digest ;-) But I >> wasn't able to break it. It also performs correctly on (non-existing SMT) >> layer (with sd span eq. single CPU). > > This is the simplest form I could come up with this morning: > > static inline int > asym_cpu_capacity_classify(struct sched_domain *sd, > const struct cpumask *cpu_map) > { > struct asym_cap_data *entry; > int i = 0, n = 0; > > list_for_each_entry(entry, &asym_cap_list, link) { > if (cpumask_intersects(sched_domain_span(sd), entry->cpu_mask)) > i++; > else > n++; > } > > if (WARN_ON_ONCE(!i) || i == 1) /* no asymmetry */ > return 0; > > if (n) /* partial asymmetry */ > return SD_ASYM_CPUCAPACITY; > > /* full asymmetry */ > return SD_ASYM_CPUCAPACITY | SD_ASYM_CPUCAPACITY_FULL; > } > > > The early termination and everything was cute; but this isn't > performance critical code and clarity is paramount. This is definitely easier to grasp. What about the missing `if (cpumask_intersects(entry->cpu_mask, cpu_map))` condition in the else path to increment n? Example: cpus = {[446 446] [871 871] [1024 1024]} So 3 asym_cap_list entries. After hp'ing out CPU4 and CPU5: DIE: 'partial asymmetry' In case we would increment n only when the condition is met, we would have `full asymmetry`. I guess we want to allow EAS task placement, hence have sd_asym_cpucapacity set in case there are only 446 and 871 left?