do_migrate_pages() calls migrate_to_node() even if task is already on a correct node

PATCH Superseded Larry Woodman 4 participants 9 messages Apr 24, 2012

Subsystems (2)

Memory management MM NUMA placement

lore

Share & tools

lore

Larry Woodman

author

2012-04-24 15:59 UTC #0 lore raw

PATCH v2 1/1 do_migrate_pages() calls migrate_to_node() even if task is already on a correct node

While moving tasks between cpusets we noticed some strange behavior. Specifically if the nodes of the destination cpuset are a subset of the nodes of the source cpuset do_migrate_pages() will move pages that are already on a node in the destination cpuset. The reason for this is do_migrate_pages() does not check whether each node in the source nodemask is in the destination nodemask before calling migrate_to_node(). If we simply do this check and skip them when the source is in the destination moving we wont move nodes that dont need to be moved.

Adding a little debug printk to migrate_to_node():

Without this change migrating tasks from a cpuset containing nodes 0-7 to a cpuset containing nodes 3-4, we migrate from ALL the nodes even if they are in the both the source and destination nodesets:

Migrating 7 to 4
Migrating 6 to 3
Migrating 5 to 4
Migrating 4 to 3
Migrating 1 to 4
Migrating 3 to 4
Migrating 0 to 3
Migrating 2 to 3

With this change we only migrate from nodes that are not in the destination nodesets:

Migrating 7 to 4
Migrating 6 to 3
Migrating 5 to 4
Migrating 2 to 3
Migrating 1 to 4
Migrating 0 to 3

This version of the patch will only skips migrating from nodes that are not in the destination nodesets if the number of nodes in the source and destination nodesets are not equal. This preserves the intended behavior that Christoph pointed out yet aviods the costly overhead of migrating them when its not necessary.

Here is timings of migrating from nodes 0-7 to 1 & 3 with and without the patch:

BEFORE PATCH -- Move times: 59, 140, 651 seconds
============

Moving 14 tasks from nodes (0-7) to nodes (1,3)
numad(8780) do_migrate_pages (mm=0xffff88081d414400 from_nodes=0xffff880818c81d28 to_nodes=0xffff880818c81ce8 flags=0x4) numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x7 dest=0x3 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x6 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x5 dest=0x3 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x4 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x2 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x1 dest=0x3 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d414400 source=0x0 dest=0x1 flags=0x4)
(Above moves repeated for each of the 14 tasks...)
PID 8890 moved to node(s) 1,3 in 59.2 seconds

Moving 20 tasks from nodes (0-7) to nodes (1,4-5)
numad(8780) do_migrate_pages (mm=0xffff88081d88c700 from_nodes=0xffff880818c81d28 to_nodes=0xffff880818c81ce8 flags=0x4) numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x7 dest=0x4 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x6 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x3 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x2 dest=0x5 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x1 dest=0x4 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d88c700 source=0x0 dest=0x1 flags=0x4)
(Above moves repeated for each of the 20 tasks...)
PID 8962 moved to node(s) 1,4-5 in 139.88 seconds

Moving 26 tasks from nodes (0-7) to nodes (1-3,5)
numad(8780) do_migrate_pages (mm=0xffff88081d5bc740 from_nodes=0xffff880818c81d28 to_nodes=0xffff880818c81ce8 flags=0x4) numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x7 dest=0x5 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x6 dest=0x3 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x5 dest=0x2 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x3 dest=0x5 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x2 dest=0x3 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x1 dest=0x2 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x0 dest=0x1 flags=0x4)
numad(8780) migrate_to_node (mm=0xffff88081d5bc740 source=0x4 dest=0x1 flags=0x4)
(Above moves repeated for each of the 26 tasks...)
PID 9058 moved to node(s) 1-3,5 in 651.45 seconds

AFTER PATCH -- Move times: 42, 56, 93 seconds
===========

Moving 14 tasks from nodes (0-7) to nodes (5,7)
numad(33209) do_migrate_pages (mm=0xffff88101d5ff140 from_nodes=0xffff88101e7b5d28 to_nodes=0xffff88101e7b5ce8 flags=0x4) numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x6 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x4 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x3 dest=0x7 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x2 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x1 dest=0x7 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d5ff140 source=0x0 dest=0x5 flags=0x4)
(Above moves repeated for each of the 14 tasks...)
PID 33221 moved to node(s) 5,7 in 41.67 seconds

Moving 20 tasks from nodes (0-7) to nodes (1,3,5)
numad(33209) do_migrate_pages (mm=0xffff88101d6c37c0 from_nodes=0xffff88101e7b5d28 to_nodes=0xffff88101e7b5ce8 flags=0x4) numad(33209) migrate_to_node (mm=0xffff88101d6c37c0 source=0x7 dest=0x3 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d6c37c0 source=0x6 dest=0x1 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d6c37c0 source=0x4 dest=0x3 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d6c37c0 source=0x2 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d6c37c0 source=0x0 dest=0x1 flags=0x4)
(Above moves repeated for each of the 20 tasks...)
PID 33289 moved to node(s) 1,3,5 in 56.3 seconds

Moving 26 tasks from nodes (0-7) to nodes (1,3,5,7)
numad(33209) do_migrate_pages (mm=0xffff88101d924400 from_nodes=0xffff88101e7b5d28 to_nodes=0xffff88101e7b5ce8 flags=0x4) numad(33209) migrate_to_node (mm=0xffff88101d924400 source=0x6 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d924400 source=0x4 dest=0x1 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d924400 source=0x2 dest=0x5 flags=0x4)
numad(33209) migrate_to_node (mm=0xffff88101d924400 source=0x0 dest=0x1 flags=0x4)
(Above moves repeated for each of the 26 tasks...)
PID 33372 moved to node(s) 1,3,5,7 in 92.67 seconds

Signed-off-by Larry Woodman<[email protected]>

mm/mempolicy.c +10 -0

index 47296fe..6c189fa 100644
@@ -1012,6 +1012,16 @@ int do_migrate_pages(struct mm_struct *mm,
 		int dest = 0;
 
 		for_each_node_mask(s, tmp) {
+
+			/* IFF there is an equal number of source and
+			 * destination nodes, maintain relative node distance
+			 * even when source and destination nodes overlap.
+			 * However, when the node weight is unequal, never move
+			 * memory out of any destination nodes */
+			if ((nodes_weight(*from_nodes) != nodes_weight(*to_nodes)) && 
+						(node_isset(s, *to_nodes)))
+				continue;
+
 			d = node_remap(s, *from_nodes, *to_nodes);
 			if (s == d)
 				continue;

Christoph Lameter

Acked-by on v2 1/1

2012-04-24 16:08 UTC #1 lore raw

Quoted 10 lines · Larry Woodman

While moving tasks between cpusets we noticed some strange behavior. Specifically if the nodes of the destination
cpuset are a subset of the nodes of the source cpuset do_migrate_pages() will move pages that are already on a node
in the destination cpuset. The reason for this is do_migrate_pages() does not check whether each node in the source
nodemask is in the destination nodemask before calling migrate_to_node(). If we simply do this check and skip them
when the source is in the destination moving we wont move nodes that dont need to be moved.

Acked-by Christoph Lameter <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

KOSAKI Motohiro

Acked-by on v2 1/1

2012-04-24 16:19 UTC #2 lore raw

In reply to Larry Woodman

Quoted 5 lines

mm/mempolicy.c +0 -0
index 47296fe..6c189fa 100644

mm/mempolicy.c int do_migrate_pages(struct mm_struct *mm, · line 1020

Quoted 9 lines

mm/mempolicy.c +6 -0

@@ -1012,6 +1012,16 @@ int do_migrate_pages(struct mm_struct *mm,
 		int dest = 0;
 
 		for_each_node_mask(s, tmp) {
+
+			/* IFF there is an equal number of source and
+			 * destination nodes, maintain relative node distance
+			 * even when source and destination nodes overlap.
+			 * However, when the node weight is unequal, never move
+			 * memory out of any destination nodes */

Please use

/*
* foo bar
*/

style comment. and this comment only explain how code work but don't explain why. I hope the comment describe HPC usecase require to migrate if src and dest have the same weight.

Otherwise looks ok. please feel free to use my ack to your next spin.

Acked-by KOSAKI Motohiro <[email protected]>

mm/mempolicy.c int do_migrate_pages(struct mm_struct *mm, · line 1024

Quoted 4 lines

mm/mempolicy.c +4 -0

+			if ((nodes_weight(*from_nodes) != nodes_weight(*to_nodes)) &&
+						(node_isset(s, *to_nodes)))
+				continue;
+

Larry Woodman

author

2012-04-24 17:16 UTC #3 lore raw

Quoted 30 lines · KOSAKI Motohiro

Quoted 5 lines
mm/mempolicy.c +0 -0
index 47296fe..6c189fa 100644
mm/mempolicy.c int do_migrate_pages(struct mm_struct *mm, · line 1020
Quoted 9 lines
mm/mempolicy.c +6 -0
@@ -1012,6 +1012,16 @@ int do_migrate_pages(struct mm_struct *mm,
         int dest = 0;
 
         for_each_node_mask(s, tmp) {
+
+            /* IFF there is an equal number of source and
+             * destination nodes, maintain relative node distance
+             * even when source and destination nodes overlap.
+             * However, when the node weight is unequal, never move
+             * memory out of any destination nodes */
Please use
/*
* foo bar
*/
style comment. and this comment only explain how code work but don't explain why. I hope the comment describe HPC usecase require to migrate if src and dest have the same weight.
Otherwise looks ok. please feel free to use my ack to your next spin.
Acked-by KOSAKI Motohiro <[email protected]>

Quoted 13 lines

Quoted 5 lines

mm/mempolicy.c +1 -0

+            if ((nodes_weight(*from_nodes) !=

nodes_weight(*to_nodes)) &&

mm/mempolicy.c +3 -0

+                        (node_isset(s, *to_nodes)))
+                continue;
+

Signature · 5 lines

To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

How does this look:

mm/mempolicy.c +18 -0

index f563fa3..b76b49a 100644
@@ -1012,6 +1012,24 @@ int do_migrate_pages(struct mm_struct *mm,
 		int dest = 0;
 
 		for_each_node_mask(s, tmp) {
+
+			/*
+			 * IFF there is an equal number of source and
+			 * destination nodes, maintain relative node distance
+			 * even when source and destination nodes overlap.
+			 * However, when the node weight is unequal/there are
+			 * a different number of source and destination nodes,
+			 * never move memory out of a source node that is also
+			 * a destination node.
+			 * 
+			 * Example: [2,3,4] -> [3,4,5] moves everything.
+			 * 	    [0-7] - > [3,4,5] moves only 0,1,2,6,7.
+			 */
+
+			if ((nodes_weight(*from_nodes) != nodes_weight(*to_nodes)) && 
+						(node_isset(s, *to_nodes)))
+				continue;
+
 			d = node_remap(s, *from_nodes, *to_nodes);
 			if (s == d)
 				continue;

KOSAKI Motohiro

Acked-by

2012-04-24 17:17 UTC #4 lore raw

Quoted 1 line

How does this look:

Acked-by KOSAKI Motohiro <[email protected]>

Rik van Riel

2012-04-24 17:21 UTC #5 lore raw

In reply to Larry Woodman

Quoted 8 lines · Larry Woodman

Quoted 6 lines · KOSAKI Motohiro
Please use
/*
* foo bar
*/

Quoted 7 lines

Quoted 5 lines
style comment. and this comment only explain how code work but don't explain why.
I hope the comment describe HPC usecase require to migrate if src and dest have the
same weight.
How does this look:

Still does not explain the "why"...

Signature · 8 lines

All rights reversed

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2012-04-24 18:17 UTC #6 lore raw

In reply to Larry Woodman

Quoted 1 line · Larry Woodman

How does this look:

Could you please send the patches inline? Its difficult to quote the attachment.

Larry Woodman

author

2012-04-24 20:08 UTC #7 lore raw

Quoted 7 lines · Christoph Lameter

Quoted 1 line · Larry Woodman
How does this look:
Could you please send the patches inline? Its difficult to quote the attachment.

Sorry all of these email clients are different.

mm/mempolicy.c +18 -0

index f563fa3..b76b49a 100644
@@ -1012,6 +1012,24 @@ int do_migrate_pages(struct mm_struct *mm,
                int dest = 0;
 
                for_each_node_mask(s, tmp) {
+
+                       /*
+                        * IFF there is an equal number of source and
+                        * destination nodes, maintain relative node distance
+                        * even when source and destination nodes overlap.
+                        * However, when the node weight is unequal/there are
+                        * a different number of source and destination nodes,
+                        * never move memory out of a source node that is also
+                        * a destination node.
+                        *
+                        * Example: [2,3,4] -> [3,4,5] moves everything.
+                        *          [0-7] - > [3,4,5] moves only 0,1,2,6,7.
+                        */
+
+                       if ((nodes_weight(*from_nodes) != nodes_weight(*to_nodes)) &&
+                                               (node_isset(s, *to_nodes)))
+                               continue;
+
                        d = node_remap(s, *from_nodes, *to_nodes);
                        if (s == d)
                                continue;

Rik van Riel

2012-04-24 20:11 UTC #8 lore raw

Quoted 10 lines · Larry Woodman

Quoted 7 lines · Christoph Lameter
Quoted 1 line · Larry Woodman
How does this look:
Could you please send the patches inline? Its difficult to quote the attachment.
Sorry all of these email clients are different.

Neither the comment or the changelog explains why you want to make this change.

Signature · 8 lines

All rights reversed

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>