Skip to contentSkip to conversation

mm/page_alloc.c: use NODE_MASK_NONE define used_mask

PATCH In review Wei Yang 3 participants 4 messages Mar 29, 2020 – Mar 30, 2020

Subsystems (2)
Memory management Page allocator
lore
Share & tools
lore

Wei Yang

author
#0 lore raw

PATCH v3 1/1 mm/page_alloc.c: use NODE_MASK_NONE define used_mask

For all 0 nodemask_t, we have already define macro NODE_MASK_NONE. Leverage this to define an all clear nodemask.

Signed-off-by Wei Yang <[email protected]>

---
v3: adjust the commit log a little
v2: use NODE_MASK_NONE as suggested by David Hildenbrand

---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
mm/page_alloc.c +1 -2
index ef790dfad6aa..dfcf2682ed40 100644
@@ -5587,14 +5587,13 @@ static void build_zonelists(pg_data_t *pgdat)
 {
 	static int node_order[MAX_NUMNODES];
 	int node, load, nr_nodes = 0;
-	nodemask_t used_mask;
+	nodemask_t used_mask = NODE_MASK_NONE;
 	int local_node, prev_node;
 
 	/* NUMA-aware ordering of nodes */
 	local_node = pgdat->node_id;
 	load = nr_online_nodes;
 	prev_node = local_node;
-	nodes_clear(used_mask);
 
 	memset(node_order, 0, sizeof(node_order));
 	while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {
Signature · 1 line
2.23.0
Reviewed-by on v3 1/1
#1 lore raw
Quoted 2 lines · Wei Yang

For all 0 nodemask_t, we have already define macro NODE_MASK_NONE. Leverage this to define an all clear nodemask.

It would be a little clearer if you used wording more like this:

Subject: [Patch v3] mm/page_alloc.c: use NODE_MASK_NONE in build_zonelists()

Slightly simplify the code by initializing user_mask with NODE_MASK_NONE, instead of later calling nodes_clear(). This saves a line of code.

Quoted 21 lines
Signed-off-by Wei Yang <[email protected]>

---
v3: adjust the commit log a little
v2: use NODE_MASK_NONE as suggested by David Hildenbrand

---
  mm/page_alloc.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
mm/page_alloc.c +1 -1
index ef790dfad6aa..dfcf2682ed40 100644
@@ -5587,14 +5587,13 @@ static void build_zonelists(pg_data_t *pgdat)
  {
  	static int node_order[MAX_NUMNODES];
  	int node, load, nr_nodes = 0;
-	nodemask_t used_mask;
+	nodemask_t used_mask = NODE_MASK_NONE;

mm/page_alloc.c static void build_zonelists(pg_data_t *pgdat) · line 5599

Quoted 10 lines
mm/page_alloc.c +0 -1
  	int local_node, prev_node;
 
  	/* NUMA-aware ordering of nodes */
  	local_node = pgdat->node_id;
  	load = nr_online_nodes;
  	prev_node = local_node;
-	nodes_clear(used_mask);
 
  	memset(node_order, 0, sizeof(node_order));
  	while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {

Honestly, I don't think it's really worth doing a patch for this, but there's nothing wrong with the diff, so:

Reviewed-by John Hubbard <[email protected]>

thanks,

Signature · 2 lines
John Hubbard
NVIDIA
author
#2 lore raw
Quoted 4 lines · John Hubbard
Quoted 2 lines · Wei Yang

For all 0 nodemask_t, we have already define macro NODE_MASK_NONE. Leverage this to define an all clear nodemask.

Quoted 8 lines

It would be a little clearer if you used wording more like this:

Subject: [Patch v3] mm/page_alloc.c: use NODE_MASK_NONE in build_zonelists()

Slightly simplify the code by initializing user_mask with NODE_MASK_NONE, instead of later calling nodes_clear(). This saves a line of code.

Thanks, the wording is better.

Quoted 34 lines
Quoted 20 lines
Signed-off-by Wei Yang <[email protected]>

---
v3: adjust the commit log a little
v2: use NODE_MASK_NONE as suggested by David Hildenbrand

---
  mm/page_alloc.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
mm/page_alloc.c +0 -1
index ef790dfad6aa..dfcf2682ed40 100644
@@ -5587,14 +5587,13 @@ static void build_zonelists(pg_data_t *pgdat)
  {
  	static int node_order[MAX_NUMNODES];
  	int node, load, nr_nodes = 0;
-	nodemask_t used_mask;

mm/page_alloc.c static void build_zonelists(pg_data_t *pgdat) · line 5598

Quoted 10 lines
mm/page_alloc.c +1 -1
+	nodemask_t used_mask = NODE_MASK_NONE;
  	int local_node, prev_node;
  	/* NUMA-aware ordering of nodes */
  	local_node = pgdat->node_id;
  	load = nr_online_nodes;
  	prev_node = local_node;
-	nodes_clear(used_mask);
  	memset(node_order, 0, sizeof(node_order));
  	while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {

Honestly, I don't think it's really worth doing a patch for this, but there's nothing wrong with the diff, so:

Quoted 2 lines
Reviewed-by John Hubbard <[email protected]>

Thanks.

Quoted 5 lines

thanks,

Signature · 2 lines
John Hubbard
NVIDIA
Signature · 2 lines
Wei Yang
Help you, Help me
Reviewed-by
#3 lore raw
In reply to John Hubbard
Quoted 41 lines · John Hubbard
Quoted 2 lines · Wei Yang

For all 0 nodemask_t, we have already define macro NODE_MASK_NONE. Leverage this to define an all clear nodemask.

It would be a little clearer if you used wording more like this:

Subject: [Patch v3] mm/page_alloc.c: use NODE_MASK_NONE in build_zonelists()

Slightly simplify the code by initializing user_mask with NODE_MASK_NONE, instead of later calling nodes_clear(). This saves a line of code.

Quoted 28 lines
Signed-off-by Wei Yang <[email protected]>

---
v3: adjust the commit log a little
v2: use NODE_MASK_NONE as suggested by David Hildenbrand

---
  mm/page_alloc.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
mm/page_alloc.c +1 -2
index ef790dfad6aa..dfcf2682ed40 100644
@@ -5587,14 +5587,13 @@ static void build_zonelists(pg_data_t *pgdat)
  {
  	static int node_order[MAX_NUMNODES];
  	int node, load, nr_nodes = 0;
-	nodemask_t used_mask;
+	nodemask_t used_mask = NODE_MASK_NONE;
  	int local_node, prev_node;
 
  	/* NUMA-aware ordering of nodes */
  	local_node = pgdat->node_id;
  	load = nr_online_nodes;
  	prev_node = local_node;
-	nodes_clear(used_mask);
Quoted 8 lines

mm/page_alloc.c static void build_zonelists(pg_data_t *pgdat) · line 5599

Quoted 3 lines
mm/page_alloc.c +0 -0
  	memset(node_order, 0, sizeof(node_order));
  	while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {

Honestly, I don't think it's really worth doing a patch for this, but there's nothing wrong with the diff, so:

Reviewed-by John Hubbard <[email protected]>

With the changed subject/description

Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks,

David / dhildenb