Given the maturity gained by cpufreq-based Frequency Invariance (FI)
support following the patches at [1], this series conditions Energy
Aware Scheduling (EAS) enablement on a frequency invariant system.
Currently, EAS can be enabled on a system without FI support, leading
to incorrect (energy-wise) task placements. As no warning is emitted,
it could take some debugging effort to track the behavior back to the
lack of FI support; this series changes that by disabling EAS
(and advertising it) when FI support is missing.
The series is structured as follows:
- 1/3 - create function that can rebuild the scheduling and EAS'
performance domains if EAS' initial conditions change
- 2/3 - condition EAS enablement on FI support
- 3/3 - arm64: rebuild scheduling and performance domains in the
case of late, counter-driven FI initialisation.
This series is dependent on the patches at [1] and based on linux-next
20200918.
[1] Most recent version at:
https://lore.kernel.org/lkml/[email protected]/
Ionela Voinescu (3):
sched/topology,schedutil: wrap sched domains rebuild
sched/topology: condition EAS enablement on FIE support
arm64: rebuild sched domains on invariance status changes
arch/arm64/include/asm/topology.h | 1 +
arch/arm64/kernel/topology.c | 10 ++++++++++
include/linux/sched/topology.h | 1 +
kernel/sched/cpufreq_schedutil.c | 9 +--------
kernel/sched/topology.c | 26 +++++++++++++++++++-------
5 files changed, 32 insertions(+), 15 deletions(-)
base-commit: b652d2a5f2a4e93d803cc33eb57fdc41ee528500
prerequisite-patch-id: 592324cdfe0735a845d827d23f4d042b66e480ae
prerequisite-patch-id: b232a586616b573b7fc320df2dcf4783f26dc169
prerequisite-patch-id: 8a8238e55f4e522eb0ee44c1d4a083cac019959a
prerequisite-patch-id: 8edd7fc97f15c7f737339d3e07dbcd6c6d99d986
prerequisite-patch-id: b24b28cd2ec4c929b770f5dd3eeb30f839f8d6ab
--
2.17.1
In order to make accurate predictions across CPUs and for all performance
states, Energy Aware Scheduling (EAS) needs frequency-invariant load
tracking signals.
EAS task placement aims to minimize energy consumption, and does so in
part by limiting the search space to only CPUs with the highest spare
capacity (CPU capacity - CPU utilization) in their performance domain.
Those candidates are the placement choices that will keep frequency at
its lowest possible and therefore save the most energy.
But without frequency invariance, a CPU's utilization is relative to the
CPU's current performance level, and not relative to its maximum
performance level, which determines its capacity. As a result, it will
fail to correctly indicate any potential spare capacity obtained by an
increase in a CPU's performance level. Therefore, a non-invariant
utilization signal would render the EAS task placement logic invalid.
Now that we properly report support for the Frequency Invariance Engine
(FIE) through arch_scale_freq_invariant() for arm and arm64 systems, we
can assert it is the case when initializing EAS. Warn and bail out
otherwise.
Signed-off-by: Ionela Voinescu <[email protected]>
Suggested-by: Quentin Perret <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
---
kernel/sched/topology.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 4073f693e2b5..348d563c2210 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -328,6 +328,7 @@ static void sched_energy_set(bool has_eas)
* 3. no SMT is detected.
* 4. the EM complexity is low enough to keep scheduling overheads low;
* 5. schedutil is driving the frequency of all CPUs of the rd;
+ * 6. frequency invariance support is present;
*
* The complexity of the Energy Model is defined as:
*
@@ -376,6 +377,12 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
goto free;
}
+ if (!arch_scale_freq_invariant()) {
+ pr_warn("rd %*pbl: Disabling EAS: frequency-invariant load tracking not supported",
+ cpumask_pr_args(cpu_map));
+ goto free;
+ }
+
for_each_cpu(i, cpu_map) {
/* Skip already covered CPUs. */
if (find_pd(pd, i))
--
2.17.1
Add the rebuild_sched_domains_energy() function to wrap the functionality
that rebuilds the scheduling domains if any of the Energy Aware Scheduling
(EAS) initialisation conditions change. This functionality is used when
schedutil is added or removed or when EAS is enabled or disabled
through the sched_energy_aware sysctl.
Therefore, create a single function that is used in both these cases and
that can be later reused.
Signed-off-by: Ionela Voinescu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: Viresh Kumar <[email protected]>
---
include/linux/sched/topology.h | 1 +
kernel/sched/cpufreq_schedutil.c | 9 +--------
kernel/sched/topology.c | 19 ++++++++++++-------
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index 9ef7bf686a9f..f122115d6c54 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -158,6 +158,7 @@ static inline struct cpumask *sched_domain_span(struct sched_domain *sd)
return to_cpumask(sd->span);
}
+extern void rebuild_sched_domains_energy(void);
extern void partition_sched_domains_locked(int ndoms_new,
cpumask_var_t doms_new[],
struct sched_domain_attr *dattr_new);
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index e39008242cf4..0337a9b025e1 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -912,16 +912,9 @@ struct cpufreq_governor *cpufreq_default_governor(void)
cpufreq_governor_init(schedutil_gov);
#ifdef CONFIG_ENERGY_MODEL
-extern bool sched_energy_update;
-extern struct mutex sched_energy_mutex;
-
static void rebuild_sd_workfn(struct work_struct *work)
{
- mutex_lock(&sched_energy_mutex);
- sched_energy_update = true;
- rebuild_sched_domains();
- sched_energy_update = false;
- mutex_unlock(&sched_energy_mutex);
+ rebuild_sched_domains_energy();
}
static DECLARE_WORK(rebuild_sd_work, rebuild_sd_workfn);
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 55c453d140e9..4073f693e2b5 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -211,6 +211,15 @@ unsigned int sysctl_sched_energy_aware = 1;
DEFINE_MUTEX(sched_energy_mutex);
bool sched_energy_update;
+void rebuild_sched_domains_energy(void)
+{
+ mutex_lock(&sched_energy_mutex);
+ sched_energy_update = true;
+ rebuild_sched_domains();
+ sched_energy_update = false;
+ mutex_unlock(&sched_energy_mutex);
+}
+
#ifdef CONFIG_PROC_SYSCTL
int sched_energy_aware_handler(struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
@@ -223,13 +232,8 @@ int sched_energy_aware_handler(struct ctl_table *table, int write,
ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
if (!ret && write) {
state = static_branch_unlikely(&sched_energy_present);
- if (state != sysctl_sched_energy_aware) {
- mutex_lock(&sched_energy_mutex);
- sched_energy_update = 1;
- rebuild_sched_domains();
- sched_energy_update = 0;
- mutex_unlock(&sched_energy_mutex);
- }
+ if (state != sysctl_sched_energy_aware)
+ rebuild_sched_domains_energy();
}
return ret;
@@ -433,6 +437,7 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
}
#else
static void free_pd(struct perf_domain *pd) { }
+void rebuild_sched_domains_energy(void) { }
#endif /* CONFIG_ENERGY_MODEL && CONFIG_CPU_FREQ_GOV_SCHEDUTIL*/
static void free_rootdomain(struct rcu_head *rcu)
--
2.17.1
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 <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/topology.h | 1 +
arch/arm64/kernel/topology.c | 10 ++++++++++
2 files changed, 11 insertions(+)
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);
--
2.17.1
On Thursday 24 Sep 2020 at 13:39:35 (+0100), Ionela Voinescu wrote:
> @@ -433,6 +437,7 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
> }
> #else
> static void free_pd(struct perf_domain *pd) { }
> +void rebuild_sched_domains_energy(void) { }
Nit: maybe make that stub static inline in a header instead? I guess LTO
and friends ought to clean that up for you, but it shouldn't hurt to give
the compiler a little bit of help here.
Otherwise, LGTM:
Acked-by: Quentin Perret <[email protected]>
> #endif /* CONFIG_ENERGY_MODEL && CONFIG_CPU_FREQ_GOV_SCHEDUTIL*/
>
> static void free_rootdomain(struct rcu_head *rcu)
> --
> 2.17.1
>
On Thursday 24 Sep 2020 at 13:39:34 (+0100), Ionela Voinescu wrote:
> Given the maturity gained by cpufreq-based Frequency Invariance (FI)
> support following the patches at [1], this series conditions Energy
> Aware Scheduling (EAS) enablement on a frequency invariant system.
>
> Currently, EAS can be enabled on a system without FI support, leading
> to incorrect (energy-wise) task placements. As no warning is emitted,
> it could take some debugging effort to track the behavior back to the
> lack of FI support; this series changes that by disabling EAS
> (and advertising it) when FI support is missing.
>
> The series is structured as follows:
> - 1/3 - create function that can rebuild the scheduling and EAS'
> performance domains if EAS' initial conditions change
> - 2/3 - condition EAS enablement on FI support
> - 3/3 - arm64: rebuild scheduling and performance domains in the
> case of late, counter-driven FI initialisation.
I'm still reading through this, but shouldn't patch 2 and 3 be swapped?
Otherwise we have a weird state at patch 2 where EAS will fail to start
(IIUC), which might not be ideal for bisection.
Thoughts?
Cheers,
Quentin
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
Hey,
On Thursday 24 Sep 2020 at 14:34:46 (+0100), Quentin Perret wrote:
> On Thursday 24 Sep 2020 at 13:39:35 (+0100), Ionela Voinescu wrote:
> > @@ -433,6 +437,7 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
> > }
> > #else
> > static void free_pd(struct perf_domain *pd) { }
> > +void rebuild_sched_domains_energy(void) { }
>
> Nit: maybe make that stub static inline in a header instead? I guess LTO
> and friends ought to clean that up for you, but it shouldn't hurt to give
> the compiler a little bit of help here.
>
Makes sense and will do!
Thank you,
Ionela.
> Otherwise, LGTM:
>
> Acked-by: Quentin Perret <[email protected]>
>
> > #endif /* CONFIG_ENERGY_MODEL && CONFIG_CPU_FREQ_GOV_SCHEDUTIL*/
> >
> > static void free_rootdomain(struct rcu_head *rcu)
> > --
> > 2.17.1
> >
On Thursday 24 Sep 2020 at 14:37:27 (+0100), Quentin Perret wrote:
> On Thursday 24 Sep 2020 at 13:39:34 (+0100), Ionela Voinescu wrote:
> > Given the maturity gained by cpufreq-based Frequency Invariance (FI)
> > support following the patches at [1], this series conditions Energy
> > Aware Scheduling (EAS) enablement on a frequency invariant system.
> >
> > Currently, EAS can be enabled on a system without FI support, leading
> > to incorrect (energy-wise) task placements. As no warning is emitted,
> > it could take some debugging effort to track the behavior back to the
> > lack of FI support; this series changes that by disabling EAS
> > (and advertising it) when FI support is missing.
> >
> > The series is structured as follows:
> > - 1/3 - create function that can rebuild the scheduling and EAS'
> > performance domains if EAS' initial conditions change
> > - 2/3 - condition EAS enablement on FI support
> > - 3/3 - arm64: rebuild scheduling and performance domains in the
> > case of late, counter-driven FI initialisation.
>
> I'm still reading through this, but shouldn't patch 2 and 3 be swapped?
> Otherwise we have a weird state at patch 2 where EAS will fail to start
> (IIUC), which might not be ideal for bisection.
>
> Thoughts?
I probably invented myself reasons for not doing it, like: without 2/3,
3/3 does not make any sense having and the scenario at 3/3 is currently
unlikely.
But it would definitely make it safer, so I'll change the order.
Thanks,
Ionela.
>
> Cheers,
> 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.