Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411523 > unrolled thread

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

Started byRob Herring <robh+dt@kernel.org>
First post2016-06-01 22:20 +0200
Last post2016-06-06 10:10 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block Rob Herring <robh+dt@kernel.org> - 2016-06-01 22:20 +0200
    Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com> - 2016-06-02 03:50 +0200
      Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block Will Deacon <will.deacon@arm.com> - 2016-06-03 11:50 +0200
        Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block Will Deacon <will.deacon@arm.com> - 2016-06-03 12:00 +0200
        Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com> - 2016-06-06 03:30 +0200
          Re: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one  memory block Hanjun Guo <hanjun.guo@linaro.org> - 2016-06-06 10:10 +0200

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

FromRob Herring <robh+dt@kernel.org>
Date2016-06-01 22:20 +0200
SubjectRe: [PATCH v2 2/5] of/numa: fix a memory@ node can only contains one memory block
Message-ID<rFkMp-4JV-7@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1411753

From"Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
Date2016-06-02 03:50 +0200
Message-ID<rFpVL-7U8-9@gated-at.bofh.it>
In reply to#1411523

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
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1412977

FromWill Deacon <will.deacon@arm.com>
Date2016-06-03 11:50 +0200
Message-ID<rFTTP-1rX-1@gated-at.bofh.it>
In reply to#1411753
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

[toc] | [prev] | [next] | [standalone]

#1412990

FromWill Deacon <will.deacon@arm.com>
Date2016-06-03 12:00 +0200
Message-ID<rFU3w-1wB-19@gated-at.bofh.it>
In reply to#1412977
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

[toc] | [prev] | [next] | [standalone]

#1414547

From"Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
Date2016-06-06 03:30 +0200
Message-ID<rGRwB-5WK-3@gated-at.bofh.it>
In reply to#1412977

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
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1414720

FromHanjun Guo <hanjun.guo@linaro.org>
Date2016-06-06 10:10 +0200
Message-ID<rGXLI-2fW-29@gated-at.bofh.it>
In reply to#1414547
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web