mem-hotplug: alloc new page from the next node if zone is MOVABLE_ZONE

PATCH In review Xishi Qiu 3 participants 7 messages Jul 22, 2016 – Jul 25, 2016

Subsystems (2)

Memory management Memory hot(un)plug

lore

Share & tools

lore

Xishi Qiu

author

2016-07-22 02:58 UTC #0 lore raw

PATCH v2 1/1 mem-hotplug: alloc new page from the next node if zone is MOVABLE_ZONE

Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.

So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.

Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.

Signed-off-by Xishi Qiu <[email protected]>

---
 mm/memory_hotplug.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

mm/memory_hotplug.c +29 -6

index e3cbdca..930a5c6 100644
@@ -1501,6 +1501,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 	return 0;
 }
 
+static struct page *new_node_page(struct page *page, unsigned long node,
+		int **result)
+{
+	if (PageHuge(page))
+		return alloc_huge_page_node(page_hstate(compound_head(page)),
+					node);
+	else
+		return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE, 0);
+}
+
 #define NR_OFFLINE_AT_ONCE_PAGES	(256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)

@@ -1510,6 +1520,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 	int move_pages = NR_OFFLINE_AT_ONCE_PAGES;
 	int not_managed = 0;
 	int ret = 0;
+	int nid = NUMA_NO_NODE;
 	LIST_HEAD(source);
 
 	for (pfn = start_pfn; pfn < end_pfn && move_pages > 0; pfn++) {

@@ -1564,12 +1575,24 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 			goto out;
 		}
 
-		/*
-		 * alloc_migrate_target should be improooooved!!
-		 * migrate_pages returns # of failed pages.
-		 */
-		ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,
-					MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+		for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+			if (!pfn_valid(pfn))
+				continue;
+			page = pfn_to_page(pfn);
+			if (zone_idx(page_zone(page)) == ZONE_MOVABLE)
+				nid = next_node_in(page_to_nid(page),
+						node_online_map);
+			break;
+		}
+
+		/* Alloc new page from the next node if possible */
+		if (nid != NUMA_NO_NODE)
+			ret = migrate_pages(&source, new_node_page, NULL,
+					nid, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+		else
+			ret = migrate_pages(&source, alloc_migrate_target, NULL,
+					0, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+
 		if (ret)
 			putback_movable_pages(&source);
 	}

Signature · 1 line

1.8.3.1

Andrew Morton

maintainer

2016-07-22 20:11 UTC #1 lore raw

Quoted 5 lines · On Fri, 22 Jul 2016 10:57:48 +0800 Xishi Qiu <[email protected]> wrote:

Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.

Quoted 7 lines

So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.
Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.

This conflicts pretty significantly with your "mem-hotplug: use different mempolicy in alloc_migrate_target()". Does it replace "mem-hotplug: use different mempolicy in alloc_migrate_target()" and your "mem-hotplug: use GFP_HIGHUSER_MOVABLE in, alloc_migrate_target()", or what?

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Xishi Qiu

author

2016-07-25 01:56 UTC #2 lore raw

Quoted 12 lines · Andrew Morton

Quoted 10 lines · On Fri, 22 Jul 2016 10:57:48 +0800 Xishi Qiu <[email protected]> wrote:
Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.
So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.

Quoted 9 lines

Quoted 2 lines
Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.
This conflicts pretty significantly with your "mem-hotplug: use different mempolicy in alloc_migrate_target()". Does it replace "mem-hotplug: use different mempolicy in alloc_migrate_target()" and your "mem-hotplug: use GFP_HIGHUSER_MOVABLE in, alloc_migrate_target()", or what?

Hi Andrew,

Yes, this patch is v2, "mem-hotplug: use different mempolicy in alloc_migrate_target()" and "mem-hotplug: use GFP_HIGHUSER_MOVABLE in alloc_migrate_target()" are v1, so just replace them.

Joonsoo and Vlastimil point that migratable pages are not always from user space, so it is not correct to use GFP_HIGHUSER_MOVABLE in alloc_migrate_target().

David points that CMA and memory offline are distinct usecases and probably deserve their own callbacks.

Thanks,
Xishi Qiu

Quoted 3 lines

.

Vlastimil Babka

2016-07-25 06:59 UTC #3 lore raw

In reply to Xishi Qiu

Quoted 10 lines · Xishi Qiu

Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.
So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.

Quoted 4 lines

Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.
Signed-off-by Xishi Qiu <[email protected]>

I think this could be simpler, if you preferred the next node regardless of whether it's movable zone or not. What are use cases for trying to offline part of non-MOVABLE zone in a node? It's not guaranteed to succeed anyway. Also if the reasoning is that the non-MOVABLE offlining preference for migration target should be instead on the *same* node, then alloc_migrate_target() would anyway prefer the node of the current CPU that happens to execute the offlining, which is random wrt the node in question. So consistently choosing remote node is IMHO better than random even for non-MOVABLE zone.

Quoted 10 lines

---
 mm/memory_hotplug.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

mm/memory_hotplug.c +0 -0

index e3cbdca..930a5c6 100644
@@ -1501,6 +1501,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 	return 0;

mm/memory_hotplug.c static unsigned long scan_movable_pages(unsigned long start, unsigned long end) · line 1511

Quoted 10 lines

mm/memory_hotplug.c +8 -0

 }
 
+static struct page *new_node_page(struct page *page, unsigned long node,
+		int **result)
+{
+	if (PageHuge(page))
+		return alloc_huge_page_node(page_hstate(compound_head(page)),
+					node);
+	else
+		return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE, 0);

You could just test for page in movable (or highmem?) zone here in the callback.

Quoted 22 lines

mm/memory_hotplug.c +3 -5

+}
+
 #define NR_OFFLINE_AT_ONCE_PAGES	(256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)

@@ -1510,6 +1520,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 	int move_pages = NR_OFFLINE_AT_ONCE_PAGES;
 	int not_managed = 0;
 	int ret = 0;
+	int nid = NUMA_NO_NODE;
 	LIST_HEAD(source);
 
 	for (pfn = start_pfn; pfn < end_pfn && move_pages > 0; pfn++) {

@@ -1564,12 +1575,24 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 			goto out;
 		}
 
-		/*
-		 * alloc_migrate_target should be improooooved!!
-		 * migrate_pages returns # of failed pages.
-		 */
-		ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1586

Quoted 10 lines

mm/memory_hotplug.c +9 -1

-					MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+		for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+			if (!pfn_valid(pfn))
+				continue;
+			page = pfn_to_page(pfn);
+			if (zone_idx(page_zone(page)) == ZONE_MOVABLE)
+				nid = next_node_in(page_to_nid(page),
+						node_online_map);
+			break;
+		}

Then you could remove the ZONE_MOVABLE check here. I'm not sure how much worth the precalculation of nid is, if it has to be a rather complicated code like this, hm.

Also, since we know that "next node in node_online_map" is in fact not optimal, what about using the opportunity to really try the best possible way? Maybe it's as simple as allocating via __alloc_pages_nodemask() with current node's zonelist (where remote nodes should be already sorted according to NUMA distance), but with current node (which would be first in the zonelist) removed from the nodemask so that it's skipped over? But check if memory offlining process didn't kill the zonelist already at this point, or something.

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1594

Quoted 8 lines

mm/memory_hotplug.c +8 -0

+
+		/* Alloc new page from the next node if possible */
+		if (nid != NUMA_NO_NODE)
+			ret = migrate_pages(&source, new_node_page, NULL,
+					nid, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+		else
+			ret = migrate_pages(&source, alloc_migrate_target, NULL,
+					0, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);

Please just use one new callback fully tailored for memory offline, instead of choosing between the two like this.

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1598

Quoted 4 lines

mm/memory_hotplug.c +0 -0

 		if (ret)
 			putback_movable_pages(&source);
 	}

Xishi Qiu

author

2016-07-25 08:37 UTC #4 lore raw

Quoted 11 lines · Vlastimil Babka

Quoted 10 lines · Xishi Qiu
Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.
So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.

Quoted 7 lines

Quoted 4 lines
Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.
Signed-off-by Xishi Qiu <[email protected]>
I think this could be simpler, if you preferred the next node regardless of whether it's movable zone or not. What are use cases for trying to offline part of non-MOVABLE zone in a node? It's not guaranteed to succeed anyway. Also if the reasoning is that the non-MOVABLE offlining preference for migration target should be instead on the *same* node, then alloc_migrate_target() would anyway prefer the node of the current CPU that happens to execute the offlining, which is random wrt the node in question. So consistently choosing remote node is IMHO better than random even for non-MOVABLE zone.

Hi Vlastimil,

use next node for movable zone, use current node for non-movable zone, right?

Quoted 12 lines

---
 mm/memory_hotplug.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

mm/memory_hotplug.c +0 -0

index e3cbdca..930a5c6 100644
@@ -1501,6 +1501,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
     return 0;
 }

Quoted 11 lines

mm/memory_hotplug.c static unsigned long scan_movable_pages(unsigned long start, unsigned long end) · line 1511
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+static struct page *new_node_page(struct page *page, unsigned long node,
+        int **result)
+{
+    if (PageHuge(page))
+        return alloc_huge_page_node(page_hstate(compound_head(page)),
+                    node);
+    else
+        return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE, 0);
You could just test for page in movable (or highmem?) zone here in the callback.

is_highmem_idx() always return 0 if CONFIG_HIGHMEM closed.
And GFP_HIGHUSER_MOVABLE will choose movable_zone first, then normal_zone.
So how about this check? if (PageHighMem() or zone == ZONE_MOVABLE) then use GFP_HIGHUSER_MOVABLE

Quoted 35 lines

Quoted 22 lines

mm/memory_hotplug.c +3 -5

+}
+
 #define NR_OFFLINE_AT_ONCE_PAGES    (256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)

@@ -1510,6 +1520,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
     int move_pages = NR_OFFLINE_AT_ONCE_PAGES;
     int not_managed = 0;
     int ret = 0;
+    int nid = NUMA_NO_NODE;
     LIST_HEAD(source);
 
     for (pfn = start_pfn; pfn < end_pfn && move_pages > 0; pfn++) {

@@ -1564,12 +1575,24 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
             goto out;
         }
 
-        /*
-         * alloc_migrate_target should be improooooved!!
-         * migrate_pages returns # of failed pages.
-         */
-        ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1586

Quoted 10 lines

mm/memory_hotplug.c +9 -1

-                    MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+            if (!pfn_valid(pfn))
+                continue;
+            page = pfn_to_page(pfn);
+            if (zone_idx(page_zone(page)) == ZONE_MOVABLE)
+                nid = next_node_in(page_to_nid(page),
+                        node_online_map);
+            break;
+        }

Then you could remove the ZONE_MOVABLE check here. I'm not sure how much worth the precalculation of nid is, if it has to be a rather complicated code like this, hm.

Quoted 2 lines

Also, since we know that "next node in node_online_map" is in fact not optimal, what about using the opportunity to really try the best possible way? Maybe it's as simple as allocating via __alloc_pages_nodemask() with current node's zonelist (where remote nodes should be already sorted according to NUMA distance), but with current node (which would be first in the zonelist) removed from the nodemask so that it's skipped over? But check if memory offlining process didn't kill the zonelist already at this point, or something.

Do you mean that call __alloc_pages_nodemask(), the zonelist is from current page's node, but it(the current page's node) is not include in the nodemask?

Thanks,
Xishi Qiu

Quoted 19 lines

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1594
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+
+        /* Alloc new page from the next node if possible */
+        if (nid != NUMA_NO_NODE)
+            ret = migrate_pages(&source, new_node_page, NULL,
+                    nid, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        else
+            ret = migrate_pages(&source, alloc_migrate_target, NULL,
+                    0, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
Please just use one new callback fully tailored for memory offline, instead of choosing between the two like this.
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1598
Quoted 4 lines
mm/memory_hotplug.c +0 -0
         if (ret)
             putback_movable_pages(&source);
     }
.

Vlastimil Babka

2016-07-25 07:52 UTC #5 lore raw

Quoted 16 lines · Xishi Qiu

Quoted 14 lines · Vlastimil Babka
Quoted 13 lines · Xishi Qiu
Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.
So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.
Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Xishi Qiu <[email protected]>
I think this could be simpler, if you preferred the next node regardless of whether it's movable zone or not. What are use cases for trying to offline part of non-MOVABLE zone in a node? It's not guaranteed to succeed anyway. Also if the reasoning is that the non-MOVABLE offlining preference for migration target should be instead on the *same* node, then alloc_migrate_target() would anyway prefer the node of the current CPU that happens to execute the offlining, which is random wrt the node in question. So consistently choosing remote node is IMHO better than random even for non-MOVABLE zone.
Hi Vlastimil,
use next node for movable zone, use current node for non-movable zone, right?

I was asking why not just next node for any zone, to make things simpler. What's the use case for offlining in non-movable zone?

Quoted 25 lines

Quoted 13 lines

---
 mm/memory_hotplug.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

mm/memory_hotplug.c +0 -0

index e3cbdca..930a5c6 100644
@@ -1501,6 +1501,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
     return 0;
 }

Quoted 11 lines

mm/memory_hotplug.c static unsigned long scan_movable_pages(unsigned long start, unsigned long end) · line 1511
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+static struct page *new_node_page(struct page *page, unsigned long node,
+        int **result)
+{
+    if (PageHuge(page))
+        return alloc_huge_page_node(page_hstate(compound_head(page)),
+                    node);
+    else
+        return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE, 0);
You could just test for page in movable (or highmem?) zone here in the callback.

Quoted 1 line

is_highmem_idx() always return 0 if CONFIG_HIGHMEM closed.

Yeah, but then it doesn't matter if __GFP_HIGHMEM is given.

Quoted 2 lines

And GFP_HIGHUSER_MOVABLE will choose movable_zone first, then normal_zone.
So how about this check? if (PageHighMem() or zone == ZONE_MOVABLE) then use GFP_HIGHUSER_MOVABLE

zone == ZONE_MOVABLE -> GFP_HIGHUSER_MOVABLE
PageHighMem() -> GFP_HIGHUSER
else GFP_USER ?

Quoted 35 lines

Quoted 22 lines

mm/memory_hotplug.c +3 -5

+}
+
 #define NR_OFFLINE_AT_ONCE_PAGES    (256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)

@@ -1510,6 +1520,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
     int move_pages = NR_OFFLINE_AT_ONCE_PAGES;
     int not_managed = 0;
     int ret = 0;
+    int nid = NUMA_NO_NODE;
     LIST_HEAD(source);
 
     for (pfn = start_pfn; pfn < end_pfn && move_pages > 0; pfn++) {

@@ -1564,12 +1575,24 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
             goto out;
         }
 
-        /*
-         * alloc_migrate_target should be improooooved!!
-         * migrate_pages returns # of failed pages.
-         */
-        ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,

mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1586

Quoted 10 lines

mm/memory_hotplug.c +9 -1

-                    MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+            if (!pfn_valid(pfn))
+                continue;
+            page = pfn_to_page(pfn);
+            if (zone_idx(page_zone(page)) == ZONE_MOVABLE)
+                nid = next_node_in(page_to_nid(page),
+                        node_online_map);
+            break;
+        }

Then you could remove the ZONE_MOVABLE check here. I'm not sure how much worth the precalculation of nid is, if it has to be a rather complicated code like this, hm.

Quoted 5 lines

Quoted 2 lines
Also, since we know that "next node in node_online_map" is in fact not optimal, what about using the opportunity to really try the best possible way? Maybe it's as simple as allocating via __alloc_pages_nodemask() with current node's zonelist (where remote nodes should be already sorted according to NUMA distance), but with current node (which would be first in the zonelist) removed from the nodemask so that it's skipped over? But check if memory offlining process didn't kill the zonelist already at this point, or something.
Do you mean that call __alloc_pages_nodemask(), the zonelist is from current page's node, but it(the current page's node) is not include in the nodemask?

Exactly.
The question is if there should be a fallback to current page's node if that fails. Only make sense if somebody wants to offline only part of the node?

Quoted 26 lines

Thanks,
Xishi Qiu
Quoted 19 lines
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1594
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+
+        /* Alloc new page from the next node if possible */
+        if (nid != NUMA_NO_NODE)
+            ret = migrate_pages(&source, new_node_page, NULL,
+                    nid, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        else
+            ret = migrate_pages(&source, alloc_migrate_target, NULL,
+                    0, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
Please just use one new callback fully tailored for memory offline, instead of choosing between the two like this.
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1598
Quoted 4 lines
mm/memory_hotplug.c +0 -0
         if (ret)
             putback_movable_pages(&source);
     }
.

Xishi Qiu

author

2016-07-25 11:02 UTC #6 lore raw

Quoted 19 lines · Vlastimil Babka

Quoted 18 lines · Xishi Qiu
Quoted 16 lines · Vlastimil Babka
Quoted 14 lines · Xishi Qiu
Memory offline could happen on both movable zone and non-movable zone. We can offline the whole node if the zone is movable zone, and if the zone is non-movable zone, we cannot offline the whole node, because some kernel memory can't be migrated.
So if we offline a node with movable zone, use prefer mempolicy to alloc new page from the next node instead of the current node or other remote nodes, because re-migrate is a waste of time and the distance of the remote nodes is often very large.
Also use GFP_HIGHUSER_MOVABLE to alloc new page if the zone is movable zone.
Signed-off-by Xishi Qiu <[email protected]>

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
I think this could be simpler, if you preferred the next node regardless of whether it's movable zone or not. What are use cases for trying to offline part of non-MOVABLE zone in a node? It's not guaranteed to succeed anyway. Also if the reasoning is that the non-MOVABLE offlining preference for migration target should be instead on the *same* node, then alloc_migrate_target() would anyway prefer the node of the current CPU that happens to execute the offlining, which is random wrt the node in question. So consistently choosing remote node is IMHO better than random even for non-MOVABLE zone.
Hi Vlastimil,
use next node for movable zone, use current node for non-movable zone, right?
I was asking why not just next node for any zone, to make things simpler. What's the use case for offlining in non-movable zone?

OK, both use the next node is much simpler.

Quoted 27 lines

Quoted 25 lines
Quoted 13 lines
Quoted 13 lines
---
 mm/memory_hotplug.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)
mm/memory_hotplug.c +0 -0
index e3cbdca..930a5c6 100644
@@ -1501,6 +1501,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
     return 0;
 }
 
Quoted 11 lines
mm/memory_hotplug.c static unsigned long scan_movable_pages(unsigned long start, unsigned long end) · line 1511
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+static struct page *new_node_page(struct page *page, unsigned long node,
+        int **result)
+{
+    if (PageHuge(page))
+        return alloc_huge_page_node(page_hstate(compound_head(page)),
+                    node);
+    else
+        return __alloc_pages_node(node, GFP_HIGHUSER_MOVABLE, 0);
You could just test for page in movable (or highmem?) zone here in the callback.
Quoted 1 line
is_highmem_idx() always return 0 if CONFIG_HIGHMEM closed.

Quoted 7 lines

Yeah, but then it doesn't matter if __GFP_HIGHMEM is given.
Quoted 2 lines
And GFP_HIGHUSER_MOVABLE will choose movable_zone first, then normal_zone.
So how about this check? if (PageHighMem() or zone == ZONE_MOVABLE) then use GFP_HIGHUSER_MOVABLE
zone == ZONE_MOVABLE -> GFP_HIGHUSER_MOVABLE
PageHighMem() -> GFP_HIGHUSER

How about use GFP_HIGHUSER_MOVABLE in the above two cases? As Joonsoo said:

"all migratable pages are not from user space. For example, blockdev file cache has __GFP_MOVABLE and migratable but it has no __GFP_HIGHMEM and __GFP_USER.
And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE."

And you said this before "GFP_USER just specifies some reclaim flags"

Quoted 1 line

else GFP_USER ?

else use GFP_USER | __GFP_MOVABLE ?

Thanks,
Xishi Qiu

Quoted 75 lines

Quoted 36 lines
Quoted 36 lines
Quoted 23 lines
mm/memory_hotplug.c +3 -5
+}
+
 #define NR_OFFLINE_AT_ONCE_PAGES    (256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
@@ -1510,6 +1520,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
     int move_pages = NR_OFFLINE_AT_ONCE_PAGES;
     int not_managed = 0;
     int ret = 0;
+    int nid = NUMA_NO_NODE;
     LIST_HEAD(source);
 
     for (pfn = start_pfn; pfn < end_pfn && move_pages > 0; pfn++) {
@@ -1564,12 +1575,24 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
             goto out;
         }
 
-        /*
-         * alloc_migrate_target should be improooooved!!
-         * migrate_pages returns # of failed pages.
-         */
-        ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1586
Quoted 10 lines
mm/memory_hotplug.c +9 -1
-                    MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+            if (!pfn_valid(pfn))
+                continue;
+            page = pfn_to_page(pfn);
+            if (zone_idx(page_zone(page)) == ZONE_MOVABLE)
+                nid = next_node_in(page_to_nid(page),
+                        node_online_map);
+            break;
+        }
Then you could remove the ZONE_MOVABLE check here. I'm not sure how much worth the precalculation of nid is, if it has to be a rather complicated code like this, hm.
Quoted 5 lines
Quoted 2 lines
Also, since we know that "next node in node_online_map" is in fact not optimal, what about using the opportunity to really try the best possible way? Maybe it's as simple as allocating via __alloc_pages_nodemask() with current node's zonelist (where remote nodes should be already sorted according to NUMA distance), but with current node (which would be first in the zonelist) removed from the nodemask so that it's skipped over? But check if memory offlining process didn't kill the zonelist already at this point, or something.
Do you mean that call __alloc_pages_nodemask(), the zonelist is from current page's node, but it(the current page's node) is not include in the nodemask?
Exactly.
The question is if there should be a fallback to current page's node if that fails. Only make sense if somebody wants to offline only part of the node?
Quoted 26 lines
Thanks,
Xishi Qiu
Quoted 19 lines
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1594
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+
+        /* Alloc new page from the next node if possible */
+        if (nid != NUMA_NO_NODE)
+            ret = migrate_pages(&source, new_node_page, NULL,
+                    nid, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
+        else
+            ret = migrate_pages(&source, alloc_migrate_target, NULL,
+                    0, MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
Please just use one new callback fully tailored for memory offline, instead of choosing between the two like this.
mm/memory_hotplug.c do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) · line 1598
Quoted 4 lines
mm/memory_hotplug.c +0 -0
         if (ret)
             putback_movable_pages(&source);
     }
.
.