Skip to contentSkip to conversation

mem-hotplug: use GFP_HIGHUSER_MOVABLE in, alloc_migrate_target()

PATCH In review Xishi Qiu 4 participants 11 messages Jul 15, 2016 – Jul 19, 2016

Subsystems (4)
lore
Share & tools
lore

Xishi Qiu

author
#0 lore raw

PATCH 1/2 mem-hotplug: use GFP_HIGHUSER_MOVABLE in, alloc_migrate_target()

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

---
 mm/page_isolation.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
mm/page_isolation.c +2 -7
index 612122b..4f32c9f 100644
@@ -282,8 +282,6 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 struct page *alloc_migrate_target(struct page *page, unsigned long private,
 				  int **resultp)
 {
-	gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
-
 	/*
 	 * TODO: allocate a destination hugepage from a nearest neighbor node,
 	 * accordance with memory policy of the user process if possible. For
@@ -293,9 +291,6 @@ struct page *alloc_migrate_target(struct page *page, unsigned long private,
 		return alloc_huge_page_node(page_hstate(compound_head(page)),
 					    next_node_in(page_to_nid(page),
 							 node_online_map));
-
-	if (PageHighMem(page))
-		gfp_mask |= __GFP_HIGHMEM;
-
-	return alloc_page(gfp_mask);
+	else
+		return alloc_page(GFP_HIGHUSER_MOVABLE);
 }
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 2/2 mem-hotplug: use different mempolicy in alloc_migrate_target()

When we offline a node, the new page should alloced from other nodes instead of the current node, because re-migrate is a waste of time.
So use prefer mempolicy for hotplug, use default mempolicy for cma.

---
 include/linux/page-isolation.h | 2 +-
 mm/memory_hotplug.c            | 5 ++++-
 mm/page_alloc.c                | 2 +-
 mm/page_isolation.c            | 8 +++++---
 4 files changed, 11 insertions(+), 6 deletions(-)
include/linux/page-isolation.h +1 -1
index 047d647..c163de3 100644
@@ -65,7 +65,7 @@ undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 			bool skip_hwpoisoned_pages);
 
-struct page *alloc_migrate_target(struct page *page, unsigned long private,
+struct page *alloc_migrate_target(struct page *page, unsigned long nid,
 				int **resultp);
 
 #endif
mm/memory_hotplug.c +4 -1
index e3cbdca..b5963bf 100644
@@ -1510,12 +1510,15 @@ 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++) {
 		if (!pfn_valid(pfn))
 			continue;
 		page = pfn_to_page(pfn);
+		if (nid == NUMA_NO_NODE)
+			nid = next_node_in(page_to_nid(page), node_online_map);
 
 		if (PageHuge(page)) {
 			struct page *head = compound_head(page);
@@ -1568,7 +1571,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 		 * alloc_migrate_target should be improooooved!!
 		 * migrate_pages returns # of failed pages.
 		 */
-		ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,
+		ret = migrate_pages(&source, alloc_migrate_target, NULL, nid,
 					MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
 		if (ret)
 			putback_movable_pages(&source);
mm/page_alloc.c +1 -1
index 6903b69..b99f1c2 100644
@@ -7322,7 +7322,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		cc->nr_migratepages -= nr_reclaimed;
 
 		ret = migrate_pages(&cc->migratepages, alloc_migrate_target,
-				    NULL, 0, cc->mode, MR_CMA);
+				    NULL, NUMA_NO_NODE, cc->mode, MR_CMA);
 	}
 	if (ret < 0) {
 		putback_movable_pages(&cc->migratepages);
mm/page_isolation.c +5 -3
index 4f32c9f..f471be6 100644
@@ -279,18 +279,20 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 	return pfn < end_pfn ? -EBUSY : 0;
 }
 
-struct page *alloc_migrate_target(struct page *page, unsigned long private,
+struct page *alloc_migrate_target(struct page *page, unsigned long nid,
 				  int **resultp)
 {
 	/*
-	 * TODO: allocate a destination hugepage from a nearest neighbor node,
+	 * hugeTLB: allocate a destination page from a nearest neighbor node,
 	 * accordance with memory policy of the user process if possible. For
 	 * now as a simple work-around, we use the next node for destination.
+	 * Normal page: use prefer mempolicy for destination if called by
+	 * hotplug, use default mempolicy for destination if called by cma.
 	 */
 	if (PageHuge(page))
 		return alloc_huge_page_node(page_hstate(compound_head(page)),
 					    next_node_in(page_to_nid(page),
 							 node_online_map));
 	else
-		return alloc_page(GFP_HIGHUSER_MOVABLE);
+		return alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);
 }
Signature · 1 line
1.8.3.1
#2 lore raw

Re: [PATCH 2/2] mem-hotplug: use different mempolicy in alloc_migrate_target()

Quoted 3 lines · Xishi Qiu

When we offline a node, the new page should alloced from other nodes instead of the current node, because re-migrate is a waste of time.

Ugh, I'm surprised that it's not the case already. Maybe the allocation from same node is already prevented indirectly somehow?

Quoted 1 line

So use prefer mempolicy for hotplug, use default mempolicy for cma.

IMHO CMA should prefer the same node as the migrated page, if anything. Current task's mempolicy shouldn't restrict it (it's likely migrating pages of somebody else) or even guide its preferences. Ideally it would keep the original page's mempolicy, but we can't afford to track that...

Quoted 42 lines
---
 include/linux/page-isolation.h | 2 +-
 mm/memory_hotplug.c            | 5 ++++-
 mm/page_alloc.c                | 2 +-
 mm/page_isolation.c            | 8 +++++---
 4 files changed, 11 insertions(+), 6 deletions(-)
include/linux/page-isolation.h +1 -1
index 047d647..c163de3 100644
@@ -65,7 +65,7 @@ undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 			bool skip_hwpoisoned_pages);
 
-struct page *alloc_migrate_target(struct page *page, unsigned long private,
+struct page *alloc_migrate_target(struct page *page, unsigned long nid,
 				int **resultp);
 
 #endif
mm/memory_hotplug.c +3 -0
index e3cbdca..b5963bf 100644
@@ -1510,12 +1510,15 @@ 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++) {
 		if (!pfn_valid(pfn))
 			continue;
 		page = pfn_to_page(pfn);
+		if (nid == NUMA_NO_NODE)
+			nid = next_node_in(page_to_nid(page), node_online_map);
 
 		if (PageHuge(page)) {
 			struct page *head = compound_head(page);

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

Quoted 1 line
mm/memory_hotplug.c +0 -0
@@ -1568,7 +1571,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 		 * alloc_migrate_target should be improooooved!!

So, is this patch improooooving it enough that we can delete the comment? If not, could the patch be improooooved? :)

Quoted 25 lines
mm/memory_hotplug.c +1 -1
 		 * migrate_pages returns # of failed pages.
 		 */
-		ret = migrate_pages(&source, alloc_migrate_target, NULL, 0,
+		ret = migrate_pages(&source, alloc_migrate_target, NULL, nid,
 					MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
 		if (ret)
 			putback_movable_pages(&source);
mm/page_alloc.c +1 -1
index 6903b69..b99f1c2 100644
@@ -7322,7 +7322,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		cc->nr_migratepages -= nr_reclaimed;
 
 		ret = migrate_pages(&cc->migratepages, alloc_migrate_target,
-				    NULL, 0, cc->mode, MR_CMA);
+				    NULL, NUMA_NO_NODE, cc->mode, MR_CMA);
 	}
 	if (ret < 0) {
 		putback_movable_pages(&cc->migratepages);
mm/page_isolation.c +0 -0
index 4f32c9f..f471be6 100644

mm/page_isolation.c int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn, · line 286

Quoted 10 lines
mm/page_isolation.c +2 -2
@@ -279,18 +279,20 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 	return pfn < end_pfn ? -EBUSY : 0;
 }
 
-struct page *alloc_migrate_target(struct page *page, unsigned long private,
+struct page *alloc_migrate_target(struct page *page, unsigned long nid,
 				  int **resultp)
 {
 	/*
-	 * TODO: allocate a destination hugepage from a nearest neighbor node,
+	 * hugeTLB: allocate a destination page from a nearest neighbor node,

for hugeTLB it's still a TODO, by removing the word the rest of comment doesn't make much sense

Quoted 4 lines
mm/page_isolation.c +1 -0
 	 * accordance with memory policy of the user process if possible. For
 	 * now as a simple work-around, we use the next node for destination.
+	 * Normal page: use prefer mempolicy for destination if called by

mm/page_isolation.c int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn, · line 298

Quoted 10 lines
mm/page_isolation.c +2 -1
+	 * hotplug, use default mempolicy for destination if called by cma.
 	 */
 	if (PageHuge(page))
 		return alloc_huge_page_node(page_hstate(compound_head(page)),
 					    next_node_in(page_to_nid(page),
 							 node_online_map));
 	else
-		return alloc_page(GFP_HIGHUSER_MOVABLE);
+		return alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);
 }

--
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>

#3 lore raw
In reply to Xishi Qiu
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Thanks.

--
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>

#4 lore raw
Quoted 9 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Right, and there's also Minchan's series for arbitrary driver page migration...

Quoted 2 lines

Thanks.

--
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>

author
#5 lore raw
In reply to Joonsoo Kim
Quoted 8 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Hi Joonsoo,

So the original code "gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;" is not correct?

Quoted 2 lines

And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE.

Can we distinguish __GFP_MOVABLE or GFP_HIGHUSER_MOVABLE when doing mem-hotplug?

Thanks,
Xishi Qiu

Quoted 5 lines

Thanks.

.

#6 lore raw
Quoted 11 lines · Xishi Qiu
Quoted 8 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Quoted 4 lines

Hi Joonsoo,

So the original code "gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;" is not correct?

It's not incorrect. GFP_USER just specifies some reclaim flags, and may perhaps restrict allocation through __GFP_HARDWALL, where the original
page could have been allocated without the restriction. But it doesn't put the place in an unexpected address range, as placing a non-highmem page into highmem could. __GFP_MOVABLE then just controls a heuristic for placement within a zone.

Quoted 5 lines
Quoted 2 lines

And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE.

Can we distinguish __GFP_MOVABLE or GFP_HIGHUSER_MOVABLE when doing mem-hotplug?

I don't understand the question here, can you rephrase with more detail? Thanks.

Quoted 11 lines

Thanks,
Xishi Qiu

Quoted 5 lines

Thanks.

.

--
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>

author
#7 lore raw
Quoted 20 lines · Vlastimil Babka
Quoted 11 lines · Xishi Qiu
Quoted 8 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Quoted 4 lines

Hi Joonsoo,

So the original code "gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;" is not correct?

It's not incorrect. GFP_USER just specifies some reclaim flags, and may perhaps restrict allocation through __GFP_HARDWALL, where the original
page could have been allocated without the restriction. But it doesn't put the place in an unexpected address range, as placing a non-highmem page into highmem could. __GFP_MOVABLE then just controls a heuristic for placement within a zone.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE.

Can we distinguish __GFP_MOVABLE or GFP_HIGHUSER_MOVABLE when doing mem-hotplug?

I don't understand the question here, can you rephrase with more detail? Thanks.

Hi Joonsoo,

When we do memory offline, and the zone is movable zone,
can we use "alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);" to alloc a new page? the nid is the next node.

Thanks,
Xishi Qiu

Quoted 15 lines
Quoted 11 lines

Thanks,
Xishi Qiu

Quoted 5 lines

Thanks.

.

.

#8 lore raw
Quoted 25 lines · Xishi Qiu
Quoted 23 lines · Vlastimil Babka
Quoted 11 lines · Xishi Qiu
Quoted 8 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Quoted 4 lines

Hi Joonsoo,

So the original code "gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;" is not correct?

It's not incorrect. GFP_USER just specifies some reclaim flags, and may perhaps restrict allocation through __GFP_HARDWALL, where the original
page could have been allocated without the restriction. But it doesn't put the place in an unexpected address range, as placing a non-highmem page into highmem could. __GFP_MOVABLE then just controls a heuristic for placement within a zone.

Quoted 3 lines
Quoted 2 lines

And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Can we distinguish __GFP_MOVABLE or GFP_HIGHUSER_MOVABLE when doing mem-hotplug?

I don't understand the question here, can you rephrase with more detail? Thanks.

Hi Joonsoo,

Above is answered by Vlastimil. :)

Quoted 3 lines

When we do memory offline, and the zone is movable zone,
can we use "alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);" to alloc a new page? the nid is the next node.

I don't know much about memory offline, but, AFAIK, memory offline could happen on non-movable zone like as ZONE_NORMAL. Perhaps, you can add "if zone of the page is movable zone then alloc with GFP_HIGHUSER_MOVABLE".

Thanks.

--
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>

author
#9 lore raw
Quoted 40 lines · Joonsoo Kim
Quoted 25 lines · Xishi Qiu
Quoted 23 lines · Vlastimil Babka
Quoted 11 lines · Xishi Qiu
Quoted 8 lines · Joonsoo Kim
Quoted 2 lines · Xishi Qiu

alloc_migrate_target() is called from migrate_pages(), and the page is always from user space, so we can add __GFP_HIGHMEM directly.

No, 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.

Quoted 4 lines

Hi Joonsoo,

So the original code "gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;" is not correct?

It's not incorrect. GFP_USER just specifies some reclaim flags, and may perhaps restrict allocation through __GFP_HARDWALL, where the original
page could have been allocated without the restriction. But it doesn't put the place in an unexpected address range, as placing a non-highmem page into highmem could. __GFP_MOVABLE then just controls a heuristic for placement within a zone.

Quoted 3 lines
Quoted 2 lines

And, zram's memory isn't GFP_HIGHUSER_MOVABLE but has __GFP_MOVABLE.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Can we distinguish __GFP_MOVABLE or GFP_HIGHUSER_MOVABLE when doing mem-hotplug?

I don't understand the question here, can you rephrase with more detail? Thanks.

Hi Joonsoo,

Above is answered by Vlastimil. :)

Quoted 3 lines

When we do memory offline, and the zone is movable zone,
can we use "alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);" to alloc a new page? the nid is the next node.

Quoted 6 lines

I don't know much about memory offline, but, AFAIK, memory offline could happen on non-movable zone like as ZONE_NORMAL. Perhaps, you can add "if zone of the page is movable zone then alloc with GFP_HIGHUSER_MOVABLE".

Thanks.

Hi Joonsoo and Vlastimil,

Memory offline could happen on both movable zone and non-movable zone, and we can offline the whole node if the zone is movable_zone(the node only has one movable_zone), and if the zone is normal_zone, we cannot offline the whole node, because some kernel memory can't be migrated.

So how about change alloc_migrate_target() to alloc memory from the next node with GFP_HIGHUSER_MOVABLE, if the offline zone is movable_zone.

And if the offline zone is normal_zone, we don't change anything, that means the new page may be from the same node.

Thanks,
Xishi Qiu

Quoted 2 lines

.

#10 lore raw
Quoted 8 lines · Xishi Qiu

Memory offline could happen on both movable zone and non-movable zone, and we can offline the whole node if the zone is movable_zone(the node only has one movable_zone), and if the zone is normal_zone, we cannot offline the whole node, because some kernel memory can't be migrated.

So how about change alloc_migrate_target() to alloc memory from the next node with GFP_HIGHUSER_MOVABLE, if the offline zone is movable_zone.

I think sharing alloc_migrate_target as a migration callback may not be worth it; CMA and memory offline are distinct usecases and probably deserve their own callbacks.

--
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>