Re: [PATCH v2 01/10] powerpc/smp: Cache node for reuse

Srikar Dronamraju
2020-07-22T08:04:49Z

* Michael Ellerman <michaele@au1.ibm.com> [2020-07-22 17:41:41]:

> Srikar Dronamraju <srikar@linux.vnet.ibm.com> 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