2020-07-21 11:39:50

by Srikar Dronamraju

[permalink] [raw]
Subject: [PATCH v2 02/10] powerpc/smp: Merge Power9 topology with Power topology

A new sched_domain_topology_level was added just for Power9. However the
same can be achieved by merging powerpc_topology with power9_topology
and makes the code more simpler especially when adding a new sched
domain.

Cc: linuxppc-dev <[email protected]>
Cc: LKML <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Nick Piggin <[email protected]>
Cc: Oliver OHalloran <[email protected]>
Cc: Nathan Lynch <[email protected]>
Cc: Michael Neuling <[email protected]>
Cc: Anton Blanchard <[email protected]>
Cc: Gautham R Shenoy <[email protected]>
Cc: Vaidyanathan Srinivasan <[email protected]>
Cc: Jordan Niethe <[email protected]>
Signed-off-by: Srikar Dronamraju <[email protected]>
---
Changelog v1 -> v2:
powerpc/smp: Merge Power9 topology with Power topology
Replaced a reference to cpu_smt_mask with per_cpu(cpu_sibling_map, cpu)
since cpu_smt_mask is only defined under CONFIG_SCHED_SMT

arch/powerpc/kernel/smp.c | 33 ++++++++++-----------------------
1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 680c0edcc59d..0e0b118d9b6e 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1315,7 +1315,7 @@ int setup_profiling_timer(unsigned int multiplier)
}

#ifdef CONFIG_SCHED_SMT
-/* cpumask of CPUs with asymetric SMT dependancy */
+/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
@@ -1328,14 +1328,6 @@ static int powerpc_smt_flags(void)
}
#endif

-static struct sched_domain_topology_level powerpc_topology[] = {
-#ifdef CONFIG_SCHED_SMT
- { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
-#endif
- { cpu_cpu_mask, SD_INIT_NAME(DIE) },
- { 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
@@ -1353,7 +1345,13 @@ static int powerpc_shared_cache_flags(void)
*/
static const struct cpumask *shared_cache_mask(int cpu)
{
- return cpu_l2_cache_mask(cpu);
+ if (shared_caches)
+ return cpu_l2_cache_mask(cpu);
+
+ if (has_big_cores)
+ return cpu_smallcore_mask(cpu);
+
+ return per_cpu(cpu_sibling_map, cpu);
}

#ifdef CONFIG_SCHED_SMT
@@ -1363,7 +1361,7 @@ static const struct cpumask *smallcore_smt_mask(int cpu)
}
#endif

-static struct sched_domain_topology_level power9_topology[] = {
+static struct sched_domain_topology_level powerpc_topology[] = {
#ifdef CONFIG_SCHED_SMT
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
#endif
@@ -1388,21 +1386,10 @@ void __init smp_cpus_done(unsigned int max_cpus)
#ifdef CONFIG_SCHED_SMT
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
- power9_topology[0].mask = smallcore_smt_mask;
powerpc_topology[0].mask = smallcore_smt_mask;
}
#endif
- /*
- * 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);
- } else {
- pr_info("Using standard scheduler topology\n");
- set_sched_topology(powerpc_topology);
- }
+ set_sched_topology(powerpc_topology);
}

#ifdef CONFIG_HOTPLUG_CPU
--
2.17.1


2020-07-22 05:49:58

by Gautham R Shenoy

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] powerpc/smp: Merge Power9 topology with Power topology

On Tue, Jul 21, 2020 at 05:08:06PM +0530, Srikar Dronamraju wrote:
> A new sched_domain_topology_level was added just for Power9. However the
> same can be achieved by merging powerpc_topology with power9_topology
> and makes the code more simpler especially when adding a new sched
> domain.
>
> Cc: linuxppc-dev <[email protected]>
> Cc: LKML <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Nick Piggin <[email protected]>
> Cc: Oliver OHalloran <[email protected]>
> Cc: Nathan Lynch <[email protected]>
> Cc: Michael Neuling <[email protected]>
> Cc: Anton Blanchard <[email protected]>
> Cc: Gautham R Shenoy <[email protected]>
> Cc: Vaidyanathan Srinivasan <[email protected]>
> Cc: Jordan Niethe <[email protected]>
> Signed-off-by: Srikar Dronamraju <[email protected]>
> ---
> Changelog v1 -> v2:
> powerpc/smp: Merge Power9 topology with Power topology
> Replaced a reference to cpu_smt_mask with per_cpu(cpu_sibling_map, cpu)
> since cpu_smt_mask is only defined under CONFIG_SCHED_SMT
>
> arch/powerpc/kernel/smp.c | 33 ++++++++++-----------------------
> 1 file changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 680c0edcc59d..0e0b118d9b6e 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1315,7 +1315,7 @@ int setup_profiling_timer(unsigned int multiplier)
> }
>
> #ifdef CONFIG_SCHED_SMT
> -/* cpumask of CPUs with asymetric SMT dependancy */
> +/* cpumask of CPUs with asymmetric SMT dependency */
> static int powerpc_smt_flags(void)
> {
> int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> @@ -1328,14 +1328,6 @@ static int powerpc_smt_flags(void)
> }
> #endif
>
> -static struct sched_domain_topology_level powerpc_topology[] = {
> -#ifdef CONFIG_SCHED_SMT
> - { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> -#endif
> - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> - { 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
> @@ -1353,7 +1345,13 @@ static int powerpc_shared_cache_flags(void)
> */
> static const struct cpumask *shared_cache_mask(int cpu)
> {
> - return cpu_l2_cache_mask(cpu);
> + if (shared_caches)
> + return cpu_l2_cache_mask(cpu);
> +
> + if (has_big_cores)
> + return cpu_smallcore_mask(cpu);
> +
> + return per_cpu(cpu_sibling_map, cpu);
> }


It might be helpful to enumerate the consequences of this change:

With this patch, on POWER7 and POWER8

SMT and CACHE domains' cpumasks will both be
per_cpu(cpu_sibling_map, cpu).

On POWER7 SMT level flags has the following
(SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING)

On POWER8 SMT level flags has the following
(SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES).

On both POWER7 and POWER8, CACHE level flags only has
SD_SHARE_PKG_RESOURCES

Thus, on both POWER7 and POWER8, since the SMT and CACHE cpumasks
are the same and since CACHE has no additional flags which SMT does
not, the parent domain CACHE will be degenerated.

Hence we will have SMT --> DIE --> NUMA as before without the
patch. So the patch introduces no behavioural change. Only change
is an additional degeneration of the CACHE domain.

On POWER9 : Baremetal.
SMT level cpumask = per_cpu(cpu_sibling_map, cpu)

Since the caches are shared for a pair of two cores,
CACHE level cpumask = cpu_l2_cache_mask(cpu)

Thus, we will have SMT --> CACHE --> DIE --> NUMA as before. No
behavioural change.

On POWER9 : LPAR
SMT level cpumask = cpu_smallcore_mask(cpu).

Since the caches are shared,
CACHE level cpumask = cpu_l2_cache_mask(cpu).

Thus, we will have SMT --> CACHE --> DIE --> NUMA as before. Again
no change in behaviour.

Reviewed-by: Gautham R. Shenoy <[email protected]>

--
Thanks and Regards
gautham.