Messages in this thread | | | From | Pavel Tatashin <> | Date | Wed, 20 Jun 2018 21:41:35 -0400 | Subject | Re: [PATCH 1/4] mm/memory_hotplug: Make add_memory_resource use __try_online_node |
| |
> I don't think __try_online_node() will ever return a value greater than > zero. I assume what was meant was
Hi Andrew and Oscar,
Actually, the new __try_online_node() returns: 1 -> a new node was allocated 0 -> node is already online -error -> an error encountered.
The function simply missing the return comment at the beginning.
Oscar, please check it via ./scripts/checkpatch.pl
Add comment explaining the return values.
And change: ret = __try_online_node (nid, start, false); new_node = !!(ret > 0); if (ret < 0) goto error; To: ret = __try_online_node (nid, start, false); if (ret < 0) goto error; new_node = ret;
Other than that the patch looks good to me, it simplifies the code. So, if the above is addressed:
Reviewed-by: Pavel Tatashin <pasha.tatashin@oracle.com>
Thank you, Pavel
> > new_node = !!(ret >= 0); > > which may as well be > > new_node = (ret >= 0); > > since both sides have bool type. > > The fact that testing didn't detect this is worrisome.... > > > + if (ret < 0) > > + goto error; > > + > > > > /* call arch's memory hotadd */ > > ret = arch_add_memory(nid, start, size, NULL, true); > > - > > if (ret < 0) > > goto error; > > > > > > ... > > >
| |