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

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

[Zhen Lei]

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

[PATCH v2 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 v2 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 | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index fb71b4e..fa85a51 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 = 0;

+	for_each_node_by_type(np, "memory") {
 		r = of_property_read_u32(np, "numa-node-id", &nid);
 		if (r == -EINVAL)
 			/*
@@ -78,21 +74,17 @@ 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)
+		if (!i || r) {
+			of_node_put(np);
+			pr_err("NUMA: bad property in memory node\n");
+			r = r ? : -EINVAL;
 			break;
+		}
 	}
-	of_node_put(np);

 	return r;
 }

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

[Rob Herring]

On Sat, May 28, 2016 at 4:22 AM, Zhen Lei <thunder.leizhen@huawei.com> wrote:
> 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 | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
> index fb71b4e..fa85a51 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 = 0;
>
> +       for_each_node_by_type(np, "memory") {
>                 r = of_property_read_u32(np, "numa-node-id", &nid);
>                 if (r == -EINVAL)
>                         /*
> @@ -78,21 +74,17 @@ 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)
> +               if (!i || r) {
> +                       of_node_put(np);
> +                       pr_err("NUMA: bad property in memory node\n");
> +                       r = r ? : -EINVAL;
>                         break;
> +               }
>         }
> -       of_node_put(np);

I believe you still need this and not the one above. You only need it
within the loop if you return. Otherwise, the last node always need to
be put.

With that, for the series:

Acked-by: Rob Herring <robh@kernel.org>

Rob

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

[Leizhen (ThunderTown)]

On 2016/6/2 4:13, Rob Herring wrote:
> On Sat, May 28, 2016 at 4:22 AM, Zhen Lei <thunder.leizhen@huawei.com> wrote:
>> 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 | 26 +++++++++-----------------
>>  1 file changed, 9 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>> index fb71b4e..fa85a51 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 = 0;
>>
>> +       for_each_node_by_type(np, "memory") {
>>                 r = of_property_read_u32(np, "numa-node-id", &nid);
>>                 if (r == -EINVAL)
>>                         /*
>> @@ -78,21 +74,17 @@ 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)
>> +               if (!i || r) {
>> +                       of_node_put(np);
>> +                       pr_err("NUMA: bad property in memory node\n");
>> +                       r = r ? : -EINVAL;
>>                         break;
>> +               }
>>         }
>> -       of_node_put(np);
> 
> I believe you still need this and not the one above. You only need it
> within the loop if you return. Otherwise, the last node always need to
> be put.

OK. Thanks.

Addition with Matthias's suggestion, I will move "return" into this patch, so that this of_node_put(np) can be safely removed.


> 
> With that, for the series:
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> Rob
> 
> .
> 

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

[Will Deacon]

On Thu, Jun 02, 2016 at 09:36:40AM +0800, Leizhen (ThunderTown) wrote:
> On 2016/6/2 4:13, Rob Herring wrote:
> > I believe you still need this and not the one above. You only need it
> > within the loop if you return. Otherwise, the last node always need to
> > be put.
> 
> OK. Thanks.
> 
> Addition with Matthias's suggestion, I will move "return" into this patch,
> so that this of_node_put(np) can be safely removed.

Do you want to include Kefeng's [1] patches in your series too? We don't
need two sets of related NUMA cleanups :)

Will

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/432715.html
--
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 v2 2/5] of/numa: fix a memory@ node can only contains one memory block

[Will Deacon]

On Fri, Jun 03, 2016 at 10:45:20AM +0100, Will Deacon wrote:
> On Thu, Jun 02, 2016 at 09:36:40AM +0800, Leizhen (ThunderTown) wrote:
> > On 2016/6/2 4:13, Rob Herring wrote:
> > > I believe you still need this and not the one above. You only need it
> > > within the loop if you return. Otherwise, the last node always need to
> > > be put.
> > 
> > OK. Thanks.
> > 
> > Addition with Matthias's suggestion, I will move "return" into this patch,
> > so that this of_node_put(np) can be safely removed.
> 
> Do you want to include Kefeng's [1] patches in your series too? We don't
> need two sets of related NUMA cleanups :)

Actually, I see you've already respun the series, do don't worry about
doing another version just for this.

Will
--
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 v2 2/5] of/numa: fix a memory@ node can only contains one memory block

[Leizhen (ThunderTown)]

On 2016/6/3 17:45, Will Deacon wrote:
> On Thu, Jun 02, 2016 at 09:36:40AM +0800, Leizhen (ThunderTown) wrote:
>> On 2016/6/2 4:13, Rob Herring wrote:
>>> I believe you still need this and not the one above. You only need it
>>> within the loop if you return. Otherwise, the last node always need to
>>> be put.
>>
>> OK. Thanks.
>>
>> Addition with Matthias's suggestion, I will move "return" into this patch,
>> so that this of_node_put(np) can be safely removed.
> 
> Do you want to include Kefeng's [1] patches in your series too? We don't
> need two sets of related NUMA cleanups :)

Yes, It's originally suggested by Joe Perches.

> 
> Will
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/432715.html
> 
> .
> 

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

[Hanjun Guo]

Hi Leizhen,

On 2016/6/6 9:24, Leizhen (ThunderTown) wrote:
>
>
> On 2016/6/3 17:45, Will Deacon wrote:
>> On Thu, Jun 02, 2016 at 09:36:40AM +0800, Leizhen (ThunderTown) wrote:
>>> On 2016/6/2 4:13, Rob Herring wrote:
>>>> I believe you still need this and not the one above. You only need it
>>>> within the loop if you return. Otherwise, the last node always need to
>>>> be put.
>>>
>>> OK. Thanks.
>>>
>>> Addition with Matthias's suggestion, I will move "return" into this patch,
>>> so that this of_node_put(np) can be safely removed.
>>
>> Do you want to include Kefeng's [1] patches in your series too? We don't
>> need two sets of related NUMA cleanups :)
>
> Yes, It's originally suggested by Joe Perches.

I think Will suggested us to add Kefeng's NUMA cleanup patches into
yours and send a new version, just see comments from Will,

 > Actually, I see you've already respun the series, do don't worry about
 > doing another version just for this.

Will, correct me if I'm wrong.

Thanks
Hanjun

[PATCH v2 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",

[PATCH v2 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 fa85a51..fb62307 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -175,13 +175,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 v2 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 +++---
 drivers/of/of_numa.c | 7 +++----
 2 files changed, 6 insertions(+), 7 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);

diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
index fb62307..3157130 100644
--- a/drivers/of/of_numa.c
+++ b/drivers/of/of_numa.c
@@ -63,7 +63,7 @@ static int __init of_numa_parse_memory_nodes(void)
 	struct device_node *np = NULL;
 	struct resource rsrc;
 	u32 nid;
-	int i, r = 0;
+	int i, r;

 	for_each_node_by_type(np, "memory") {
 		r = of_property_read_u32(np, "numa-node-id", &nid);
@@ -81,12 +81,11 @@ static int __init of_numa_parse_memory_nodes(void)
 		if (!i || r) {
 			of_node_put(np);
 			pr_err("NUMA: bad property in memory node\n");
-			r = r ? : -EINVAL;
-			break;
+			return r ? : -EINVAL;
 		}
 	}

-	return r;
+	return 0;
 }

 static int __init of_numa_parse_distance_map_v1(struct device_node *map)

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

[Matthias Brugger]

On 28/05/16 11:22, 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-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---

Which kernel version is this patch based on?

Regards,
Matthias

>   arch/arm64/mm/numa.c | 6 +++---
>   drivers/of/of_numa.c | 7 +++----
>   2 files changed, 6 insertions(+), 7 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);
>
> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
> index fb62307..3157130 100644
> --- a/drivers/of/of_numa.c
> +++ b/drivers/of/of_numa.c
> @@ -63,7 +63,7 @@ static int __init of_numa_parse_memory_nodes(void)
>   	struct device_node *np = NULL;
>   	struct resource rsrc;
>   	u32 nid;
> -	int i, r = 0;
> +	int i, r;
>
>   	for_each_node_by_type(np, "memory") {
>   		r = of_property_read_u32(np, "numa-node-id", &nid);
> @@ -81,12 +81,11 @@ static int __init of_numa_parse_memory_nodes(void)
>   		if (!i || r) {
>   			of_node_put(np);
>   			pr_err("NUMA: bad property in memory node\n");
> -			r = r ? : -EINVAL;
> -			break;
> +			return r ? : -EINVAL;
>   		}
>   	}
>
> -	return r;
> +	return 0;
>   }
>

Well this is fixing changes you introduced in this patch-set. Any reason 
this is not part of patch 2?

>   static int __init of_numa_parse_distance_map_v1(struct device_node *map)
> --
> 2.5.0
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
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 v2 5/5] arm64/numa: avoid inconsistent information to be printed

[Leizhen (ThunderTown)]

On 2016/5/31 17:07, Matthias Brugger wrote:
> 
> 
> On 28/05/16 11:22, 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>
>> ---
> 
> Which kernel version is this patch based on?

Base on mainline(git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git), I git pulled about 3-5 days ago, the last commit-id is dc03c0f.

And thess patches base on https://lkml.org/lkml/2016/5/24/679 series(acpi numa) as David Daney's requirement.

> 
> Regards,
> Matthias
> 
>>   arch/arm64/mm/numa.c | 6 +++---
>>   drivers/of/of_numa.c | 7 +++----
>>   2 files changed, 6 insertions(+), 7 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);
>>
>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>> index fb62307..3157130 100644
>> --- a/drivers/of/of_numa.c
>> +++ b/drivers/of/of_numa.c
>> @@ -63,7 +63,7 @@ static int __init of_numa_parse_memory_nodes(void)
>>       struct device_node *np = NULL;
>>       struct resource rsrc;
>>       u32 nid;
>> -    int i, r = 0;
>> +    int i, r;
>>
>>       for_each_node_by_type(np, "memory") {
>>           r = of_property_read_u32(np, "numa-node-id", &nid);
>> @@ -81,12 +81,11 @@ static int __init of_numa_parse_memory_nodes(void)
>>           if (!i || r) {
>>               of_node_put(np);
>>               pr_err("NUMA: bad property in memory node\n");
>> -            r = r ? : -EINVAL;
>> -            break;
>> +            return r ? : -EINVAL;
>>           }
>>       }
>>
>> -    return r;
>> +    return 0;
>>   }
>>
> 
> Well this is fixing changes you introduced in this patch-set. Any reason this is not part of patch 2?

Because they fixed two different problems.

> 
>>   static int __init of_numa_parse_distance_map_v1(struct device_node *map)
>> -- 
>> 2.5.0
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> .
> 

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

[Leizhen (ThunderTown)]

On 2016/5/31 19:27, Leizhen (ThunderTown) wrote:
> 
> 
> On 2016/5/31 17:07, Matthias Brugger wrote:
>>
>>
>> On 28/05/16 11:22, 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>
>>> ---
>>
>> Which kernel version is this patch based on?
> 
> Base on mainline(git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git), I git pulled about 3-5 days ago, the last commit-id is dc03c0f.
> 
> And thess patches base on https://lkml.org/lkml/2016/5/24/679 series(acpi numa) as David Daney's requirement.
> 
>>
>> Regards,
>> Matthias
>>
>>>   arch/arm64/mm/numa.c | 6 +++---
>>>   drivers/of/of_numa.c | 7 +++----
>>>   2 files changed, 6 insertions(+), 7 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);
>>>
>>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>>> index fb62307..3157130 100644
>>> --- a/drivers/of/of_numa.c
>>> +++ b/drivers/of/of_numa.c
>>> @@ -63,7 +63,7 @@ static int __init of_numa_parse_memory_nodes(void)
>>>       struct device_node *np = NULL;
>>>       struct resource rsrc;
>>>       u32 nid;
>>> -    int i, r = 0;
>>> +    int i, r;
>>>
>>>       for_each_node_by_type(np, "memory") {
>>>           r = of_property_read_u32(np, "numa-node-id", &nid);
>>> @@ -81,12 +81,11 @@ static int __init of_numa_parse_memory_nodes(void)
>>>           if (!i || r) {
>>>               of_node_put(np);
>>>               pr_err("NUMA: bad property in memory node\n");
>>> -            r = r ? : -EINVAL;
>>> -            break;
>>> +            return r ? : -EINVAL;
>>>           }
>>>       }
>>>
>>> -    return r;
>>> +    return 0;
>>>   }
>>>
>>
>> Well this is fixing changes you introduced in this patch-set. Any reason this is not part of patch 2?
> 
> Because they fixed two different problems.

Hi, Matthias

I thougth it again on my way home yesterday. Yeah, you're right, move this part to patch 2, will make these two
patches looks more well. I put it here before, because for "No numa configuration" case, it originally returns error
code, so that it can not walk to "if (nodes_empty(numa_nodes_parsed))".

	ret = init_func();
	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;
   +    }

Regards,
Zhen Lei

> 
>>
>>>   static int __init of_numa_parse_distance_map_v1(struct device_node *map)
>>> -- 
>>> 2.5.0
>>>
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>
>> .
>>