Subject: Re: [RFC, 1/5] powerpc:numa Add numa_cpu_lookup function to update lookup table

From: Raghavendra K T <[email-masked]>
Date: Tue, 06 Oct 2015 16:03:34 +0530

On 10/06/2015 03:47 PM, Michael Ellerman wrote:
> On Sun, 2015-27-09 at 18:29:09 UTC, Raghavendra K T wrote:
>> We access numa_cpu_lookup_table array directly in all the places
>> to read/update numa cpu lookup information. Instead use a helper
>> function to update.
>>
>> This is helpful in changing the way numa<-->cpu mapping in single
>> place when needed.
>>
>> This is a cosmetic change, no change in functionality.
>>
>> Signed-off-by: Raghavendra K T <[email-masked]>
>> Signed-off-by: Raghavendra K T <[email-masked]>
>> ---
>>   arch/powerpc/include/asm/mmzone.h |  2 +-
>>   arch/powerpc/kernel/smp.c         | 10 +++++-----
>>   arch/powerpc/mm/numa.c            | 28 +++++++++++++++++-----------
>>   3 files changed, 23 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mmzone.h b/arch/powerpc/include/asm/mmzone.h
>> index 7b58917..c24a5f4 100644
>> --- a/arch/powerpc/include/asm/mmzone.h
>> +++ b/arch/powerpc/include/asm/mmzone.h
>> @@ -29,7 +29,7 @@ extern struct pglist_data *node_data[];
>>    * Following are specific to this numa platform.
>>    */
>>
>> -extern int numa_cpu_lookup_table[];
>> +extern int numa_cpu_lookup(int cpu);
>
> Can you rename it better :)
>
> Something like cpu_to_nid().

Good name. sure.

>
> Although maybe nid is wrong given the rest of the series.

May be not. The current plan is to rename (after discussing with Nish)

chipid to pnid (physical nid)
and nid to vnid (virtual nid)

within powerpc numa.c
[reasoning chipid is applicable only to OPAL, since we want to handle
powerkvm, powervm and baremetal we need a generic name ]

But 'nid' naming will be retained which is applicable for generic
kernel interactions.

>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 8b9502a..d5e6eee 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -52,7 +52,6 @@ int numa_cpu_lookup_table[NR_CPUS];
>>   cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
>>   struct pglist_data *node_data[MAX_NUMNODES];
>>
>> -EXPORT_SYMBOL(numa_cpu_lookup_table);
>>   EXPORT_SYMBOL(node_to_cpumask_map);
>>   EXPORT_SYMBOL(node_data);
>>
>> @@ -134,19 +133,25 @@ static int __init fake_numa_create_new_node(unsigned long end_pfn,
>>   	return 0;
>>   }
>>
>> -static void reset_numa_cpu_lookup_table(void)
>> +int numa_cpu_lookup(int cpu)
>>   {
>> -	unsigned int cpu;
>> -
>> -	for_each_possible_cpu(cpu)
>> -		numa_cpu_lookup_table[cpu] = -1;
>> +	return numa_cpu_lookup_table[cpu];
>>   }
>> +EXPORT_SYMBOL(numa_cpu_lookup);
>
> I don't see you changing any modular code that uses this, or any macros that
> might be used by modules, so I don't see why this needs to be exported?
>
> I think you just added it because num_cpu_lookup_table was exported?
>

arch/powerpc/kernel/smp.c uses it.