power9 shared caches support
This is a pretty substantial rework of the last patch that should address most of the comment mpe had, namely:
1. Removed the random cpu_feature(ARCH_300) check. The new series always builds the cache mask and the new scheduler topology is used if we detect a mismatch of the thread and
2. In the previous series the online and offlining paths were symmetrical. Althoug this is nice in theory it resulted in a bit unnecessary indirection. To simplify things it now only parse the device-tree, etc in the onlining path. When offlining a CPU all we need to do is walk the masks of the offlining CPU.
There's also fixes for various nitpicks.
PATCH 1/4 powerpc/smp: Use cpu_to_chip_id() to find core siblings
When building the CPU scheduler topology the kernel uses the ibm,chipid property from the devicetree to group logical CPUs. Currently the DT search for this property is open-coded in smp.c and this functionality is a duplication of what's in cpu_to_chip_id() already. This patch removes the existing search in favor of that.
It's worth mentioning that the semantics of the search are different in cpu_to_chip_id(). When there is no ibm,chipid in the CPUs node it will also search /cpus and / for the property, but this should not effect the output topology.
--- arch/powerpc/kernel/smp.c | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-)
arch/powerpc/kernel/smp.c +11 -26
@@ -829,19 +829,11 @@ EXPORT_SYMBOL_GPL(cpu_first_thread_of_core);static void traverse_siblings_chip_id (int cpu,bool add,int chipid) { -const struct cpumask *mask; -struct device_node *np; -int i, plen; -const __be32 *prop; +const struct cpumask *mask = add ? cpu_online_mask : cpu_present_mask; +int i; - mask = add ? cpu_online_mask : cpu_present_mask;for_each_cpu (i, mask) { - np =of_get_cpu_node (i,NULL ); -if (!np) -continue ; - prop =of_get_property (np,"ibm,chip-id" , &plen); -if (prop && plen ==sizeof (int ) && -of_read_number (prop,1 ) == chipid) { +if (cpu_to_chip_id (i) == chipid) {if (add) {cpumask_set_cpu (cpu,cpu_core_mask (i));cpumask_set_cpu (i,cpu_core_mask (cpu));
@@ -850,7 +842,6 @@ static void traverse_siblings_chip_id(int cpu, bool add, int chipid)cpumask_clear_cpu (i,cpu_core_mask (cpu)); } } -of_node_put (np); } }
@@ -880,21 +871,15 @@ static void traverse_core_siblings(int cpu, bool add) {struct device_node *l2_cache, *np;const struct cpumask *mask; -int i, chip, plen; -const __be32 *prop; +int chip_id; +int i; -/* First see if we have ibm,chip-id properties in cpu nodes */ - np =of_get_cpu_node (cpu,NULL ); -if (np) { - chip =-1 ; - prop =of_get_property (np,"ibm,chip-id" , &plen); -if (prop && plen ==sizeof (int )) - chip =of_read_number (prop,1 ); -of_node_put (np); -if (chip >=0 ) { -traverse_siblings_chip_id (cpu, add, chip); -return ; - } +/* threads that share a chip-id are considered siblings */ + chip_id =cpu_to_chip_id (cpu); + +if (chip_id >=0 ) { +traverse_siblings_chip_id (cpu, add, chip_id); +return ; } l2_cache =cpu_to_l2cache (cpu);
Signature · 1 line
2.9.4
Re: [1/4] powerpc/smp: Use cpu_to_chip_id() to find core siblings
Quoted 6 lines · Oliver O'Halloran
When building the CPU scheduler topology the kernel uses the ibm,chipid property from the devicetree to group logical CPUs. Currently the DT search for this property is open-coded in
smp.cand this functionality is a duplication of what's incpu_to_chip_id()already. This patch removes the existing search in favor of that.
Quoted 6 lines
It's worth mentioning that the semantics of the search are different in
cpu_to_chip_id(). When there is no ibm,chipid in the CPUs node it will also search /cpus and / for the property, but this should not effect the output topology.Signed-off-by Oliver O'Halloran <[email protected]>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/e3d8b67e2c60dcd35661d34df249f2
cheers
PATCH 2/4 powerpc/smp: Rework CPU topology construction
In reply to Oliver O'HalloranThe CPU scheduler topology is constructed from a number of per-cpu cpumasks which describe which sets of logical CPUs are related in some fashion. Current code that handles constructing these masks when CPUs are hot(un)plugged can be simplified a bit by exploiting the fact that the scheduler requires higher levels of the toplogy (e.g package level groupings) to be supersets of the lower levels (e.g. threas in a core). This patch reworks the cpumask construction to be simpler and easier to extend with extra topology levels.
--- arch/powerpc/kernel/smp.c | 148 +++++++++++++++++++++++++--------------------- 1 file changed, 82 insertions(+), 66 deletions(-)
arch/powerpc/kernel/smp.c +82 -66
@@ -569,6 +569,24 @@ static void smp_store_cpu_info(int id)#endif } +/* +* Relationships between CPUs are maintained in a set of per-cpu cpumasks so +* rather than just passing around the cpumask we pass around a function that +* returns the that cpumask for the given CPU. +*/ +static void set_cpus_related (int i,int j,struct cpumask *(*get_cpumask)(int )) +{ +cpumask_set_cpu (i,get_cpumask (j)); +cpumask_set_cpu (j,get_cpumask (i)); +} + +static void set_cpus_unrelated (int i,int j, +struct cpumask *(*get_cpumask)(int )) +{ +cpumask_clear_cpu (i,get_cpumask (j)); +cpumask_clear_cpu (j,get_cpumask (i)); +} +void __init smp_prepare_cpus (unsigned int max_cpus) {unsigned int cpu;
@@ -600,6 +618,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus) } } +/* Init the cpumasks so the boot CPU is related to itself */ cpumask_set_cpu (boot_cpuid,cpu_sibling_mask (boot_cpuid));cpumask_set_cpu (boot_cpuid,cpu_core_mask (boot_cpuid));
@@ -827,24 +846,6 @@ int cpu_first_thread_of_core(int core) }EXPORT_SYMBOL_GPL (cpu_first_thread_of_core); -static void traverse_siblings_chip_id (int cpu,bool add,int chipid) -{ -const struct cpumask *mask = add ? cpu_online_mask : cpu_present_mask; -int i; - -for_each_cpu (i, mask) { -if (cpu_to_chip_id (i) == chipid) { -if (add) { -cpumask_set_cpu (cpu,cpu_core_mask (i)); -cpumask_set_cpu (i,cpu_core_mask (cpu)); - }else { -cpumask_clear_cpu (cpu,cpu_core_mask (i)); -cpumask_clear_cpu (i,cpu_core_mask (cpu)); - } - } - } -} -/* Must be called when no change can occur to cpu_present_mask, * i.e. during cpu online or offline. */
@@ -867,46 +868,83 @@ static struct device_node *cpu_to_l2cache(int cpu)return cache; } -static void traverse_core_siblings (int cpu,bool add) +static bool update_mask_by_l2 (int cpu,struct cpumask *(*mask_fn)(int )) {struct device_node *l2_cache, *np; -const struct cpumask *mask; -int chip_id;int i; -/* threads that share a chip-id are considered siblings */ - chip_id =cpu_to_chip_id (cpu); - -if (chip_id >=0 ) { -traverse_siblings_chip_id (cpu, add, chip_id); -return ; - } - l2_cache =cpu_to_l2cache (cpu); - mask = add ? cpu_online_mask : cpu_present_mask; -for_each_cpu (i, mask) { +if (!l2_cache) +return false; + +for_each_cpu (i, cpu_online_mask) { +/* +* when updating the marks the current CPU has not been marked +* online, but we need to update the cache masks +*/ np =cpu_to_l2cache (i);if (!np)continue ; -if (np == l2_cache) { -if (add) { -cpumask_set_cpu (cpu,cpu_core_mask (i)); -cpumask_set_cpu (i,cpu_core_mask (cpu)); - }else { -cpumask_clear_cpu (cpu,cpu_core_mask (i)); -cpumask_clear_cpu (i,cpu_core_mask (cpu)); - } - } + +if (np == l2_cache) +set_cpus_related (cpu, i, mask_fn); +of_node_put (np); }of_node_put (l2_cache); + +return true; +} + +static void remove_cpu_from_masks (int cpu) +{ +int i; + +/* NB: cpu_core_mask is a superset of the others */ +for_each_cpu (i,cpu_core_mask (cpu)) { +set_cpus_unrelated (cpu, i, cpu_core_mask); +set_cpus_unrelated (cpu, i, cpu_sibling_mask); + } +} + +static void add_cpu_to_masks (int cpu) +{ +int first_thread =cpu_first_thread_sibling (cpu); +int chipid =cpu_to_chip_id (cpu); +int i; + +/* +* This CPU will not be in the online mask yet so we need to manually +* add it to it's own thread sibling mask. +*/ +cpumask_set_cpu (cpu,cpu_sibling_mask (cpu)); + +for (i = first_thread; i < first_thread + threads_per_core; i++) +if (cpu_online (i)) +set_cpus_related (i, cpu, cpu_sibling_mask); + +/* +* Copy the thread sibling into core sibling mask, and +* add CPUs that share a chip or an L2 to the core sibling +* mask. +*/ +for_each_cpu (i,cpu_sibling_mask (cpu)) +set_cpus_related (cpu, i, cpu_core_mask); + +if (chipid ==-1 ) { +update_mask_by_l2 (cpu, cpu_core_mask); +return ; + } + +for_each_cpu (i, cpu_online_mask) +if (cpu_to_chip_id (i) == chipid) +set_cpus_related (cpu, i, cpu_core_mask); }/* Activate a secondary processor. */ void start_secondary (void *unused) {unsigned int cpu =smp_processor_id (); -int i, base;mmgrab (&init_mm); current->active_mm = &init_mm;
@@ -929,22 +967,8 @@ void start_secondary(void *unused)vdso_getcpu_init ();#endif -/* Update sibling maps */ - base =cpu_first_thread_sibling (cpu); -for (i =0 ; i < threads_per_core; i++) { -if (cpu_is_offline (base + i) && (cpu != base + i)) -continue ; -cpumask_set_cpu (cpu,cpu_sibling_mask (base + i)); -cpumask_set_cpu (base + i,cpu_sibling_mask (cpu)); - -/* cpu_core_map should be a superset of -* cpu_sibling_map even if we don't have cache -* information, so update the former here, too. -*/ -cpumask_set_cpu (cpu,cpu_core_mask (base + i)); -cpumask_set_cpu (base + i,cpu_core_mask (cpu)); - } -traverse_core_siblings (cpu, true); +/* Update topology CPU masks */ +add_cpu_to_masks (cpu);set_numa_node (numa_cpu_lookup_table[cpu]);set_numa_mem (local_memory_node (numa_cpu_lookup_table[cpu]));
@@ -1015,7 +1039,6 @@ void __init smp_cpus_done(unsigned int max_cpus)int __cpu_disable (void ) {int cpu =smp_processor_id (); -int base, i;int err;if (!smp_ops->cpu_disable)
@@ -1026,14 +1049,7 @@ int __cpu_disable(void)return err;/* Update sibling maps */ - base =cpu_first_thread_sibling (cpu); -for (i =0 ; i < threads_per_core && base + i < nr_cpu_ids; i++) { -cpumask_clear_cpu (cpu,cpu_sibling_mask (base + i)); -cpumask_clear_cpu (base + i,cpu_sibling_mask (cpu)); -cpumask_clear_cpu (cpu,cpu_core_mask (base + i)); -cpumask_clear_cpu (base + i,cpu_core_mask (cpu)); - } -traverse_core_siblings (cpu, false); +remove_cpu_from_masks (cpu);return 0 ; }
Signature · 1 line
2.9.4
PATCH 3/4 powerpc/smp: Add cpu_l2_cache_map
In reply to Oliver O'HalloranWe want to add an extra level to the CPU scheduler topology to account for cores which share a cache. To do this we need to build a cpumask for each CPU that indicates which CPUs share this cache to use as an input to the scheduler.
--- arch/powerpc/include/asm/smp.h | 6 ++++++ arch/powerpc/kernel/smp.c | 23 +++++++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-)
arch/powerpc/include/asm/smp.h +6 -0
@@ -97,6 +97,7 @@ static inline void set_hard_smp_processor_id(int cpu, int phys)#endif DECLARE_PER_CPU (cpumask_var_t, cpu_sibling_map); +DECLARE_PER_CPU (cpumask_var_t, cpu_l2_cache_map);DECLARE_PER_CPU (cpumask_var_t, cpu_core_map);static inline struct cpumask *cpu_sibling_mask (int cpu)
@@ -109,6 +110,11 @@ static inline struct cpumask *cpu_core_mask(int cpu)return per_cpu (cpu_core_map, cpu); } +static inline struct cpumask *cpu_l2_cache_mask (int cpu) +{ +return per_cpu (cpu_l2_cache_map, cpu); +} +extern int cpu_to_core_id (int cpu);/* Since OpenPIC has only 4 IPIs, we use slightly different message numbers.
arch/powerpc/kernel/smp.c +17 -6
@@ -74,9 +74,11 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };struct thread_info *secondary_ti;DEFINE_PER_CPU (cpumask_var_t, cpu_sibling_map); +DEFINE_PER_CPU (cpumask_var_t, cpu_l2_cache_map);DEFINE_PER_CPU (cpumask_var_t, cpu_core_map);EXPORT_PER_CPU_SYMBOL (cpu_sibling_map); +EXPORT_PER_CPU_SYMBOL (cpu_l2_cache_map);EXPORT_PER_CPU_SYMBOL (cpu_core_map);/* SMP operations for this machine */
@@ -606,6 +608,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus)for_each_possible_cpu (cpu) {zalloc_cpumask_var_node (&per_cpu (cpu_sibling_map, cpu),GFP_KERNEL ,cpu_to_node (cpu)); +zalloc_cpumask_var_node (&per_cpu (cpu_l2_cache_map, cpu), +GFP_KERNEL ,cpu_to_node (cpu));zalloc_cpumask_var_node (&per_cpu (cpu_core_map, cpu),GFP_KERNEL ,cpu_to_node (cpu));/*
@@ -620,6 +624,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)/* Init the cpumasks so the boot CPU is related to itself */ cpumask_set_cpu (boot_cpuid,cpu_sibling_mask (boot_cpuid)); +cpumask_set_cpu (boot_cpuid,cpu_l2_cache_mask (boot_cpuid));cpumask_set_cpu (boot_cpuid,cpu_core_mask (boot_cpuid));if (smp_ops && smp_ops->probe)
@@ -903,6 +908,7 @@ static void remove_cpu_from_masks(int cpu)/* NB: cpu_core_mask is a superset of the others */ for_each_cpu (i,cpu_core_mask (cpu)) {set_cpus_unrelated (cpu, i, cpu_core_mask); +set_cpus_unrelated (cpu, i, cpu_l2_cache_mask);set_cpus_unrelated (cpu, i, cpu_sibling_mask); } }
@@ -924,17 +930,22 @@ static void add_cpu_to_masks(int cpu)set_cpus_related (i, cpu, cpu_sibling_mask);/* -* Copy the thread sibling into core sibling mask, and -* add CPUs that share a chip or an L2 to the core sibling -* mask. +* Copy the thread sibling mask into the cache sibling mask +* and mark any CPUs that share an L2 with this CPU. */ for_each_cpu (i,cpu_sibling_mask (cpu)) +set_cpus_related (cpu, i, cpu_l2_cache_mask); +update_mask_by_l2 (cpu, cpu_l2_cache_mask); + +/* +* Copy the cache sibling mask into core sibling mask and mark +* any CPUs on the same chip as this CPU. +*/ +for_each_cpu (i,cpu_l2_cache_mask (cpu))set_cpus_related (cpu, i, cpu_core_mask); -if (chipid ==-1 ) { -update_mask_by_l2 (cpu, cpu_core_mask); +if (chipid ==-1 )return ; - }for_each_cpu (i, cpu_online_mask)if (cpu_to_chip_id (i) == chipid)
Signature · 1 line
2.9.4
PATCH 4/4 powerpc/smp: Add Power9 scheduler topology
In reply to Oliver O'HalloranIn previous generations of Power processors each core had a private L2 cache. The Power 9 processor has a slightly different design where the L2 cache is shared among pairs of cores rather than being completely private.
Making the scheduler aware of this cache sharing allows the scheduler to make better migration decisions. For example, if two CPU heavy tasks share a core then one task can be migrated to the paired core to improve throughput. Under the existing three level topology the task could be migrated to any core on the same chip, while with the new topology it would be preferentially migrated to the paired core so it remains cache-hot.
--- arch/powerpc/kernel/smp.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)
arch/powerpc/kernel/smp.c +51 -1
@@ -952,6 +952,8 @@ static void add_cpu_to_masks(int cpu)set_cpus_related (cpu, i, cpu_core_mask); } +static bool shared_caches; +/* Activate a secondary processor. */ void start_secondary (void *unused) {
@@ -981,6 +983,13 @@ void start_secondary(void *unused)/* Update topology CPU masks */ add_cpu_to_masks (cpu); +/* +* Check for any shared caches. Note that this must be done on a +* per-core basis because one core in the pair might be disabled. +*/ +if (!cpumask_equal (cpu_l2_cache_mask (cpu),cpu_sibling_mask (cpu))) + shared_caches = true; +set_numa_node (numa_cpu_lookup_table[cpu]);set_numa_mem (local_memory_node (numa_cpu_lookup_table[cpu]));
@@ -1022,6 +1031,35 @@ static struct sched_domain_topology_level powerpc_topology[] = { { NULL, }, }; +/* +* P9 has a slightly odd architecture where pairs of cores share an L2 cache. +* This topology makes it *much* cheaper to migrate tasks between adjacent cores +* since the migrated task remains cache hot. We want to take advantage of this +* at the scheduler level so an extra topology level is required. +*/ +static int powerpc_shared_cache_flags (void ) +{ +return SD_SHARE_PKG_RESOURCES ; +} + +/* +* We can't just pass cpu_l2_cache_mask() directly because +* returns a non-const pointer and the compiler barfs on that. +*/ +static const struct cpumask *shared_cache_mask (int cpu) +{ +return cpu_l2_cache_mask (cpu); +} + +static struct sched_domain_topology_level power9_topology[] = { +#ifdef CONFIG_SCHED_SMT + { cpu_smt_mask, powerpc_smt_flags,SD_INIT_NAME (SMT ) }, +#endif + { shared_cache_mask, powerpc_shared_cache_flags,SD_INIT_NAME (CACHE ) }, + { cpu_cpu_mask,SD_INIT_NAME (DIE ) }, + {NULL , }, +}; +static __init long smp_setup_cpu_workfn (void *data__always_unused ) { smp_ops->setup_cpu (boot_cpuid);
@@ -1043,7 +1081,19 @@ void __init smp_cpus_done(unsigned int max_cpus)dump_numa_cpu_topology (); -set_sched_topology (powerpc_topology); +/* +* If any CPU detects that it's sharing a cache with another CPU then +* use the deeper topology that is aware of this sharing. +*/ +if (shared_caches) { +pr_info ("Using shared cache scheduler topology\n" ); +set_sched_topology (power9_topology); +/* HACK: warn if we're using this on anything by P9 */ +WARN_ON ((mfspr (SPRN_PVR ) &0xffffff00 ) !=0x004e0100 ); + }else { +pr_info ("Using standard scheduler topology\n" ); +set_sched_topology (powerpc_topology); + } }#ifdef CONFIG_HOTPLUG_CPU
Signature · 1 line
2.9.4
Michael Ellerman
reviewerRe: [PATCH 4/4] powerpc/smp: Add Power9 scheduler topology
Quoted 8 lines · Oliver O'Halloran
arch/powerpc/kernel/smp.c +0
-0@@ -1043,7 +1081,19 @@ void __init smp_cpus_done(unsigned int max_cpus)dump_numa_cpu_topology ();
arch/powerpc/kernel/smp.c void __init smp_cpus_done(unsigned int max_cpus) · line 1092
Quoted 10 lines
arch/powerpc/kernel/smp.c +9
-1-set_sched_topology (powerpc_topology); +/* +* If any CPU detects that it's sharing a cache with another CPU then +* use the deeper topology that is aware of this sharing. +*/ +if (shared_caches) { +pr_info ("Using shared cache scheduler topology\n" ); +set_sched_topology (power9_topology); +/* HACK: warn if we're using this on anything by P9 */ +WARN_ON ((mfspr (SPRN_PVR ) &0xffffff00 ) !=0x004e0100 );
Um, yuck.
Why are we doing that?
cheers
Re: [PATCH 4/4] powerpc/smp: Add Power9 scheduler topology
In reply to Oliver O'Halloranarch/powerpc/kernel/smp.c static struct sched_domain_topology_level powerpc_topology[] = { · line 1062
Quoted 9 lines
arch/powerpc/kernel/smp.c +9
-0+static struct sched_domain_topology_level power9_topology[] = { +#ifdef CONFIG_SCHED_SMT + { cpu_smt_mask, powerpc_smt_flags,SD_INIT_NAME (SMT ) }, +#endif + { shared_cache_mask, powerpc_shared_cache_flags,SD_INIT_NAME (CACHE ) }, + { cpu_cpu_mask,SD_INIT_NAME (DIE ) }, + {NULL , }, +}; +
Should we name this domain as MC instead of CACHE. Then it would be compatible with other archs like x86/arm/s390. MC stands for Multi-Core so it gels with what we have.
Also we could enable CONFIG_SCHED_MC and reuse cpu_core_flags() instead of adding powerpc_shared_cache_flags.