Re: ["RFC PATCH" 2/2] powerpc/mm: Conslidate numa_enable check and min_common_depth check

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

On 6/29/19 9:09 PM, Vaibhav Jain wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> 
>> Update min_common_depth = -1 if numa is disabled. This
>> help us to avoid checking for both in different code paths.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   arch/powerpc/mm/numa.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index f6d68baeaa96..c84062a390cc 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -212,7 +212,7 @@ static int associativity_to_nid(const __be32 *associativity)
>>   {
>>   	int nid = NUMA_NO_NODE;
>>   
>> -	if (min_common_depth == -1 || !numa_enabled)
>> +	if (min_common_depth == -1)
>>   		goto out;
>>   
>>   	if (of_read_number(associativity, 1) >= min_common_depth)
>> @@ -625,6 +625,7 @@ static int __init parse_numa_properties(void)
>>   
>>   	if (numa_enabled == 0) {
>>   		printk(KERN_WARNING "NUMA disabled by user\n");
>> +		min_common_depth = -1;
>>   		return -1;
>>   	}
> 
> I would prefer updating the definition of variable 'min_common_depth' to
> 
> static int min_common_depth = -1;
> 
> This would handle the case where someone calls 'associativity_to_nid()' and
> other functions that read 'min_common_depth' and get an invalid result
> back. And also handle the case where kernel is booted with 'numa = off'.
> 

Sure. As mentioned in another email, I am wondering whether all that 
min_common_depth check should be if !numa_enabled. That makes it much 
easy to read. I will respin once i get more clarity on 
of_drconf_to_nid_single usage.

> Also the init value 'min_common_depth == 0' indicates that the
> first word in "ibm,associativity" array represents the node-id which is
> wrong. Instead its the length of the "ibm,associativity" array.
> 

-aneesh