On Fri, 1 Jun 2018 14:53:18 +0200 osalvador@xxxxxxxxxxxxxxxxxx wrote:
> From: Oscar Salvador <osalvador@xxxxxxx>
>
> add_memory_resource() contains code to allocate a new node in case
> it is necessary.
> Since try_online_node() also hast some code for this purpose,
> let us make use of that and remove duplicate code.
>
> This introduces __try_online_node(), which is called by add_memory_resource()
> and try_online_node().
> __try_online_node() has two new parameters, start_addr of the node,
> and if the node should be onlined and registered right away.
> This is always wanted if we are calling from do_cpu_up(), but not
> when we are calling from memhotplug code.
> Nothing changes from the point of view of the users of try_online_node(),
> since try_online_node passes start_addr=0 and online_node=true to
> __try_online_node().
>
> ...
>
> @@ -1126,17 +1136,14 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
> */
> memblock_add_node(start, size, nid);
>
> - new_node = !node_online(nid);
> - if (new_node) {
> - pgdat = hotadd_new_pgdat(nid, start);
> - ret = -ENOMEM;
> - if (!pgdat)
> - goto error;
> - }
> + ret = __try_online_node (nid, start, false);
> + new_node = !!(ret > 0);
I don't think __try_online_node() will ever return a value greater than
zero. I assume what was meant was
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;
>
>
> ...
>
