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@xxxxxxxxxx>
--
Thanks,
David / dhildenb
