> 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@xxxxxxxxxx>
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;
> >
> >
> > ...
> >
>
