While cpu_to_node is inline function with access to per_cpu variable.
However when using repeatedly, it may be cleaner to cache it in a local
variable.
Also fix a build error in a some weird config.
"error: _numa_cpu_lookup_table_ undeclared"
No functional change
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]>
Reviewed-by: Gautham R. Shenoy <[email protected]>
Signed-off-by: Srikar Dronamraju <[email protected]>
---
arch/powerpc/kernel/smp.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 73199470c265..680c0edcc59d 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -843,7 +843,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
DBG("smp_prepare_cpus\n");
- /*
+ /*
* setup_cpu may need to be called on the boot cpu. We havent
* spun any cpus up but lets be paranoid.
*/
@@ -854,20 +854,24 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
cpu_callin_map[boot_cpuid] = 1;
for_each_possible_cpu(cpu) {
+ int node = cpu_to_node(cpu);
+
zalloc_cpumask_var_node(&per_cpu(cpu_sibling_map, cpu),
- GFP_KERNEL, cpu_to_node(cpu));
+ GFP_KERNEL, node);
zalloc_cpumask_var_node(&per_cpu(cpu_l2_cache_map, cpu),
- GFP_KERNEL, cpu_to_node(cpu));
+ GFP_KERNEL, node);
zalloc_cpumask_var_node(&per_cpu(cpu_core_map, cpu),
- GFP_KERNEL, cpu_to_node(cpu));
+ GFP_KERNEL, node);
+#ifdef CONFIG_NEED_MULTIPLE_NODES
/*
* numa_node_id() works after this.
*/
if (cpu_present(cpu)) {
- set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
- set_cpu_numa_mem(cpu,
- local_memory_node(numa_cpu_lookup_table[cpu]));
+ node = numa_cpu_lookup_table[cpu];
+ set_cpu_numa_node(cpu, node);
+ set_cpu_numa_mem(cpu, local_memory_node(node));
}
+#endif
}
/* Init the cpumasks so the boot CPU is related to itself */
--
2.17.1
Srikar Dronamraju <[email protected]> writes:
> While cpu_to_node is inline function with access to per_cpu variable.
> However when using repeatedly, it may be cleaner to cache it in a local
> variable.
It's not clear what "cleaner" is supposed to mean. Are you saying it
makes the source clearer, or the generated code?
I'm not sure it will make any difference to the latter.
> Also fix a build error in a some weird config.
> "error: _numa_cpu_lookup_table_ undeclared"
Separate patch please.
> No functional change
The ifdef change means that's not true.
> 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]>
> Reviewed-by: Gautham R. Shenoy <[email protected]>
> Signed-off-by: Srikar Dronamraju <[email protected]>
> ---
> arch/powerpc/kernel/smp.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 73199470c265..680c0edcc59d 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -843,7 +843,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>
> DBG("smp_prepare_cpus\n");
>
> - /*
> + /*
> * setup_cpu may need to be called on the boot cpu. We havent
> * spun any cpus up but lets be paranoid.
> */
> @@ -854,20 +854,24 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> cpu_callin_map[boot_cpuid] = 1;
>
> for_each_possible_cpu(cpu) {
> + int node = cpu_to_node(cpu);
> +
Does cpu_to_node() even work here?
Doesn't look like it to me.
More fallout from 8c272261194d ("powerpc/numa: Enable USE_PERCPU_NUMA_NODE_ID") ?
> zalloc_cpumask_var_node(&per_cpu(cpu_sibling_map, cpu),
> - GFP_KERNEL, cpu_to_node(cpu));
> + GFP_KERNEL, node);
> zalloc_cpumask_var_node(&per_cpu(cpu_l2_cache_map, cpu),
> - GFP_KERNEL, cpu_to_node(cpu));
> + GFP_KERNEL, node);
> zalloc_cpumask_var_node(&per_cpu(cpu_core_map, cpu),
> - GFP_KERNEL, cpu_to_node(cpu));
> + GFP_KERNEL, node);
> +#ifdef CONFIG_NEED_MULTIPLE_NODES
> /*
> * numa_node_id() works after this.
> */
> if (cpu_present(cpu)) {
> - set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
> - set_cpu_numa_mem(cpu,
> - local_memory_node(numa_cpu_lookup_table[cpu]));
> + node = numa_cpu_lookup_table[cpu];
> + set_cpu_numa_node(cpu, node);
> + set_cpu_numa_mem(cpu, local_memory_node(node));
> }
> +#endif
> }
>
> /* Init the cpumasks so the boot CPU is related to itself */
> --
> 2.17.1
cheers
* Michael Ellerman <[email protected]> [2020-07-22 17:41:41]:
> Srikar Dronamraju <[email protected]> writes:
> > While cpu_to_node is inline function with access to per_cpu variable.
> > However when using repeatedly, it may be cleaner to cache it in a local
> > variable.
>
> It's not clear what "cleaner" is supposed to mean. Are you saying it
> makes the source clearer, or the generated code?
>
> I'm not sure it will make any difference to the latter.
I meant the source code, I am okay dropping the hunks that try to cache
cpu_to_node.
>
> > Also fix a build error in a some weird config.
> > "error: _numa_cpu_lookup_table_ undeclared"
>
> Separate patch please.
Okay, will do.
>
> > No functional change
>
> The ifdef change means that's not true.
Okay
> > @@ -854,20 +854,24 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > cpu_callin_map[boot_cpuid] = 1;
> >
> > for_each_possible_cpu(cpu) {
> > + int node = cpu_to_node(cpu);
> > +
>
> Does cpu_to_node() even work here?
Except in the case where NUMA is not enabled, (when cpu_to_node would return
-1), It should work here since numa initialization would have happened by
now. It cpu_to_node(cpu) should work once numa_setup_cpu() /
map_cpu_to_node() gets called. And those are being called before this.
>
> Doesn't look like it to me.
>
> More fallout from 8c272261194d ("powerpc/numa: Enable USE_PERCPU_NUMA_NODE_ID") ?
>
> > }
> >
> > /* Init the cpumasks so the boot CPU is related to itself */
--
Thanks and Regards
Srikar Dronamraju