[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200722080424.GF9290@linux.vnet.ibm.com>
Date: Wed, 22 Jul 2020 13:34:24 +0530
From: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To: Michael Ellerman <michaele@....ibm.com>
Cc: linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Valentin Schneider <valentin.schneider@....com>,
Nick Piggin <npiggin@....ibm.com>,
Oliver OHalloran <oliveroh@....ibm.com>,
Nathan Lynch <nathanl@...ux.ibm.com>,
Michael Neuling <mikey@...ux.ibm.com>,
Anton Blanchard <anton@....ibm.com>,
Gautham R Shenoy <ego@...ux.vnet.ibm.com>,
Vaidyanathan Srinivasan <svaidy@...ux.ibm.com>,
Jordan Niethe <jniethe5@...il.com>
Subject: Re: [PATCH v2 01/10] powerpc/smp: Cache node for reuse
* Michael Ellerman <michaele@....ibm.com> [2020-07-22 17:41:41]:
> Srikar Dronamraju <srikar@...ux.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
Powered by blists - more mailing lists