| Message ID | 20180320094312.24081-7-dietmar.eggemann@arm.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Tue, Mar 20, 2018 at 09:43:12AM +0000, Dietmar Eggemann wrote: > From: Quentin Perret <quentin.perret@arm.com> > > Energy Aware Scheduling (EAS) has to be started from the arch code. Ok, but: > This commit enables it from the arch topology driver for arm/arm64 > systems, hence enabling a better support for Arm big.LITTLE and future > DynamIQ architectures. Why does this have to be in the driver core code just for those specific types of cpus? > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Quentin Perret <quentin.perret@arm.com> > --- > drivers/base/arch_topology.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 52ec5174bcb1..e2206ea16538 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -15,6 +15,7 @@ > #include <linux/slab.h> > #include <linux/string.h> > #include <linux/sched/topology.h> > +#include <linux/sched/energy.h> > > DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; > > @@ -204,6 +205,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, > free_raw_capacity(); > pr_debug("cpu_capacity: parsing done\n"); > schedule_work(&parsing_done_work); > + init_sched_energy(); This is not arch-specific code only for arm. Don't you have a ARM cpu bringup code somewhere? Shouldn't this call be in there? It feels odd that this scheduler change is buried way down here... thanks, greg k-h
On 03/20/2018 10:49 AM, Greg Kroah-Hartman wrote: > On Tue, Mar 20, 2018 at 09:43:12AM +0000, Dietmar Eggemann wrote: >> From: Quentin Perret <quentin.perret@arm.com> >> >> Energy Aware Scheduling (EAS) has to be started from the arch code. > > Ok, but: > >> This commit enables it from the arch topology driver for arm/arm64 >> systems, hence enabling a better support for Arm big.LITTLE and future >> DynamIQ architectures. > > Why does this have to be in the driver core code just for those specific > types of cpus? The arch_topology driver is shared functionality between arm and arm64 arch. So far it handles the correct setting of per-cpu cpu_scale values which are then used by the task scheduler to set the correct cpu capacity values. This allows to provide cpu invariant load tracking for arm and arm64 big.LITTLE systems. The driver was initially created so that we didn't had to duplicate the exact code in the arm and arm64 arch. Big.Little platforms have to set appropriate cpu node capacity-dmips-mhz properties (e.g. arch/arm64/boot/dts/arm/juno.dts) to enable the start the functionality of the driver. The cpu_scale value depends on the capacity-dmips-mhz property as well as on the max frequency of a logical cpu (hence the dependency to cpufreq). A corner case would be a big.little platform purely based on max frequency differences (e.g. Google Pixel, Qualcomm MSM8996 Snapdragon 821, Quad-core (2x2.15 GHz Kryo & 2x1.6 GHz Kryo)). For such a system capacity-dmips-mhz should be set to the same value for all logical cpus (e.g. 1024). We would like to use the same code (shared between arm and arm64) to initialize the energy model and start EAS for arm and arm64 systems. >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Signed-off-by: Quentin Perret <quentin.perret@arm.com> >> --- >> drivers/base/arch_topology.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> index 52ec5174bcb1..e2206ea16538 100644 >> --- a/drivers/base/arch_topology.c >> +++ b/drivers/base/arch_topology.c >> @@ -15,6 +15,7 @@ >> #include <linux/slab.h> >> #include <linux/string.h> >> #include <linux/sched/topology.h> >> +#include <linux/sched/energy.h> >> >> DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; >> >> @@ -204,6 +205,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, >> free_raw_capacity(); >> pr_debug("cpu_capacity: parsing done\n"); >> schedule_work(&parsing_done_work); >> + init_sched_energy(); > > This is not arch-specific code only for arm. The driver requires CONFIG_GENERIC_ARCH_TOPOLOGY which is currently only set in arch/arm/Kconfig and arch/arm64/Kconfig. > Don't you have a ARM cpu bringup code somewhere? Shouldn't this call be > in there? It feels odd that this scheduler change is buried way down > here... The big benefit is that we don't have to duplicate code for arm and arm64. In the current form, EAS should only be started when cpufreq is initialized for all possible cpus. And we don't have to signal back from the driver to the arch that cpufreq is initialized for all possible cpus. I agree that none of this is obvious so the patch should explain the requirements and design better. -- Dietmar [...]
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 52ec5174bcb1..e2206ea16538 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -15,6 +15,7 @@ #include <linux/slab.h> #include <linux/string.h> #include <linux/sched/topology.h> +#include <linux/sched/energy.h> DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; @@ -204,6 +205,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, free_raw_capacity(); pr_debug("cpu_capacity: parsing done\n"); schedule_work(&parsing_done_work); + init_sched_energy(); } return 0;