The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org --- arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC + bool "Multi-core scheduler support" + depends on SMP && ARM_CPU_TOPOLOGY + default n + 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 SMP && ARM_CPU_TOPOLOGY + default n + 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 MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY + bool "Support cpu topology definition" + depends on SMP && CPU_V7 + help + Support Arm cpu topology definition. The MPIDR register defines + affinity between processors which is used to set the cpu + topology of an Arm System. + config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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(unsigned int cpu); + +#else + +#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {}; + +#endif + #include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy; + + store_cpu_topology(cpuid); + }
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
+ init_cpu_topology(); + smp_store_cpu_info(smp_processor_id());
/* diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..d61723c --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,133 @@ +/* + * arch/arm/kernel/topology.c + * + * Copyright (C) 2011 vincent.guittot@linaro.org + * + * 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/cacheflush.h> +#include <asm/topology.h> + +#define hard_smp_mpidr() \ + ({ \ + unsigned int cpunum; \ + __asm__("mrc p15, 0, %0, c0, c0, 5" \ + : "=r" (cpunum)); \ + cpunum; \ + }) + +struct cputopo_arm cpu_topology[NR_CPUS]; + +const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{ + return &(cpu_topology[cpu].core_sibling); +} + +/* + * store_cpu_topology is called at boot when only one cpu is running + * and with the mutex cpu_hotplug.lock locked, when several cpus have booted, + * which prevents simultaneous write access to cpu_topology array + */ +void store_cpu_topology(unsigned int cpuid) +{ + struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]); + unsigned int mpidr; + unsigned int cpu; + + /* If the cpu topology has been already set, just return */ + if (cpuid_topo->core_id != -1) + return; + + mpidr = hard_smp_mpidr(); + + /* create cpu topology mapping */ + if (mpidr & (0x3 << 30)) { + /* + * This is a multiprocessor system + * multiprocessor format & multiprocessor mode field are set + */ + + if (mpidr & (0x1 << 24)) { + /* core performance interdependency */ + cpuid_topo->thread_id = (mpidr & 0x3); + cpuid_topo->core_id = ((mpidr >> 8) & 0xF); + cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF); + } else { + /* normal core interdependency */ + cpuid_topo->thread_id = -1; + cpuid_topo->core_id = (mpidr & 0x3); + cpuid_topo->socket_id = ((mpidr >> 8) & 0xF); + } + } else { + /* + * This is an uniprocessor system + * we are in multiprocessor format but uniprocessor system + * or in the old uniprocessor format + */ + + cpuid_topo->thread_id = -1; + cpuid_topo->core_id = 0; + cpuid_topo->socket_id = -1; + } + + /* update core and thread sibling masks */ + for_each_possible_cpu(cpu) { + struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]); + + if (cpuid_topo->socket_id == cpu_topo->socket_id) { + 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) { + cpumask_set_cpu(cpuid, + &cpu_topo->thread_sibling); + if (cpu != cpuid) + cpumask_set_cpu(cpu, + &cpuid_topo->thread_sibling); + } + } + } + smp_wmb(); + + printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n", + cpuid, cpu_topology[cpuid].thread_id, + cpu_topology[cpuid].core_id, + cpu_topology[cpuid].socket_id, mpidr); + +} + +/* + * init_cpu_topology is called at boot when only one cpu is running + * which prevent simultaneous write access to cpu_topology array + */ +void init_cpu_topology(void) +{ + unsigned int cpu; + + /* init core mask */ + 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(); +}
Hello,
Vincent Guittot, le Thu 16 Jun 2011 10:49:13 +0200, a écrit :
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Cool! Could you check that the hwloc tool gets also gets this information from userland through /sys, and/or send me the output of the hwloc-gather-topology tool from hwloc so we can add an testcase for this?
Samuel
On 16 June 2011 10:55, Samuel Thibault samuel.thibault@ens-lyon.org wrote:
Hello,
Vincent Guittot, le Thu 16 Jun 2011 10:49:13 +0200, a écrit :
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Cool! Could you check that the hwloc tool gets also gets this information from userland through /sys, and/or send me the output of the hwloc-gather-topology tool from hwloc so we can add an testcase for this?
The output of hwloc-gather-topology is :
Machine (phys=0 local=280840KB total=280840KB) Socket #0 (phys=3) Core #0 (phys=0) PU #0 (phys=0) Core #1 (phys=1) PU #1 (phys=1) depth 0: 1 Machine (type #1) depth 1: 1 Socket (type #3) depth 2: 2 Cores (type #5) depth 3: 2 PUs (type #6) Topology not from this system
let me know if it's what you want
Samuel
Vincent Guittot, le Thu 16 Jun 2011 11:44:30 +0200, a écrit :
The output of hwloc-gather-topology is :
Machine (phys=0 local=280840KB total=280840KB) Socket #0 (phys=3) Core #0 (phys=0) PU #0 (phys=0) Core #1 (phys=1) PU #1 (phys=1) depth 0: 1 Machine (type #1) depth 1: 1 Socket (type #3) depth 2: 2 Cores (type #5) depth 3: 2 PUs (type #6) Topology not from this system
let me know if it's what you want
This looks good for a bicore socket, yes. Could you send me the tarball it created, so we can include it in the hwloc testsuite?
Do you have a example with smt cores?
Samuel
On 16 June 2011 11:47, Samuel Thibault samuel.thibault@ens-lyon.org wrote:
Vincent Guittot, le Thu 16 Jun 2011 11:44:30 +0200, a écrit :
The output of hwloc-gather-topology is :
Machine (phys=0 local=280840KB total=280840KB) Socket #0 (phys=3) Core #0 (phys=0) PU #0 (phys=0) Core #1 (phys=1) PU #1 (phys=1) depth 0: 1 Machine (type #1) depth 1: 1 Socket (type #3) depth 2: 2 Cores (type #5) depth 3: 2 PUs (type #6) Topology not from this system
let me know if it's what you want
This looks good for a bicore socket, yes. Could you send me the tarball it created, so we can include it in the hwloc testsuite?
yes, I will send it to you
Do you have a example with smt cores?
no, I haven't
Samuel
On 06/16/2011 10:49 AM, Vincent Guittot wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Signed-off-by: Vincent Guittotvincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP&& ARM_CPU_TOPOLOGY
ARM_CPU_TOPOLOGY depends on SMP, so the check can be reduced to
depends on ARM_CPU_TOPOLOGY
- default n
- help
Multi-core scheduler support improves the CPU scheduler's decisionmaking when dealing with multi-core CPU chips at a cost of slightlyincreased overhead in some places. If unsure say N here.+config SCHED_SMT
- bool "SMT scheduler support"
- depends on SMP&& ARM_CPU_TOPOLOGY
depends on SMT && ARM_CPU_TOPOLOGY ?
- default n
- help
Improves the CPU scheduler's decision making when dealing withMultiThreading at a cost of slightly increased overhead in someplaces. If unsure say N here.- config MMU bool "MMU-based Paged Memory Management Support" default y
@@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP&& CPU_V7
- help
Support Arm cpu topology definition. The MPIDR register definesaffinity between processors which is used to set the cputopology of an Arm System.- config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K
diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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;
I am not sure how that deals with the rest of the functions prototype but wouldn't u16 be more adequate ?
- 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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
AFAIK the convention is to declare static inline noop functions.
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/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include<asm/cacheflush.h> #include<asm/cpu.h> #include<asm/cputype.h> +#include<asm/topology.h> #include<asm/mmu_context.h> #include<asm/pgtable.h> #include<asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info =&per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
- }
If the store_cpu_topology function is called once, can it be changed to a __cpuinit function, declared as a subsys_initcall and removed from here ?
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
Why do you need to call the init function here ?
On the other architecture I see:
static int __init topology_init(void) { ... }
subsys_initcall(topology_init);
Isn't possible to use the same way ? (with the benefit to save two declarations in the header).
[ ... ]
+struct cputopo_arm cpu_topology[NR_CPUS];
IMO, you can define it static here no ?
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return&(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo =&(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
return;
If the code calls store_cpu_topology but with no effect because it was already called before, that means it shouldn't be called at all, no ? IMHO, this test should be removed or at least add a WARN_ONCE.
- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr& (0x3<< 30)) {
/** This is a multiprocessor system* multiprocessor format& multiprocessor mode field are set*/if (mpidr& (0x1<< 24)) {/* core performance interdependency */cpuid_topo->thread_id = (mpidr& 0x3);cpuid_topo->core_id = ((mpidr>> 8)& 0xF);cpuid_topo->socket_id = ((mpidr>> 16)& 0xFF);} else {/* normal core interdependency */cpuid_topo->thread_id = -1;cpuid_topo->core_id = (mpidr& 0x3);cpuid_topo->socket_id = ((mpidr>> 8)& 0xF);}- } else {
/** This is an uniprocessor system* we are in multiprocessor format but uniprocessor system* or in the old uniprocessor format*/cpuid_topo->thread_id = -1;cpuid_topo->core_id = 0;cpuid_topo->socket_id = -1;- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
struct cputopo_arm *cpu_topo =&(cpu_topology[cpu]);if (cpuid_topo->socket_id == cpu_topo->socket_id) {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) {cpumask_set_cpu(cpuid,&cpu_topo->thread_sibling);if (cpu != cpuid)cpumask_set_cpu(cpu,&cpuid_topo->thread_sibling);}}- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n",
cpuid, cpu_topology[cpuid].thread_id,cpu_topology[cpuid].core_id,cpu_topology[cpuid].socket_id, mpidr);+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- for_each_possible_cpu(cpu) {
struct cputopo_arm *cpu_topo =&(cpu_topology[cpu]);cpu_topo->thread_id = -1;cpu_topo->core_id = -1;
nit : extra space
cpu_topo->socket_id = -1;cpumask_clear(&cpu_topo->core_sibling);cpumask_clear(&cpu_topo->thread_sibling);- }
- smp_wmb();
+}
On 16 June 2011 12:49, Daniel Lezcano daniel.lezcano@linaro.org wrote:
On 06/16/2011 10:49 AM, Vincent Guittot wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Signed-off-by: Vincent Guittotvincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP&& ARM_CPU_TOPOLOGY
ARM_CPU_TOPOLOGY depends on SMP, so the check can be reduced to
depends on ARM_CPU_TOPOLOGY
you're right
- default n
- 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 SMP&& ARM_CPU_TOPOLOGY
depends on SMT && ARM_CPU_TOPOLOGY ?
SMP is the right one but it can be reduced to : depends on ARM_CPU_TOPOLOGY like SCHED_MC,
- default n
- 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 MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP&& CPU_V7
- help
- Support Arm cpu topology definition. The MPIDR register defines
- affinity between processors which is used to set the cpu
- topology of an Arm System.
config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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;
I am not sure how that deals with the rest of the functions prototype but wouldn't u16 be more adequate ?
I have used int to be aligned on register size and minimize register manipulation
- 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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
AFAIK the convention is to declare static inline noop functions.
static inline void init_cpu_topology(void) { }; static inline void store_cpu_topology(unsigned int cpuid) { };
ok
+#endif
#include<asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include<asm/cacheflush.h> #include<asm/cpu.h> #include<asm/cputype.h> +#include<asm/topology.h> #include<asm/mmu_context.h> #include<asm/pgtable.h> #include<asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info =&per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
}
If the store_cpu_topology function is called once, can it be changed to a __cpuinit function, declared as a subsys_initcall and removed from here ?
it must be called once on each cpu before the call of sched_init_smp
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
Why do you need to call the init function here ?
this function must be called before the 1st call to smp_store_cpu_info
On the other architecture I see:
static int __init topology_init(void) { ... }
subsys_initcall(topology_init);
Isn't possible to use the same way ? (with the benefit to save two declarations in the header).
[ ... ]
+struct cputopo_arm cpu_topology[NR_CPUS];
IMO, you can define it static here no ?
This array is used by "#define topology_xxx" and "#define xx_capable" which are used by the scheduler and the topology driver
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return&(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have
booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo =&(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
- return;
If the code calls store_cpu_topology but with no effect because it was already called before, that means it shouldn't be called at all, no ? IMHO, this test should be removed or at least add a WARN_ONCE.
We will call smp_store_cpu_info each time the cpu will be plugged. But once set, we don't need to update topology information
- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr& (0x3<< 30)) {
- /*
- * This is a multiprocessor system
- * multiprocessor format& multiprocessor mode field are
set
- */
- if (mpidr& (0x1<< 24)) {
- /* core performance interdependency */
- cpuid_topo->thread_id = (mpidr& 0x3);
- cpuid_topo->core_id = ((mpidr>> 8)& 0xF);
- cpuid_topo->socket_id = ((mpidr>> 16)& 0xFF);
- } else {
- /* normal core interdependency */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = (mpidr& 0x3);
- cpuid_topo->socket_id = ((mpidr>> 8)& 0xF);
- }
- } else {
- /*
- * This is an uniprocessor system
- * we are in multiprocessor format but uniprocessor system
- * or in the old uniprocessor format
- */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo =&(cpu_topology[cpu]);
- if (cpuid_topo->socket_id == cpu_topo->socket_id) {
- 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) {
- cpumask_set_cpu(cpuid,
- &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
&cpuid_topo->thread_sibling);
- }
- }
- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr
%x\n",
- cpuid, cpu_topology[cpuid].thread_id,
- cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo =&(cpu_topology[cpu]);
- cpu_topo->thread_id = -1;
- cpu_topo->core_id = -1;
nit : extra space
ok
- cpu_topo->socket_id = -1;
- cpumask_clear(&cpu_topo->core_sibling);
- cpumask_clear(&cpu_topo->thread_sibling);
- }
- smp_wmb();
+}
On 11 Jun 16, Vincent Guittot wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Probably worth adding a note that sched_mc/sched_smt are disabled by default and need to be enabled by writing to their respective sysfs files.
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Multi-core scheduler support improves the CPU scheduler's decisionmaking when dealing with multi-core CPU chips at a cost of slightlyincreased overhead in some places. If unsure say N here.+config SCHED_SMT
- bool "SMT scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Improves the CPU scheduler's decision making when dealing withMultiThreading at a cost of slightly increased overhead in someplaces. If unsure say N here.config MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
Support Arm cpu topology definition. The MPIDR register definesaffinity between processors which is used to set the cputopology of an Arm System.config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
+#endif
#include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
}
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
init_cpu_topology();
smp_store_cpu_info(smp_processor_id());
/*
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..d61723c --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,133 @@ +/*
- arch/arm/kernel/topology.c
- Copyright (C) 2011 vincent.guittot@linaro.org
This should be
Copyright (C) 2011 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/init.h> +#include <linux/percpu.h> +#include <linux/node.h> +#include <linux/nodemask.h> +#include <linux/sched.h>
+#include <asm/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
unsigned int cpunum; \__asm__("mrc p15, 0, %0, c0, c0, 5" \: "=r" (cpunum)); \cpunum; \- })
+struct cputopo_arm cpu_topology[NR_CPUS];
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return &(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
return;- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr & (0x3 << 30)) {
Use #defines for the MPIDR bit-fields
#define MPIDR_U_BITMASK (0x3 << 30)
BTW, Bit 31 seems to always be 1, do you really want that in the mask?
/** This is a multiprocessor system* multiprocessor format & multiprocessor mode field are set*/if (mpidr & (0x1 << 24)) {
According to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388f/CIHCHAD...
bit 24 is SBZ. Where is this redefined for MT?
/* core performance interdependency */cpuid_topo->thread_id = (mpidr & 0x3);cpuid_topo->core_id = ((mpidr >> 8) & 0xF);cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);
Define BITMASKs for these bits too.
} else {/* normal core interdependency */cpuid_topo->thread_id = -1;cpuid_topo->core_id = (mpidr & 0x3);cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);}- } else {
/** This is an uniprocessor system* we are in multiprocessor format but uniprocessor system* or in the old uniprocessor format*/cpuid_topo->thread_id = -1;cpuid_topo->core_id = 0;cpuid_topo->socket_id = -1;- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);if (cpuid_topo->socket_id == cpu_topo->socket_id) {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) {cpumask_set_cpu(cpuid,&cpu_topo->thread_sibling);if (cpu != cpuid)cpumask_set_cpu(cpu,&cpuid_topo->thread_sibling);}}- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n",
cpuid, cpu_topology[cpuid].thread_id,cpu_topology[cpuid].core_id,cpu_topology[cpuid].socket_id, mpidr);+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- 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();
+}
1.7.4.1
On 16 June 2011 13:48, Amit Kucheria amit.kucheria@linaro.org wrote:
On 11 Jun 16, Vincent Guittot wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Probably worth adding a note that sched_mc/sched_smt are disabled by default and need to be enabled by writing to their respective sysfs files.
ok
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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 MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
- Support Arm cpu topology definition. The MPIDR register defines
- affinity between processors which is used to set the cpu
- topology of an Arm System.
config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
+#endif
#include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
}
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
smp_store_cpu_info(smp_processor_id());
/* diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..d61723c --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,133 @@ +/*
- arch/arm/kernel/topology.c
- Copyright (C) 2011 vincent.guittot@linaro.org
This should be
Copyright (C) 2011 Linaro Limited.
Written by: Vincent Guittot
ok
- 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/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
- unsigned int cpunum; \
- __asm__("mrc p15, 0, %0, c0, c0, 5" \
- : "=r" (cpunum)); \
- cpunum; \
- })
+struct cputopo_arm cpu_topology[NR_CPUS];
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return &(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
- return;
- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr & (0x3 << 30)) {
Use #defines for the MPIDR bit-fields
#define MPIDR_U_BITMASK (0x3 << 30)
BTW, Bit 31 seems to always be 1, do you really want that in the mask?
According to http://infocenter.arm.com/help/topic/com.arm.doc.ddi0406b/index.html
we need to test both bit 31 and bit 24
- /*
- * This is a multiprocessor system
- * multiprocessor format & multiprocessor mode field are set
- */
- if (mpidr & (0x1 << 24)) {
According to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388f/CIHCHAD...
bit 24 is SBZ. Where is this redefined for MT?
- /* core performance interdependency */
- cpuid_topo->thread_id = (mpidr & 0x3);
- cpuid_topo->core_id = ((mpidr >> 8) & 0xF);
- cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);
Define BITMASKs for these bits too.
ok
- } else {
- /* normal core interdependency */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = (mpidr & 0x3);
- cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);
- }
- } else {
- /*
- * This is an uniprocessor system
- * we are in multiprocessor format but uniprocessor system
- * or in the old uniprocessor format
- */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
- if (cpuid_topo->socket_id == cpu_topo->socket_id) {
- 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) {
- cpumask_set_cpu(cpuid,
- &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
- &cpuid_topo->thread_sibling);
- }
- }
- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n",
- cpuid, cpu_topology[cpuid].thread_id,
- cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- 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();
+}
1.7.4.1
I have some doubts about the bit fields of the MPIDR register. Comments added below. On 16 June 2011 14:19, Vincent Guittot vincent.guittot@linaro.org wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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 MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
- Support Arm cpu topology definition. The MPIDR register defines
- affinity between processors which is used to set the cpu
- topology of an Arm System.
config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
+#endif
#include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
}
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
smp_store_cpu_info(smp_processor_id());
/* diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..d61723c --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,133 @@ +/*
- arch/arm/kernel/topology.c
- Copyright (C) 2011 vincent.guittot@linaro.org
- 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/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
- unsigned int cpunum; \
- __asm__("mrc p15, 0, %0, c0, c0, 5" \
- : "=r" (cpunum)); \
- cpunum; \
- })
+struct cputopo_arm cpu_topology[NR_CPUS];
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return &(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
- return;
- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr & (0x3 << 30)) {
I checked the A9 reference manual. It says bit 30 if 0 denotes that processor is part of multiprocessor system.
- /*
- * This is a multiprocessor system
- * multiprocessor format & multiprocessor mode field are set
- */
- if (mpidr & (0x1 << 24)) {
- /* core performance interdependency */
- cpuid_topo->thread_id = (mpidr & 0x3);
- cpuid_topo->core_id = ((mpidr >> 8) & 0xF);
- cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);
- } else {
- /* normal core interdependency */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = (mpidr & 0x3);
- cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);
- }
- } else {
- /*
- * This is an uniprocessor system
- * we are in multiprocessor format but uniprocessor system
- * or in the old uniprocessor format
- */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
- if (cpuid_topo->socket_id == cpu_topo->socket_id) {
- 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) {
- cpumask_set_cpu(cpuid,
- &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
- &cpuid_topo->thread_sibling);
- }
- }
- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n",
- cpuid, cpu_topology[cpuid].thread_id,
- cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- 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();
+}
1.7.4.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On 16 June 2011 13:55, Amit Kachhap amit.kachhap@linaro.org wrote:
I have some doubts about the bit fields of the MPIDR register. Comments added below. On 16 June 2011 14:19, Vincent Guittot vincent.guittot@linaro.org wrote:
The affinity between Arm processors is defined in the MPIDR register. We can identify which processors are in the same cluster, and which ones have performance interdependency. The cpu topology of an Arm platform can be set thanks to this register and this topology is then used by sched_mc and sched_smt.
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
arch/arm/Kconfig | 26 ++++++++ arch/arm/include/asm/topology.h | 33 ++++++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/smp.c | 6 ++ arch/arm/kernel/topology.c | 133 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..bacf9af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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 MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
- Support Arm cpu topology definition. The MPIDR register defines
- affinity between processors which is used to set the cpu
- topology of an Arm System.
config ARM_ERRATA_411920 bool "ARM errata: Invalidation of the Instruction Cache operation can fail" depends on CPU_V6 || CPU_V6K diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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(unsigned int cpu);
+#else
+#define init_cpu_topology() {}; +#define store_cpu_topology(cpuid) {};
+#endif
#include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
}
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
smp_store_cpu_info(smp_processor_id());
/* diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..d61723c --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,133 @@ +/*
- arch/arm/kernel/topology.c
- Copyright (C) 2011 vincent.guittot@linaro.org
- 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/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
- unsigned int cpunum; \
- __asm__("mrc p15, 0, %0, c0, c0, 5" \
- : "=r" (cpunum)); \
- cpunum; \
- })
+struct cputopo_arm cpu_topology[NR_CPUS];
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return &(cpu_topology[cpu].core_sibling);
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]);
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
- return;
- mpidr = hard_smp_mpidr();
- /* create cpu topology mapping */
- if (mpidr & (0x3 << 30)) {
I checked the A9 reference manual. It says bit 30 if 0 denotes that processor is part of multiprocessor system.
You're right, i'm going to update my test
- /*
- * This is a multiprocessor system
- * multiprocessor format & multiprocessor mode field are set
- */
- if (mpidr & (0x1 << 24)) {
- /* core performance interdependency */
- cpuid_topo->thread_id = (mpidr & 0x3);
- cpuid_topo->core_id = ((mpidr >> 8) & 0xF);
- cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);
- } else {
- /* normal core interdependency */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = (mpidr & 0x3);
- cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);
- }
- } else {
- /*
- * This is an uniprocessor system
- * we are in multiprocessor format but uniprocessor system
- * or in the old uniprocessor format
- */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
- if (cpuid_topo->socket_id == cpu_topo->socket_id) {
- 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) {
- cpumask_set_cpu(cpuid,
- &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
- &cpuid_topo->thread_sibling);
- }
- }
- }
- smp_wmb();
- printk(KERN_INFO "cpu %u : thread %d cpu %d, socket %d, mpidr %x\n",
- cpuid, cpu_topology[cpuid].thread_id,
- cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- 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();
+}
1.7.4.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Thu, Jun 16, 2011 at 10:49:13AM +0200, Vincent Guittot wrote:
@@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Multi-core scheduler support improves the CPU scheduler's decisionmaking when dealing with multi-core CPU chips at a cost of slightlyincreased overhead in some places. If unsure say N here.+config SCHED_SMT
- bool "SMT scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Improves the CPU scheduler's decision making when dealing withMultiThreading at a cost of slightly increased overhead in someplaces. If unsure say N here.
Quick question: the patch doesn't actually seem to use the SCHED_MC and SCHED_SMT config bits; is that actually correct?
On 16 June 2011 15:24, Christian Robottom Reis kiko@linaro.org wrote:
On Thu, Jun 16, 2011 at 10:49:13AM +0200, Vincent Guittot wrote:
@@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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.
Quick question: the patch doesn't actually seem to use the SCHED_MC and SCHED_SMT config bits; is that actually correct?
The default config of the patch is to disable sched_mc and sched_smt. The 1st goal is to let each platform enable and test the feature. According to feedback, we could then change the default state of the configuration
-- Christian Robottom Reis | [+55] 16 9112 6430 | http://launchpad.net/~kiko Linaro Engineering VP | [ +1] 612 216 4935 | http://async.com.br/~kiko
On 06/16/2011 01:49 AM, Vincent Guittot wrote:
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Multi-core scheduler support improves the CPU scheduler's decisionmaking when dealing with multi-core CPU chips at a cost of slightlyincreased overhead in some places. If unsure say N here.+config SCHED_SMT
- bool "SMT scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Improves the CPU scheduler's decision making when dealing withMultiThreading at a cost of slightly increased overhead in someplaces. If unsure say N here.
The default is already n so you can drop those two lines.
* This is a multiprocessor system* multiprocessor format & multiprocessor mode field are set*/if (mpidr & (0x1 << 24)) {/* core performance interdependency */cpuid_topo->thread_id = (mpidr & 0x3);cpuid_topo->core_id = ((mpidr >> 8) & 0xF);cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);} else {/* normal core interdependency */cpuid_topo->thread_id = -1;cpuid_topo->core_id = (mpidr & 0x3);cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);}
The ARM ARM says these fields are IMPLEMENTATION DEFINED meaning that different vendors may attribute different meaning to these fields if they wish. Does that mean this should be a platform_*() function?
On 16 June 2011 21:40, Stephen Boyd sboyd@codeaurora.org wrote:
On 06/16/2011 01:49 AM, Vincent Guittot wrote:
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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.
The default is already n so you can drop those two lines.
ok
- * This is a multiprocessor system
- * multiprocessor format & multiprocessor mode field are set
- */
- if (mpidr & (0x1 << 24)) {
- /* core performance interdependency */
- cpuid_topo->thread_id = (mpidr & 0x3);
- cpuid_topo->core_id = ((mpidr >> 8) & 0xF);
- cpuid_topo->socket_id = ((mpidr >> 16) & 0xFF);
- } else {
- /* normal core interdependency */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = (mpidr & 0x3);
- cpuid_topo->socket_id = ((mpidr >> 8) & 0xF);
- }
The ARM ARM says these fields are IMPLEMENTATION DEFINED meaning that different vendors may attribute different meaning to these fields if they wish. Does that mean this should be a platform_*() function?
The ARM ARM also provides a recommended use of the fields of this register and the TRM of each Cortex adds some details. On the cortex A9, each platform can only set the value of the Cluster ID with the CLUSTERID pins. I have tried to consolidate the value of MPIDR across several platforms and they all match with the description.
Have you got an example of a MPIDR register which doesn't match with the implementation ?
-- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On 06/16/2011 11:54 PM, Vincent Guittot wrote:
On 16 June 2011 21:40, Stephen Boyd sboyd@codeaurora.org wrote:
The ARM ARM says these fields are IMPLEMENTATION DEFINED meaning that different vendors may attribute different meaning to these fields if they wish. Does that mean this should be a platform_*() function?
The ARM ARM also provides a recommended use of the fields of this register and the TRM of each Cortex adds some details. On the cortex A9, each platform can only set the value of the Cluster ID with the CLUSTERID pins. I have tried to consolidate the value of MPIDR across several platforms and they all match with the description.
Have you got an example of a MPIDR register which doesn't match with the implementation ?
Not that I know of. I'm more concerned with how the ARM ARM has two recommended usages for these fields depending on virtualization or not. I suppose we can handle that issue when it arises (or does your implementation already handle that?)
On Tue, Jun 21, 2011 at 01:36:15PM -0700, Stephen Boyd wrote:
On 06/16/2011 11:54 PM, Vincent Guittot wrote:
On 16 June 2011 21:40, Stephen Boyd sboyd@codeaurora.org wrote:
The ARM ARM says these fields are IMPLEMENTATION DEFINED meaning that different vendors may attribute different meaning to these fields if they wish. Does that mean this should be a platform_*() function?
The ARM ARM also provides a recommended use of the fields of this register and the TRM of each Cortex adds some details. On the cortex A9, each platform can only set the value of the Cluster ID with the CLUSTERID pins. I have tried to consolidate the value of MPIDR across several platforms and they all match with the description.
Have you got an example of a MPIDR register which doesn't match with the implementation ?
Not that I know of. I'm more concerned with how the ARM ARM has two recommended usages for these fields depending on virtualization or not. I suppose we can handle that issue when it arises (or does your implementation already handle that?)
According to the ARM ARM:
MPIDR provides a mechanism with up to three levels of affinity information, but the meaning of those levels of affinity is entirely IMPLEMENTATION DEFINED.
So we can't really tell the meaning of the affinity bits. There are two recommended ways indeed (with or without virtualisation) which are not that different with regards to the topology (just introducing another level for virtual CPUs).
But I think a more general solution would be for the CPU topology to be provided via the FDT.
On 11 Jun 22, Catalin Marinas wrote:
On Tue, Jun 21, 2011 at 01:36:15PM -0700, Stephen Boyd wrote:
On 06/16/2011 11:54 PM, Vincent Guittot wrote:
On 16 June 2011 21:40, Stephen Boyd sboyd@codeaurora.org wrote:
The ARM ARM says these fields are IMPLEMENTATION DEFINED meaning that different vendors may attribute different meaning to these fields if they wish. Does that mean this should be a platform_*() function?
The ARM ARM also provides a recommended use of the fields of this register and the TRM of each Cortex adds some details. On the cortex A9, each platform can only set the value of the Cluster ID with the CLUSTERID pins. I have tried to consolidate the value of MPIDR across several platforms and they all match with the description.
Have you got an example of a MPIDR register which doesn't match with the implementation ?
Not that I know of. I'm more concerned with how the ARM ARM has two recommended usages for these fields depending on virtualization or not. I suppose we can handle that issue when it arises (or does your implementation already handle that?)
According to the ARM ARM:
MPIDR provides a mechanism with up to three levels of affinity information, but the meaning of those levels of affinity is entirely IMPLEMENTATION DEFINED.
So we can't really tell the meaning of the affinity bits. There are two recommended ways indeed (with or without virtualisation) which are not that different with regards to the topology (just introducing another level for virtual CPUs).
But I think a more general solution would be for the CPU topology to be provided via the FDT.
Agreed. That will be the next step.
We decided on doing it this way to allow non-DT-enabled platforms to be able to use the feature and to allow DT-enabled platforms to settle down in the mean time.
On Thu, Jun 16, 2011 at 10:49:13AM +0200, Vincent Guittot wrote:
@@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Multi-core scheduler support improves the CPU scheduler's decisionmaking when dealing with multi-core CPU chips at a cost of slightlyincreased overhead in some places. If unsure say N here.+config SCHED_SMT
- bool "SMT scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- help
Improves the CPU scheduler's decision making when dealing withMultiThreading at a cost of slightly increased overhead in someplaces. If unsure say N here.
Why place these in the "system type" menu? Wouldn't it be better to place them along side ARM_CPU_TOPOLOGY, and place that along side the SMP option too?
config MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
Support Arm cpu topology definition. The MPIDR register definesaffinity between processors which is used to set the cputopology of an Arm System.
Is there a reason you'd want to disable this?
Please also note that it's "ARM" not "Arm".
diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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))
The thing which naggs me about this is that its a static array, and we know from x86 that static arrays of per-cpu data have various issues (cache line bouncing, as well as limitations when we end up with large numbers of CPUs.)
Lastly, x86 seems to do this:
#define arch_provides_topology_pointers yes
and the only effect I can find of that define is in drivers/base/topology.c:
#ifdef arch_provides_topology_pointers ... unsigned int cpu = dev->id; \ return show_cpumap(0, topology_##name(cpu), buf); \ ... #else ... return show_cpumap(0, topology_##name(dev->id), buf); \ ... #endif
dev->id is type 'u32' which devolves to 'unsigned int' on all supported arches. So it looks to me like this arch_provides_topology_pointers define is completely pointless and we just have useless obfuscation for the hell of it.
That's not a criticism of your patch, it's pointing out a difference between x86 and our implementation.
+#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/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
unsigned int cpunum; \__asm__("mrc p15, 0, %0, c0, c0, 5" \: "=r" (cpunum)); \cpunum; \- })
Please add a definition for CPUID_MPIDR to arch/arm/include/asm/cputype.h and a read_cpuid_mpidr() function, and use that instead.
On 16 June 2011 23:13, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Thu, Jun 16, 2011 at 10:49:13AM +0200, Vincent Guittot wrote:
@@ -219,6 +219,24 @@ source "kernel/Kconfig.freezer"
menu "System Type"
+config SCHED_MC
- bool "Multi-core scheduler support"
- depends on SMP && ARM_CPU_TOPOLOGY
- default n
- 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 SMP && ARM_CPU_TOPOLOGY
- default n
- 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.
Why place these in the "system type" menu? Wouldn't it be better to place them along side ARM_CPU_TOPOLOGY, and place that along side the SMP option too?
yes, it's a better place
config MMU bool "MMU-based Paged Memory Management Support" default y @@ -1062,6 +1080,14 @@ if !MMU source "arch/arm/Kconfig-nommu" endif
+config ARM_CPU_TOPOLOGY
- bool "Support cpu topology definition"
- depends on SMP && CPU_V7
- help
- Support Arm cpu topology definition. The MPIDR register defines
- affinity between processors which is used to set the cpu
- topology of an Arm System.
Is there a reason you'd want to disable this?
In fact, I only want to disable sched_mc and sched_smt. I'm going to add default y for ARM_CPU_TOPOLOGY
Please also note that it's "ARM" not "Arm".
OK
diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..cb90d0a 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +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))
The thing which naggs me about this is that its a static array, and we know from x86 that static arrays of per-cpu data have various issues (cache line bouncing, as well as limitations when we end up with large numbers of CPUs.)
The array is updated sequentially by each processor during boot. Then, it should be used by one cpu when building the sched_domain and when calling the topology sysfs entry. We should not have so many cases where several cpu are accessing simultaneously the cells of the array.
Lastly, x86 seems to do this:
#define arch_provides_topology_pointers yes
and the only effect I can find of that define is in drivers/base/topology.c:
#ifdef arch_provides_topology_pointers ... unsigned int cpu = dev->id; \ return show_cpumap(0, topology_##name(cpu), buf); \ ... #else ... return show_cpumap(0, topology_##name(dev->id), buf); \ ... #endif
dev->id is type 'u32' which devolves to 'unsigned int' on all supported arches. So it looks to me like this arch_provides_topology_pointers define is completely pointless and we just have useless obfuscation for the hell of it.
That's not a criticism of your patch, it's pointing out a difference between x86 and our implementation.
I haven't set arch_provides_topology_pointers because I can't find a difference between setting it or not
+#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/cacheflush.h> +#include <asm/topology.h>
+#define hard_smp_mpidr() \
- ({ \
- unsigned int cpunum; \
- __asm__("mrc p15, 0, %0, c0, c0, 5" \
- : "=r" (cpunum)); \
- cpunum; \
- })
Please add a definition for CPUID_MPIDR to arch/arm/include/asm/cputype.h and a read_cpuid_mpidr() function, and use that instead.
ok I can directly use read_cpuid with a definition for CPUID_MPIDR
-
Amit Kachhap -
Amit Kucheria -
Catalin Marinas -
Christian Robottom Reis -
Daniel Lezcano -
Russell King - ARM Linux -
Samuel Thibault -
Stephen Boyd -
Vincent Guittot