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

[David Hildenbrand <david@redhat.com>]

On 16.04.20 19:25, David Hildenbrand wrote:
> 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.
> 

Forgot to

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb