Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
On 三, 2014-08-27 at 09:41 +0800, Li Zhong wrote:
> On 二, 2014-08-26 at 08:10 -0500, Nathan Fontenot wrote:
> > On 08/25/2014 02:22 AM, Li Zhong wrote:
> > > 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 saw something like following reported:
> > > [0.00] 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().
> > >
> > > Signed-off-by: Li Zhong
> > > ---
> > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > > index d7737a5..3a9061e 100644
> > > --- a/arch/powerpc/mm/numa.c
> > > +++ b/arch/powerpc/mm/numa.c
> > > @@ -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);
> > > }
> > > }
> > >
> >
> > 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: c000fea4 ti: c000fea8 task.ti: c000fea8
> > NIP: c01afad8 LR: c0193b68 CTR:
> > REGS: c000fea839e0 TRAP: 0300 Not tainted
> > (3.16.0-202712-g9e81330-dirty)
> > MSR: 80019033 CR: 2400 XER: 2004
> > CFAR: c00084d4 DAR: 1690 DSISR: 4000 SOFTE: 1
> > GPR00: c0b6db9c c000fea83c60 c0cd0628 1688
> > GPR04: 0001 c000fea83c80 0990
> > GPR08: c0d531e0 c0d66218 c0d60628
> > GPR12: cec6 c000bc88
> > GPR16:
> > GPR20: c0c21b88 c0c03738
> > GPR24: c0c03638 c0d24b10 c0c03638 c0c03738
> > GPR28: 0080 0080 c0d208e8 0010
> > NIP [c01afad8] next_zones_zonelist+0x8/0xa0
> > LR [c0193b68] local_memory_node+0x38/0x60
> > Call Trace:
> > [c000fea83c60] [c000fea83c90] 0xc000fea83c90 (unreliable)
> > [c000fea83c90] [c0b6db9c] smp_prepare_cpus+0x16c/0x278
> > [c000fea83d00] [c0b64098] kernel_init_freeable+0x150/0x340
> > [c000fea83dc0] [c000bca4] kernel_init+0x24/0x140
> > [c000fea83e30] [c0009560] ret_from_kernel_thread+0x5c/0x7c
> > Instruction dump:
> > e9230038 39490f00 7fa35040 409c000c 38630780 4e800020 7d234b78 4b64
> > 6000 6042 2c25 40c2004c <81230008> 7f892040 419d0014 4830
> > ---[ end trace cb88537fdc8fa200 ]---
> >
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
> >
> > 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...
>
> 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.
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
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
On 二, 2014-08-26 at 08:10 -0500, Nathan Fontenot wrote:
> On 08/25/2014 02:22 AM, Li Zhong wrote:
> > 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 saw something like following reported:
> > [0.00] 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().
> >
> > Signed-off-by: Li Zhong
> > ---
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index d7737a5..3a9061e 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -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);
> > }
> > }
> >
>
> 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: c000fea4 ti: c000fea8 task.ti: c000fea8
> NIP: c01afad8 LR: c0193b68 CTR:
> REGS: c000fea839e0 TRAP: 0300 Not tainted
> (3.16.0-202712-g9e81330-dirty)
> MSR: 80019033 CR: 2400 XER: 2004
> CFAR: c00084d4 DAR: 1690 DSISR: 4000 SOFTE: 1
> GPR00: c0b6db9c c000fea83c60 c0cd0628 1688
> GPR04: 0001 c000fea83c80 0990
> GPR08: c0d531e0 c0d66218 c0d60628
> GPR12: cec6 c000bc88
> GPR16:
> GPR20: c0c21b88 c0c03738
> GPR24: c0c03638 c0d24b10 c0c03638 c0c03738
> GPR28: 0080 0080 c0d208e8 0010
> NIP [c01afad8] next_zones_zonelist+0x8/0xa0
> LR [c0193b68] local_memory_node+0x38/0x60
> Call Trace:
> [c000fea83c60] [c000fea83c90] 0xc000fea83c90 (unreliable)
> [c000fea83c90] [c0b6db9c] smp_prepare_cpus+0x16c/0x278
> [c000fea83d00] [c0b64098] kernel_init_freeable+0x150/0x340
> [c000fea83dc0] [c000bca4] kernel_init+0x24/0x140
> [c000fea83e30] [c0009560] ret_from_kernel_thread+0x5c/0x7c
> Instruction dump:
> e9230038 39490f00 7fa35040 409c000c 38630780 4e800020 7d234b78 4b64
> 6000 6042 2c25 40c2004c <81230008> 7f892040 419d0014 4830
> ---[ end trace cb88537fdc8fa200 ]---
>
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
>
> 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...
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
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index d7737a5..b827f2e 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -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;
> }
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
Hi Nathan,
On 26.08.2014 [08:10:14 -0500], Nathan Fontenot wrote:
> On 08/25/2014 02:22 AM, Li Zhong wrote:
> > 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 saw something like following reported:
> > [0.00] 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().
> >
> > Signed-off-by: Li Zhong
> > ---
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index d7737a5..3a9061e 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -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);
> > }
> > }
> >
>
> 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
On 08/25/2014 02:22 AM, Li Zhong wrote:
> 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 saw something like following reported:
> [0.00] 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().
>
> Signed-off-by: Li Zhong
> ---
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index d7737a5..3a9061e 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -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);
> }
> }
>
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: c000fea4 ti: c000fea8 task.ti: c000fea8
NIP: c01afad8 LR: c0193b68 CTR:
REGS: c000fea839e0 TRAP: 0300 Not tainted (3.16.0-202712-g9e81330-dirty)
MSR: 80019033 CR: 2400 XER: 2004
CFAR: c00084d4 DAR: 1690 DSISR: 4000 SOFTE: 1
GPR00: c0b6db9c c000fea83c60 c0cd0628 1688
GPR04: 0001 c000fea83c80 0990
GPR08: c0d531e0 c0d66218 c0d60628
GPR12: cec6 c000bc88
GPR16:
GPR20: c0c21b88 c0c03738
GPR24: c0c03638 c0d24b10 c0c03638 c0c03738
GPR28: 0080 0080 c0d208e8 0010
NIP [c01afad8] next_zones_zonelist+0x8/0xa0
LR [c0193b68] local_memory_node+0x38/0x60
Call Trace:
[c000fea83c60] [c000fea83c90] 0xc000fea83c90 (unreliable)
[c000fea83c90] [c0b6db9c] smp_prepare_cpus+0x16c/0x278
[c000fea83d00] [c0b64098] kernel_init_freeable+0x150/0x340
[c000fea83dc0] [c000bca4] kernel_init+0x24/0x140
[c000fea83e30] [c0009560] ret_from_kernel_thread+0x5c/0x7c
Instruction dump:
e9230038 39490f00 7fa35040 409c000c 38630780 4e800020 7d234b78 4b64
6000 6042 2c25 40c2004c <81230008> 7f892040 419d0014 4830
---[ end trace cb88537fdc8fa200 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
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...
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index d7737a5..b827f2e 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -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);
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powerpc: 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 saw something like following reported:
[0.00] 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().
Signed-off-by: Li Zhong
---
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index d7737a5..3a9061e 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -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);
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
