Fix warning reported by verify_cpu_node_mapping()
RFC PATCH 1/1 Fix warning reported by verify_cpu_node_mapping()
With commit 2fabf084b, during boottime, cpu_numa_callback() is called earlier(before their online) for each cpu, and verify_cpu_node_mapping() uses cpu_to_node() to check whether siblings are in the same node.
It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu numa_node cpu_to_node() uses hasn't been set up yet (which will be set up in smp_prepare_cpus()).
So I could see something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
node!
As we don't actually do the checking during this early stage, so maybe we could directly call numa_setup_cpu() in do_init_bootmem()?
--- arch/powerpc/mm/numa.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/mm/numa.c +1 -2
@@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu); } }
Quoted 4 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.
Quoted 8 lines
It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).So I could see something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
You mean you did see this, right? (as opposed to "could" based upon code inspection or something)
Quoted 5 lines
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem()?Signed-off-by Li Zhong <[email protected]>
Quoted 9 lines
--- arch/powerpc/mm/numa.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/powerpc/mm/numa.c +0
-0
arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1131
Quoted 6 lines
arch/powerpc/mm/numa.c +1
-2@@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu);
This is a good change, thanks for catching it. I must have glossed over those messages in my testing, my apologies!
-Nish
Quoted 10 lines · Nishanth Aravamudan
Quoted 9 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).
Quoted 6 lines
Quoted 3 lines
So I could see something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
You mean you did see this, right? (as opposed to "could" based upon code inspection or something)
Yes, I did see the warnings. Seems I didn't express it precisely in English ...
Quoted 8 lines
Quoted 6 lines
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem()?Signed-off-by Li Zhong <[email protected]>
Thank you for the review,
Zhong
Quoted 21 lines
Quoted 10 lines
--- arch/powerpc/mm/numa.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/powerpc/mm/numa.c +0
-0arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1131
Quoted 6 lines
arch/powerpc/mm/numa.c +1
-2@@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu);This is a good change, thanks for catching it. I must have glossed over those messages in my testing, my apologies!
-Nish
Quoted 26 lines · Li Zhong
Quoted 10 lines · Nishanth Aravamudan
Quoted 9 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).Quoted 6 lines
Quoted 3 lines
So I could see something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
You mean you did see this, right? (as opposed to "could" based upon code inspection or something)
Yes, I did see the warnings. Seems I didn't express it precisely in English ...
Quoted 5 lines
Quoted 5 lines
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem()?
Quoted 27 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Li Zhong <[email protected]>Thank you for the review,
Zhong
Quoted 19 lines
Quoted 10 lines
--- arch/powerpc/mm/numa.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/powerpc/mm/numa.c +0
-0arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1131
Quoted 6 lines
arch/powerpc/mm/numa.c +1
-2@@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu);This is a good change, thanks for catching it. I must have glossed over those messages in my testing, my apologies!
Actually, thinking about this more, do you think it makes more sense to do:
for_each_present_cpu(cpu) in this loop? That is, at boot, ensure all present (but possibly offline) CPUs have their NUMA mapping set up. CPUs that aren't present (but are possible) might trigger other warnings, right? (e.g., the WARN_ON(1) in numa_setup_cpu)
-Nish
Quoted 55 lines · Nishanth Aravamudan
Quoted 26 lines · Li Zhong
Quoted 10 lines · Nishanth Aravamudan
Quoted 9 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).Quoted 6 lines
Quoted 3 lines
So I could see something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
You mean you did see this, right? (as opposed to "could" based upon code inspection or something)
Yes, I did see the warnings. Seems I didn't express it precisely in English ...
Quoted 5 lines
Quoted 5 lines
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem()?Quoted 27 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Li Zhong <[email protected]>Thank you for the review,
Zhong
Quoted 19 lines
Quoted 10 lines
--- arch/powerpc/mm/numa.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/powerpc/mm/numa.c +0
-0arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1131
Quoted 6 lines
arch/powerpc/mm/numa.c +1
-2@@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu);This is a good change, thanks for catching it. I must have glossed over those messages in my testing, my apologies!
Quoted 7 lines
Actually, thinking about this more, do you think it makes more sense to do:
for_each_present_cpu(cpu)in this loop? That is, at boot, ensure all present (but possibly offline) CPUs have their NUMA mapping set up. CPUs that aren't present (but are possible) might trigger other warnings, right? (e.g., theWARN_ON(1)innuma_setup_cpu)
After reading the code that set up the cpu masks, I think you are right. I will send a new version with this fixed.
Thanks, Zhong
Quoted 2 lines
-Nish
PATCH v2 1/1 powerpc: Fix warning reported by verify_cpu_node_mapping()
In reply to Nishanth AravamudanWith commit 2fabf084b, during boottime, cpu_numa_callback() is called earlier(before their online) for each cpu, and verify_cpu_node_mapping() uses cpu_to_node() to check whether siblings are in the same node.
It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu numa_node cpu_to_node() uses hasn't been set up yet (which will be set up in smp_prepare_cpus()).
So I saw something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
node!
As we don't actually do the checking during this early stage, so maybe we could directly call numa_setup_cpu() in do_init_bootmem().
Also, as Nish suggested, here it's better to use present cpu mask instead of possible mask to avoid warning in numa_setup_cpu().
---
arch/powerpc/mm/numa.c +2 -3
@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)* even before we online them, so that we can use cpu_to_{node,mem} * early in boot, cf. smp_prepare_cpus(). */ -for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +for_each_present_cpu (cpu) { +numa_setup_cpu ((unsigned long )cpu); } }
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
Quoted 27 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).So I saw something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem().Also, as Nish suggested, here it's better to use present cpu mask instead of possible mask to avoid warning in
numa_setup_cpu().Signed-off-by Li Zhong <[email protected]>---
arch/powerpc/mm/numa.c +0
-0@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)* even before we online them, so that we can use cpu_to_{node,mem}
arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1134
Quoted 10 lines
arch/powerpc/mm/numa.c +2
-3* early in boot, cf. smp_prepare_cpus(). */ -for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +for_each_present_cpu (cpu) { +numa_setup_cpu ((unsigned long )cpu); } }
I am getting the following error on my system booting with this patch.
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-202712-g9e81330-dirty #42 task: c0000000fea40000 ti: c0000000fea80000 task.ti: c0000000fea80000 NIP: c0000000001afad8 LR: c000000000193b68 CTR: 0000000000000000 REGS: c0000000fea839e0 TRAP: 0300 Not tainted (3.16.0-202712-g9e81330-dirty) MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24000000 XER: 20000004 CFAR: c0000000000084d4 DAR: 0000000000001690 DSISR: 40000000 SOFTE: 1
GPR00: c000000000b6db9c c0000000fea83c60 c000000000cd0628 0000000000001688
GPR04: 0000000000000001 0000000000000000 c0000000fea83c80 0000000009900000
GPR08: c000000000d531e0 c000000000d66218 c000000000d60628 ffffffffffffffff
GPR12: ffffffffffffffff c00000000ec60000 c00000000000bc88 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 c000000000c21b88 c000000000c03738
GPR24: c000000000c03638 c000000000d24b10 c000000000c03638 c000000000c03738
GPR28: 0000000000000080 0000000000000080 c000000000d208e8 0000000000000010 NIP [c0000000001afad8] next_zones_zonelist+0x8/0xa0
LR [c000000000193b68] local_memory_node+0x38/0x60
Call Trace:
[c0000000fea83c60] [c0000000fea83c90] 0xc0000000fea83c90 (unreliable) [c0000000fea83c90] [c000000000b6db9c] smp_prepare_cpus+0x16c/0x278 [c0000000fea83d00] [c000000000b64098] kernel_init_freeable+0x150/0x340 [c0000000fea83dc0] [c00000000000bca4] kernel_init+0x24/0x140 [c0000000fea83e30] [c000000000009560] ret_from_kernel_thread+0x5c/0x7c Instruction dump:e9230038 39490f00 7fa35040 409c000c 38630780 4e800020 7d234b78 4bffff64 60000000 60420000 2c250000 40c2004c <81230008> 7f892040 419d0014 48000030
---[ end trace cb88537fdc8fa200 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
I think the loop needs to go back to initializing all possibe cpus instead of only the present cpus. We can add a check for present cpus in numa_setup_cpu() to avoid printing the WARN_ON() for cpus that are not present, something like the following...
arch/powerpc/mm/numa.c +3 -3
@@ -554,7 +554,8 @@ static int numa_setup_cpu(unsigned long lcpu) cpu =of_get_cpu_node (lcpu,NULL );if (!cpu) { -WARN_ON (1 ); +if (cpu_present (lcpu)) +WARN_ON (1 ); nid =0 ; goto out; }
@@ -1128,8 +1129,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu); } }
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
Hi Nathan,
Quoted 32 lines · Nathan Fontenot
Quoted 31 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).So I saw something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem().Also, as Nish suggested, here it's better to use present cpu mask instead of possible mask to avoid warning in
numa_setup_cpu().Signed-off-by Li Zhong <[email protected]>---
arch/powerpc/mm/numa.c +0
-2@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)* even before we online them, so that we can use cpu_to_{node,mem} * early in boot, cf. smp_prepare_cpus(). */ -for_each_possible_cpu (cpu) { - cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
Quoted 8 lines
arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1135
Quoted 6 lines
arch/powerpc/mm/numa.c +2
-1- (void *)(unsigned long)cpu); +for_each_present_cpu (cpu) { +numa_setup_cpu ((unsigned long )cpu); } }I am getting the following error on my system booting with this patch.
With the patch below, you don't get the error, I assume? Does it boot fully in that case?
-Nish
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
In reply to Nathan FontenotQuoted 66 lines · Nathan Fontenot
Quoted 27 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).So I saw something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem().Also, as Nish suggested, here it's better to use present cpu mask instead of possible mask to avoid warning in
numa_setup_cpu().Signed-off-by Li Zhong <[email protected]>---
arch/powerpc/mm/numa.c +0
-0@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)* even before we online them, so that we can use cpu_to_{node,mem} arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1134
Quoted 10 lines
arch/powerpc/mm/numa.c +2
-3* early in boot, cf. smp_prepare_cpus(). */ -for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +for_each_present_cpu (cpu) { +numa_setup_cpu ((unsigned long )cpu); } }I am getting the following error on my system booting with this patch.
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-202712-g9e81330-dirty #42 task: c0000000fea40000 ti: c0000000fea80000 task.ti: c0000000fea80000 NIP: c0000000001afad8 LR: c000000000193b68 CTR: 0000000000000000 REGS: c0000000fea839e0 TRAP: 0300 Not tainted (3.16.0-202712-g9e81330-dirty) MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24000000 XER: 20000004 CFAR: c0000000000084d4 DAR: 0000000000001690 DSISR: 40000000 SOFTE: 1GPR00:
c000000000b6db9cc0000000fea83c60c000000000cd06280000000000001688
GPR04: 0000000000000001 0000000000000000c0000000fea83c800000000009900000
GPR08:c000000000d531e0c000000000d66218c000000000d60628ffffffffffffffff
GPR12: ffffffffffffffffc00000000ec60000c00000000000bc880000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000c000000000c21b88c000000000c03738
GPR24:c000000000c03638c000000000d24b10c000000000c03638c000000000c03738
GPR28: 0000000000000080 0000000000000080c000000000d208e80000000000000010 NIP [c0000000001afad8]next_zones_zonelist+0x8/0xa0
LR [c000000000193b68]local_memory_node+0x38/0x60Call Trace:[
c0000000fea83c60] [c0000000fea83c90]0xc0000000fea83c90(unreliable) [c0000000fea83c90] [c000000000b6db9c]smp_prepare_cpus+0x16c/0x278[c0000000fea83d00] [c000000000b64098]kernel_init_freeable+0x150/0x340[c0000000fea83dc0] [c00000000000bca4]kernel_init+0x24/0x140[c0000000fea83e30] [c000000000009560]ret_from_kernel_thread+0x5c/0x7cInstruction dump:e923003839490f007fa35040409c000c386307804e8000207d234b784bffff6460000000 604200002c25000040c2004c<81230008>7f892040419d001448000030
Quoted 8 lines
---[ end trace cb88537fdc8fa200 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000bI think the loop needs to go back to initializing all possibe cpus instead of only the present cpus. We can add a check for present cpus in
numa_setup_cpu()to avoid printing theWARN_ON()for cpus that are not present, something like the following...
Ah, yes, seems the panic was caused by smp_prepare_cpus() using uninitialized numa_cpu_lookup_table for cpus which are possible but not present during boottime.
However, by following change, it seems those cpus will be set to node 0 at boottime, and not be changed after they become present, because of the following check in numa_setup_cpu():
if ((nid = numa_cpu_lookup_table[lcpu]) >= 0) {
map_cpu_to_node(lcpu, nid);
return nid;
}Maybe we could change the smp_prepare_cpus() to set numa information for present cpus instead?
And for those possible, !present cpus, we could do the setup after they are started.
Thanks, Zhong
Quoted 26 lines
arch/powerpc/mm/numa.c +3
-3@@ -554,7 +554,8 @@ static int numa_setup_cpu(unsigned long lcpu) cpu =of_get_cpu_node (lcpu,NULL );if (!cpu) { -WARN_ON (1 ); +if (cpu_present (lcpu)) +WARN_ON (1 ); nid =0 ; goto out; }@@ -1128,8 +1129,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu); } }
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
Quoted 88 lines · Li Zhong
Quoted 66 lines · Nathan Fontenot
Quoted 27 lines · Li Zhong
With commit
2fabf084b, during boottime,cpu_numa_callback()is called earlier(before their online) for each cpu, andverify_cpu_node_mapping()usescpu_to_node()to check whether siblings are in the same node.It skips the checking for siblings that are not online yet. So the only check done here is for the bootcpu, which is online at that time. But the per-cpu
numa_nodecpu_to_node()uses hasn't been set up yet (which will be set up insmp_prepare_cpus()).So I saw something like following reported:
[ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the samenode!
As we don't actually do the checking during this early stage, so maybe we could directly call
numa_setup_cpu()indo_init_bootmem().Also, as Nish suggested, here it's better to use present cpu mask instead of possible mask to avoid warning in
numa_setup_cpu().Signed-off-by Li Zhong <[email protected]>---
arch/powerpc/mm/numa.c +0
-0@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)* even before we online them, so that we can use cpu_to_{node,mem} arch/powerpc/mm/numa.c void __init do_init_bootmem(void) · line 1134
Quoted 10 lines
arch/powerpc/mm/numa.c +2
-3* early in boot, cf. smp_prepare_cpus(). */ -for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +for_each_present_cpu (cpu) { +numa_setup_cpu ((unsigned long )cpu); } }I am getting the following error on my system booting with this patch.
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-202712-g9e81330-dirty #42 task: c0000000fea40000 ti: c0000000fea80000 task.ti: c0000000fea80000 NIP: c0000000001afad8 LR: c000000000193b68 CTR: 0000000000000000 REGS: c0000000fea839e0 TRAP: 0300 Not tainted (3.16.0-202712-g9e81330-dirty) MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24000000 XER: 20000004 CFAR: c0000000000084d4 DAR: 0000000000001690 DSISR: 40000000 SOFTE: 1GPR00:
c000000000b6db9cc0000000fea83c60c000000000cd06280000000000001688
GPR04: 0000000000000001 0000000000000000c0000000fea83c800000000009900000
GPR08:c000000000d531e0c000000000d66218c000000000d60628ffffffffffffffff
GPR12: ffffffffffffffffc00000000ec60000c00000000000bc880000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000c000000000c21b88c000000000c03738
GPR24:c000000000c03638c000000000d24b10c000000000c03638c000000000c03738
GPR28: 0000000000000080 0000000000000080c000000000d208e80000000000000010 NIP [c0000000001afad8]next_zones_zonelist+0x8/0xa0
LR [c000000000193b68]local_memory_node+0x38/0x60Call Trace:[
c0000000fea83c60] [c0000000fea83c90]0xc0000000fea83c90(unreliable) [c0000000fea83c90] [c000000000b6db9c]smp_prepare_cpus+0x16c/0x278[c0000000fea83d00] [c000000000b64098]kernel_init_freeable+0x150/0x340[c0000000fea83dc0] [c00000000000bca4]kernel_init+0x24/0x140[c0000000fea83e30] [c000000000009560]ret_from_kernel_thread+0x5c/0x7cInstruction dump:e923003839490f007fa35040409c000c386307804e8000207d234b784bffff6460000000 604200002c25000040c2004c<81230008>7f892040419d001448000030Quoted 8 lines
---[ end trace cb88537fdc8fa200 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000bI think the loop needs to go back to initializing all possibe cpus instead of only the present cpus. We can add a check for present cpus in
numa_setup_cpu()to avoid printing theWARN_ON()for cpus that are not present, something like the following...Ah, yes, seems the panic was caused by
smp_prepare_cpus()using uninitializednuma_cpu_lookup_tablefor cpus which are possible but not present during boottime.However, by following change, it seems those cpus will be set to node 0 at boottime, and not be changed after they become present, because of the following check in
numa_setup_cpu():if ((nid = numa_cpu_lookup_table[lcpu]) >= 0) { map_cpu_to_node(lcpu, nid); return nid; }
Quoted 5 lines
Maybe we could change the
smp_prepare_cpus()to set numa information for present cpus instead?And for those possible, !present cpus, we could do the setup after they are started.
Hi, Nathan, Nish,
I did some draft code based on the above approach, and will send it out. Could you please help to have a review?
I split the code to separate patches, so each small patch addressed only one small issue.
Thanks, Zhong
Quoted 30 lines
Thanks, Zhong
Quoted 26 lines
arch/powerpc/mm/numa.c +3
-3@@ -554,7 +554,8 @@ static int numa_setup_cpu(unsigned long lcpu) cpu =of_get_cpu_node (lcpu,NULL );if (!cpu) { -WARN_ON (1 ); +if (cpu_present (lcpu)) +WARN_ON (1 ); nid =0 ; goto out; }@@ -1128,8 +1129,7 @@ void __init do_init_bootmem(void)* early in boot, cf. smp_prepare_cpus(). */ for_each_possible_cpu (cpu) { -cpu_numa_callback (&ppc64_numa_nb,CPU_UP_PREPARE , - (void *)(unsigned long )cpu); +numa_setup_cpu ((unsigned long )cpu); } }