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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Thanks,

David / dhildenb