| Message ID | 20200924123937.20938-4-ionela.voinescu@arm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | condition EAS enablement on FI support | expand |
On Thursday 24 Sep 2020 at 13:39:37 (+0100), Ionela Voinescu wrote: > For arm64 this affects the task scheduler behavior which builds its > scheduling domain hierarchy well before the late counter-based FI init. > During that process it will disable EAS due to its dependency on FI. Does it mean we get a warn on every boot, even though this is a perfectly normal scenario? Thanks, Quentin
On Thursday 24 Sep 2020 at 14:39:25 (+0100), Quentin Perret wrote: > On Thursday 24 Sep 2020 at 13:39:37 (+0100), Ionela Voinescu wrote: > > For arm64 this affects the task scheduler behavior which builds its > > scheduling domain hierarchy well before the late counter-based FI init. > > During that process it will disable EAS due to its dependency on FI. > > Does it mean we get a warn on every boot, even though this is a > perfectly normal scenario? > Yes, we will get a few "Disabling EAS: frequency-invariant load tracking not supported" warnings until the final rebuild of the sched domains finds FI supported and enables EAS (silently this time, which possibly makes things worse). We have the same behavior for removing and adding the schedutil governor. I'm not sure what is a good way of fixing this.. I could add more info to the warning to suggest it might be temporary ("Disabling EAS: frequency-invariant load tracking currently not supported"). For further debugging there are the additional prints guarded by sched_debug(). I'll look over the code some more to see if other ideas pop out. Any suggestions are appreciated. Many thanks for the review, Ionela. > Thanks, > Quentin
Hey Ionela, On Thursday 24 Sep 2020 at 17:10:02 (+0100), Ionela Voinescu wrote: > I'm not sure what is a good way of fixing this.. I could add more info > to the warning to suggest it might be temporary ("Disabling EAS: > frequency-invariant load tracking currently not supported"). For further > debugging there are the additional prints guarded by sched_debug(). > > I'll look over the code some more to see if other ideas pop out. Any > suggestions are appreciated. Right, I'm not seeing anything perfect here, but I think I'd be personally happy with this message being entirely guarded by sched_debug(), like we do for asym CPU capacities for instance. It's not easy to see if EAS has started at all w/o sched debug anyway, so I expect folks who need it to enable the debug stuff during bring-up. With a descriptive enough warn message, that should be just fine. But that's my 2p, so I'm happy to hear if others disagree. Thanks, Quentin
On 25/09/2020 15:59, Quentin Perret wrote: > Hey Ionela, > > On Thursday 24 Sep 2020 at 17:10:02 (+0100), Ionela Voinescu wrote: >> I'm not sure what is a good way of fixing this.. I could add more info >> to the warning to suggest it might be temporary ("Disabling EAS: >> frequency-invariant load tracking currently not supported"). For further >> debugging there are the additional prints guarded by sched_debug(). >> >> I'll look over the code some more to see if other ideas pop out. Any >> suggestions are appreciated. > > Right, I'm not seeing anything perfect here, but I think I'd be > personally happy with this message being entirely guarded by > sched_debug(), like we do for asym CPU capacities for instance. > > It's not easy to see if EAS has started at all w/o sched debug anyway, > so I expect folks who need it to enable the debug stuff during > bring-up. With a descriptive enough warn message, that should be just > fine. But that's my 2p, so I'm happy to hear if others disagree. Are you discussing a scenario where the system doesn't have FI via CPUfreq but only via AMU? And then we would get the pr_warn "rd %*pbl: Disabling EAS: frequency-invariant load tracking not supported" in (1)-(3)? (1) initial sd build (2) update_topology_flags_workfn() (3) rebuild_sched_domains_energy() (4) init_amu_fie() Today (e.g. on Juno( we start EAS within (1) root@juno:~# dmesg | grep "build_perf_domains\|EAS" [ 3.491304] *** build_perf_domains: rd 0-5 [ 3.574226] sched_energy_set: starting EAS <--- !!! [ 3.847584] *** build_perf_domains: rd 0-5 [ 3.928227] *** build_perf_domains: rd 0-5 And on a future AMU FI only system it would look like: Disabling EAS: frequency-invariant load tracking not supported" Disabling EAS: frequency-invariant load tracking not supported" Disabling EAS: frequency-invariant load tracking not supported" sched_energy_set: starting EAS I guess it's a good idea to put all those warnings which indicate why EAS can't be started under sched_debug(). The warning "rd %*pbl: CPUs do not have asymmetric capacities" already is. This one is actually very similar to the FI related one, since 'asymmetric capacities' could only exist starting with (2) (big.LITTLE based entirely on CPUfreq diffs)
Hi guys, On Monday 28 Sep 2020 at 13:55:49 (+0200), Dietmar Eggemann wrote: > On 25/09/2020 15:59, Quentin Perret wrote: > > Hey Ionela, > > > > On Thursday 24 Sep 2020 at 17:10:02 (+0100), Ionela Voinescu wrote: > >> I'm not sure what is a good way of fixing this.. I could add more info > >> to the warning to suggest it might be temporary ("Disabling EAS: > >> frequency-invariant load tracking currently not supported"). For further > >> debugging there are the additional prints guarded by sched_debug(). > >> > >> I'll look over the code some more to see if other ideas pop out. Any > >> suggestions are appreciated. > > > > Right, I'm not seeing anything perfect here, but I think I'd be > > personally happy with this message being entirely guarded by > > sched_debug(), like we do for asym CPU capacities for instance. > > > > It's not easy to see if EAS has started at all w/o sched debug anyway, > > so I expect folks who need it to enable the debug stuff during > > bring-up. With a descriptive enough warn message, that should be just > > fine. But that's my 2p, so I'm happy to hear if others disagree. > > Are you discussing a scenario where the system doesn't have FI via > CPUfreq but only via AMU? And then we would get the pr_warn > > "rd %*pbl: Disabling EAS: frequency-invariant load tracking not > supported" > Yes, that is correct. Unfortunately for !sched_debug, even if we have FI via AMUs, the EAS enablement message "sched_energy_set: starting EAS" won't appear, and therefore one would only see the warnings above, giving the wrong impression that EAS is disabled. > in (1)-(3)? > > (1) initial sd build > (2) update_topology_flags_workfn() > (3) rebuild_sched_domains_energy() > (4) init_amu_fie() > > Today (e.g. on Juno( we start EAS within (1) > > root@juno:~# dmesg | grep "build_perf_domains\|EAS" > [ 3.491304] *** build_perf_domains: rd 0-5 > [ 3.574226] sched_energy_set: starting EAS <--- !!! > [ 3.847584] *** build_perf_domains: rd 0-5 > [ 3.928227] *** build_perf_domains: rd 0-5 > > And on a future AMU FI only system it would look like: > > Disabling EAS: frequency-invariant load tracking not supported" > Disabling EAS: frequency-invariant load tracking not supported" > Disabling EAS: frequency-invariant load tracking not supported" > sched_energy_set: starting EAS > Correct (with the same mention that "sched_energy_set: starting EAS" is guarded by sched debug). > I guess it's a good idea to put all those warnings which indicate why > EAS can't be started under sched_debug(). > > The warning "rd %*pbl: CPUs do not have asymmetric capacities" already > is. This one is actually very similar to the FI related one, since > 'asymmetric capacities' could only exist starting with (2) (big.LITTLE > based entirely on CPUfreq diffs) Yes, this seems the right solution, as suggested by Quentin as well. I'll do this together with the other suggestions and submit v2. Thank you both, Ionela.
diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h index 7cb519473fbd..9394101e3c08 100644 --- a/arch/arm64/include/asm/topology.h +++ b/arch/arm64/include/asm/topology.h @@ -16,6 +16,7 @@ int pcibus_to_node(struct pci_bus *bus); #include <linux/arch_topology.h> +void rebuild_sched_domains_energy(void); #ifdef CONFIG_ARM64_AMU_EXTN /* * Replace task scheduler's default counter-based diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index 543c67cae02f..2a9b69fdabc9 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -213,6 +213,7 @@ static DEFINE_STATIC_KEY_FALSE(amu_fie_key); static int __init init_amu_fie(void) { + bool invariance_status = topology_scale_freq_invariant(); cpumask_var_t valid_cpus; bool have_policy = false; int ret = 0; @@ -255,6 +256,15 @@ static int __init init_amu_fie(void) if (!topology_scale_freq_invariant()) static_branch_disable(&amu_fie_key); + /* + * Task scheduler behavior depends on frequency invariance support, + * either cpufreq or counter driven. If the support status changes as + * a result of counter initialisation and use, retrigger the build of + * scheduling domains to ensure the information is propagated properly. + */ + if (invariance_status != topology_scale_freq_invariant()) + rebuild_sched_domains_energy(); + free_valid_mask: free_cpumask_var(valid_cpus);
Task scheduler behavior depends on frequency invariance (FI) support and the resulting invariant load tracking signals. For example, in order to make accurate predictions across CPUs for all performance states, Energy Aware Scheduling (EAS) needs frequency-invariant load tracking signals and therefore it has a direct dependency on FI. If a platform is found lacking FI support, EAS is disabled. While arch_scale_freq_invariant() will see changes in FI support, it could return different values during system initialisation. Such a scenario will happen for a system that does not support cpufreq driven FI, but does support counter-driven FI. For such a system, arch_scale_freq_invariant() will return false if called before counter based FI initialisation, but change its status to true after it. For arm64 this affects the task scheduler behavior which builds its scheduling domain hierarchy well before the late counter-based FI init. During that process it will disable EAS due to its dependency on FI. Two points of early calls to arch_scale_freq_invariant() which determine EAS enablement are: - (1) drivers/base/arch_topology.c:126 <<update_topology_flags_workfn>> rebuild_sched_domains(); This will happen after CPU capacity initialisation. - (2) kernel/sched/cpufreq_schedutil.c:917 <<rebuild_sd_workfn>> rebuild_sched_domains_energy(); -->rebuild_sched_domains(); This will happen during sched_cpufreq_governor_change() for the schedutil cpufreq governor. Therefore, if there is a change in FI support status after counter init, use the existing rebuild_sched_domains_energy() function to trigger a rebuild of the scheduling and performance domains that in turn determine the enablement of EAS. Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> --- arch/arm64/include/asm/topology.h | 1 + arch/arm64/kernel/topology.c | 10 ++++++++++ 2 files changed, 11 insertions(+)