lkml.org 
[lkml]   [2018]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] mm/memory_hotplug: Make add_memory_resource use __try_online_node
On Wed, Jun 20, 2018 at 09:41:35PM -0400, Pavel Tatashin wrote:
> > 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>

Hi Pavel,

I'll do so, thanks!

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

--
Oscar Salvador
SUSE L3

\
 
 \ /
  Last update: 2018-06-21 09:34    [W:0.142 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site