Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1325303 > unrolled thread

[PATCH v3 0/6] CPUs capacity information for heterogeneous systems

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/6] CPUs capacity information for heterogeneous systems Juri Lelli <juri.lelli@arm.com> - 2016-02-03 13:00 +0100
    [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization Juri Lelli <juri.lelli@arm.com> - 2016-02-03 13:00 +0100
      Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-02-08 13:30 +0100
        Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization Mark Brown <broonie@kernel.org> - 2016-02-08 14:20 +0100
          Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-02-08 14:50 +0100
    [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute Juri Lelli <juri.lelli@arm.com> - 2016-02-03 13:10 +0100
      Re: [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-02-05 18:20 +0100
        Re: [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute Juri Lelli <juri.lelli@arm.com> - 2016-02-05 18:50 +0100
    [PATCH v3 5/6] arm: add sysfs cpu_capacity attribute Juri Lelli <juri.lelli@arm.com> - 2016-02-03 13:10 +0100
    [PATCH v3 3/6] arm: Enable dynamic CPU capacity initialization Juri Lelli <juri.lelli@arm.com> - 2016-02-03 13:10 +0100
    Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous  systems Steve Muckle <steve.muckle@linaro.org> - 2016-02-09 01:10 +0100
      Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous  systems Juri Lelli <juri.lelli@arm.com> - 2016-02-09 11:40 +0100
        Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous  systems Steve Muckle <steve.muckle@linaro.org> - 2016-02-09 18:40 +0100
          Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous  systems Juri Lelli <juri.lelli@arm.com> - 2016-02-09 18:50 +0100

#1325303 — [PATCH v3 0/6] CPUs capacity information for heterogeneous systems

Hi all,

this is take 3 of "CPUs capacity information for heterogeneous systems"
patchset [1]; some context follows.

ARM systems may be configured to have CPUs with different power/performance
characteristics within the same chip. In this case, additional information has
to be made available to the kernel (the scheduler in particular) for it to be
aware of such differences and take decisions accordingly. This RFC stems from
the ongoing discussion about introducing a simple platform energy cost model to
guide scheduling decisions (a.k.a Energy Aware Scheduling [3]), but also aims
to be an independent track aimed to standardise the way we make the scheduler
aware of heterogenous CPU systems. With these patches and in addition patches
from [3] (that make the scheduler wakeup paths aware of heterogenous CPU
systems) we enable the scheduler to have good default performance on such
systems. In addition, we get a clearly defined way of providing the scheduler
with needed information about CPU capacity on such systems.

CPU capacity is defined in this context as a number that provides the scheduler
information about CPUs heterogeneity. Such heterogeneity can come from
micro-architectural differences (e.g., ARM big.LITTLE systems) or maximum
frequency at which CPUs can run (e.g., SMP systems with multiple frequency
domains and different max frequencies). Heterogeneity in this context is about
differing performance characteristics; in practice, the binding that we propose
in this RFC tries to capture a first-order approximation of the relative
performance of CPUs.

Several approaches for providing CPUs capacity information have been already
discussed on the list:

 v1: DT + sysfs [1]

 v2: Dynamic profiling at boot [2]

Third version of this patchset proposes what seems to be the solution we agreed
upon (see [2] for reference) to the problem of how do we init CPUs original
capacity: we run a bogus benchmark (stealing int_sqrt from lib/ we run that in
a loop to perform some integer computation, better benchmarks are welcome)
on the first cpu of each frequency domain (assuming no u-arch differences
inside domains), measure time to complete a fixed number of iterations and then
normalize results to SCHED_CAPACITY_SCALE (1024). This time around we also
added a boot time parameter to disable profiling at boot (as it can be time
consuming) and sysfs attributes with which default values can be overwritten.
The proposed solution is basically putting together bits of v1 and v2 that are
considered valuable and acceptable for mainline.

What follows gives you and idea of the kind of results you can expect comparing
the dynamic approach to profiling in userspace:

                          LITTLE      big

 TC2-userspace_profile     430        1024
 TC2-dynamic_profile      ~490        1024

 JUNO-userspace_profile    446        1024
 JUNO-dynamic_profile     ~424        1024

This time around we also decided to remove the RFC tag; even if patches might
still need some degree of improvement and discussion, there seems to be general
consensus about the idea behind the current solution (i.e., nobody NAKed it yet 
:)).

Patches high level description:

 o 01/06 cleans up how cpu_scale is initialized in arm (already landed on
   Russell's patch system)
 o 02/06 introduces dynamic profiling of CPUs capacity at boot
 o [03-04]/06 enable dynamic profiling for arm and arm64.
 o [05-06]/06 introduce sysfs attribute for arm and arm64.

The patchset is based on top of mainline as of today (4.5-rc2).

In case you would like to test this out, I pushed a branch here:

 git://linux-arm.org/linux-jl.git upstream/default_caps_v3

This branch contains additional patches, useful to better understand how CPU
capacity information is actually used by the scheduler. Discussion regarding
these additional patches will be started with a different posting in the
future. We just didn't want to make discussion too broad, as we realize that
this set can be controversial already on its own.

Comments, concerns and rants are more than welcome!

Best,

- Juri

[1] v1 - https://lkml.org/lkml/2015/11/23/391
[2] v2 - https://lkml.org/lkml/2016/1/8/417
[3] https://lkml.org/lkml/2015/7/7/754

Juri Lelli (6):
  ARM: initialize cpu_scale to its default
  drivers/cpufreq: implement init_cpu_capacity_default()
  arm: Enable dynamic CPU capacity initialization
  arm64: Enable dynamic CPU capacity initialization
  arm: add sysfs cpu_capacity attribute
  arm64: add sysfs cpu_capacity attribute

 Documentation/kernel-parameters.txt |   4 +
 arch/arm/kernel/topology.c          |  79 +++++++++++++++-
 arch/arm64/kernel/topology.c        |  85 ++++++++++++++++++
 drivers/cpufreq/Makefile            |   2 +-
 drivers/cpufreq/cpufreq.c           |   1 +
 drivers/cpufreq/cpufreq_capacity.c  | 174 ++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h             |   2 +
 7 files changed, 342 insertions(+), 5 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq_capacity.c

-- 
2.7.0

[toc] | [next] | [standalone]

#1325304 — [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization

Define arch_wants_init_cpu_capacity() to return true; so that
cpufreq_init_cpu_capacity() can go ahead and profile CPU capacities
at boot time.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm64/kernel/topology.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 3b75d63..f2513a6 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -35,6 +35,11 @@ void set_capacity_scale(unsigned int cpu, unsigned long capacity)
 	per_cpu(cpu_scale, cpu) = capacity;
 }
 
+bool arch_wants_init_cpu_capacity(void)
+{
+	return true;
+}
+
 static int __init get_cpu_for_node(struct device_node *node)
 {
 	struct device_node *cpu_node;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1328993 — Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization

On 03/02/16 11:59, Juri Lelli wrote:
> Define arch_wants_init_cpu_capacity() to return true; so that
> cpufreq_init_cpu_capacity() can go ahead and profile CPU capacities
> at boot time.

[...]

>  
> +bool arch_wants_init_cpu_capacity(void)
> +{
> +	return true;

Isn't this a little bit too simple? Not every ARM/ARM64 platform is a
heterogeneous one.

You could add code to compare the cpu node 'compatible' properties
(required) and only return true if they differ, which would let you
detect uarch based heterogeneity.

In case of max. frequency based heterogeneity (clusters consisting of
same cpu types but running at different max. frequency), you're at the
mercy of cpu node 'clock-frequency' properties (optional).
We might argue that for these platforms, providing cpu node
'clock-frequency' properties is necessary.
The 'struct cpu_efficiency table_efficiency[]' based approach in ARM
already faces this problem.

> +}
> +
>  static int __init get_cpu_for_node(struct device_node *node)
>  {
>  	struct device_node *cpu_node;
> 

[toc] | [prev] | [next] | [standalone]

#1329027 — Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization

[Multipart message — attachments visible in raw view] — view raw

On Mon, Feb 08, 2016 at 12:28:39PM +0000, Dietmar Eggemann wrote:
> On 03/02/16 11:59, Juri Lelli wrote:

> > +bool arch_wants_init_cpu_capacity(void)
> > +{
> > +	return true;

> Isn't this a little bit too simple? Not every ARM/ARM64 platform is a
> heterogeneous one.

Does it matter?  Is there any problem with doing the callibration and
having it say that all the CPUs performs very similarly?  My
understanding was that this was simply saying it was worth checking to
see if there was some asymmetry.

[toc] | [prev] | [next] | [standalone]

#1329065 — Re: [PATCH v3 4/6] arm64: Enable dynamic CPU capacity initialization

On 08/02/16 13:13, Mark Brown wrote:
> On Mon, Feb 08, 2016 at 12:28:39PM +0000, Dietmar Eggemann wrote:
>> On 03/02/16 11:59, Juri Lelli wrote:
> 
>>> +bool arch_wants_init_cpu_capacity(void)
>>> +{
>>> +	return true;
> 
>> Isn't this a little bit too simple? Not every ARM/ARM64 platform is a
>> heterogeneous one.
> 
> Does it matter?  Is there any problem with doing the callibration and
> having it say that all the CPUs performs very similarly?  My
> understanding was that this was simply saying it was worth checking to
> see if there was some asymmetry.
> 

No, the calibration would work on any platform. I can see your point,
you want to have this feature not depend on dt.

[toc] | [prev] | [next] | [standalone]

#1325312 — [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute

Add a sysfs cpu_capacity attribute with which it is possible to read and
write (thus over-writing default values) CPUs capacity. This might be
useful in situation where there is no way to get proper default values
at boot time.

The new attribute shows up as:

 /sys/devices/system/cpu/cpu*/cpu_capacity

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Brown <broonie@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm64/kernel/topology.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index f2513a6..f05cc07 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -40,6 +40,74 @@ bool arch_wants_init_cpu_capacity(void)
 	return true;
 }
 
+#ifdef CONFIG_PROC_SYSCTL
+#include <asm/cpu.h>
+#include <linux/string.h>
+static ssize_t show_cpu_capacity(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	ssize_t rc;
+	int cpunum = cpu->dev.id;
+	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
+
+	rc = sprintf(buf, "%lu\n", capacity);
+
+	return rc;
+}
+
+static ssize_t store_cpu_capacity(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	int this_cpu = cpu->dev.id, i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (count) {
+		char *p = (char *) buf;
+
+		ret = kstrtoul(p, 0, &new_capacity);
+		if (ret)
+			return ret;
+		if (new_capacity > SCHED_CAPACITY_SCALE)
+			return -EINVAL;
+
+		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+			set_capacity_scale(i, new_capacity);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(cpu_capacity,
+		   0644,
+		   show_cpu_capacity,
+		   store_cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+	int i;
+	struct device *cpu;
+
+	for_each_possible_cpu(i) {
+		cpu = get_cpu_device(i);
+		if (!cpu) {
+			pr_err("%s: too early to get CPU%d device!\n",
+			       __func__, i);
+			continue;
+		}
+		device_create_file(cpu, &dev_attr_cpu_capacity);
+	}
+
+	return 0;
+}
+late_initcall(register_cpu_capacity_sysctl);
+#endif
+
 static int __init get_cpu_for_node(struct device_node *node)
 {
 	struct device_node *cpu_node;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1327954 — Re: [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute

Hi Juri,

On 03/02/16 11:59, Juri Lelli wrote:
> Add a sysfs cpu_capacity attribute with which it is possible to read and
> write (thus over-writing default values) CPUs capacity. This might be
> useful in situation where there is no way to get proper default values
> at boot time.
> 
> The new attribute shows up as:
> 
>  /sys/devices/system/cpu/cpu*/cpu_capacity
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Brown <broonie@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>  arch/arm64/kernel/topology.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index f2513a6..f05cc07 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -40,6 +40,74 @@ bool arch_wants_init_cpu_capacity(void)
>  	return true;
>  }
>  
> +#ifdef CONFIG_PROC_SYSCTL
> +#include <asm/cpu.h>
> +#include <linux/string.h>
> +static ssize_t show_cpu_capacity(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct cpu *cpu = container_of(dev, struct cpu, dev);
> +	ssize_t rc;
> +	int cpunum = cpu->dev.id;
> +	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);

Gives me an implicit declaration of function ‘arch_scale_cpu_capacity’
error [without the commit fbc899610e1a ("arm64: Update
arch_scale_cpu_capacity() to reflect change to define") on your
git://linux-arm.org/linux-jl.git upstream/default_caps_v3 branch].

Why don't you just return cpu_scale

@@ -49,10 +49,8 @@ static ssize_t show_cpu_capacity(struct device *dev,
 {
        struct cpu *cpu = container_of(dev, struct cpu, dev);
        ssize_t rc;
-       int cpunum = cpu->dev.id;
-       unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);

-       rc = sprintf(buf, "%lu\n", capacity);
+       rc = sprintf(buf, "%lu\n", per_cpu(cpu_scale, cpu->dev.id));

        return rc;
 }

to get rid of this dependency?

-- Dietmar

[...]

[toc] | [prev] | [next] | [standalone]

#1327966 — Re: [PATCH v3 6/6] arm64: add sysfs cpu_capacity attribute

Hi,

On 05/02/16 17:19, Dietmar Eggemann wrote:
> Hi Juri,
> 
> On 03/02/16 11:59, Juri Lelli wrote:
> > Add a sysfs cpu_capacity attribute with which it is possible to read and
> > write (thus over-writing default values) CPUs capacity. This might be
> > useful in situation where there is no way to get proper default values
> > at boot time.
> > 
> > The new attribute shows up as:
> > 
> >  /sys/devices/system/cpu/cpu*/cpu_capacity
> > 
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Brown <broonie@linaro.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > ---
> >  arch/arm64/kernel/topology.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> > index f2513a6..f05cc07 100644
> > --- a/arch/arm64/kernel/topology.c
> > +++ b/arch/arm64/kernel/topology.c
> > @@ -40,6 +40,74 @@ bool arch_wants_init_cpu_capacity(void)
> >  	return true;
> >  }
> >  
> > +#ifdef CONFIG_PROC_SYSCTL
> > +#include <asm/cpu.h>
> > +#include <linux/string.h>
> > +static ssize_t show_cpu_capacity(struct device *dev,
> > +				 struct device_attribute *attr,
> > +				 char *buf)
> > +{
> > +	struct cpu *cpu = container_of(dev, struct cpu, dev);
> > +	ssize_t rc;
> > +	int cpunum = cpu->dev.id;
> > +	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
> 
> Gives me an implicit declaration of function ‘arch_scale_cpu_capacity’
> error [without the commit fbc899610e1a ("arm64: Update
> arch_scale_cpu_capacity() to reflect change to define") on your
> git://linux-arm.org/linux-jl.git upstream/default_caps_v3 branch].
> 
> Why don't you just return cpu_scale
> 
> @@ -49,10 +49,8 @@ static ssize_t show_cpu_capacity(struct device *dev,
>  {
>         struct cpu *cpu = container_of(dev, struct cpu, dev);
>         ssize_t rc;
> -       int cpunum = cpu->dev.id;
> -       unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
> 
> -       rc = sprintf(buf, "%lu\n", capacity);
> +       rc = sprintf(buf, "%lu\n", per_cpu(cpu_scale, cpu->dev.id));
> 
>         return rc;
>  }
> 
> to get rid of this dependency?
> 

Right! I'll fix this in the next version.

Thanks,

- Juri

[toc] | [prev] | [next] | [standalone]

#1325313 — [PATCH v3 5/6] arm: add sysfs cpu_capacity attribute

Add a sysfs cpu_capacity attribute with which it is possible to read and
write (thus over-writing default values) CPUs capacity. This might be
useful in situation where there is no way to get proper default values
at boot time.

The new attribute shows up as:

 /sys/devices/system/cpu/cpu*/cpu_capacity

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm/kernel/topology.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 7d7fc2c..9ffba65 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -57,6 +57,74 @@ bool arch_wants_init_cpu_capacity(void)
 	return true;
 }
 
+#ifdef CONFIG_PROC_SYSCTL
+#include <asm/cpu.h>
+#include <linux/string.h>
+static ssize_t show_cpu_capacity(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	ssize_t rc;
+	int cpunum = cpu->dev.id;
+	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
+
+	rc = sprintf(buf, "%lu\n", capacity);
+
+	return rc;
+}
+
+static ssize_t store_cpu_capacity(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	int this_cpu = cpu->dev.id, i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (count) {
+		char *p = (char *) buf;
+
+		ret = kstrtoul(p, 0, &new_capacity);
+		if (ret)
+			return ret;
+		if (new_capacity > SCHED_CAPACITY_SCALE)
+			return -EINVAL;
+
+		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+			set_capacity_scale(i, new_capacity);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(cpu_capacity,
+		   0644,
+		   show_cpu_capacity,
+		   store_cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+	int i;
+	struct device *cpu;
+
+	for_each_possible_cpu(i) {
+		cpu = get_cpu_device(i);
+		if (!cpu) {
+			pr_err("%s: too early to get CPU%d device!\n",
+			       __func__, i);
+			continue;
+		}
+		device_create_file(cpu, &dev_attr_cpu_capacity);
+	}
+
+	return 0;
+}
+late_initcall(register_cpu_capacity_sysctl);
+#endif
+
 #ifdef CONFIG_OF
 struct cpu_efficiency {
 	const char *compatible;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1325322 — [PATCH v3 3/6] arm: Enable dynamic CPU capacity initialization

Define arch_wants_init_cpu_capacity() to return true; so that
cpufreq_init_cpu_capacity() can go ahead and profile CPU capacities
at boot time.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm/kernel/topology.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index c9c87a5..7d7fc2c 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -52,6 +52,11 @@ void set_capacity_scale(unsigned int cpu, unsigned long capacity)
 	per_cpu(cpu_scale, cpu) = capacity;
 }
 
+bool arch_wants_init_cpu_capacity(void)
+{
+	return true;
+}
+
 #ifdef CONFIG_OF
 struct cpu_efficiency {
 	const char *compatible;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1329731 — Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous systems

Hi Juri,

On 02/03/2016 03:59 AM, Juri Lelli wrote:
>  v1: DT + sysfs [1]
> 
>  v2: Dynamic profiling at boot [2]
> 
> Third version of this patchset proposes what seems to be the solution we agreed
> upon (see [2] for reference) to the problem of how do we init CPUs original
> capacity: we run a bogus benchmark (stealing int_sqrt from lib/ we run that in
> a loop to perform some integer computation, better benchmarks are welcome)
> on the first cpu of each frequency domain (assuming no u-arch differences
> inside domains), measure time to complete a fixed number of iterations and then
> normalize results to SCHED_CAPACITY_SCALE (1024). This time around we also
> added a boot time parameter to disable profiling at boot (as it can be time
> consuming) and sysfs attributes with which default values can be overwritten.
> The proposed solution is basically putting together bits of v1 and v2 that are
> considered valuable and acceptable for mainline.

I'm still concerned that there's no way to obtain optimal boot time on a
heterogeneous system. Either the dynamic benchmarking is enabled, adding
1 sec, or the benchmarking is skipped, and task distribution on the
heterogeneous CPUs is determined by the platform's CPU numbering and
chance, potentially impacting performance nondeterministically until
userspace sets the correct capacity values via sysfs.

I believe you tested the impact on boot time of using equal capacity
values and saw little difference. I'm wondering though, what was the CPU
numbering on that target?

thanks,
Steve

[toc] | [prev] | [next] | [standalone]

#1330057 — Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous systems

Hi Steve,

On 08/02/16 15:59, Steve Muckle wrote:
> Hi Juri,
> 
> On 02/03/2016 03:59 AM, Juri Lelli wrote:
> >  v1: DT + sysfs [1]
> > 
> >  v2: Dynamic profiling at boot [2]
> > 
> > Third version of this patchset proposes what seems to be the solution we agreed
> > upon (see [2] for reference) to the problem of how do we init CPUs original
> > capacity: we run a bogus benchmark (stealing int_sqrt from lib/ we run that in
> > a loop to perform some integer computation, better benchmarks are welcome)
> > on the first cpu of each frequency domain (assuming no u-arch differences
> > inside domains), measure time to complete a fixed number of iterations and then
> > normalize results to SCHED_CAPACITY_SCALE (1024). This time around we also
> > added a boot time parameter to disable profiling at boot (as it can be time
> > consuming) and sysfs attributes with which default values can be overwritten.
> > The proposed solution is basically putting together bits of v1 and v2 that are
> > considered valuable and acceptable for mainline.
> 
> I'm still concerned that there's no way to obtain optimal boot time on a
> heterogeneous system. Either the dynamic benchmarking is enabled, adding
> 1 sec, or the benchmarking is skipped, and task distribution on the
> heterogeneous CPUs is determined by the platform's CPU numbering and
> chance, potentially impacting performance nondeterministically until
> userspace sets the correct capacity values via sysfs.
> 
> I believe you tested the impact on boot time of using equal capacity
> values and saw little difference. I'm wondering though, what was the CPU
> numbering on that target?
> 

My targets (Juno and TC2) had big cluster on 1,2 and little on the
remaining cpus. Why do you think this might matter?

Anyway, IMHO boot time performance is not what we are targeting here, so
I wouldn't be too worried about this particular point.

Best,

- Juri

[toc] | [prev] | [next] | [standalone]

#1330524 — Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous systems

On 02/09/2016 02:37 AM, Juri Lelli wrote:
>> I'm still concerned that there's no way to obtain optimal boot time on a
>> > heterogeneous system. Either the dynamic benchmarking is enabled, adding
>> > 1 sec, or the benchmarking is skipped, and task distribution on the
>> > heterogeneous CPUs is determined by the platform's CPU numbering and
>> > chance, potentially impacting performance nondeterministically until
>> > userspace sets the correct capacity values via sysfs.
>> > 
>> > I believe you tested the impact on boot time of using equal capacity
>> > values and saw little difference. I'm wondering though, what was the CPU
>> > numbering on that target?
>> > 
>
> My targets (Juno and TC2) had big cluster on 1,2 and little on the
> remaining cpus. Why do you think this might matter?

There's a natural bias in the scheduler AFAIK towards lower-numbered
CPUs since they are typically scanned in numerically ascending order. So
when all capacities are initially defaulted to be the same I think
you'll be more likely to use the lower numbered CPUs.

I'd be curious what the performance penalty is on a b.L system where the
lowest numbered CPUs are small. I don't have such a target but maybe
it's possible to compare booting just with bigs vs just with littles, at
least until userspace intializes and a script can bring up the others,
which is the same point at which capacities could be properly set. That
would give something of an upper bound.

> Anyway, IMHO boot time performance is not what we are targeting here, so
> I wouldn't be too worried about this particular point.

It may not be the most important thing but it is a factor worth
considering - as mentioned earlier there are applications where boot
time is critical such as automotive. It seems unfortunate that actual
performance may be left on the table due to (IMO anyway) a tenuous
concern over DT semantics. But it looks like that may just be my
position :/ .

thanks,
Steve

[toc] | [prev] | [next] | [standalone]

#1330541 — Re: [PATCH v3 0/6] CPUs capacity information for heterogeneous systems

On 09/02/16 09:30, Steve Muckle wrote:
> On 02/09/2016 02:37 AM, Juri Lelli wrote:
> >> I'm still concerned that there's no way to obtain optimal boot time on a
> >> > heterogeneous system. Either the dynamic benchmarking is enabled, adding
> >> > 1 sec, or the benchmarking is skipped, and task distribution on the
> >> > heterogeneous CPUs is determined by the platform's CPU numbering and
> >> > chance, potentially impacting performance nondeterministically until
> >> > userspace sets the correct capacity values via sysfs.
> >> > 
> >> > I believe you tested the impact on boot time of using equal capacity
> >> > values and saw little difference. I'm wondering though, what was the CPU
> >> > numbering on that target?
> >> > 
> >
> > My targets (Juno and TC2) had big cluster on 1,2 and little on the
> > remaining cpus. Why do you think this might matter?
> 
> There's a natural bias in the scheduler AFAIK towards lower-numbered
> CPUs since they are typically scanned in numerically ascending order. So
> when all capacities are initially defaulted to be the same I think
> you'll be more likely to use the lower numbered CPUs.
> 
> I'd be curious what the performance penalty is on a b.L system where the
> lowest numbered CPUs are small. I don't have such a target but maybe
> it's possible to compare booting just with bigs vs just with littles, at
> least until userspace intializes and a script can bring up the others,
> which is the same point at which capacities could be properly set. That
> would give something of an upper bound.
> 

Yeah. I could run some tests along this line. It should give us a rough
idea about how much we are leaving on the table.

> > Anyway, IMHO boot time performance is not what we are targeting here, so
> > I wouldn't be too worried about this particular point.
> 
> It may not be the most important thing but it is a factor worth
> considering - as mentioned earlier there are applications where boot
> time is critical such as automotive. It seems unfortunate that actual
> performance may be left on the table due to (IMO anyway) a tenuous
> concern over DT semantics. But it looks like that may just be my
> position :/ .
> 

Don't get me wrong Steve. I agree with you and I tried to defend the DT
approach as much as I could. I still think that it is the best solution
(much more cleaner and simpler), but it seems that there is no way we
can make it happen. Or has this discussion we are having changed things
in the meantime? :)

Thanks,

- Juri

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web