Just moving the powerpc_topology description above.
This will help in using functions in this file and avoid declarations.
No other functional changes
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
arch/powerpc/kernel/smp.c | 104 +++++++++++++++++++-------------------
1 file changed, 52 insertions(+), 52 deletions(-)
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v1 -> v2:
Set cpumask after verifying l2-cache. (Gautham)
arch/powerpc/kernel/smp.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
Move topology fixup based on the platform attributes into its own
function which is called just before set_sched_topology.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v2 -> v3:
Rewrote changelog (Gautham)
Renamed to powerpc/smp: Move topology fixups into a new function
arch/powerpc/kernel/smp.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
@@ -1362,6 +1362,16 @@ int setup_profiling_timer(unsigned int multiplier)return0;}+staticvoidfixup_topology(void)+{+#ifdef CONFIG_SCHED_SMT+if(has_big_cores){+pr_info("Big cores detected but using small core scheduling\n");+powerpc_topology[0].mask=smallcore_smt_mask;+}+#endif+}+void__initsmp_cpus_done(unsignedintmax_cpus){/*
@@ -1375,12 +1385,7 @@ void __init smp_cpus_done(unsigned int max_cpus)dump_numa_cpu_topology();-#ifdef CONFIG_SCHED_SMT-if(has_big_cores){-pr_info("Big cores detected but using small core scheduling\n");-powerpc_topology[0].mask=smallcore_smt_mask;-}-#endif+fixup_topology();set_sched_topology(powerpc_topology);}
In start_secondary, even if shared_cache was already set, system does a
redundant match for cpumask. This redundant check can be removed by
checking if shared_cache is already set.
While here, localize the sibling_mask variable to within the if
condition.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v4 ->v5:
Retain cache domain, no need for generalization
(Michael Ellerman, Peter Zijlstra,
Valentin Schneider, Gautham R. Shenoy)
Changelog v1 -> v2:
Moved shared_cache topology fixup to fixup_topology (Gautham)
arch/powerpc/kernel/smp.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
Add support for grouping cores based on the device-tree classification.
- The last domain in the associativity domains always refers to the
core.
- If primary reference domain happens to be the penultimate domain in
the associativity domains device-tree property, then there are no
coregroups. However if its not a penultimate domain, then there are
coregroups. There can be more than one coregroup. For now we would be
interested in the last or the smallest coregroups, i.e one sub-group
per DIE.
Currently there are no firmwares that are exposing this grouping. Hence
allow the basis for grouping to be abstract. Once the firmware starts
using this grouping, code would be added to detect the type of grouping
and adjust the sd domain flags accordingly.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v4->v5:
Updated commit msg with current abstract nature of the coregroups
(Michael Ellerman)
Changelog v1 -> v2:
Explained Coregroup in commit msg (Michael Ellerman)
arch/powerpc/include/asm/smp.h | 1 +
arch/powerpc/kernel/smp.c | 1 +
arch/powerpc/mm/numa.c | 34 +++++++++++++++++++++-------------
3 files changed, 23 insertions(+), 13 deletions(-)
Add percpu coregroup maps and masks to create coregroup domain.
If a coregroup doesn't exist, the coregroup domain will be degenerated
in favour of SMT/CACHE domain. Do note this patch is only creating stubs
for cpu_to_coregroup_id. The actual cpu_to_coregroup_id implementation
would be in a subsequent patch.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R. Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v4 ->v5:
Updated commit msg to specify actual implementation of
cpu_to_coregroup_id is in a subsequent patch (Michael Ellerman)
Changelog v3 ->v4:
if coregroup_support doesn't exist, update MC mask to the next
smaller domain mask.
Changelog v2 -> v3:
Add optimization for mask updation under coregroup_support
Changelog v1 -> v2:
Moved coregroup topology fixup to fixup_topology (Gautham)
arch/powerpc/include/asm/topology.h | 10 ++++++
arch/powerpc/kernel/smp.c | 54 ++++++++++++++++++++++++++++-
arch/powerpc/mm/numa.c | 5 +++
3 files changed, 68 insertions(+), 1 deletion(-)
@@ -1374,9 +1423,12 @@ static void fixup_topology(void)#ifdef CONFIG_SCHED_SMTif(has_big_cores){pr_info("Big cores detected but using small core scheduling\n");-powerpc_topology[0].mask=smallcore_smt_mask;+powerpc_topology[smt_idx].mask=smallcore_smt_mask;}#endif++if(!has_coregroup_support())+powerpc_topology[mc_idx].mask=powerpc_topology[cache_idx].mask;}void__initsmp_cpus_done(unsignedintmax_cpus)
Lookup the coregroup id from the associativity array.
If unable to detect the coregroup id, fallback on the core id.
This way, ensure sched_domain degenerates and an extra sched domain is
not created.
Ideally this function should have been implemented in
arch/powerpc/kernel/smp.c. However if its implemented in mm/numa.c, we
don't need to find the primary domain again.
If the device-tree mentions more than one coregroup, then kernel
implements only the last or the smallest coregroup, which currently
corresponds to the penultimate domain in the device-tree.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v1 -> v2:
Move coregroup_enabled before getting associativity (Gautham)
arch/powerpc/mm/numa.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
If allocated earlier and the search fails, then cpu_l1_cache_map cpumask
is unnecessarily cleared. However cpu_l1_cache_map can be allocated /
cleared after we search thread group.
Please note CONFIG_CPUMASK_OFFSTACK is not set on Powerpc. Hence cpumask
allocated by zalloc_cpumask_var_node is never freed.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v4 ->v5:
Updated commit msg on why cpumask need not be freed.
(Michael Ellerman)
arch/powerpc/kernel/smp.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
@@ -860,6 +860,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)GFP_KERNEL,cpu_to_node(cpu));zalloc_cpumask_var_node(&per_cpu(cpu_core_map,cpu),GFP_KERNEL,cpu_to_node(cpu));+#ifdef CONFIG_NEED_MULTIPLE_NODES/**numa_node_id()worksafterthis.*/
@@ -868,6 +869,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)set_cpu_numa_mem(cpu,local_memory_node(numa_cpu_lookup_table[cpu]));}+#endif}/* Init the cpumasks so the boot CPU is related to itself */
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 <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v1 -> v2:
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 | 25 +++----------------------
1 file changed, 3 insertions(+), 22 deletions(-)
@@ -1313,7 +1313,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 */staticintpowerpc_smt_flags(void){intflags=SD_SHARE_CPUCAPACITY|SD_SHARE_PKG_RESOURCES;
@@ -1326,14 +1326,6 @@ static int powerpc_smt_flags(void)}#endif-staticstructsched_domain_topology_levelpowerpc_topology[]={-#ifdef CONFIG_SCHED_SMT-{cpu_smt_mask,powerpc_smt_flags,SD_INIT_NAME(SMT)},-#endif-{cpu_cpu_mask,SD_INIT_NAME(DIE)},-{NULL,},-};-/**P9hasaslightlyoddarchitecturewherepairsofcoresshareanL2cache.*Thistopologymakesit*much*cheapertomigratetasksbetweenadjacentcores
@@ -1386,21 +1378,10 @@ void __init smp_cpus_done(unsigned int max_cpus)#ifdef CONFIG_SCHED_SMTif(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-/*-*IfanyCPUdetectsthatit'ssharingacachewithanotherCPUthen-*usethedeepertopologythatisawareofthissharing.-*/-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
From: Gautham R Shenoy <hidden> Date: 2020-08-12 05:38:30
Hi Srikar,
On Mon, Aug 10, 2020 at 12:48:30PM +0530, Srikar Dronamraju wrote:
In start_secondary, even if shared_cache was already set, system does a
redundant match for cpumask. This redundant check can be removed by
checking if shared_cache is already set.
While here, localize the sibling_mask variable to within the if
condition.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
The change looks good to me.
Reviewed-by: Gautham R. Shenoy <redacted>
quoted hunk
---
Changelog v4 ->v5:
Retain cache domain, no need for generalization
(Michael Ellerman, Peter Zijlstra,
Valentin Schneider, Gautham R. Shenoy)
Changelog v1 -> v2:
Moved shared_cache topology fixup to fixup_topology (Gautham)
arch/powerpc/kernel/smp.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2020-09-11 11:57:51
Srikar Dronamraju [off-list ref] writes:
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
* Michael Ellerman [off-list ref] [2020-09-11 21:55:23]:
Srikar Dronamraju [off-list ref] writes:
quoted
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
Thanks Michael for the report and also for identifying the patch and also
giving an easy reproducer. That made my task easy. (My only problem was all
my PowerKVM hosts had a old compiler that refuse to compile never kernels.)
So in this setup, CPU doesn't have a l2-cache. And in that scenario, we
miss updating the l2-cache domain. Actually the initial patch had this
exact code. However it was my mistake. I should have reassessed it before
making changes suggested by Gautham.
Patch below. Do let me know if you want me to send the patch separately.
I haven't tried Mambo. But the problem report looks similar.
Did I miss a lead-up patch?
No
--
Thanks and Regards
Srikar Dronamraju
------------------------->8--------------------------------------------8<------------
From b25d47b01b7195b1df19083a4043fa6a87a901a3 Mon Sep 17 00:00:00 2001
From: Srikar Dronamraju <redacted>
Date: Thu, 9 Jul 2020 13:33:38 +0530
Subject: [PATCH v5.1 05/10] powerpc/smp: Dont assume l2-cache to be superset of
sibling
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v1 -> v2:
Set cpumask after verifying l2-cache. (Gautham)
Changelog v5 -> v5.1:
Set cpumask before verifying l2-cache. (Michael Ellerman)
arch/powerpc/kernel/smp.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2020-09-13 01:47:32
Srikar Dronamraju [off-list ref] writes:
* Michael Ellerman [off-list ref] [2020-09-11 21:55:23]:
quoted
Srikar Dronamraju [off-list ref] writes:
quoted
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
Thanks Michael for the report and also for identifying the patch and also
giving an easy reproducer. That made my task easy. (My only problem was all
my PowerKVM hosts had a old compiler that refuse to compile never kernels.)
So in this setup, CPU doesn't have a l2-cache. And in that scenario, we
miss updating the l2-cache domain. Actually the initial patch had this
exact code. However it was my mistake. I should have reassessed it before
making changes suggested by Gautham.
Patch below. Do let me know if you want me to send the patch separately.
I haven't tried Mambo. But the problem report looks similar.
The patch fixes qemu, and I don't see the crash on mambo, but I still
see:
[ 0.010536] smp: Bringing up secondary CPUs ...
[ 0.019189] smp: Brought up 2 nodes, 8 CPUs
[ 0.019210] numa: Node 0 CPUs: 0-3
[ 0.019235] numa: Node 1 CPUs: 4-7
[ 0.023669] BUG: arch topology borken
[ 0.023690] the SMT domain not a subset of the CACHE domain
[ 0.023726] BUG: arch topology borken
[ 0.023747] the CACHE domain not a subset of the MC domain
[ 0.023808] BUG: arch topology borken
[ 0.023829] the SMT domain not a subset of the CACHE domain
[ 0.023865] BUG: arch topology borken
[ 0.023887] the CACHE domain not a subset of the MC domain
[ 0.023948] BUG: arch topology borken
[ 0.023969] the SMT domain not a subset of the CACHE domain
[ 0.024005] BUG: arch topology borken
[ 0.024026] the CACHE domain not a subset of the MC domain
[ 0.024087] BUG: arch topology borken
[ 0.024108] the SMT domain not a subset of the CACHE domain
[ 0.024144] BUG: arch topology borken
[ 0.024165] the CACHE domain not a subset of the MC domain
[ 0.024227] BUG: arch topology borken
[ 0.024248] the SMT domain not a subset of the CACHE domain
[ 0.024284] BUG: arch topology borken
[ 0.024305] the CACHE domain not a subset of the MC domain
[ 0.024366] BUG: arch topology borken
[ 0.024387] the SMT domain not a subset of the CACHE domain
[ 0.024423] BUG: arch topology borken
[ 0.024444] the CACHE domain not a subset of the MC domain
[ 0.024505] BUG: arch topology borken
[ 0.024527] the SMT domain not a subset of the CACHE domain
[ 0.024563] BUG: arch topology borken
[ 0.024584] the CACHE domain not a subset of the MC domain
[ 0.024645] BUG: arch topology borken
[ 0.024666] the SMT domain not a subset of the CACHE domain
[ 0.024702] BUG: arch topology borken
[ 0.024723] the CACHE domain not a subset of the MC domain
That's the p9 mambo model, using skiboot.tcl from skiboot, with CPUS=2,
THREADS=4 and MAMBO_NUMA=1.
Node layout is:
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000000000000-0x00000000ffffffff]
[ 0.000000] node 1: [mem 0x0000200000000000-0x00002000ffffffff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x00000000ffffffff]
[ 0.000000] On node 0 totalpages: 65536
[ 0.000000] Initmem setup node 1 [mem 0x0000200000000000-0x00002000ffffffff]
[ 0.000000] On node 1 totalpages: 65536
There aren't any l2-cache properties in the device-tree under cpus.
I'll try and have a closer look tonight.
cheers
* Michael Ellerman [off-list ref] [2020-09-13 11:46:41]:
Srikar Dronamraju [off-list ref] writes:
quoted
* Michael Ellerman [off-list ref] [2020-09-11 21:55:23]:
quoted
Srikar Dronamraju [off-list ref] writes:
quoted
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
I'm seeing oopses with this:
The patch fixes qemu, and I don't see the crash on mambo, but I still
see:
[ 0.010536] smp: Bringing up secondary CPUs ...
[ 0.019189] smp: Brought up 2 nodes, 8 CPUs
[ 0.019210] numa: Node 0 CPUs: 0-3
[ 0.019235] numa: Node 1 CPUs: 4-7
[ 0.024444] the CACHE domain not a subset of the MC domain
[ 0.024505] BUG: arch topology borken
[ 0.024527] the SMT domain not a subset of the CACHE domain
[ 0.024563] BUG: arch topology borken
[ 0.024584] the CACHE domain not a subset of the MC domain
[ 0.024645] BUG: arch topology borken
[ 0.024666] the SMT domain not a subset of the CACHE domain
[ 0.024702] BUG: arch topology borken
[ 0.024723] the CACHE domain not a subset of the MC domain
That's the p9 mambo model, using skiboot.tcl from skiboot, with CPUS=2,
THREADS=4 and MAMBO_NUMA=1.
I was able to reproduce with
qemu-system-ppc64 -nographic -vga none -M pseries -cpu POWER8 \
-kernel build~/vmlinux \
-m 2G,slots=2,maxmem=4G \
-object memory-backend-ram,size=1G,id=m0 \
-object memory-backend-ram,size=1G,id=m1 \
-numa node,nodeid=0,memdev=m0 \
-numa node,nodeid=1,memdev=m1 \
-smp 8,threads=4,sockets=2,maxcpus=8 \
If the CPU doesn't have a l2-cache element, then CPU not only has to set
itself in the cpu_l2_cache but also the siblings. Otherwise it will so
happen that the Siblings will have 4 Cpus set, and the Cache domain will
have just one cpu set, leading to this BUG message.
Patch follows this mail.
Node layout is:
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000000000000-0x00000000ffffffff]
[ 0.000000] node 1: [mem 0x0000200000000000-0x00002000ffffffff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x00000000ffffffff]
[ 0.000000] On node 0 totalpages: 65536
[ 0.000000] Initmem setup node 1 [mem 0x0000200000000000-0x00002000ffffffff]
[ 0.000000] On node 1 totalpages: 65536
There aren't any l2-cache properties in the device-tree under cpus.
I'll try and have a closer look tonight.
cheers
Fix to make it work where CPUs dont have a l2-cache element.
------------>8-----------------------------------------8<---------------------
From b25d47b01b7195b1df19083a4043fa6a87a901a3 Mon Sep 17 00:00:00 2001
From: Srikar Dronamraju <redacted>
Date: Thu, 9 Jul 2020 13:33:38 +0530
Subject: [PATCH v5.2 05/10] powerpc/smp: Dont assume l2-cache to be superset of
sibling
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
Cc: linuxppc-dev <redacted>
Cc: LKML <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <redacted>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <redacted>
Cc: Michael Neuling <redacted>
Cc: Gautham R Shenoy <redacted>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <redacted>
Cc: Jordan Niethe <redacted>
Cc: Vaidyanathan Srinivasan <redacted>
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog v1 -> v2:
Set cpumask after verifying l2-cache. (Gautham)
Changelog v5 -> v5.2:
If cpu has no l2-cache set cpumask as per its
sibling mask. (Michael Ellerman)
arch/powerpc/kernel/smp.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
On Fri, Sep 11, 2020 at 09:55:23PM +1000, Michael Ellerman wrote:
Srikar Dronamraju [off-list ref] writes:
quoted
Current code assumes that cpumask of cpus sharing a l2-cache mask will
always be a superset of cpu_sibling_mask.
Lets stop that assumption. cpu_l2_cache_mask is a superset of
cpu_sibling_mask if and only if shared_caches is set.
PowerKVM guest vCPUs does not yet have L2 and L3 cache elements
I had this bug raised some time ago, probably related?
https://bugs.launchpad.net/qemu/+bug/1774605
Regards,
-Satheesh.