diff mbox series

[v5,1/6] powerpc/pseries: rename min_common_depth to primary_domain_index

Message ID 20210628151117.545935-2-aneesh.kumar@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series Add support for FORM2 associativity | expand
Related show

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (0f7a719601eb957c10d417c62bd5f65080b5a409)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 0 warnings, 1 checks, 119 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Aneesh Kumar K V June 28, 2021, 3:11 p.m. UTC 
No functional change in this patch.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/mm/numa.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

David Gibson July 22, 2021, 1:59 a.m. UTC | #1 
On Mon, Jun 28, 2021 at 08:41:12PM +0530, Aneesh Kumar K.V wrote:
> No functional change in this patch.

The new name does not match how you describe "primary domain index" in
the documentation from patch 6/6.  There it comes from the values in
associativity-reference-points, but here it simply comes from the
lengths of all the associativity properties.

> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  arch/powerpc/mm/numa.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index f2bf98bdcea2..8365b298ec48 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -51,7 +51,7 @@ EXPORT_SYMBOL(numa_cpu_lookup_table);
>  EXPORT_SYMBOL(node_to_cpumask_map);
>  EXPORT_SYMBOL(node_data);
>  
> -static int min_common_depth;
> +static int primary_domain_index;
>  static int n_mem_addr_cells, n_mem_size_cells;
>  static int form1_affinity;
>  
> @@ -232,8 +232,8 @@ static int associativity_to_nid(const __be32 *associativity)
>  	if (!numa_enabled)
>  		goto out;
>  
> -	if (of_read_number(associativity, 1) >= min_common_depth)
> -		nid = of_read_number(&associativity[min_common_depth], 1);
> +	if (of_read_number(associativity, 1) >= primary_domain_index)
> +		nid = of_read_number(&associativity[primary_domain_index], 1);
>  
>  	/* POWER4 LPAR uses 0xffff as invalid node */
>  	if (nid == 0xffff || nid >= nr_node_ids)
> @@ -284,9 +284,9 @@ int of_node_to_nid(struct device_node *device)
>  }
>  EXPORT_SYMBOL(of_node_to_nid);
>  
> -static int __init find_min_common_depth(void)
> +static int __init find_primary_domain_index(void)
>  {
> -	int depth;
> +	int index;
>  	struct device_node *root;
>  
>  	if (firmware_has_feature(FW_FEATURE_OPAL))
> @@ -326,7 +326,7 @@ static int __init find_min_common_depth(void)
>  	}
>  
>  	if (form1_affinity) {
> -		depth = of_read_number(distance_ref_points, 1);
> +		index = of_read_number(distance_ref_points, 1);
>  	} else {
>  		if (distance_ref_points_depth < 2) {
>  			printk(KERN_WARNING "NUMA: "
> @@ -334,7 +334,7 @@ static int __init find_min_common_depth(void)
>  			goto err;
>  		}
>  
> -		depth = of_read_number(&distance_ref_points[1], 1);
> +		index = of_read_number(&distance_ref_points[1], 1);
>  	}
>  
>  	/*
> @@ -348,7 +348,7 @@ static int __init find_min_common_depth(void)
>  	}
>  
>  	of_node_put(root);
> -	return depth;
> +	return index;
>  
>  err:
>  	of_node_put(root);
> @@ -437,16 +437,16 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
>  	int nid = default_nid;
>  	int rc, index;
>  
> -	if ((min_common_depth < 0) || !numa_enabled)
> +	if ((primary_domain_index < 0) || !numa_enabled)
>  		return default_nid;
>  
>  	rc = of_get_assoc_arrays(&aa);
>  	if (rc)
>  		return default_nid;
>  
> -	if (min_common_depth <= aa.array_sz &&
> +	if (primary_domain_index <= aa.array_sz &&
>  	    !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
> -		index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
> +		index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
>  		nid = of_read_number(&aa.arrays[index], 1);
>  
>  		if (nid == 0xffff || nid >= nr_node_ids)
> @@ -708,18 +708,18 @@ static int __init parse_numa_properties(void)
>  		return -1;
>  	}
>  
> -	min_common_depth = find_min_common_depth();
> +	primary_domain_index = find_primary_domain_index();
>  
> -	if (min_common_depth < 0) {
> +	if (primary_domain_index < 0) {
>  		/*
> -		 * if we fail to parse min_common_depth from device tree
> +		 * if we fail to parse primary_domain_index from device tree
>  		 * mark the numa disabled, boot with numa disabled.
>  		 */
>  		numa_enabled = false;
> -		return min_common_depth;
> +		return primary_domain_index;
>  	}
>  
> -	dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
> +	dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
>  
>  	/*
>  	 * Even though we connect cpus to numa domains later in SMP
> @@ -919,14 +919,14 @@ static void __init find_possible_nodes(void)
>  			goto out;
>  	}
>  
> -	max_nodes = of_read_number(&domains[min_common_depth], 1);
> +	max_nodes = of_read_number(&domains[primary_domain_index], 1);
>  	for (i = 0; i < max_nodes; i++) {
>  		if (!node_possible(i))
>  			node_set(i, node_possible_map);
>  	}
>  
>  	prop_length /= sizeof(int);
> -	if (prop_length > min_common_depth + 2)
> +	if (prop_length > primary_domain_index + 2)
>  		coregroup_enabled = 1;
>  
>  out:
> @@ -1259,7 +1259,7 @@ int cpu_to_coregroup_id(int cpu)
>  		goto out;
>  
>  	index = of_read_number(associativity, 1);
> -	if (index > min_common_depth + 1)
> +	if (index > primary_domain_index + 1)
>  		return of_read_number(&associativity[index - 1], 1);
>  
>  out:
David Gibson July 22, 2021, 2:36 a.m. UTC | #2 
On Thu, Jul 22, 2021 at 11:59:15AM +1000, David Gibson wrote:
> On Mon, Jun 28, 2021 at 08:41:12PM +0530, Aneesh Kumar K.V wrote:
> > No functional change in this patch.
> 
> The new name does not match how you describe "primary domain index" in
> the documentation from patch 6/6.  There it comes from the values in
> associativity-reference-points, but here it simply comes from the
> lengths of all the associativity properties.

No, sorry, I misread this code... misled by the old name, so it's a
good thing you're changing it.

But.. I'm still not sure the new name is accurate, either...

[snip]
> >  	if (form1_affinity) {
> > -		depth = of_read_number(distance_ref_points, 1);
> > +		index = of_read_number(distance_ref_points, 1);

AFACIT distance_ref_points hasn't been altered from the
of_get_property() at this point, so isn't this setting depth / index
to the number of entries in ref-points, rather than the value of the
first entry (which is what primary domain index is supposed to be).

> >  	} else {
> >  		if (distance_ref_points_depth < 2) {
> >  			printk(KERN_WARNING "NUMA: "
> > @@ -334,7 +334,7 @@ static int __init find_min_common_depth(void)
> >  			goto err;
> >  		}
> >  
> > -		depth = of_read_number(&distance_ref_points[1], 1);
> > +		index = of_read_number(&distance_ref_points[1], 1);
> >  	}
> >  
> >  	/*
> > @@ -348,7 +348,7 @@ static int __init find_min_common_depth(void)
> >  	}
> >  
> >  	of_node_put(root);
> > -	return depth;
> > +	return index;
> >  
> >  err:
> >  	of_node_put(root);
> > @@ -437,16 +437,16 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> >  	int nid = default_nid;
> >  	int rc, index;
> >  
> > -	if ((min_common_depth < 0) || !numa_enabled)
> > +	if ((primary_domain_index < 0) || !numa_enabled)
> >  		return default_nid;
> >  
> >  	rc = of_get_assoc_arrays(&aa);
> >  	if (rc)
> >  		return default_nid;
> >  
> > -	if (min_common_depth <= aa.array_sz &&
> > +	if (primary_domain_index <= aa.array_sz &&
> >  	    !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
> > -		index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
> > +		index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
> >  		nid = of_read_number(&aa.arrays[index], 1);
> >  
> >  		if (nid == 0xffff || nid >= nr_node_ids)
> > @@ -708,18 +708,18 @@ static int __init parse_numa_properties(void)
> >  		return -1;
> >  	}
> >  
> > -	min_common_depth = find_min_common_depth();
> > +	primary_domain_index = find_primary_domain_index();
> >  
> > -	if (min_common_depth < 0) {
> > +	if (primary_domain_index < 0) {
> >  		/*
> > -		 * if we fail to parse min_common_depth from device tree
> > +		 * if we fail to parse primary_domain_index from device tree
> >  		 * mark the numa disabled, boot with numa disabled.
> >  		 */
> >  		numa_enabled = false;
> > -		return min_common_depth;
> > +		return primary_domain_index;
> >  	}
> >  
> > -	dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
> > +	dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
> >  
> >  	/*
> >  	 * Even though we connect cpus to numa domains later in SMP
> > @@ -919,14 +919,14 @@ static void __init find_possible_nodes(void)
> >  			goto out;
> >  	}
> >  
> > -	max_nodes = of_read_number(&domains[min_common_depth], 1);
> > +	max_nodes = of_read_number(&domains[primary_domain_index], 1);
> >  	for (i = 0; i < max_nodes; i++) {
> >  		if (!node_possible(i))
> >  			node_set(i, node_possible_map);
> >  	}
> >  
> >  	prop_length /= sizeof(int);
> > -	if (prop_length > min_common_depth + 2)
> > +	if (prop_length > primary_domain_index + 2)
> >  		coregroup_enabled = 1;
> >  
> >  out:
> > @@ -1259,7 +1259,7 @@ int cpu_to_coregroup_id(int cpu)
> >  		goto out;
> >  
> >  	index = of_read_number(associativity, 1);
> > -	if (index > min_common_depth + 1)
> > +	if (index > primary_domain_index + 1)
> >  		return of_read_number(&associativity[index - 1], 1);
> >  
> >  out:
>
Aneesh Kumar K V July 22, 2021, 5:17 a.m. UTC | #3 
On 7/22/21 8:06 AM, David Gibson wrote:
> On Thu, Jul 22, 2021 at 11:59:15AM +1000, David Gibson wrote:
>> On Mon, Jun 28, 2021 at 08:41:12PM +0530, Aneesh Kumar K.V wrote:
>>> No functional change in this patch.
>>
>> The new name does not match how you describe "primary domain index" in
>> the documentation from patch 6/6.  There it comes from the values in
>> associativity-reference-points, but here it simply comes from the
>> lengths of all the associativity properties.
> 
> No, sorry, I misread this code... misled by the old name, so it's a
> good thing you're changing it.
> 
> But.. I'm still not sure the new name is accurate, either...
> 
> [snip]
>>>   	if (form1_affinity) {
>>> -		depth = of_read_number(distance_ref_points, 1);
>>> +		index = of_read_number(distance_ref_points, 1);
> 
> AFACIT distance_ref_points hasn't been altered from the
> of_get_property() at this point, so isn't this setting depth / index
> to the number of entries in ref-points, rather than the value of the
> first entry (which is what primary domain index is supposed to be).
> 

ibm,associativity-reference-points property format is as below.

# lsprop  ibm,associativity-reference-points
ibm,associativity-reference-points
                  00000004 00000002

it doesn't have the number of elements as the first item.

For FORM1 1 element is the NUMA boundary index/primary_domain_index
For FORM0 2 element is the NUMA boundary index/primary_domain_index.


>>>   	} else {
>>>   		if (distance_ref_points_depth < 2) {
>>>   			printk(KERN_WARNING "NUMA: "
>>> @@ -334,7 +334,7 @@ static int __init find_min_common_depth(void)
>>>   			goto err;
>>>   		}
>>>   
>>> -		depth = of_read_number(&distance_ref_points[1], 1);
>>> +		index = of_read_number(&distance_ref_points[1], 1);
>>>   	}
>>>   
>>>   	/*
>>> @@ -348,7 +348,7 @@ static int __init find_min_common_depth(void)
>>>   	}
>>>   
>>>   	of_node_put(root);
>>> -	return depth;
>>> +	return index;
>>>   
>>>   err:
>>>   	of_node_put(root);
>>> @@ -437,16 +437,16 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
>>>   	int nid = default_nid;
>>>   	int rc, index;
>>>   
>>> -	if ((min_common_depth < 0) || !numa_enabled)
>>> +	if ((primary_domain_index < 0) || !numa_enabled)
>>>   		return default_nid;
>>>   
>>>   	rc = of_get_assoc_arrays(&aa);
>>>   	if (rc)
>>>   		return default_nid;
>>>   
>>> -	if (min_common_depth <= aa.array_sz &&
>>> +	if (primary_domain_index <= aa.array_sz &&
>>>   	    !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
>>> -		index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
>>> +		index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
>>>   		nid = of_read_number(&aa.arrays[index], 1);
>>>   
>>>   		if (nid == 0xffff || nid >= nr_node_ids)
>>> @@ -708,18 +708,18 @@ static int __init parse_numa_properties(void)
>>>   		return -1;
>>>   	}
>>>   
>>> -	min_common_depth = find_min_common_depth();
>>> +	primary_domain_index = find_primary_domain_index();
>>>   
>>> -	if (min_common_depth < 0) {
>>> +	if (primary_domain_index < 0) {
>>>   		/*
>>> -		 * if we fail to parse min_common_depth from device tree
>>> +		 * if we fail to parse primary_domain_index from device tree
>>>   		 * mark the numa disabled, boot with numa disabled.
>>>   		 */
>>>   		numa_enabled = false;
>>> -		return min_common_depth;
>>> +		return primary_domain_index;
>>>   	}
>>>   
>>> -	dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
>>> +	dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
>>>   
>>>   	/*
>>>   	 * Even though we connect cpus to numa domains later in SMP
>>> @@ -919,14 +919,14 @@ static void __init find_possible_nodes(void)
>>>   			goto out;
>>>   	}
>>>   
>>> -	max_nodes = of_read_number(&domains[min_common_depth], 1);
>>> +	max_nodes = of_read_number(&domains[primary_domain_index], 1);
>>>   	for (i = 0; i < max_nodes; i++) {
>>>   		if (!node_possible(i))
>>>   			node_set(i, node_possible_map);
>>>   	}
>>>   
>>>   	prop_length /= sizeof(int);
>>> -	if (prop_length > min_common_depth + 2)
>>> +	if (prop_length > primary_domain_index + 2)
>>>   		coregroup_enabled = 1;
>>>   
>>>   out:
>>> @@ -1259,7 +1259,7 @@ int cpu_to_coregroup_id(int cpu)
>>>   		goto out;
>>>   
>>>   	index = of_read_number(associativity, 1);
>>> -	if (index > min_common_depth + 1)
>>> +	if (index > primary_domain_index + 1)
>>>   		return of_read_number(&associativity[index - 1], 1);
>>>   
>>>   out:
>>
> 
> 
>
David Gibson July 26, 2021, 2:28 a.m. UTC | #4 
On Thu, Jul 22, 2021 at 10:47:49AM +0530, Aneesh Kumar K.V wrote:
> On 7/22/21 8:06 AM, David Gibson wrote:
> > On Thu, Jul 22, 2021 at 11:59:15AM +1000, David Gibson wrote:
> > > On Mon, Jun 28, 2021 at 08:41:12PM +0530, Aneesh Kumar K.V wrote:
> > > > No functional change in this patch.
> > > 
> > > The new name does not match how you describe "primary domain index" in
> > > the documentation from patch 6/6.  There it comes from the values in
> > > associativity-reference-points, but here it simply comes from the
> > > lengths of all the associativity properties.
> > 
> > No, sorry, I misread this code... misled by the old name, so it's a
> > good thing you're changing it.
> > 
> > But.. I'm still not sure the new name is accurate, either...
> > 
> > [snip]
> > > >   	if (form1_affinity) {
> > > > -		depth = of_read_number(distance_ref_points, 1);
> > > > +		index = of_read_number(distance_ref_points, 1);
> > 
> > AFACIT distance_ref_points hasn't been altered from the
> > of_get_property() at this point, so isn't this setting depth / index
> > to the number of entries in ref-points, rather than the value of the
> > first entry (which is what primary domain index is supposed to be).
> > 
> 
> ibm,associativity-reference-points property format is as below.
> 
> # lsprop  ibm,associativity-reference-points
> ibm,associativity-reference-points
>                  00000004 00000002
> 
> it doesn't have the number of elements as the first item.
> 
> For FORM1 1 element is the NUMA boundary index/primary_domain_index
> For FORM0 2 element is the NUMA boundary index/primary_domain_index.

Sorry, my bad.  I foolishly expected consistency from PAPR.
diff mbox series

Patch

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index f2bf98bdcea2..8365b298ec48 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -51,7 +51,7 @@  EXPORT_SYMBOL(numa_cpu_lookup_table);
 EXPORT_SYMBOL(node_to_cpumask_map);
 EXPORT_SYMBOL(node_data);
 
-static int min_common_depth;
+static int primary_domain_index;
 static int n_mem_addr_cells, n_mem_size_cells;
 static int form1_affinity;
 
@@ -232,8 +232,8 @@  static int associativity_to_nid(const __be32 *associativity)
 	if (!numa_enabled)
 		goto out;
 
-	if (of_read_number(associativity, 1) >= min_common_depth)
-		nid = of_read_number(&associativity[min_common_depth], 1);
+	if (of_read_number(associativity, 1) >= primary_domain_index)
+		nid = of_read_number(&associativity[primary_domain_index], 1);
 
 	/* POWER4 LPAR uses 0xffff as invalid node */
 	if (nid == 0xffff || nid >= nr_node_ids)
@@ -284,9 +284,9 @@  int of_node_to_nid(struct device_node *device)
 }
 EXPORT_SYMBOL(of_node_to_nid);
 
-static int __init find_min_common_depth(void)
+static int __init find_primary_domain_index(void)
 {
-	int depth;
+	int index;
 	struct device_node *root;
 
 	if (firmware_has_feature(FW_FEATURE_OPAL))
@@ -326,7 +326,7 @@  static int __init find_min_common_depth(void)
 	}
 
 	if (form1_affinity) {
-		depth = of_read_number(distance_ref_points, 1);
+		index = of_read_number(distance_ref_points, 1);
 	} else {
 		if (distance_ref_points_depth < 2) {
 			printk(KERN_WARNING "NUMA: "
@@ -334,7 +334,7 @@  static int __init find_min_common_depth(void)
 			goto err;
 		}
 
-		depth = of_read_number(&distance_ref_points[1], 1);
+		index = of_read_number(&distance_ref_points[1], 1);
 	}
 
 	/*
@@ -348,7 +348,7 @@  static int __init find_min_common_depth(void)
 	}
 
 	of_node_put(root);
-	return depth;
+	return index;
 
 err:
 	of_node_put(root);
@@ -437,16 +437,16 @@  int of_drconf_to_nid_single(struct drmem_lmb *lmb)
 	int nid = default_nid;
 	int rc, index;
 
-	if ((min_common_depth < 0) || !numa_enabled)
+	if ((primary_domain_index < 0) || !numa_enabled)
 		return default_nid;
 
 	rc = of_get_assoc_arrays(&aa);
 	if (rc)
 		return default_nid;
 
-	if (min_common_depth <= aa.array_sz &&
+	if (primary_domain_index <= aa.array_sz &&
 	    !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
-		index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
+		index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
 		nid = of_read_number(&aa.arrays[index], 1);
 
 		if (nid == 0xffff || nid >= nr_node_ids)
@@ -708,18 +708,18 @@  static int __init parse_numa_properties(void)
 		return -1;
 	}
 
-	min_common_depth = find_min_common_depth();
+	primary_domain_index = find_primary_domain_index();
 
-	if (min_common_depth < 0) {
+	if (primary_domain_index < 0) {
 		/*
-		 * if we fail to parse min_common_depth from device tree
+		 * if we fail to parse primary_domain_index from device tree
 		 * mark the numa disabled, boot with numa disabled.
 		 */
 		numa_enabled = false;
-		return min_common_depth;
+		return primary_domain_index;
 	}
 
-	dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
+	dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
 
 	/*
 	 * Even though we connect cpus to numa domains later in SMP
@@ -919,14 +919,14 @@  static void __init find_possible_nodes(void)
 			goto out;
 	}
 
-	max_nodes = of_read_number(&domains[min_common_depth], 1);
+	max_nodes = of_read_number(&domains[primary_domain_index], 1);
 	for (i = 0; i < max_nodes; i++) {
 		if (!node_possible(i))
 			node_set(i, node_possible_map);
 	}
 
 	prop_length /= sizeof(int);
-	if (prop_length > min_common_depth + 2)
+	if (prop_length > primary_domain_index + 2)
 		coregroup_enabled = 1;
 
 out:
@@ -1259,7 +1259,7 @@  int cpu_to_coregroup_id(int cpu)
 		goto out;
 
 	index = of_read_number(associativity, 1);
-	if (index > min_common_depth + 1)
+	if (index > primary_domain_index + 1)
 		return of_read_number(&associativity[index - 1], 1);
 
 out: