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

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

[Waiman Long]

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

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

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/nodemask.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
index 5a30ad5..ba07661 100644
--- a/include/linux/nodemask.h
+++ b/include/linux/nodemask.h
@@ -97,6 +97,8 @@
 
 typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t;
 extern nodemask_t _unused_nodemask_arg_;
+extern int nr_node_ids;
+extern int nr_online_nodes;
 
 /**
  * nodemask_pr_args - printf args to output a nodemask
@@ -108,7 +110,7 @@
 				__nodemask_pr_bits(maskp)
 static inline unsigned int __nodemask_pr_numnodes(const nodemask_t *m)
 {
-	return m ? MAX_NUMNODES : 0;
+	return m ? nr_node_ids : 0;
 }
 static inline const unsigned long *__nodemask_pr_bits(const nodemask_t *m)
 {
@@ -444,9 +446,6 @@ static inline int next_memory_node(int nid)
 	return next_node(nid, node_states[N_MEMORY]);
 }
 
-extern int nr_node_ids;
-extern int nr_online_nodes;
-
 static inline void node_set_online(int nid)
 {
 	node_set_state(nid, N_ONLINE);
-- 
1.8.3.1

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

[Andrew Morton]

On Tue, 18 Dec 2018 12:30:31 -0500 Waiman Long <longman@redhat.com> 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.

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

[Waiman Long]

On 12/18/2018 03:41 PM, Andrew Morton wrote:
> On Tue, 18 Dec 2018 12:30:31 -0500 Waiman Long <longman@redhat.com> 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