[PATCH v3 0/5] fix some type infos and bugs for arm64/of numa

[PATCH v3 0/5] fix some type infos and bugs for arm64/of numa

[Zhen Lei]

v2 -> v3:
1. Adjust patch2 and patch5 according to Matthias Brugger's advice, to make the
   patches looks more well. The final code have no change. 

v1 -> v2:
1. Base on https://lkml.org/lkml/2016/5/24/679
2. Rewrote of_numa_parse_memory_nodes according to Rob Herring's advice. So that it looks more clear.
3. Rewrote patch 5 because some scenes were not considered before.

Zhen Lei (5):
  of/numa: remove a duplicated pr_debug information
  of/numa: fix a memory@ node can only contains one memory block
  arm64/numa: add nid check for memory block
  of/numa: remove a duplicated warning
  arm64/numa: avoid inconsistent information to be printed

 arch/arm64/mm/numa.c | 11 ++++++++---
 drivers/of/of_numa.c | 42 ++++++++++++------------------------------
 2 files changed, 20 insertions(+), 33 deletions(-)

--
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/5] of/numa: remove a duplicated pr_debug information

[Zhen Lei]

This information will be printed in the subfunction numa_add_memblk.
They are not the same, but very similar.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/of/of_numa.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index ed5a097..fb71b4e 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -88,10 +88,6 @@ static int __init of_numa_parse_memory_nodes(void)
 			break;
 		}

-		pr_debug("NUMA:  base = %llx len = %llx, node = %u\n",
-			 rsrc.start, rsrc.end - rsrc.start + 1, nid);
-
-
 		r = numa_add_memblk(nid, rsrc.start, rsrc.end + 1);
 		if (r)
 			break;

[PATCH v3 2/5] of/numa: fix a memory@ node can only contains one memory block

[Zhen Lei]

For a normal memory@ devicetree node, its reg property can contains more
memory blocks.

Because we don't known how many memory blocks maybe contained, so we try
from index=0, increase 1 until error returned(the end).

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/of/of_numa.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index fb71b4e..7b3fbdc 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -63,13 +63,9 @@ static int __init of_numa_parse_memory_nodes(void)
 	struct device_node *np = NULL;
 	struct resource rsrc;
 	u32 nid;
-	int r = 0;
-
-	for (;;) {
-		np = of_find_node_by_type(np, "memory");
-		if (!np)
-			break;
+	int i, r;

+	for_each_node_by_type(np, "memory") {
 		r = of_property_read_u32(np, "numa-node-id", &nid);
 		if (r == -EINVAL)
 			/*
@@ -78,23 +74,18 @@ static int __init of_numa_parse_memory_nodes(void)
 			 * "numa-node-id" property
 			 */
 			continue;
-		else if (r)
-			/* some other error */
-			break;

-		r = of_address_to_resource(np, 0, &rsrc);
-		if (r) {
-			pr_err("NUMA: bad reg property in memory node\n");
-			break;
-		}
+		for (i = 0; !r && !of_address_to_resource(np, i, &rsrc); i++)
+			r = numa_add_memblk(nid, rsrc.start, rsrc.end + 1);

-		r = numa_add_memblk(nid, rsrc.start, rsrc.end + 1);
-		if (r)
-			break;
+		if (!i || r) {
+			of_node_put(np);
+			pr_err("NUMA: bad property in memory node\n");
+			return r ? : -EINVAL;
+		}
 	}
-	of_node_put(np);

-	return r;
+	return 0;
 }

 static int __init of_numa_parse_distance_map_v1(struct device_node *map)

[PATCH v3 3/5] arm64/numa: add nid check for memory block

[Zhen Lei]

Use the same tactic to cpu and numa-distance nodes.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 arch/arm64/mm/numa.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index c7fe3ec..2601660 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -141,6 +141,11 @@ int __init numa_add_memblk(int nid, u64 start, u64 end)
 {
 	int ret;

+	if (nid >= MAX_NUMNODES) {
+		pr_warn("NUMA: Node id %u exceeds maximum value\n", nid);
+		return -EINVAL;
+	}
+
 	ret = memblock_set_node(start, (end - start), &memblock.memory, nid);
 	if (ret < 0) {
 		pr_err("NUMA: memblock [0x%llx - 0x%llx] failed to add on node %d\n",

Re: [PATCH v3 3/5] arm64/numa: add nid check for memory block

[Will Deacon]

On Thu, Jun 02, 2016 at 10:28:09AM +0800, Zhen Lei wrote:
> Use the same tactic to cpu and numa-distance nodes.

Sorry, I don't understand... :/

Will

> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm64/mm/numa.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index c7fe3ec..2601660 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -141,6 +141,11 @@ int __init numa_add_memblk(int nid, u64 start, u64 end)
>  {
>  	int ret;
> 
> +	if (nid >= MAX_NUMNODES) {
> +		pr_warn("NUMA: Node id %u exceeds maximum value\n", nid);
> +		return -EINVAL;
> +	}
> +
>  	ret = memblock_set_node(start, (end - start), &memblock.memory, nid);
>  	if (ret < 0) {
>  		pr_err("NUMA: memblock [0x%llx - 0x%llx] failed to add on node %d\n",
> --
> 2.5.0
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] arm64/numa: add nid check for memory block

[Leizhen (ThunderTown)]

On 2016/6/3 17:52, Will Deacon wrote:
> On Thu, Jun 02, 2016 at 10:28:09AM +0800, Zhen Lei wrote:
>> Use the same tactic to cpu and numa-distance nodes.
> 
> Sorry, I don't understand... :/

In function of_numa_parse_cpu_nodes:
for_each_child_of_node(cpus, np) {
	...
	r = of_property_read_u32(np, "numa-node-id", &nid);
	...
	if (nid >= MAX_NUMNODES)									//check nid
			pr_warn("NUMA: Node id %u exceeds maximum value\n", nid);			//print warning info
	...


In function numa_set_distance:
	if (from >= numa_distance_cnt || to >= numa_distance_cnt ||					//check nid
			from < 0 || to < 0) {
		pr_warn_once("NUMA: Warning: node ids are out of bound, from=%d to=%d distance=%d\n",	//print warning info
			    from, to, distance);
		return;
	}

Both these two functions will check that whether nid(configured in dts, the subnodes of
cpus and distance-map) is right or not. So memory@ should also be checked.


	memory@c00000 {
		device_type = "memory";
		reg = <0x0 0xc00000 0x0 0x80000000>;
		/* node 0 */
		numa-node-id = <0>;				//have not been checked yet.
	};							//suppose I configued a wrong nid, it will not print any warning info

	cpus {
		#address-cells = <2>;
		#size-cells = <0>;

		cpu@0 {
			device_type = "cpu";
			compatible =  "arm,armv8";
			reg = <0x0 0x0>;
			enable-method = "psci";
			/* node 0 */
			numa-node-id = <0>;			//checked in of_numa_parse_cpu_nodes
		};

	distance-map {
		compatible = "numa-distance-map-v1";
		distance-matrix = <0 0 10>,			//checked in of_numa_parse_distance_map_v1 --> numa_set_distance
				  <0 1 20>,
				  <1 1 10>;
	};

> 
> Will
> 
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  arch/arm64/mm/numa.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>> index c7fe3ec..2601660 100644
>> --- a/arch/arm64/mm/numa.c
>> +++ b/arch/arm64/mm/numa.c
>> @@ -141,6 +141,11 @@ int __init numa_add_memblk(int nid, u64 start, u64 end)
>>  {
>>  	int ret;
>>
>> +	if (nid >= MAX_NUMNODES) {
>> +		pr_warn("NUMA: Node id %u exceeds maximum value\n", nid);
>> +		return -EINVAL;
>> +	}
>> +
>>  	ret = memblock_set_node(start, (end - start), &memblock.memory, nid);
>>  	if (ret < 0) {
>>  		pr_err("NUMA: memblock [0x%llx - 0x%llx] failed to add on node %d\n",
>> --
>> 2.5.0
>>
>>
> 
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/5] of/numa: remove a duplicated warning

[Zhen Lei]

This warning has been printed in of_numa_parse_cpu_nodes before.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/of/of_numa.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index 7b3fbdc..3157130 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -174,13 +174,8 @@ int of_node_to_nid(struct device_node *device)
 			np->name);
 	of_node_put(np);

-	if (!r) {
-		if (nid >= MAX_NUMNODES)
-			pr_warn("NUMA: Node id %u exceeds maximum value\n",
-				nid);
-		else
-			return nid;
-	}
+	if (!r)
+		return nid;

 	return NUMA_NO_NODE;
 }
--
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 5/5] arm64/numa: avoid inconsistent information to be printed

[Zhen Lei]

numa_init(of_numa_init) may returned error because of numa configuration
error. So "No NUMA configuration found" is inaccurate. In fact, specific
configuration error information should be immediately printed by the
testing branch.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 arch/arm64/mm/numa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index 2601660..1b9622c 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -338,8 +338,10 @@ static int __init numa_init(int (*init_func)(void))
 	if (ret < 0)
 		return ret;

-	if (nodes_empty(numa_nodes_parsed))
+	if (nodes_empty(numa_nodes_parsed)) {
+		pr_info("No NUMA configuration found\n");
 		return -EINVAL;
+	}

 	ret = numa_register_nodes();
 	if (ret < 0)
@@ -370,8 +372,6 @@ static int __init dummy_numa_init(void)

 	if (numa_off)
 		pr_info("NUMA disabled\n"); /* Forced off on command line. */
-	else
-		pr_info("No NUMA configuration found\n");
 	pr_info("NUMA: Faking a node at [mem %#018Lx-%#018Lx]\n",
 	       0LLU, PFN_PHYS(max_pfn) - 1);

Re: [PATCH v3 5/5] arm64/numa: avoid inconsistent information to be printed

[Will Deacon]

On Thu, Jun 02, 2016 at 10:28:11AM +0800, Zhen Lei wrote:
> numa_init(of_numa_init) may returned error because of numa configuration
> error. So "No NUMA configuration found" is inaccurate. In fact, specific
> configuration error information should be immediately printed by the
> testing branch.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  arch/arm64/mm/numa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Looks fine to me, but this doesn't apply against -rc1.

Will

> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index 2601660..1b9622c 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -338,8 +338,10 @@ static int __init numa_init(int (*init_func)(void))
>  	if (ret < 0)
>  		return ret;
> 
> -	if (nodes_empty(numa_nodes_parsed))
> +	if (nodes_empty(numa_nodes_parsed)) {
> +		pr_info("No NUMA configuration found\n");
>  		return -EINVAL;
> +	}
> 
>  	ret = numa_register_nodes();
>  	if (ret < 0)
> @@ -370,8 +372,6 @@ static int __init dummy_numa_init(void)
> 
>  	if (numa_off)
>  		pr_info("NUMA disabled\n"); /* Forced off on command line. */
> -	else
> -		pr_info("No NUMA configuration found\n");
>  	pr_info("NUMA: Faking a node at [mem %#018Lx-%#018Lx]\n",
>  	       0LLU, PFN_PHYS(max_pfn) - 1);
> 
> --
> 2.5.0
> 
> 

Re: [PATCH v3 5/5] arm64/numa: avoid inconsistent information to be printed

[Leizhen (ThunderTown)]

On 2016/6/3 17:55, Will Deacon wrote:
> On Thu, Jun 02, 2016 at 10:28:11AM +0800, Zhen Lei wrote:
>> numa_init(of_numa_init) may returned error because of numa configuration
>> error. So "No NUMA configuration found" is inaccurate. In fact, specific
>> configuration error information should be immediately printed by the
>> testing branch.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  arch/arm64/mm/numa.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Looks fine to me, but this doesn't apply against -rc1.

Oh,

These patched based on https://lkml.org/lkml/2016/5/24/679 series.

> 
> Will
> 
>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>> index 2601660..1b9622c 100644
>> --- a/arch/arm64/mm/numa.c
>> +++ b/arch/arm64/mm/numa.c
>> @@ -338,8 +338,10 @@ static int __init numa_init(int (*init_func)(void))
>>  	if (ret < 0)
>>  		return ret;
>>
>> -	if (nodes_empty(numa_nodes_parsed))
>> +	if (nodes_empty(numa_nodes_parsed)) {
>> +		pr_info("No NUMA configuration found\n");
>>  		return -EINVAL;
>> +	}
>>
>>  	ret = numa_register_nodes();
>>  	if (ret < 0)
>> @@ -370,8 +372,6 @@ static int __init dummy_numa_init(void)
>>
>>  	if (numa_off)
>>  		pr_info("NUMA disabled\n"); /* Forced off on command line. */
>> -	else
>> -		pr_info("No NUMA configuration found\n");
>>  	pr_info("NUMA: Faking a node at [mem %#018Lx-%#018Lx]\n",
>>  	       0LLU, PFN_PHYS(max_pfn) - 1);
>>
>> --
>> 2.5.0
>>
>>
> 
> .
>