Messages in this thread | | | Subject | Re: [PATCH v6 07/14] sched/topology: Introduce sched_energy_present static key | From | Dietmar Eggemann <> | Date | Thu, 6 Sep 2018 16:49:47 -0700 |
| |
On 09/06/2018 02:29 AM, Quentin Perret wrote: > Hi Dietmar, > > On Wednesday 05 Sep 2018 at 23:06:38 (-0700), Dietmar Eggemann wrote: >> On 08/20/2018 02:44 AM, Quentin Perret wrote: >>> In order to ensure a minimal performance impact on non-energy-aware >>> systems, introduce a static_key guarding the access to Energy-Aware >>> Scheduling (EAS) code. >>> >>> The static key is set iff all the following conditions are met for at >>> least one root domain: >>> 1. all online CPUs of the root domain are covered by the Energy >>> Model (EM); >>> 2. the complexity of the root domain's EM is low enough to keep >>> scheduling overheads low; >>> 3. the root domain has an asymmetric CPU capacity topology (detected >>> by looking for the SD_ASYM_CPUCAPACITY flag in the sched_domain >>> hierarchy). >> >> This is pretty much the list (+ is schedutil running) of conditions to set >> rd->pd != NULL in build_perf_domains(). > > Yes, exactly. I actually loop over the rds to check if one of them has a > pd != NULL in order to enable/disable the static key. So the check for > those conditions is always done at the rd level. > >> So when testing 'static_branch_unlikely(&sched_energy_present) && >> rcu_dereference(rd->pd)' don't you test two times the same thing? > > Well, not exactly. You could have two rds in your system, and only one > of the two has an Energy Model. The static key is just a performance > optimization for !EAS systems really. But I must admit it sort of lost a
Ah, that's correct. But the static key could be exchanged by a sched feature, that's the important bit here.
> bit of its interest over the versions. I mean, it's not that clear now > that a static key is a better option than a sched_feat as you suggest > below. > >> Also, if let's say somebody wants to run another EM user (e.g. a thermal >> governor, like IPA) but not EAS on a asymmetric CPU capacity system. This >> can't be achieved with the current static branch approach > > I assume you're talking about IPA once migrated to using the EM > framework ? In this case, I think you're right, we won't have a single
Right, in case we will have multiple user of the EM in the future.
> tunable left to enable/disable EAS. On a big.LITTLE platform, if you > want IPA, EAS will always be enabled by side effect ... > > That's a very good point actually. I think some people will not be happy > with that. There are big.LITTLE users (not very many of them, but still) > who don't care that much about energy, but do about performance. And > those guys might want to use IPA without EAS. So I guess we really need > a new knob.
I guess so too.
>> So what about using a (disabled by default ?) sched_feature + rd->pd != NULL >> instead? > > Right, that's an option. I could remove the static key and > sched_energy_start() altogether and replace all the > "if (static_branch_unlikely(&sched_energy_present))" by > "if (sched_feat(ENERGY_AWARE))" for example. That should be equivalent > to what I did with the static key from a performance standpoint. But that > would mean users have to manually flip switches to get EAS up and > running ... I assume it's the price to pay for configurability. > > Another option would be a KConfig option + static key. I could keep all > of the ifdefery inside an accessor function like the following: > > static inline bool sched_energy_aware(void) > { > #ifdef CONFIG_SCHED_ENERGY > return static_branch_likely(&sched_energy_present); > #else > return false; > #endif > } > > Now, I understand that scheduler-related KConfig options aren't welcome > in general, so I tend to prefer the sched_feat option. > > Thoughts ?
I would prefer a sched_feature. I guess it has to be disabled by default so that other systems don't have to check rcu_dereference(rd->pd) in the wakeup path.
But since at the beginning EAS will be the only user of the EM there is no need to change the static key sched_energy_present right now.
| |