Re: [PATCH v4] mm/memory_hotplug: refrain from adding memory into an impossible node

On 16.04.20 19:23, Verma, Vishal L wrote: > On Thu, 2020-04-16 at 19:12 +0200, David Hildenbrand wrote: >> On 16.04.20 19:10, Vishal Verma wrote: >>> >>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >>> index 0a54ffac8c68..ddd3347edd54 100644 >>> --- a/mm/memory_hotplug.c >>> +++ b/mm/memory_hotplug.c >>> @@ -1005,6 +1005,11 @@ int __ref add_memory_resource(int nid, struct resource *res) >>> if (ret) >>> return ret; >>> >>> + if (!node_possible(nid)) { >>> + WARN(1, "node %d was absent from the node_possible_map\n", nid); >>> + return -ENXIO; >> >> Nit: I suggest using "-EINVAL" instead (e.g., returned via >> check_hotplug_memory_range). >> >> Not sure if we should pr_err() instead of WARN (see e.g., >> check_hotplug_memory_range) >> > Hm, I'm happy to make the changes, but EINVAL to me suggests there is a > problem in the way this was called by the user. And in this case there > really might not be much the user can change in case fo buggy firmware. Yeah, but introducing new return codes callers might not expected might create IMHO other issues. > > Same thing with the WARN - make the potential firmware bug much more > obvious and visible. > Yeah, but I doubt this is really necessary. No strong feelings.