[Linux-stable-mirror] [PATCH] mm: migrate: fix an incorrect call of prep_transhuge_page()
From: Zi Yan zi.yan@cs.rutgers.edu
In [1], Andrea reported that during memory hotplug/hot remove prep_transhuge_page() is called incorrectly on non-THP pages for migration, when THP is on but THP migration is not enabled. This leads to a bad state of target pages for migration.
This patch fixes it by only calling prep_transhuge_page() when we are certain that the target page is THP.
[1] https://lkml.org/lkml/2017/11/20/411
Cc: stable@vger.kernel.org # v4.14 Fixes: 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration") Reported-by: Andrea Reale ar@linux.vnet.ibm.com Signed-off-by: Zi Yan zi.yan@cs.rutgers.edu Cc: Naoya Horiguchi n-horiguchi@ah.jp.nec.com Cc: "Jérôme Glisse" jglisse@redhat.com --- include/linux/migrate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 895ec0c4942e..a2246cf670ba 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -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;
On Mon, 20 Nov 2017 21:18:55 -0500 Zi Yan zi.yan@sent.com wrote:
In [1], Andrea reported that during memory hotplug/hot remove prep_transhuge_page() is called incorrectly on non-THP pages for migration, when THP is on but THP migration is not enabled. This leads to a bad state of target pages for migration.
This patch fixes it by only calling prep_transhuge_page() when we are certain that the target page is THP.
What are the user-visible effects of the bug?
On Mon 20-11-17 21:18:55, Zi Yan wrote:
From: Zi Yan zi.yan@cs.rutgers.edu
In [1], Andrea reported that during memory hotplug/hot remove prep_transhuge_page() is called incorrectly on non-THP pages for migration, when THP is on but THP migration is not enabled. This leads to a bad state of target pages for migration.
This patch fixes it by only calling prep_transhuge_page() when we are certain that the target page is THP.
lkml.org tends to be quite unstable so a http://lkml.kernel.org/r/%24msg-id is usually a preferred way.
Cc: stable@vger.kernel.org # v4.14 Fixes: 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration") Reported-by: Andrea Reale ar@linux.vnet.ibm.com Signed-off-by: Zi Yan zi.yan@cs.rutgers.edu Cc: Naoya Horiguchi n-horiguchi@ah.jp.nec.com Cc: "Jérôme Glisse" jglisse@redhat.com
include/linux/migrate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 895ec0c4942e..a2246cf670ba 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -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);
I would keep the two checks consistent. But that leads to a more interesting question. new_page_nodemask does
if (thp_migration_supported() && PageTransHuge(page)) { order = HPAGE_PMD_ORDER; gfp_mask |= GFP_TRANSHUGE; }
How come it is safe to allocate an order-0 page if !thp_migration_supported() when we are about to migrate THP? This doesn't make any sense to me. Are we working around this somewhere else? Why shouldn't we simply return NULL here?
Nayoa, could you explain please? 8135d8926c08 ("mm: memory_hotplug: memory hotremove supports thp migration") changelog is less than satisfactory.
On Wed 22-11-17 09:54:16, Michal Hocko wrote:
On Mon 20-11-17 21:18:55, Zi Yan wrote:
[...]
diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 895ec0c4942e..a2246cf670ba 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -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);
I would keep the two checks consistent. But that leads to a more interesting question. new_page_nodemask does
if (thp_migration_supported() && PageTransHuge(page)) { order = HPAGE_PMD_ORDER; gfp_mask |= GFP_TRANSHUGE; }
And one more question/note. Why do we need thp_migration_supported in the first place? 9c670ea37947 ("mm: thp: introduce CONFIG_ARCH_ENABLE_THP_MIGRATION") says : Introduce CONFIG_ARCH_ENABLE_THP_MIGRATION to limit thp migration : functionality to x86_64, which should be safer at the first step.
but why is unsafe to enable the feature on other arches which support THP? Is there any plan to do the next step and remove this config option?
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Michal Hocko -
Zi Yan