mm/migrate: do not call prep_transhuge_page if !thp_migration_supported

PATCH In review Andrea Reale 2 participants 4 messages Nov 20, 2017 MM migration

lore

Share & tools

lore

Andrea Reale

author

2017-11-20 16:17 UTC #0 lore raw

cover mm/migrate: do not call prep_transhuge_page if !thp_migration_supported

Hi all,

While working on a second round of patches to introduce memory hotplug / hot remove to arm64 (original attempt at [1]), and trying to rebase them on 4.14, I stumbled in some problematic behaviour during page migration when offlining for hot remove, whereas target pages for migration where found in a bad state.

I believe that this behaviour may be originating from commit 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration") and it is triggered when migrating pages for builds that have no ARCH_ENABLE_THP_MIGRATION but have thp support on (arm64, in my case). Specifically, my understanding is that prep_transhuge_page should not be called in linux/migrate.h:new_page_nodemask if !thp_migration_supported because, in that case, new_page would not be a thp.

A tiny patch follows, showing what I mean. Please, let me know if my understanding is bogus.

Best regards,
Andrea

[1] https://lkml.org/lkml/2017/4/11/536

Andrea Reale (1):
mm/migrate: do not call prep_transhuge_page if
!thp_migration_supported

 include/linux/migrate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Signature · 1 line

2.7.4

Andrea Reale

author

2017-11-20 16:18 UTC #1 lore raw

PATCH 1/1 mm/migrate: do not call prep_transhuge_page if !thp_migration_supported

new_page_nodemask in linux/migrate.h should not call prep_transhuge_page if thp_migration_support is false.

Fixes commit 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration")

Signed-off-by Andrea Reale <[email protected]>

---
 include/linux/migrate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/migrate.h +1 -1

index 895ec0c..725eac5 100644
@@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
 	new_page = __alloc_pages_nodemask(gfp_mask, order,
 				preferred_nid, nodemask);
 
-	if (new_page && PageTransHuge(page))
+	if (thp_migration_supported() && new_page && PageTransHuge(page))
 		prep_transhuge_page(new_page);
 
 	return new_page;

Signature · 1 line

2.7.4

Zi Yan

2017-11-20 16:33 UTC #2 lore raw

Hi Andrea,

Thanks for pointing this out.

I think the problem is that we should not prep_transhuge_page(new_page) unless new_page is allocated and PageTransHuge(). And several lines above, the allocation flags and the order is changed to make new_page a THP only if thp_migration_supported() is true.

Does the patch below look better?

include/linux/migrate.h +1 -1

index 895ec0c..725eac5 100644
@@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
 	new_page = __alloc_pages_nodemask(gfp_mask, order,
 				preferred_nid, nodemask);
 
-	if (new_page && PageTransHuge(page))
+	if (new_page && PageTransHuge(new_page))
 		prep_transhuge_page(new_page);
 
 	return new_page;

Signature · 3 lines

--
Best Regards
Yan Zi

Quoted 26 lines · Andrea Reale

new_page_nodemask in linux/migrate.h should not call prep_transhuge_page if thp_migration_support is false.
Fixes commit 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration")
Signed-off-by Andrea Reale <[email protected]>
---
 include/linux/migrate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/migrate.h +1 -1
index 895ec0c..725eac5 100644
@@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
 	new_page = __alloc_pages_nodemask(gfp_mask, order,
 				preferred_nid, nodemask);
 
-	if (new_page && PageTransHuge(page))
+	if (thp_migration_supported() && new_page && PageTransHuge(page))
 		prep_transhuge_page(new_page);
 
 	return new_page;
Signature · 1 line
2.7.4

Andrea Reale

author

2017-11-20 17:07 UTC #3 lore raw

Hi Yan Zi,

thanks for the feedback. Yes, it looks definitely less redundant :)

Regards,
Andrea

Quoted 59 lines · Zi Yan

Hi Andrea,
Thanks for pointing this out.
I think the problem is that we should not prep_transhuge_page(new_page) unless new_page is allocated and PageTransHuge(). And several lines above, the allocation flags and the order is changed to make new_page a THP only if thp_migration_supported() is true.
Does the patch below look better?
include/linux/migrate.h +1 -1
index 895ec0c..725eac5 100644
@@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
 	new_page = __alloc_pages_nodemask(gfp_mask, order,
 				preferred_nid, nodemask);
 
-	if (new_page && PageTransHuge(page))
+	if (new_page && PageTransHuge(new_page))
 		prep_transhuge_page(new_page);
 
 	return new_page;
Signature · 3 lines
--
Best Regards
Yan Zi
Quoted 26 lines · Andrea Reale
new_page_nodemask in linux/migrate.h should not call prep_transhuge_page if thp_migration_support is false.
Fixes commit 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration")
Signed-off-by Andrea Reale <[email protected]>
---
 include/linux/migrate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/migrate.h +1 -1
index 895ec0c..725eac5 100644
@@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
 	new_page = __alloc_pages_nodemask(gfp_mask, order,
 				preferred_nid, nodemask);
 
-	if (new_page && PageTransHuge(page))
+	if (thp_migration_supported() && new_page && PageTransHuge(page))
 		prep_transhuge_page(new_page);
 
 	return new_page;
Signature · 1 line
2.7.4