Re: [PATCH] include/linux/nodemask.h: Use nr_node_ids instead of MAX_NUMNODES in nodemask_pr_args

From: Waiman Long
Date: Tue Dec 18 2018 - 16:41:22 EST


On 12/18/2018 03:41 PM, Andrew Morton wrote:
> On Tue, 18 Dec 2018 12:30:31 -0500 Waiman Long <longman@xxxxxxxxxx> wrote:
>
>> When viewing the /proc/<pid>/status file, one can see output lines like
>>
>> Cpus_allowed: ffffffff,ffffffff,ffffffff
>> Cpus_allowed_list: 0-95
>> Mems_allowed: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,0000000f
>> Mems_allowed_list: 0-3
> Oh dear.
>
>> It looks really strange that so many bits are displayed in the
>> "Mems_allowed:" line. Whereas the "Cpus_allowed:" line is perfectly
>> fine.
>>
>> It is because the nodemask_pr_args() macro uses MAX_NUMNODES as the
>> number of nodes to iterate. The cpumask_pr_args() macro uses nr_cpu_ids
>> instead of MAX_CPUS. For nodes, there is a corresponding nr_node_ids.
>> So it makes sense to use nr_node_ids instead to be consistent with
>> "Cpus_allowed:".
>>
>> With that change, the "Mems_allowed:" line becomes
>>
>> Mems_allowed: f
> OK, I guess. There is a risk of breaking existing userspace, but any
> half-decent parser should treat the above as equivalent.
>
> However there are quite a lot of nodemask_pr_args() callers in the tree
> and this patch potentially changes behaviour at all those sites.
> Please work through all those callers and show how the output is
> altered so we can consider the patch's complete effect.
>
>
The /proc/<pid>/status file is the only one that will be affected by
this patch. I believe the "Mems_allowed_list" is what most users are
using, not the "Mems_allowed".

Cheers,
Longman