[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In non-heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 146 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7fd473367e9b..eae508ca816a 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -26,6 +26,29 @@
 #include <asm/smp_plat.h>
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int cluster_id;
 
@@ -143,9 +166,50 @@ static void __init parse_cluster(struct device_node *cluster)
 		cluster_id++;
 }
 
+#ifdef CONFIG_OF
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = (unsigned long)(-1);
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int alloc_size, cpu;
+
+	alloc_size = nr_cpu_ids * sizeof(*__cpu_capacity);
+	__cpu_capacity = kzalloc(alloc_size, GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -161,10 +225,88 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		/* check if the cpu is marked as "disabled", if so ignore */
+		if (!of_device_is_available(cn))
+			continue;
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -213,6 +355,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -232,6 +376,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->socket_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.1

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In non-heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 145 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 5a2724b3d4b7..68ccf4f4f258 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -23,6 +23,29 @@
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int cluster_id;
 
@@ -140,9 +163,49 @@ static void __init parse_cluster(struct device_node *cluster)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = (unsigned long)(-1);
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int alloc_size, cpu;
+
+	alloc_size = nr_cpu_ids * sizeof(*__cpu_capacity);
+	__cpu_capacity = kzalloc(alloc_size, GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -158,10 +221,88 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		/* check if the cpu is marked as "disabled", if so ignore */
+		if (!of_device_is_available(cn))
+			continue;
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -210,6 +351,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -229,6 +372,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->socket_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.2

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Thu, Dec 19, 2013 at 08:06:14PM +0000, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> In non-heterogeneous systems like big.LITTLE systems the scheduler will be
> able to make better use of the available cores if we provide power numbers
> to it indicating their relative performance. Do this by parsing the CPU
> nodes in the DT.
> 
> This code currently has no effect as no information on the relative
> performance of the cores is provided.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
>  arch/arm64/kernel/topology.c | 145 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c

[...]

> +/*
> + * Iterate all CPUs' descriptor in DT and compute the efficiency
> + * (as per table_efficiency). Also calculate a middle efficiency
> + * as close as possible to  (max{eff_i} - min{eff_i}) / 2
> + * This is later used to scale the cpu_power field such that an
> + * 'average' CPU is of middle power. Also see the comments near
> + * table_efficiency[] and update_cpu_power().
> + */
>  static void __init parse_dt_topology(void)
>  {
> +	const struct cpu_efficiency *cpu_eff;
>  	struct device_node *cn;
> +	unsigned long min_capacity = (unsigned long)(-1);

ULONG_MAX ?

> +	unsigned long max_capacity = 0;
> +	unsigned long capacity = 0;
> +	int alloc_size, cpu;
> +
> +	alloc_size = nr_cpu_ids * sizeof(*__cpu_capacity);
> +	__cpu_capacity = kzalloc(alloc_size, GFP_NOWAIT);

kcalloc ? BTW this patch should include slab.h not the previous ones,
because that's the only patch where memory allocation takes place unless
I am missing something.

>  	cn = of_find_node_by_path("/cpus");
>  	if (!cn) {
> @@ -158,10 +221,88 @@ static void __init parse_dt_topology(void)
>  	if (!cn)
>  		return;
>  	parse_cluster(cn);
> +
> +	for_each_possible_cpu(cpu) {
> +		const u32 *rate;
> +		int len;
> +
> +		/* Too early to use cpu->of_node */
> +		cn = of_get_cpu_node(cpu, NULL);
> +		if (!cn) {
> +			pr_err("Missing device node for CPU %d\n", cpu);
> +			continue;
> +		}
> +
> +		/* check if the cpu is marked as "disabled", if so ignore */
> +		if (!of_device_is_available(cn))
> +			continue;

It is time we defined what a "disabled" CPU means in ARM world, I need to
have a proper look into this since this topic has been brought up before.

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Tue, Jan 07, 2014 at 01:05:40PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Dec 19, 2013 at 08:06:14PM +0000, Mark Brown wrote:

> > +		/* check if the cpu is marked as "disabled", if so ignore */
> > +		if (!of_device_is_available(cn))
> > +			continue;

> It is time we defined what a "disabled" CPU means in ARM world, I need to
> have a proper look into this since this topic has been brought up before.

What is the confusion here - why would there be something architecture
specific going on?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140107/c7d57827/attachment.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Tue, Jan 07, 2014 at 01:38:29PM +0000, Mark Brown wrote:
> On Tue, Jan 07, 2014 at 01:05:40PM +0000, Lorenzo Pieralisi wrote:
> > On Thu, Dec 19, 2013 at 08:06:14PM +0000, Mark Brown wrote:
> 
> > > +		/* check if the cpu is marked as "disabled", if so ignore */
> > > +		if (!of_device_is_available(cn))
> > > +			continue;
> 
> > It is time we defined what a "disabled" CPU means in ARM world, I need to
> > have a proper look into this since this topic has been brought up before.
> 
> What is the confusion here - why would there be something architecture
> specific going on?

I think this check was added following this thread discussion:

http://lkml.indiana.edu/hypermail/linux/kernel/1306.0/03663.html

So my question is: what does "disabled" mean ? A CPU present in HW
that can't/must not be booted ?

ePAPR v1.1 page 43:

"disabled". The CPU is in a quiescent state. A quiescent CPU is in a state
where it cannot interfere with the normal operation of other CPUs, nor can
its state be affected by the normal operation of other running CPUs, except
by an explicit method for enabling or reenabling the quiescent CPU (see the
enable-method property).

This means that a "disabled" CPU can be booted with eg PSCI but that is
not what the thread in the link above wants to achieve. Furthermore, if
we add the check in topology.c, the check must be executed also when
building the cpu_logical_map, otherwise a "disabled" cpu would be marked
possible and then booted, am I wrong ?

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Tue, Jan 07, 2014 at 02:29:29PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Jan 07, 2014 at 01:38:29PM +0000, Mark Brown wrote:
> > On Tue, Jan 07, 2014 at 01:05:40PM +0000, Lorenzo Pieralisi wrote:

> > > It is time we defined what a "disabled" CPU means in ARM world, I need to
> > > have a proper look into this since this topic has been brought up before.

> > What is the confusion here - why would there be something architecture
> > specific going on?

> I think this check was added following this thread discussion:

> http://lkml.indiana.edu/hypermail/linux/kernel/1306.0/03663.html

> So my question is: what does "disabled" mean ? A CPU present in HW
> that can't/must not be booted ?

Yes, that would seem to be the obvious meaning and consistent with ePAPR
(in so far as we're paying a blind bit of notice to ePAPR, see other
threads...).

> ePAPR v1.1 page 43:

> "disabled". The CPU is in a quiescent state. A quiescent CPU is in a state
> where it cannot interfere with the normal operation of other CPUs, nor can
> its state be affected by the normal operation of other running CPUs, except
> by an explicit method for enabling or reenabling the quiescent CPU (see the
> enable-method property).

> This means that a "disabled" CPU can be booted with eg PSCI but that is
> not what the thread in the link above wants to achieve. Furthermore, if

I think that's just another bit of ill considered wording from ePAPR
that doesn't really reflect reality; it seems like what they're trying
to shoot for there is administratively down.  

At the very least that means hot unplugged, and it seems reasonable to
read that as being stronger than that.  The current ARM implementation
is more conservative since it doesn't provide any way to put the core on
line but it does seem more likely to match what a system integrator
would be trying to achieve and it also matches the standard meaning of
disabled.

> we add the check in topology.c, the check must be executed also when
> building the cpu_logical_map, otherwise a "disabled" cpu would be marked
> possible and then booted, am I wrong ?

Right, this is a separate issue in the SMP enumeration code - it should
be paying attention to the property and at the very least defaulting the
core to being unplugged, though like I say I do find the ARM meaning
more sane.

In any case I don't vastly care, I guess I'll drop this for now.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140107/b9590c39/attachment-0001.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Tue, Jan 07, 2014 at 03:06:31PM +0000, Mark Brown wrote:
> On Tue, Jan 07, 2014 at 02:29:29PM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Jan 07, 2014 at 01:38:29PM +0000, Mark Brown wrote:
> > > On Tue, Jan 07, 2014 at 01:05:40PM +0000, Lorenzo Pieralisi wrote:
> 
> > > > It is time we defined what a "disabled" CPU means in ARM world, I need to
> > > > have a proper look into this since this topic has been brought up before.
> 
> > > What is the confusion here - why would there be something architecture
> > > specific going on?
> 
> > I think this check was added following this thread discussion:
> 
> > http://lkml.indiana.edu/hypermail/linux/kernel/1306.0/03663.html
> 
> > So my question is: what does "disabled" mean ? A CPU present in HW
> > that can't/must not be booted ?
> 
> Yes, that would seem to be the obvious meaning and consistent with ePAPR
> (in so far as we're paying a blind bit of notice to ePAPR, see other
> threads...).

Just playing devil's advocate and trying to reuse ePAPR bindings as much as
possible, provided they define what we need on ARM. In this case it seems they
do not.

> > ePAPR v1.1 page 43:
> 
> > "disabled". The CPU is in a quiescent state. A quiescent CPU is in a state
> > where it cannot interfere with the normal operation of other CPUs, nor can
> > its state be affected by the normal operation of other running CPUs, except
> > by an explicit method for enabling or reenabling the quiescent CPU (see the
> > enable-method property).
> 
> > This means that a "disabled" CPU can be booted with eg PSCI but that is
> > not what the thread in the link above wants to achieve. Furthermore, if
> 
> I think that's just another bit of ill considered wording from ePAPR
> that doesn't really reflect reality; it seems like what they're trying
> to shoot for there is administratively down.  
> 
> At the very least that means hot unplugged, and it seems reasonable to
> read that as being stronger than that.  The current ARM implementation
> is more conservative since it doesn't provide any way to put the core on
> line but it does seem more likely to match what a system integrator
> would be trying to achieve and it also matches the standard meaning of
> disabled.

What do you mean by ARM implementation ? The status property is currently
ignored on ARM. I'd agree with what you are saying but that should be
specified in DT bindings.

> > we add the check in topology.c, the check must be executed also when
> > building the cpu_logical_map, otherwise a "disabled" cpu would be marked
> > possible and then booted, am I wrong ?
> 
> Right, this is a separate issue in the SMP enumeration code - it should
> be paying attention to the property and at the very least defaulting the
> core to being unplugged, though like I say I do find the ARM meaning
> more sane.

Again, I tend to agree, since this means that the CPU is there but
simply is not a "possible" one. To be debated.

> In any case I don't vastly care, I guess I'll drop this for now.

Yes, I think dropping the check is fine for now, we can add it if/when
we achieve consensus, that should not be a big deal.

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Tue, Jan 07, 2014 at 05:56:42PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Jan 07, 2014 at 03:06:31PM +0000, Mark Brown wrote:

> > At the very least that means hot unplugged, and it seems reasonable to
> > read that as being stronger than that.  The current ARM implementation
> > is more conservative since it doesn't provide any way to put the core on
> > line but it does seem more likely to match what a system integrator
> > would be trying to achieve and it also matches the standard meaning of
> > disabled.

> What do you mean by ARM implementation ? The status property is currently
> ignored on ARM. I'd agree with what you are saying but that should be
> specified in DT bindings.

The 32 bit ARM implementation.  That code was supposed to be just
cut'n'pasted from there, though I see now it wasn't...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140107/59c818df/attachment.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In non-heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 145 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index e77c6b0844be..e5140b03b342 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -23,6 +23,29 @@
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int cluster_id;
 
@@ -144,9 +167,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -162,10 +225,88 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		/* check if the cpu is marked as "disabled", if so ignore */
+		if (!of_device_is_available(cn))
+			continue;
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -214,6 +355,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -233,6 +376,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->socket_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.2

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Wed, Jan 08, 2014 at 05:10:12PM +0000, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>

[...]

> +		/* check if the cpu is marked as "disabled", if so ignore */
> +		if (!of_device_is_available(cn))
> +			continue;

This check must go. As I mentioned, either we define what this means properly
or to me the status property of cpu nodes must not be relied upon on ARM.

I can add a patch on top of your series that defines the behaviour for both
arm32 and arm64 when we reach agreement and code initializing
cpu_logical_map should be patched as well, accordingly.

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Wed, Jan 08, 2014 at 06:39:39PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Jan 08, 2014 at 05:10:12PM +0000, Mark Brown wrote:
> > From: Mark Brown <broonie@linaro.org>

> > +		/* check if the cpu is marked as "disabled", if so ignore */
> > +		if (!of_device_is_available(cn))
> > +			continue;

> This check must go. As I mentioned, either we define what this means properly
> or to me the status property of cpu nodes must not be relied upon on ARM.

Meh, sorry - forgot to squash in that patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140108/81ab1777/attachment-0001.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In non-heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 141 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index e77c6b0844be..e1751e0e6142 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -23,6 +23,29 @@
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int cluster_id;
 
@@ -144,9 +167,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -162,10 +225,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -214,6 +351,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -233,6 +372,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->socket_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.2

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Robin Randhawa]

Hi Mark.

On 08/01/2014, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> In non-heterogeneous systems like big.LITTLE systems the scheduler will be

Minor nit: 'Heterogeneous systems' or 'non-homogeneous
systems' would be more appropriate.

Robin

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 144 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7ef0d783ffff..2748b252d4e7 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -89,7 +113,7 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 	bool leaf = true;
 	bool has_cores = false;
 	struct device_node *c;
-	static int cluster_id = 0;
+	static int cluster_id;
 	int core_id = 0;
 	int i;
 
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -207,6 +345,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -226,6 +366,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->socket_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.2

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Sun, Jan 12, 2014 at 07:20:40PM +0000, Mark Brown wrote:

[...]

> @@ -89,7 +113,7 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
>  	bool leaf = true;
>  	bool has_cores = false;
>  	struct device_node *c;
> -	static int cluster_id = 0;
> +	static int cluster_id;

It has to be __initdata, and the line change above does not belong in
this patch but patch 1.

[...]

>  static void __init parse_dt_topology(void)
>  {
> +	const struct cpu_efficiency *cpu_eff;
>  	struct device_node *cn;
> +	unsigned long min_capacity = ULONG_MAX;
> +	unsigned long max_capacity = 0;
> +	unsigned long capacity = 0;
> +	int cpu;
> +
> +	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
> +				 GFP_NOWAIT);
>  
>  	cn = of_find_node_by_path("/cpus");
>  	if (!cn) {
> @@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
>  	if (!cn)
>  		return;
>  	parse_cluster(cn, 0);
> +
> +	for_each_possible_cpu(cpu) {
> +		const u32 *rate;
> +		int len;
> +
> +		/* Too early to use cpu->of_node */
> +		cn = of_get_cpu_node(cpu, NULL);
> +		if (!cn) {
> +			pr_err("Missing device node for CPU %d\n", cpu);
> +			continue;
> +		}
> +
> +		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
> +			if (of_device_is_compatible(cn, cpu_eff->compatible))
> +				break;
> +
> +		if (cpu_eff->compatible == NULL) {
> +			pr_warn("%s: Unknown CPU type\n", cn->full_name);
> +			continue;
> +		}
> +
> +		rate = of_get_property(cn, "clock-frequency", &len);
> +		if (!rate || len != 4) {
> +			pr_err("%s: Missing clock-frequency property\n",
> +				cn->full_name);
> +			continue;
> +		}

I am wondering why we spit an error for a property that in practice is
optional. Either we make it required, or we drop the error output.

Actually this is not defined anywhere apart from the ePAPR, which
defines this property as required, but following your attempt to
standardize it for ARM, I gather it should be considered optional.

If it is optional, should we really print an error ? (I know it is the
same on arm32, I am questioning that code too).

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Mon, Jan 13, 2014 at 04:40:21PM +0000, Lorenzo Pieralisi wrote:
> On Sun, Jan 12, 2014 at 07:20:40PM +0000, Mark Brown wrote:

> > @@ -89,7 +113,7 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
> >  	bool leaf = true;
> >  	bool has_cores = false;
> >  	struct device_node *c;
> > -	static int cluster_id = 0;
> > +	static int cluster_id;

> It has to be __initdata, and the line change above does not belong in
> this patch but patch 1.

I would really have expected static data from a function marked init to
end up marked appropriately, but whatever.

> > +		rate = of_get_property(cn, "clock-frequency", &len);
> > +		if (!rate || len != 4) {
> > +			pr_err("%s: Missing clock-frequency property\n",
> > +				cn->full_name);
> > +			continue;
> > +		}

> I am wondering why we spit an error for a property that in practice is
> optional. Either we make it required, or we drop the error output.

> Actually this is not defined anywhere apart from the ePAPR, which
> defines this property as required, but following your attempt to
> standardize it for ARM, I gather it should be considered optional.

It's already standard in the spec we claim to be following...

> If it is optional, should we really print an error ? (I know it is the
> same on arm32, I am questioning that code too).

For big.LITTLE systems with the current implementation this information
is required in order to scale the relative performances of the cores -
such a system the maximum frequencies of the cores may vary as well as
their type (or indeed theoretically even only their maximum frequency).
We could at some future time end up evolving the code so that this
information is acquired from cpufreq or somewhere but that's something
that should probably happen kernel wide as part of the scheduler work
rather than going off and doing something custom.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140113/27886b75/attachment.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Mon, Jan 13, 2014 at 05:01:56PM +0000, Mark Brown wrote:
> On Mon, Jan 13, 2014 at 04:40:21PM +0000, Lorenzo Pieralisi wrote:
> > On Sun, Jan 12, 2014 at 07:20:40PM +0000, Mark Brown wrote:
> 
> > > @@ -89,7 +113,7 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
> > >  	bool leaf = true;
> > >  	bool has_cores = false;
> > >  	struct device_node *c;
> > > -	static int cluster_id = 0;
> > > +	static int cluster_id;
> 
> > It has to be __initdata, and the line change above does not belong in
> > this patch but patch 1.
> 
> I would really have expected static data from a function marked init to
> end up marked appropriately, but whatever.

I would not expect that.

> > > +		rate = of_get_property(cn, "clock-frequency", &len);
> > > +		if (!rate || len != 4) {
> > > +			pr_err("%s: Missing clock-frequency property\n",
> > > +				cn->full_name);
> > > +			continue;
> > > +		}
> 
> > I am wondering why we spit an error for a property that in practice is
> > optional. Either we make it required, or we drop the error output.
> 
> > Actually this is not defined anywhere apart from the ePAPR, which
> > defines this property as required, but following your attempt to
> > standardize it for ARM, I gather it should be considered optional.
> 
> It's already standard in the spec we claim to be following...

So is it required ?

> > If it is optional, should we really print an error ? (I know it is the
> > same on arm32, I am questioning that code too).
> 
> For big.LITTLE systems with the current implementation this information
> is required in order to scale the relative performances of the cores -
> such a system the maximum frequencies of the cores may vary as well as
> their type (or indeed theoretically even only their maximum frequency).
> We could at some future time end up evolving the code so that this
> information is acquired from cpufreq or somewhere but that's something
> that should probably happen kernel wide as part of the scheduler work
> rather than going off and doing something custom.

I was just referring to this thread, whose outcome is unclear to me.

http://archive.arm.linux.org.uk/lurker/message/20131206.115707.24b095f4.en.html

I am not questioning why it is needed, I am just asking whether it is
optional or not. If it is, getting error messages in the kernel log does not
seem correct.

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Tue, Jan 14, 2014 at 10:12:36AM +0000, Lorenzo Pieralisi wrote:
> On Mon, Jan 13, 2014 at 05:01:56PM +0000, Mark Brown wrote:

> > I would really have expected static data from a function marked init to
> > end up marked appropriately, but whatever.

> I would not expect that.

Really?  If something is local to a function marked init it seems like
the __init ought to carry over to it.

> > > > +		rate = of_get_property(cn, "clock-frequency", &len);

> > It's already standard in the spec we claim to be following...

> So is it required ?

That's what ePAPR says.  If that's good decision making on the part of
ePAPR or not is a separate question.

> I was just referring to this thread, whose outcome is unclear to me.

> http://archive.arm.linux.org.uk/lurker/message/20131206.115707.24b095f4.en.html

> I am not questioning why it is needed, I am just asking whether it is
> optional or not. If it is, getting error messages in the kernel log does not
> seem correct.

At present we don't really have a better way to get the information so
we're relying on it; until the scheduler is able to talk to cpufreq not
providing this information means we won't be able to provide a relative
performance estimate to the scheduler.  This means that we probably
ought to be telling the user if we couldn't figure out the top frequency
for the core.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140114/81b8c90d/attachment.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Lorenzo Pieralisi]

On Tue, Jan 14, 2014 at 12:13:43PM +0000, Mark Brown wrote:
> On Tue, Jan 14, 2014 at 10:12:36AM +0000, Lorenzo Pieralisi wrote:
> > On Mon, Jan 13, 2014 at 05:01:56PM +0000, Mark Brown wrote:
> 
> > > I would really have expected static data from a function marked init to
> > > end up marked appropriately, but whatever.
> 
> > I would not expect that.
> 
> Really?  If something is local to a function marked init it seems like
> the __init ought to carry over to it.

Yes, for the same reason as static variables declared in a function do
not end up in the .text section. You want the variable to be in the
.init.data section and the compiler initialize it to 0 for you. If it was not
declared as __initdata it would be added to the .bss section and zeroed
by the kernel.

> 
> > > > > +		rate = of_get_property(cn, "clock-frequency", &len);
> 
> > > It's already standard in the spec we claim to be following...
> 
> > So is it required ?
> 
> That's what ePAPR says.  If that's good decision making on the part of
> ePAPR or not is a separate question.
> 
> > I was just referring to this thread, whose outcome is unclear to me.
> 
> > http://archive.arm.linux.org.uk/lurker/message/20131206.115707.24b095f4.en.html
> 
> > I am not questioning why it is needed, I am just asking whether it is
> > optional or not. If it is, getting error messages in the kernel log does not
> > seem correct.
> 
> At present we don't really have a better way to get the information so
> we're relying on it; until the scheduler is able to talk to cpufreq not
> providing this information means we won't be able to provide a relative
> performance estimate to the scheduler.  This means that we probably
> ought to be telling the user if we couldn't figure out the top frequency
> for the core.

It is not the top frequency, it is the current frequency. Leaving the
log is fine by me, but actually implies that the ePAPR must be followed,
ie the property is "required".

Lorenzo

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

On Tue, Jan 14, 2014 at 01:23:19PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Jan 14, 2014 at 12:13:43PM +0000, Mark Brown wrote:
> > On Tue, Jan 14, 2014 at 10:12:36AM +0000, Lorenzo Pieralisi wrote:
> > > On Mon, Jan 13, 2014 at 05:01:56PM +0000, Mark Brown wrote:

> > > > I would really have expected static data from a function marked init to
> > > > end up marked appropriately, but whatever.

> > > I would not expect that.

> > Really?  If something is local to a function marked init it seems like
> > the __init ought to carry over to it.

> Yes, for the same reason as static variables declared in a function do
> not end up in the .text section. You want the variable to be in the
> .init.data section and the compiler initialize it to 0 for you. If it was not
> declared as __initdata it would be added to the .bss section and zeroed
> by the kernel.

I understand why it might happen from an implementation point of view
but still not what I would expect to happen - I'd have expected that
annotations applied to a function would be able to automatically do the
right thing with their data.

> It is not the top frequency, it is the current frequency. Leaving the
> log is fine by me, but actually implies that the ePAPR must be followed,
> ie the property is "required".

Tweaking the semantics there was half the point of my patch (since
having the current frequency makes no sense in the context of FDT or
anything else without a running firmware), the other bit was just making
this more discoverable since while we say we're following ePAPR I don't
think anyone actually looks at it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140114/da87ecbb/attachment.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 142 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 061e3b813b9f..951b55e24f13 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -207,6 +345,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -226,6 +366,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->cluster_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.8.5.2

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Add basic CPU topology support to arm64, based on the existing pre-v8
code and some work done by Mark Hambleton.  This patch does not
implement any topology discovery support since that should be based on
information from firmware, it merely implements the scaffolding for
integration of topology support in the architecture.

The goal is to separate the architecture hookup for providing topology
information from the DT parsing in order to ease review and avoid
blocking the architecture code (which will be built on by other work)
with the DT code review by providing something simple and basic.

A following patch will implement support for parsing the DT topology
bindings for ARM, similar patches will be needed for ACPI.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/Kconfig                | 24 +++++++++++
 arch/arm64/include/asm/topology.h | 39 +++++++++++++++++
 arch/arm64/kernel/Makefile        |  1 +
 arch/arm64/kernel/smp.c           | 11 +++++
 arch/arm64/kernel/topology.c      | 91 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 166 insertions(+)
 create mode 100644 arch/arm64/include/asm/topology.h
 create mode 100644 arch/arm64/kernel/topology.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc7202a..fea7b477676b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -164,6 +164,30 @@ config SMP
 
 	  If you don't know what to do here, say N.
 
+config CPU_TOPOLOGY
+	bool "Support CPU topology definition"
+	depends on SMP
+	default y
+	help
+	  Support CPU topology definition, based on configuration
+	  provided by the firmware.
+
+config SCHED_MC
+	bool "Multi-core scheduler support"
+	depends on CPU_TOPOLOGY
+	help
+	  Multi-core scheduler support improves the CPU scheduler's decision
+	  making when dealing with multi-core CPU chips at a cost of slightly
+	  increased overhead in some places. If unsure say N here.
+
+config SCHED_SMT
+	bool "SMT scheduler support"
+	depends on CPU_TOPOLOGY
+	help
+	  Improves the CPU scheduler's decision making when dealing with
+	  MultiThreading at a cost of slightly increased overhead in some
+	  places. If unsure say N here.
+
 config NR_CPUS
 	int "Maximum number of CPUs (2-32)"
 	range 2 32
diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
new file mode 100644
index 000000000000..c8a47e8f452b
--- /dev/null
+++ b/arch/arm64/include/asm/topology.h
@@ -0,0 +1,39 @@
+#ifndef __ASM_TOPOLOGY_H
+#define __ASM_TOPOLOGY_H
+
+#ifdef CONFIG_CPU_TOPOLOGY
+
+#include <linux/cpumask.h>
+
+struct cpu_topology {
+	int thread_id;
+	int core_id;
+	int cluster_id;
+	cpumask_t thread_sibling;
+	cpumask_t core_sibling;
+};
+
+extern struct cpu_topology cpu_topology[NR_CPUS];
+
+#define topology_physical_package_id(cpu)	(cpu_topology[cpu].cluster_id)
+#define topology_core_id(cpu)		(cpu_topology[cpu].core_id)
+#define topology_core_cpumask(cpu)	(&cpu_topology[cpu].core_sibling)
+#define topology_thread_cpumask(cpu)	(&cpu_topology[cpu].thread_sibling)
+
+#define mc_capable()	(cpu_topology[0].cluster_id != -1)
+#define smt_capable()	(cpu_topology[0].thread_id != -1)
+
+void init_cpu_topology(void);
+void store_cpu_topology(unsigned int cpuid);
+const struct cpumask *cpu_coregroup_mask(int cpu);
+
+#else
+
+static inline void init_cpu_topology(void) { }
+static inline void store_cpu_topology(unsigned int cpuid) { }
+
+#endif
+
+#include <asm-generic/topology.h>
+
+#endif /* _ASM_ARM_TOPOLOGY_H */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 2d4554b13410..252b62181532 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -20,6 +20,7 @@ arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o
 arm64-obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND)	+= sleep.o suspend.o
 arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
+arm64-obj-$(CONFIG_CPU_TOPOLOGY)	+= topology.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 7cfb92a4ab66..9660750f34ba 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -114,6 +114,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 	return ret;
 }
 
+static void smp_store_cpu_info(unsigned int cpuid)
+{
+	store_cpu_topology(cpuid);
+}
+
 /*
  * This is the secondary CPU boot entry.  We're using this CPUs
  * idle thread stack, but a set of temporary page tables.
@@ -152,6 +157,8 @@ asmlinkage void secondary_start_kernel(void)
 	 */
 	notify_cpu_starting(cpu);
 
+	smp_store_cpu_info(cpu);
+
 	/*
 	 * OK, now it's safe to let the boot CPU continue.  Wait for
 	 * the CPU migration code to notice that the CPU is online
@@ -390,6 +397,10 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	int err;
 	unsigned int cpu, ncores = num_possible_cpus();
 
+	init_cpu_topology();
+
+	smp_store_cpu_info(smp_processor_id());
+
 	/*
 	 * are we trying to boot more cores than exist?
 	 */
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
new file mode 100644
index 000000000000..a6d4ed2d69c0
--- /dev/null
+++ b/arch/arm64/kernel/topology.c
@@ -0,0 +1,91 @@
+/*
+ * arch/arm64/kernel/topology.c
+ *
+ * Copyright (C) 2011,2013 Linaro Limited.
+ *
+ * Based on the arm32 version written by Vincent Guittot in turn based on
+ * arch/sh/kernel/topology.c
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+#include <linux/node.h>
+#include <linux/nodemask.h>
+#include <linux/sched.h>
+
+#include <asm/topology.h>
+
+/*
+ * cpu topology table
+ */
+struct cpu_topology cpu_topology[NR_CPUS];
+EXPORT_SYMBOL_GPL(cpu_topology);
+
+const struct cpumask *cpu_coregroup_mask(int cpu)
+{
+	return &cpu_topology[cpu].core_sibling;
+}
+
+static void update_siblings_masks(unsigned int cpuid)
+{
+	struct cpu_topology *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
+	int cpu;
+
+	/* update core and thread sibling masks */
+	for_each_possible_cpu(cpu) {
+		cpu_topo = &cpu_topology[cpu];
+
+		if (cpuid_topo->cluster_id != cpu_topo->cluster_id)
+			continue;
+
+		cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
+		if (cpu != cpuid)
+			cpumask_set_cpu(cpu, &cpuid_topo->core_sibling);
+
+		if (cpuid_topo->core_id != cpu_topo->core_id)
+			continue;
+
+		cpumask_set_cpu(cpuid, &cpu_topo->thread_sibling);
+		if (cpu != cpuid)
+			cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
+	}
+	smp_wmb();
+}
+
+void store_cpu_topology(unsigned int cpuid)
+{
+	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
+
+	/* DT should have been parsed by the time we get here */
+	if (cpuid_topo->core_id == -1)
+		pr_info("CPU%u: No topology information configured\n", cpuid);
+	else
+		update_siblings_masks(cpuid);
+}
+
+/*
+ * init_cpu_topology is called at boot when only one cpu is running
+ * which prevent simultaneous write access to cpu_topology array
+ */
+void __init init_cpu_topology(void)
+{
+	unsigned int cpu;
+
+	/* init core mask and power*/
+	for_each_possible_cpu(cpu) {
+		struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+
+		cpu_topo->thread_id = -1;
+		cpu_topo->core_id =  -1;
+		cpu_topo->cluster_id = -1;
+		cpumask_clear(&cpu_topo->core_sibling);
+		cpumask_clear(&cpu_topo->thread_sibling);
+	}
+	smp_wmb();
+}
-- 
1.9.0.rc3

[PATCH 2/4] arm64: topology: Add support for topology DT bindings

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Add support for parsing the explicit topology bindings to discover the
topology of the system.

Since it is not currently clear how to map multi-level clusters for the
scheduler all leaf clusters are presented to the scheduler at the same
level. This should be enough to provide good support for current systems.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 143 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index a6d4ed2d69c0..061e3b813b9f 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -17,10 +17,150 @@
 #include <linux/percpu.h>
 #include <linux/node.h>
 #include <linux/nodemask.h>
+#include <linux/of.h>
 #include <linux/sched.h>
 
 #include <asm/topology.h>
 
+#ifdef CONFIG_OF
+static int __init get_cpu_for_node(struct device_node *node)
+{
+	struct device_node *cpu_node;
+	int cpu;
+
+	cpu_node = of_parse_phandle(node, "cpu", 0);
+	if (!cpu_node)
+		return -1;
+
+	for_each_possible_cpu(cpu) {
+		if (of_get_cpu_node(cpu, NULL) == cpu_node)
+			return cpu;
+	}
+
+	pr_crit("Unable to find CPU node for %s\n", cpu_node->full_name);
+	return -1;
+}
+
+static void __init parse_core(struct device_node *core, int cluster_id,
+			      int core_id)
+{
+	char name[10];
+	bool leaf = true;
+	int i = 0;
+	int cpu;
+	struct device_node *t;
+
+	do {
+		snprintf(name, sizeof(name), "thread%d", i);
+		t = of_get_child_by_name(core, name);
+		if (t) {
+			leaf = false;
+			cpu = get_cpu_for_node(t);
+			if (cpu >= 0) {
+				cpu_topology[cpu].cluster_id = cluster_id;
+				cpu_topology[cpu].core_id = core_id;
+				cpu_topology[cpu].thread_id = i;
+			} else {
+				pr_err("%s: Can't get CPU for thread\n",
+				       t->full_name);
+			}
+		}
+		i++;
+	} while (t);
+
+	cpu = get_cpu_for_node(core);
+	if (cpu >= 0) {
+		if (!leaf) {
+			pr_err("%s: Core has both threads and CPU\n",
+			       core->full_name);
+			return;
+		}
+
+		cpu_topology[cpu].cluster_id = cluster_id;
+		cpu_topology[cpu].core_id = core_id;
+	} else if (leaf) {
+		pr_err("%s: Can't get CPU for leaf core\n", core->full_name);
+	}
+}
+
+static void __init parse_cluster(struct device_node *cluster, int depth)
+{
+	char name[10];
+	bool leaf = true;
+	bool has_cores = false;
+	struct device_node *c;
+	static int __initdata cluster_id;
+	int core_id = 0;
+	int i;
+
+	/*
+	 * First check for child clusters; we currently ignore any
+	 * information about the nesting of clusters and present the
+	 * scheduler with a flat list of them.
+	 */
+	i = 0;
+	do {
+		snprintf(name, sizeof(name), "cluster%d", i);
+		c = of_get_child_by_name(cluster, name);
+		if (c) {
+			parse_cluster(c, depth + 1);
+			leaf = false;
+		}
+		i++;
+	} while (c);
+
+	/* Now check for cores */
+	i = 0;
+	do {
+		snprintf(name, sizeof(name), "core%d", i);
+		c = of_get_child_by_name(cluster, name);
+		if (c) {
+			has_cores = true;
+
+			if (depth == 0)
+				pr_err("%s: cpu-map children should be clusters\n",
+				       c->full_name);
+
+			if (leaf)
+				parse_core(c, cluster_id, core_id++);
+			else
+				pr_err("%s: Non-leaf cluster with core %s\n",
+				       cluster->full_name, name);
+		}
+		i++;
+	} while (c);
+
+	if (leaf && !has_cores)
+		pr_warn("%s: empty cluster\n", cluster->full_name);
+
+	if (leaf)
+		cluster_id++;
+}
+
+static void __init parse_dt_topology(void)
+{
+	struct device_node *cn;
+
+	cn = of_find_node_by_path("/cpus");
+	if (!cn) {
+		pr_err("No CPU information found in DT\n");
+		return;
+	}
+
+	/*
+	 * When topology is provided cpu-map is essentially a root
+	 * cluster with restricted subnodes.
+	 */
+	cn = of_find_node_by_name(cn, "cpu-map");
+	if (!cn)
+		return;
+	parse_cluster(cn, 0);
+}
+
+#else
+static inline void parse_dt_topology(void) {}
+#endif
+
 /*
  * cpu topology table
  */
@@ -87,5 +227,8 @@ void __init init_cpu_topology(void)
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
 	}
+
+	parse_dt_topology();
+
 	smp_wmb();
 }
-- 
1.9.0.rc3

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 142 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 061e3b813b9f..951b55e24f13 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -207,6 +345,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -226,6 +366,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->cluster_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.9.0.rc3

[PATCH 4/4] arm64: topology: Provide relative power numbers for cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Provide performance numbers to the scheduler to help it fill the cores in
the system on big.LITTLE systems. With the current scheduler this may
perform poorly for applications that try to do OpenMP style work over all
cores but should help for more common workloads. The current 32 bit ARM
implementation provides a similar estimate so this helps ensure that
work to improve big.LITTLE systems on ARMv7 systems performs similarly
on ARMv8 systems.

The power numbers are the same as for ARMv7 since it seems that the
expected differential between the big and little cores is very similar on
both ARMv7 and ARMv8.  In both ARMv7 and ARMv8 cases the numbers were
based on the published DMIPS numbers.

These numbers are just an initial and basic approximation for use with
the current scheduler, it is likely that both experience with silicon
and ongoing work on improving the scheduler will lead to further tuning
or will tune automatically at runtime and so make the specific choice of
numbers here less critical.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 951b55e24f13..0728264e14ee 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -177,6 +177,8 @@ struct cpu_efficiency {
  * use the default SCHED_POWER_SCALE value for cpu_scale.
  */
 static const struct cpu_efficiency table_efficiency[] = {
+	{ "arm,cortex-a57", 3891 },
+	{ "arm,cortex-a53", 2048 },
 	{ NULL, },
 };
 
-- 
1.9.0.rc3

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Catalin Marinas]

On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
> +static void update_siblings_masks(unsigned int cpuid)
> +{
> +	struct cpu_topology *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
> +	int cpu;
> +
> +	/* update core and thread sibling masks */
> +	for_each_possible_cpu(cpu) {
> +		cpu_topo = &cpu_topology[cpu];
> +
> +		if (cpuid_topo->cluster_id != cpu_topo->cluster_id)
> +			continue;
> +
> +		cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
> +		if (cpu != cpuid)
> +			cpumask_set_cpu(cpu, &cpuid_topo->core_sibling);
> +
> +		if (cpuid_topo->core_id != cpu_topo->core_id)
> +			continue;
> +
> +		cpumask_set_cpu(cpuid, &cpu_topo->thread_sibling);
> +		if (cpu != cpuid)
> +			cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
> +	}
> +	smp_wmb();

I now noticed there are a couple of smp_wmb() calls in this patch. What
are they for?

-- 
Catalin

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:

> > +		if (cpu != cpuid)
> > +			cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
> > +	}
> > +	smp_wmb();

> I now noticed there are a couple of smp_wmb() calls in this patch. What
> are they for?

To be honest I mostly cargo culted them from the ARM implementation; I
did look a bit but didn't fully dig into it - it seemed they were
required to ensure that the updates for the new CPU are visible over all
CPUs.  Vincent?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/fc8ec600/attachment-0001.sig>

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Vincent Guittot]

On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
>> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
>
>> > +           if (cpu != cpuid)
>> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
>> > +   }
>> > +   smp_wmb();
>
>> I now noticed there are a couple of smp_wmb() calls in this patch. What
>> are they for?
>
> To be honest I mostly cargo culted them from the ARM implementation; I
> did look a bit but didn't fully dig into it - it seemed they were
> required to ensure that the updates for the new CPU are visible over all
> CPUs.  Vincent?

Yes that's it. we must ensure that updates are made visible to other CPUs

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Will Deacon]

On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
> >
> >> > +           if (cpu != cpuid)
> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
> >> > +   }
> >> > +   smp_wmb();
> >
> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
> >> are they for?
> >
> > To be honest I mostly cargo culted them from the ARM implementation; I
> > did look a bit but didn't fully dig into it - it seemed they were
> > required to ensure that the updates for the new CPU are visible over all
> > CPUs.  Vincent?
> 
> Yes that's it. we must ensure that updates are made visible to other CPUs

In relation to what? The smp_* barriers ensure ordering of observability
between a number of independent accesses, so you must be ensuring
ordering against something else. Also, you need to guarantee ordering on the
read-side too -- how is this achieved? I can't see any smp_rmb calls from a
quick grep, so I assume you're making use of address dependencies?

/confused

Will

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Vincent Guittot]

On 11 February 2014 11:34, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
>> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
>> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
>> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
>> >
>> >> > +           if (cpu != cpuid)
>> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
>> >> > +   }
>> >> > +   smp_wmb();
>> >
>> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
>> >> are they for?
>> >
>> > To be honest I mostly cargo culted them from the ARM implementation; I
>> > did look a bit but didn't fully dig into it - it seemed they were
>> > required to ensure that the updates for the new CPU are visible over all
>> > CPUs.  Vincent?
>>
>> Yes that's it. we must ensure that updates are made visible to other CPUs
>
> In relation to what? The smp_* barriers ensure ordering of observability
> between a number of independent accesses, so you must be ensuring
> ordering against something else. Also, you need to guarantee ordering on the
> read-side too -- how is this achieved? I can't see any smp_rmb calls from a
> quick grep, so I assume you're making use of address dependencies?

The boot sequence ensures the rmb

Vincent

>
> /confused
>
> Will

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Catalin Marinas]

On Tue, Feb 11, 2014 at 01:18:56PM +0000, Vincent Guittot wrote:
> On 11 February 2014 11:34, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
> >> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
> >> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
> >> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
> >> >
> >> >> > +           if (cpu != cpuid)
> >> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
> >> >> > +   }
> >> >> > +   smp_wmb();
> >> >
> >> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
> >> >> are they for?
> >> >
> >> > To be honest I mostly cargo culted them from the ARM implementation; I
> >> > did look a bit but didn't fully dig into it - it seemed they were
> >> > required to ensure that the updates for the new CPU are visible over all
> >> > CPUs.  Vincent?
> >>
> >> Yes that's it. we must ensure that updates are made visible to other CPUs
> >
> > In relation to what? The smp_* barriers ensure ordering of observability
> > between a number of independent accesses, so you must be ensuring
> > ordering against something else. Also, you need to guarantee ordering on the
> > read-side too -- how is this achieved? I can't see any smp_rmb calls from a
> > quick grep, so I assume you're making use of address dependencies?
> 
> The boot sequence ensures the rmb

As Will said, smp_*mb() do not ensure absolute visibility, only relative
to subsequent memory accesses on the same processor. So just placing a
barrier at the end of a function does not mean much, it only shows half
of the problem it is trying to solve.

How are the secondary CPUs using this information? AFAICT, secondaries
call smp_store_cpu_info() which also go through each CPU in
update_siblings_mask(). Is there any race here that smp_wmb() is trying
to solve?

I guess for secondaries you could move the barrier just before
set_cpu_online(), this way it is clear that we want any previous writes
to become visible when this CPU is marked online. For the primary, any
memory writes should become visible before the CPU is started. One
synchronisation point is the pen release, depending on the smp_ops. I
think that's already covered by code like arch/arm/mach-*/platsmp.c.

So my proposal is to remove the smp_wmb() from topology.c and add it
where it is relevant as described above. If we have some race in
topology.c (like for example we may later decide to start more
secondaries at the same time), it needs to be solved using spinlocks.

-- 
Catalin

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Vincent Guittot]

On 11 February 2014 15:07, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Feb 11, 2014 at 01:18:56PM +0000, Vincent Guittot wrote:
>> On 11 February 2014 11:34, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
>> >> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
>> >> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
>> >> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
>> >> >
>> >> >> > +           if (cpu != cpuid)
>> >> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
>> >> >> > +   }
>> >> >> > +   smp_wmb();
>> >> >
>> >> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
>> >> >> are they for?
>> >> >
>> >> > To be honest I mostly cargo culted them from the ARM implementation; I
>> >> > did look a bit but didn't fully dig into it - it seemed they were
>> >> > required to ensure that the updates for the new CPU are visible over all
>> >> > CPUs.  Vincent?
>> >>
>> >> Yes that's it. we must ensure that updates are made visible to other CPUs
>> >
>> > In relation to what? The smp_* barriers ensure ordering of observability
>> > between a number of independent accesses, so you must be ensuring
>> > ordering against something else. Also, you need to guarantee ordering on the
>> > read-side too -- how is this achieved? I can't see any smp_rmb calls from a
>> > quick grep, so I assume you're making use of address dependencies?
>>
>> The boot sequence ensures the rmb
>
> As Will said, smp_*mb() do not ensure absolute visibility, only relative
> to subsequent memory accesses on the same processor. So just placing a
> barrier at the end of a function does not mean much, it only shows half
> of the problem it is trying to solve.

OK, that's probably the shortcut that has been made, we want to drain
the write buffer to make modification available to other cpus and I
though smp_wmb and the associated mb(ishst) was there for that
purpose.

Vincent

>
> How are the secondary CPUs using this information? AFAICT, secondaries
> call smp_store_cpu_info() which also go through each CPU in
> update_siblings_mask(). Is there any race here that smp_wmb() is trying
> to solve?
>
> I guess for secondaries you could move the barrier just before
> set_cpu_online(), this way it is clear that we want any previous writes
> to become visible when this CPU is marked online. For the primary, any
> memory writes should become visible before the CPU is started. One
> synchronisation point is the pen release, depending on the smp_ops. I
> think that's already covered by code like arch/arm/mach-*/platsmp.c.
>
> So my proposal is to remove the smp_wmb() from topology.c and add it
> where it is relevant as described above. If we have some race in
> topology.c (like for example we may later decide to start more
> secondaries at the same time), it needs to be solved using spinlocks.
>
> --
> Catalin

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

On Tue, Feb 11, 2014 at 03:46:04PM +0100, Vincent Guittot wrote:
> On 11 February 2014 15:07, Catalin Marinas <catalin.marinas@arm.com> wrote:

> > As Will said, smp_*mb() do not ensure absolute visibility, only relative
> > to subsequent memory accesses on the same processor. So just placing a
> > barrier at the end of a function does not mean much, it only shows half
> > of the problem it is trying to solve.

> OK, that's probably the shortcut that has been made, we want to drain
> the write buffer to make modification available to other cpus and I
> though smp_wmb and the associated mb(ishst) was there for that
> purpose.

It does also explain where I got to trying to figure out what exactly
the mechanism was!

> > So my proposal is to remove the smp_wmb() from topology.c and add it
> > where it is relevant as described above. If we have some race in
> > topology.c (like for example we may later decide to start more
> > secondaries at the same time), it needs to be solved using spinlocks.

I'll just repost a version dropping them from the topology series and
separately add the barriers elsewhere.  The 32 bit ARM implementation
probably ought to be fixed as well.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140211/af5fc293/attachment-0001.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 142 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index a757297992a1..0f5a5d0cbbf0 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -206,6 +344,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_info("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -225,6 +365,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->cluster_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.9.0.rc3

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Vincent Guittot]

On 11 February 2014 15:07, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Feb 11, 2014 at 01:18:56PM +0000, Vincent Guittot wrote:
>> On 11 February 2014 11:34, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
>> >> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
>> >> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
>> >> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
>> >> >
>> >> >> > +           if (cpu != cpuid)
>> >> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
>> >> >> > +   }
>> >> >> > +   smp_wmb();
>> >> >
>> >> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
>> >> >> are they for?
>> >> >
>> >> > To be honest I mostly cargo culted them from the ARM implementation; I
>> >> > did look a bit but didn't fully dig into it - it seemed they were
>> >> > required to ensure that the updates for the new CPU are visible over all
>> >> > CPUs.  Vincent?
>> >>
>> >> Yes that's it. we must ensure that updates are made visible to other CPUs
>> >
>> > In relation to what? The smp_* barriers ensure ordering of observability
>> > between a number of independent accesses, so you must be ensuring
>> > ordering against something else. Also, you need to guarantee ordering on the
>> > read-side too -- how is this achieved? I can't see any smp_rmb calls from a
>> > quick grep, so I assume you're making use of address dependencies?
>>
>> The boot sequence ensures the rmb
>
> As Will said, smp_*mb() do not ensure absolute visibility, only relative
> to subsequent memory accesses on the same processor. So just placing a

It's my time to be a bit confused, if smp_*mb() do not ensure absolute
visibility on other CPUs, how can we ensure that ?

> barrier at the end of a function does not mean much, it only shows half
> of the problem it is trying to solve.
>
> How are the secondary CPUs using this information? AFAICT, secondaries
> call smp_store_cpu_info() which also go through each CPU in
> update_siblings_mask(). Is there any race here that smp_wmb() is trying
> to solve?

The fields will be used to construct topology so we must ensure their
visibility

Vincent
>
> I guess for secondaries you could move the barrier just before
> set_cpu_online(), this way it is clear that we want any previous writes
> to become visible when this CPU is marked online. For the primary, any
> memory writes should become visible before the CPU is started. One
> synchronisation point is the pen release, depending on the smp_ops. I
> think that's already covered by code like arch/arm/mach-*/platsmp.c.
>
> So my proposal is to remove the smp_wmb() from topology.c and add it
> where it is relevant as described above. If we have some race in
> topology.c (like for example we may later decide to start more
> secondaries at the same time), it needs to be solved using spinlocks.
>
> --
> Catalin

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Catalin Marinas]

On Wed, Feb 12, 2014 at 08:04:54AM +0000, Vincent Guittot wrote:
> On 11 February 2014 15:07, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Feb 11, 2014 at 01:18:56PM +0000, Vincent Guittot wrote:
> >> On 11 February 2014 11:34, Will Deacon <will.deacon@arm.com> wrote:
> >> > On Tue, Feb 11, 2014 at 08:15:19AM +0000, Vincent Guittot wrote:
> >> >> On 10 February 2014 17:46, Mark Brown <broonie@kernel.org> wrote:
> >> >> > On Mon, Feb 10, 2014 at 04:22:31PM +0000, Catalin Marinas wrote:
> >> >> >> On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:
> >> >> >
> >> >> >> > +           if (cpu != cpuid)
> >> >> >> > +                   cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
> >> >> >> > +   }
> >> >> >> > +   smp_wmb();
> >> >> >
> >> >> >> I now noticed there are a couple of smp_wmb() calls in this patch. What
> >> >> >> are they for?
> >> >> >
> >> >> > To be honest I mostly cargo culted them from the ARM implementation; I
> >> >> > did look a bit but didn't fully dig into it - it seemed they were
> >> >> > required to ensure that the updates for the new CPU are visible over all
> >> >> > CPUs.  Vincent?
> >> >>
> >> >> Yes that's it. we must ensure that updates are made visible to other CPUs
> >> >
> >> > In relation to what? The smp_* barriers ensure ordering of observability
> >> > between a number of independent accesses, so you must be ensuring
> >> > ordering against something else. Also, you need to guarantee ordering on the
> >> > read-side too -- how is this achieved? I can't see any smp_rmb calls from a
> >> > quick grep, so I assume you're making use of address dependencies?
> >>
> >> The boot sequence ensures the rmb
> >
> > As Will said, smp_*mb() do not ensure absolute visibility, only relative
> > to subsequent memory accesses on the same processor. So just placing a
> 
> It's my time to be a bit confused, if smp_*mb() do not ensure absolute
> visibility on other CPUs, how can we ensure that ?

smb_wmb()/smb_rmb() do not provide any waiting, they are not
synchronisation primitives. You have to use spinlocks or some other
polling (and of course, barriers for relative ordering of memory
reads/writes).

> > barrier at the end of a function does not mean much, it only shows half
> > of the problem it is trying to solve.
> >
> > How are the secondary CPUs using this information? AFAICT, secondaries
> > call smp_store_cpu_info() which also go through each CPU in
> > update_siblings_mask(). Is there any race here that smp_wmb() is trying
> > to solve?
> 
> The fields will be used to construct topology so we must ensure their
> visibility

I wonder whether you need spinlocks around the topology updating code.

-- 
Catalin

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

On Wed, Feb 12, 2014 at 10:27:16AM +0000, Catalin Marinas wrote:
> On Wed, Feb 12, 2014 at 08:04:54AM +0000, Vincent Guittot wrote:

> > The fields will be used to construct topology so we must ensure their
> > visibility

> I wonder whether you need spinlocks around the topology updating code.

It certainly feels a lot safer to have them; if we can't convince
ourselves that the code is safe without them then there's very little
cost in having them so we may as well err on the side of doing that -
does that seem reasonable?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/a4dd9428/attachment.sig>

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Lorenzo Pieralisi]

On Mon, Feb 10, 2014 at 01:02:01PM +0000, Mark Brown wrote:

[...]

> +void store_cpu_topology(unsigned int cpuid)
> +{
> +	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
> +
> +	/* DT should have been parsed by the time we get here */
> +	if (cpuid_topo->core_id == -1)
> +		pr_info("CPU%u: No topology information configured\n", cpuid);
> +	else
> +		update_siblings_masks(cpuid);

If the DT does not contain a proper topology the scheduler seem to go for a
toss. I tried to track it down and it seems it expects topology cpumasks to be
initialized regardless (eg to possible mask), they cannot be left empty.

Either update_siblings_masks is called regardless or possible mask must
be copied to the topology masks.

I will have a thorough look to check if the scheduler has a fall-back
mechanism.

On top of that, the pr_info message is quite annoying and should be
probably downgraded or removed altogether.

Furthermore leaving core_id as -1 should be fine, but I have to have a
proper look into the scheduler topology code to countercheck that.

Lorenzo

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

On Fri, Feb 21, 2014 at 03:01:40PM +0000, Lorenzo Pieralisi wrote:

> If the DT does not contain a proper topology the scheduler seem to go for a
> toss. I tried to track it down and it seems it expects topology cpumasks to be
> initialized regardless (eg to possible mask), they cannot be left empty.

Could you be more specific please?  I didn't notice anything particular
in my testing but then the fact that it's a model does obscure a lot of
things.

> On top of that, the pr_info message is quite annoying and should be
> probably downgraded or removed altogether.

This was deliberate - since we are not willing to use the MPIDR
information to discover the topology we need to get the information into
the DT bindings in order to discover it.  Even in a SMP system there is
a difference in how closely attached the cores are so it seems like we
should be expecting a description of the topology.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140222/c5be7caa/attachment.sig>

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Lorenzo Pieralisi]

On Sat, Feb 22, 2014 at 02:06:02AM +0000, Mark Brown wrote:
> On Fri, Feb 21, 2014 at 03:01:40PM +0000, Lorenzo Pieralisi wrote:
> 
> > If the DT does not contain a proper topology the scheduler seem to go for a
> > toss. I tried to track it down and it seems it expects topology cpumasks to be
> > initialized regardless (eg to possible mask), they cannot be left empty.
> 
> Could you be more specific please?  I didn't notice anything particular
> in my testing but then the fact that it's a model does obscure a lot of
> things.

I will send you a backtrace, config file, commit.

Problem is hit when CONFIG_SCHED_SMT is on and there is no cpu-map in
the dts.

What's a model ? And if you mean the processor model, what can it possibly
obscure as long as this patch is concerned ?

> > On top of that, the pr_info message is quite annoying and should be
> > probably downgraded or removed altogether.
> 
> This was deliberate - since we are not willing to use the MPIDR
> information to discover the topology we need to get the information into
> the DT bindings in order to discover it.  Even in a SMP system there is
> a difference in how closely attached the cores are so it seems like we
> should be expecting a description of the topology.

We have to make a decision. Either we rely on MPIDR_EL1 as a fallback or
we barf on missing topology nodes (or we just set-up a flat topology if
the cpu-map is missing and do not log anything).

Thanks,
Lorenzo

[PATCH 1/4] arm64: topology: Implement basic CPU topology support

[Mark Brown]

On Sat, Feb 22, 2014 at 12:26:48PM +0000, Lorenzo Pieralisi wrote:
> On Sat, Feb 22, 2014 at 02:06:02AM +0000, Mark Brown wrote:
> > On Fri, Feb 21, 2014 at 03:01:40PM +0000, Lorenzo Pieralisi wrote:

> > > If the DT does not contain a proper topology the scheduler seem to go for a
> > > toss. I tried to track it down and it seems it expects topology cpumasks to be
> > > initialized regardless (eg to possible mask), they cannot be left empty.

> > Could you be more specific please?  I didn't notice anything particular
> > in my testing but then the fact that it's a model does obscure a lot of
> > things.

> I will send you a backtrace, config file, commit.

> Problem is hit when CONFIG_SCHED_SMT is on and there is no cpu-map in
> the dts.

Interesting...  I did test this incrementally during development but
didn't see any issues, though as there have been many stylistic updates
so I have to confess it's probably been a while since I ran that test,
I'll take a look when I have a stable enough network connection to run
the models (I'm on a Shinkansen at the minute so my connection keeps
dropping out).

> What's a model ? And if you mean the processor model, what can it possibly
> obscure as long as this patch is concerned ?

A fast or foundation model.  Your only description was "went for a toss"
so I had no idea what the problem was, I was guessing that this was some
sort of issue with performance like only using one core or something.
If it's a crash then using the models won't make a difference but for
performance issues the emulation means it's not always apparent when
using the system if the kernel is performing poorly or if the emulation
is just slow.

> > > On top of that, the pr_info message is quite annoying and should be
> > > probably downgraded or removed altogether.

> > This was deliberate - since we are not willing to use the MPIDR
> > information to discover the topology we need to get the information into
> > the DT bindings in order to discover it.  Even in a SMP system there is
> > a difference in how closely attached the cores are so it seems like we
> > should be expecting a description of the topology.

> We have to make a decision. Either we rely on MPIDR_EL1 as a fallback or
> we barf on missing topology nodes (or we just set-up a flat topology if
> the cpu-map is missing and do not log anything).

Indeed.  My personal preference would be that we fall back to MPIDR if
we don't have topology information from the firmware (since it's always
possible that the silicon does the right thing) or failing that we
insist on topology information from the firmware.  Once systems are out
in the wild it's potentially painful to get the data added to DTs so
pushing for information in case we need it in the future seems like the
safest approach in cases like this where it's not going to be too much
work to provide.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140223/6988707f/attachment-0001.sig>

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 142 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index cda9a48..b330e79 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -206,6 +344,8 @@ void store_cpu_topology(unsigned int cpuid)
 		pr_warn("CPU%u: No topology information configured\n", cpuid);
 	else
 		update_siblings_masks(cpuid);
+
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -225,6 +365,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->cluster_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.9.0.rc3

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 141 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 13ef616..8a9b325 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 #ifdef CONFIG_OF
 static int __init get_cpu_for_node(struct device_node *node)
 {
@@ -137,9 +161,49 @@ static void __init parse_cluster(struct device_node *cluster, int depth)
 		cluster_id++;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static void __init parse_dt_topology(void)
 {
+	const struct cpu_efficiency *cpu_eff;
 	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
 
 	cn = of_find_node_by_path("/cpus");
 	if (!cn) {
@@ -155,10 +219,84 @@ static void __init parse_dt_topology(void)
 	if (!cn)
 		return;
 	parse_cluster(cn, 0);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
 }
 
 #else
 static inline void parse_dt_topology(void) {}
+static inline void update_cpu_power(unsigned int cpuid) {}
 #endif
 
 /*
@@ -212,6 +350,7 @@ static void update_siblings_masks(unsigned int cpuid)
 void store_cpu_topology(unsigned int cpuid)
 {
 	update_siblings_masks(cpuid);
+	update_cpu_power(cpuid);
 }
 
 /*
@@ -231,6 +370,8 @@ void __init init_cpu_topology(void)
 		cpu_topo->cluster_id = -1;
 		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_clear(&cpu_topo->thread_sibling);
+
+		set_power_scale(cpu, SCHED_POWER_SCALE);
 	}
 
 	parse_dt_topology();
-- 
1.9.0

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---

 - Split into a separate function from the rest of the DT parse since
   the CPU performance information isn't directly part of the topology
   binding.

 arch/arm64/kernel/topology.c | 153 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index d0cb687fb7f5..516dd2bed7d6 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 static int __init get_cpu_for_node(struct device_node *node)
 {
 	struct device_node *cpu_node;
@@ -151,6 +175,38 @@ static int __init parse_cluster(struct device_node *cluster, int depth)
 	return 0;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static int __init parse_dt_topology(void)
 {
 	struct device_node *cn, *map;
@@ -194,6 +250,91 @@ out:
 	return ret;
 }
 
+static void __init parse_dt_cpu_power(void)
+{
+	const struct cpu_efficiency *cpu_eff;
+	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
+}
+
 /*
  * cpu topology table
  */
@@ -242,6 +383,7 @@ static void update_siblings_masks(unsigned int cpuid)
 void store_cpu_topology(unsigned int cpuid)
 {
 	update_siblings_masks(cpuid);
+	update_cpu_power(cpuid);
 }
 
 static void __init reset_cpu_topology(void)
@@ -262,6 +404,14 @@ static void __init reset_cpu_topology(void)
 	}
 }
 
+static void __init reset_cpu_power(void)
+{
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu)
+		set_power_scale(cpu, SCHED_POWER_SCALE);
+}
+
 void __init init_cpu_topology(void)
 {
 	reset_cpu_topology();
@@ -272,4 +422,7 @@ void __init init_cpu_topology(void)
 	 */
 	if (parse_dt_topology())
 		reset_cpu_topology();
+
+	reset_cpu_power();
+	parse_dt_cpu_power();
 }
-- 
1.9.1

[PATCH 3/4] arm64: topology: Tell the scheduler about the relative power of cores

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

In heterogeneous systems like big.LITTLE systems the scheduler will be
able to make better use of the available cores if we provide power numbers
to it indicating their relative performance. Do this by parsing the CPU
nodes in the DT.

This code currently has no effect as no information on the relative
performance of the cores is provided.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm64/kernel/topology.c | 153 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index a326f5b37546..a912f3a7c804 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,9 +19,33 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 
 #include <asm/topology.h>
 
+/*
+ * cpu power table
+ * This per cpu data structure describes the relative capacity of each core.
+ * On a heteregenous system, cores don't have the same computation capacity
+ * and we reflect that difference in the cpu_power field so the scheduler can
+ * take this difference into account during load balance. A per cpu structure
+ * is preferred because each CPU updates its own cpu_power field during the
+ * load balance except for idle cores. One idle core is selected to run the
+ * rebalance_domains for all idle cores and the cpu_power can be updated
+ * during this sequence.
+ */
+static DEFINE_PER_CPU(unsigned long, cpu_scale);
+
+unsigned long arch_scale_freq_power(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_power_scale(unsigned int cpu, unsigned long power)
+{
+	per_cpu(cpu_scale, cpu) = power;
+}
+
 static int __init get_cpu_for_node(struct device_node *node)
 {
 	struct device_node *cpu_node;
@@ -159,6 +183,38 @@ static int __init parse_cluster(struct device_node *cluster, int depth)
 	return 0;
 }
 
+struct cpu_efficiency {
+	const char *compatible;
+	unsigned long efficiency;
+};
+
+/*
+ * Table of relative efficiency of each processors
+ * The efficiency value must fit in 20bit and the final
+ * cpu_scale value must be in the range
+ *   0 < cpu_scale < 3*SCHED_POWER_SCALE/2
+ * in order to return@most 1 when DIV_ROUND_CLOSEST
+ * is used to compute the capacity of a CPU.
+ * Processors that are not defined in the table,
+ * use the default SCHED_POWER_SCALE value for cpu_scale.
+ */
+static const struct cpu_efficiency table_efficiency[] = {
+	{ NULL, },
+};
+
+static unsigned long *__cpu_capacity;
+#define cpu_capacity(cpu)	__cpu_capacity[cpu]
+
+static unsigned long middle_capacity = 1;
+
+/*
+ * Iterate all CPUs' descriptor in DT and compute the efficiency
+ * (as per table_efficiency). Also calculate a middle efficiency
+ * as close as possible to  (max{eff_i} - min{eff_i}) / 2
+ * This is later used to scale the cpu_power field such that an
+ * 'average' CPU is of middle power. Also see the comments near
+ * table_efficiency[] and update_cpu_power().
+ */
 static int __init parse_dt_topology(void)
 {
 	struct device_node *cn, *map;
@@ -202,6 +258,91 @@ out:
 	return ret;
 }
 
+static void __init parse_dt_cpu_power(void)
+{
+	const struct cpu_efficiency *cpu_eff;
+	struct device_node *cn;
+	unsigned long min_capacity = ULONG_MAX;
+	unsigned long max_capacity = 0;
+	unsigned long capacity = 0;
+	int cpu;
+
+	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
+				 GFP_NOWAIT);
+
+	for_each_possible_cpu(cpu) {
+		const u32 *rate;
+		int len;
+
+		/* Too early to use cpu->of_node */
+		cn = of_get_cpu_node(cpu, NULL);
+		if (!cn) {
+			pr_err("Missing device node for CPU %d\n", cpu);
+			continue;
+		}
+
+		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
+			if (of_device_is_compatible(cn, cpu_eff->compatible))
+				break;
+
+		if (cpu_eff->compatible == NULL) {
+			pr_warn("%s: Unknown CPU type\n", cn->full_name);
+			continue;
+		}
+
+		rate = of_get_property(cn, "clock-frequency", &len);
+		if (!rate || len != 4) {
+			pr_err("%s: Missing clock-frequency property\n",
+				cn->full_name);
+			continue;
+		}
+
+		capacity = ((be32_to_cpup(rate)) >> 20) * cpu_eff->efficiency;
+
+		/* Save min capacity of the system */
+		if (capacity < min_capacity)
+			min_capacity = capacity;
+
+		/* Save max capacity of the system */
+		if (capacity > max_capacity)
+			max_capacity = capacity;
+
+		cpu_capacity(cpu) = capacity;
+	}
+
+	/* If min and max capacities are equal we bypass the update of the
+	 * cpu_scale because all CPUs have the same capacity. Otherwise, we
+	 * compute a middle_capacity factor that will ensure that the capacity
+	 * of an 'average' CPU of the system will be as close as possible to
+	 * SCHED_POWER_SCALE, which is the default value, but with the
+	 * constraint explained near table_efficiency[].
+	 */
+	if (min_capacity == max_capacity)
+		return;
+	else if (4 * max_capacity < (3 * (max_capacity + min_capacity)))
+		middle_capacity = (min_capacity + max_capacity)
+				>> (SCHED_POWER_SHIFT+1);
+	else
+		middle_capacity = ((max_capacity / 3)
+				>> (SCHED_POWER_SHIFT-1)) + 1;
+}
+
+/*
+ * Look for a customed capacity of a CPU in the cpu_topo_data table during the
+ * boot. The update of all CPUs is in O(n^2) for heteregeneous system but the
+ * function returns directly for SMP system.
+ */
+static void update_cpu_power(unsigned int cpu)
+{
+	if (!cpu_capacity(cpu))
+		return;
+
+	set_power_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+
+	pr_info("CPU%u: update cpu_power %lu\n",
+		cpu, arch_scale_freq_power(NULL, cpu));
+}
+
 /*
  * cpu topology table
  */
@@ -249,6 +390,7 @@ static void update_siblings_masks(unsigned int cpuid)
 void store_cpu_topology(unsigned int cpuid)
 {
 	update_siblings_masks(cpuid);
+	update_cpu_power(cpuid);
 }
 
 static void __init reset_cpu_topology(void)
@@ -269,6 +411,14 @@ static void __init reset_cpu_topology(void)
 	}
 }
 
+static void __init reset_cpu_power(void)
+{
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu)
+		set_power_scale(cpu, SCHED_POWER_SCALE);
+}
+
 void __init init_cpu_topology(void)
 {
 	reset_cpu_topology();
@@ -279,4 +429,7 @@ void __init init_cpu_topology(void)
 	 */
 	if (parse_dt_topology())
 		reset_cpu_topology();
+
+	reset_cpu_power();
+	parse_dt_cpu_power();
 }
-- 
1.9.2