| Message ID | 1464427377-12712-3-git-send-email-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > . >
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
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
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 > > . >
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
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; }
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(-) -- 2.5.0