On 14/10/2019 18:03, Valentin Schneider wrote:
On 14/10/2019 14:52, Quentin Perret wrote:
Right, but that's not possible by definition -- static keys aren't variables. The static keys for asym CPUs and for EAS are just to optimize the case when they're disabled, but when they _are_ enabled, you have no choice but do another per-rd check.
Bleh, right, realized my nonsense after sending the email.
And to clarify what I tried to say before, it might be possible to 'count' the number of RDs that have SD_ASYM_CPUCAPACITY set using static_branch_inc()/dec(), like we do for the SMT static key. I remember trying to do something like that for EAS, but that was easier said than done ... :)
Gotcha, the reason I didn't go with this is that I wanted to maintain the relationship between the key and the flag (you either have both or none). It feels icky to have the key set and to have a NULL sd_asym_cpucapacity pointer.
An alternative might be to have a separate counter for asymmetric rd's, always disable the key on domain destruction and use that counter to figure out if we need to restore it. If we don't care about having a NULL SD pointer while the key is set, we could use the included counter as you're suggesting.
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.
Valentin's patch makes sure that sched_asym_cpucapacity is correctly set when the sd hierarchy is rebuild due to CPU hp. 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.
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.