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 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 | 12 +++++ arch/arm64/kernel/topology.c | 95 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 171 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 6d4dd22ee4b7..00fcd490b3be 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -154,6 +154,30 @@ config SMP
If you don't know what to do here, say N.
+config ARM_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 ARM_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 ARM_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..58b8b84adcd2 --- /dev/null +++ b/arch/arm64/include/asm/topology.h @@ -0,0 +1,39 @@ +#ifndef _ASM_ARM_TOPOLOGY_H +#define _ASM_ARM_TOPOLOGY_H + +#ifdef CONFIG_ARM_CPU_TOPOLOGY + +#include <linux/cpumask.h> + +struct cputopo_arm { + int thread_id; + int core_id; + int socket_id; + cpumask_t thread_sibling; + cpumask_t core_sibling; +}; + +extern struct cputopo_arm cpu_topology[NR_CPUS]; + +#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_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].socket_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 5ba2fd43a75b..2d145e38ad49 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o +arm64-obj-$(CONFIG_ARM_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 a0c2ca602cf8..0271fbde5363 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -113,6 +113,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle) return ret; }
+static void __cpuinit 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. @@ -150,6 +155,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 @@ -388,6 +395,11 @@ 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..b078afa6958d --- /dev/null +++ b/arch/arm64/kernel/topology.c @@ -0,0 +1,95 @@ +/* + * arch/arm64/kernel/topology.c + * + * Copyright (C) 2011,2013 Linaro Limited. + * Written by: Vincent Guittot + * + * 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/export.h> +#include <linux/init.h> +#include <linux/percpu.h> +#include <linux/node.h> +#include <linux/nodemask.h> +#include <linux/sched.h> +#include <linux/slab.h> + +#include <asm/cputype.h> +#include <asm/smp_plat.h> +#include <asm/topology.h> + +/* + * cpu topology table + */ +struct cputopo_arm 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 cputopo_arm *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->socket_id != cpu_topo->socket_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 cputopo_arm *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 cputopo_arm *cpu_topo = &(cpu_topology[cpu]); + + cpu_topo->thread_id = -1; + cpu_topo->core_id = -1; + cpu_topo->socket_id = -1; + cpumask_clear(&cpu_topo->core_sibling); + cpumask_clear(&cpu_topo->thread_sibling); + } + smp_wmb(); +}
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 | 145 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+)
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index b078afa6958d..7fd473367e9b 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -18,6 +18,7 @@ #include <linux/percpu.h> #include <linux/node.h> #include <linux/nodemask.h> +#include <linux/of.h> #include <linux/sched.h> #include <linux/slab.h>
@@ -25,6 +26,147 @@ #include <asm/smp_plat.h> #include <asm/topology.h>
+#ifdef CONFIG_OF +static int cluster_id; + +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) { + pr_crit("%s: Unable to parse CPU phandle\n", node->full_name); + 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 core_id) +{ + char name[10]; + bool leaf = true; + int i, cpu; + struct device_node *t; + + i = 0; + 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) { + pr_info("CPU%d: socket %d core %d thread %d\n", + cpu, cluster_id, core_id, i); + cpu_topology[cpu].socket_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; + } + + pr_info("CPU%d: socket %d core %d\n", + cpu, cluster_id, core_id); + cpu_topology[cpu].socket_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) +{ + char name[10]; + bool leaf = true; + bool has_cores = false; + struct device_node *c; + 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); + 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 (leaf) + parse_core(c, 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; + } + + /* + * If topology is provided as a cpu-map it is essentially a + * root cluster. + */ + cn = of_find_node_by_name(cn, "cpu-map"); + if (!cn) + return; + parse_cluster(cn); +} + +#else +static inline void parse_dt_topology(void) {} +#endif + /* * cpu topology table */ @@ -91,5 +233,8 @@ void __init init_cpu_topology(void) cpumask_clear(&cpu_topo->core_sibling); cpumask_clear(&cpu_topo->thread_sibling); } + + parse_dt_topology(); + smp_wmb(); }
On Mon, Dec 16, 2013 at 04:49:23PM +0000, Mark Brown wrote:
[...]
+#ifdef CONFIG_OF +static int cluster_id;
+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) {
pr_crit("%s: Unable to parse CPU phandle\n", node->full_name);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 core_id) +{
- char name[10];
- bool leaf = true;
- int i, cpu;
- struct device_node *t;
- i = 0;
- 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) {
I think that's wrong. If cpu == -1 that should be skipped.
pr_info("CPU%d: socket %d core %d thread %d\n",cpu, cluster_id, core_id, i);cpu_topology[cpu].socket_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;}pr_info("CPU%d: socket %d core %d\n",cpu, cluster_id, core_id);cpu_topology[cpu].socket_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) +{
- char name[10];
- bool leaf = true;
- bool has_cores = false;
- struct device_node *c;
- 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);leaf = false;}i++;- } while (c);
A cpu-map can only contain cluster nodes, this is not verified here, but it has to be. Put it differently, a core node cannot be a cpu-map direct child, a long winded way to say cpu-map cannot be parsed by this function as it is.
- /* 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 (leaf)parse_core(c, core_id++);elsepr_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;- }
- /*
* If topology is provided as a cpu-map it is essentially a* root cluster.
No, because it can't contain core nodes as direct children.
"a cpu-map's child nodes can be: one or more cluster nodes" the bindings say :)
Apart from these minor remarks, I think we should aim for consolidating these parsing functions, after all they are all pretty similar bar minor corner cases, or at least factor out the parsing/enumeration loops.
What do you think ?
Thanks, Lorenzo
On Tue, Dec 17, 2013 at 05:40:37PM +0000, Lorenzo Pieralisi wrote:
On Mon, Dec 16, 2013 at 04:49:23PM +0000, Mark Brown wrote:
- i = 0;
- 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) {
I think that's wrong. If cpu == -1 that should be skipped.
Yup, good spot.
- do {
snprintf(name, sizeof(name), "cluster%d", i);c = of_get_child_by_name(cluster, name);if (c) {parse_cluster(c);leaf = false;}i++;- } while (c);
A cpu-map can only contain cluster nodes, this is not verified here, but it has to be. Put it differently, a core node cannot be a cpu-map direct child, a long winded way to say cpu-map cannot be parsed by this function as it is.
Well, it can be parsed totally happily but we're not as strict with the validation as we might want be - we'll parse valid bindings successfully but also accept out of spec bindings (but then they're using undefined behaviour so anything could happen including the kernel trying to do something sensible with what it was handed).
It might just make sense to change the binding here, saying the cpu_map is a root cluster seems reasonable to me, it doesn't hurt to have the extra level but it doesn't seem to buy us anything either. But we could also add a validation check for unwanted properties, I'm not that fussed between any of these options.
Apart from these minor remarks, I think we should aim for consolidating these parsing functions, after all they are all pretty similar bar minor corner cases, or at least factor out the parsing/enumeration loops.
What do you think ?
I thought about that and did poke at it but it didn't seem worth the effort for the very small number of uses - adding a callback for the action didn't seem to be doing anything for the readability and starting to define macros didn't fill me with great joy. I didn't want to put anything in of.h as bindings that can use the existing iterators are generally more idiomatic.
It may be there's some nice way of writing the factoring out but I didn't think of it.
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 at 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();
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 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. 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. In both ARMv7 and ARMv8 cases the numbers were based on the published DMIPS numbers.
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 eae508ca816a..d080ba77a812 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -183,6 +183,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, }, };
Mark,
On Mon, Dec 16, 2013 at 04:49:22PM +0000, Mark Brown wrote:
+void store_cpu_topology(unsigned int cpuid);
I still think this function is not needed at all and therefore should not be exported.
I understand this patch is there to provide the infrastructure, it is not clear to me why store_cpu_topology() should be part of it, even if we had to parse affinity levels in the MPIDR bitfields.
[...]
+static void __cpuinit smp_store_cpu_info(unsigned int cpuid) +{
- store_cpu_topology(cpuid);
As I mentioned this need not be done here. Parse the topology from DT, (cpu_logical_map or cpu-map) in one go and get rid of store_cpu_topology().
/*
- This is the secondary CPU boot entry. We're using this CPUs
- idle thread stack, but a set of temporary page tables.
@@ -150,6 +155,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
@@ -388,6 +395,11 @@ 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..b078afa6958d --- /dev/null +++ b/arch/arm64/kernel/topology.c @@ -0,0 +1,95 @@ +/*
- arch/arm64/kernel/topology.c
- Copyright (C) 2011,2013 Linaro Limited.
- Written by: Vincent Guittot
You should probably update the header, just a nitpick though.
- 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/export.h> +#include <linux/init.h> +#include <linux/percpu.h> +#include <linux/node.h> +#include <linux/nodemask.h> +#include <linux/sched.h> +#include <linux/slab.h>
+#include <asm/cputype.h> +#include <asm/smp_plat.h>
I think that some of these includes are not needed, they were needed in previous versions, but not in this patch as it stands.
Lorenzo
On Tue, Dec 17, 2013 at 05:06:25PM +0000, Lorenzo Pieralisi wrote:
On Mon, Dec 16, 2013 at 04:49:22PM +0000, Mark Brown wrote:
+void store_cpu_topology(unsigned int cpuid);
I still think this function is not needed at all and therefore should not be exported.
+static void __cpuinit smp_store_cpu_info(unsigned int cpuid) +{
- store_cpu_topology(cpuid);
As I mentioned this need not be done here. Parse the topology from DT, (cpu_logical_map or cpu-map) in one go and get rid of store_cpu_topology().
That's what we're actually doing for parsing, the parsing all happens at once.
I do think it's useful to to have the function there partly just as a hook but more for the warning it generates if we manage to boot a core that we didn't hook up with the topology information. This seems like it'll be helpful for debugging issues - we'll actively warn if there are CPUs that have been omitted from the definition in the firmware. The other things the function does could be done elsewhere I think, it didn't seem worth moving them though.
- arch/arm64/kernel/topology.c
- Copyright (C) 2011,2013 Linaro Limited.
- Written by: Vincent Guittot
You should probably update the header, just a nitpick though.
Well, most of this still is Vincent's code with the serial numbers filed off but yeah.
+#include <linux/cpu.h> +#include <linux/cpumask.h> +#include <linux/export.h> +#include <linux/init.h> +#include <linux/percpu.h> +#include <linux/node.h> +#include <linux/nodemask.h> +#include <linux/sched.h> +#include <linux/slab.h>
+#include <asm/cputype.h> +#include <asm/smp_plat.h>
I think that some of these includes are not needed, they were needed in previous versions, but not in this patch as it stands.
Some can probably go. I don't think some of those were even needed on arm.
On Tue, Dec 17, 2013 at 07:55:19PM +0000, Mark Brown wrote:
On Tue, Dec 17, 2013 at 05:06:25PM +0000, Lorenzo Pieralisi wrote:
On Mon, Dec 16, 2013 at 04:49:22PM +0000, Mark Brown wrote:
+void store_cpu_topology(unsigned int cpuid);
I still think this function is not needed at all and therefore should not be exported.
+static void __cpuinit smp_store_cpu_info(unsigned int cpuid) +{
- store_cpu_topology(cpuid);
As I mentioned this need not be done here. Parse the topology from DT, (cpu_logical_map or cpu-map) in one go and get rid of store_cpu_topology().
That's what we're actually doing for parsing, the parsing all happens at once.
I do think it's useful to to have the function there partly just as a hook but more for the warning it generates if we manage to boot a core that we didn't hook up with the topology information. This seems like it'll be helpful for debugging issues - we'll actively warn if there are CPUs that have been omitted from the definition in the firmware. The other things the function does could be done elsewhere I think, it didn't seem worth moving them though.
I think you have a point, the question is whether we need that function to run the check you mention or not, we can run the check for all possible cpus after completing the topology parsing. Actually, we might be building topology for cores that do not come online as well, I do not think this is a problem, but I have to cast a proper look into that before jumping to conclusions, thanks for pointing that out.
Lorenzo
On Wed, Dec 18, 2013 at 05:39:42PM +0000, Lorenzo Pieralisi wrote:
On Tue, Dec 17, 2013 at 07:55:19PM +0000, Mark Brown wrote:
I do think it's useful to to have the function there partly just as a hook but more for the warning it generates if we manage to boot a core that we didn't hook up with the topology information. This seems like
I think you have a point, the question is whether we need that function to run the check you mention or not, we can run the check for all possible cpus after completing the topology parsing. Actually, we might be
Yeah, it should be possible to do it elsewhere but then since the SMP code is already enumerating all the cores it's going to boot it seemed sensible to keep piggybacking on that and not duplicate the logic.
building topology for cores that do not come online as well, I do not think this is a problem, but I have to cast a proper look into that before jumping to conclusions, thanks for pointing that out.
I'd expect that could reasonably happen if cores are administratively disabled for some reason - that's one of the reasons for the status marking in DT.
linaro-kernel@lists.linaro.org
-
Lorenzo Pieralisi -
Mark Brown