On Tuesday 19 Jun 2018 at 18:20:42 (+0200), Peter Zijlstra wrote: > Right, so I would not do that many things at once. Also be more explicit > about what data structure, and why.
OK, I can probably split that patch in two smaller patches. One that introduces and enables the static_key (or something else to replace it, see my reply below); and another one to create the list of frequency domains. > That said, I think the whole for_each_freq_domain() thing as done is > broken. You've completely ignored the arguments to > partition_sched_domains(). What happens if you create partitions right > along the frequency domains? > > So you really want an argument to for_each_freq_domain() to indicate > who's frequency domains you want to iterate. And then I think it's > easiest if you hook into build_sched_domains() instead, because you > really want a list per root_domain I suspect Hmm right, the current implementation is broken with multiple root domains... We rarely use that in mobile but the code should work regardless I suppose. I agree having one list per root_domain should be better. I'll change that in v4. But I also think the idea of a global static_key is broken then. We need a per root_domain thing as well. The SD_ASYM_CPUCAPACITY flag might be set on one hierarchy and not the other for ex. Not sure if attaching a static_key to each root_domain really makes sense though ... Would replacing the static_key by a flag attached to the root_domain be reasonable ? The CONFIG_ENERGY_MODEL config option could help us make sure there is no performance impact when EAS can't be used with something like this: static bool sched_energy_enabled(struct root_domain *rd) { #ifdef CONFIG_ENERGY_MODEL return rd->eas_enabled; #else return false; #endif } Or maybe a sched_feat instead of that ifdefery ? > (and an empty list if there is but one entry on). Sorry but I didn't understand that ... Thanks, Quentin