[PATCH v2] mm: migrate: fix missing update page_private to hugetlb_page_subpool

[PATCH v2] mm: migrate: fix missing update page_private to hugetlb_page_subpool

[Muchun Song]

Since commit d6995da31122 ("hugetlb: use page.private for hugetlb specific
page flags") converts page.private for hugetlb specific page flags. We
should use hugetlb_page_subpool() to get the subpool pointer instead of
page_private().

This 'could' prevent the migration of hugetlb pages. page_private(hpage)
is now used for hugetlb page specific flags.  At migration time, the
only flag which could be set is HPageVmemmapOptimized.  This flag will
only be set if the new vmemmap reduction feature is enabled.  In
addition, !page_mapping() implies an anonymous mapping.  So, this will
prevent migration of hugetb pages in anonymous mappings if the vmemmap
reduction feature is enabled.

In addition, that if statement checked for the rare race condition of a
page being migrated while in the process of being freed.  Since that
check is now wrong, we could leak hugetlb subpool usage counts.

The commit forgot to update it in the page migration routine. So fix it.

Fixes: d6995da31122 ("hugetlb: use page.private for hugetlb specific page flags")
Reported-by: Anshuman Khandual <anshuman.khandual@arm.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
---
Changeslog in v2:
 1. Update commit log suggested by Mike and Andrew.
 2. Fix compiler error when !CONFIG_HUGETLB_PAGE reported by Randy.

 Thanks.

 include/linux/hugetlb.h | 5 +++++
 mm/migrate.c            | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 9f192ce44088..e11a2e9aa514 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -915,6 +915,11 @@ static inline bool is_hugetlb_free_vmemmap_enabled(void)
 #else	/* CONFIG_HUGETLB_PAGE */
 struct hstate {};
 
+static inline struct hugepage_subpool *hugetlb_page_subpool(struct page *hpage)
+{
+	return NULL;
+}
+
 static inline int isolate_or_dissolve_huge_page(struct page *page,
 						struct list_head *list)
 {
diff --git a/mm/migrate.c b/mm/migrate.c
index 066b1e4e8bde..0692fc2638aa 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1292,7 +1292,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
 	 * page_mapping() set, hugetlbfs specific move page routine will not
 	 * be called and we could leak usage counts for subpools.
 	 */
-	if (page_private(hpage) && !page_mapping(hpage)) {
+	if (hugetlb_page_subpool(hpage) && !page_mapping(hpage)) {
 		rc = -EBUSY;
 		goto out_unlock;
 	}
-- 
2.11.0

Re: [PATCH v2] mm: migrate: fix missing update page_private to hugetlb_page_subpool

[Michal Hocko]

On Fri 21-05-21 10:27:47, Muchun Song wrote:
> Since commit d6995da31122 ("hugetlb: use page.private for hugetlb specific
> page flags") converts page.private for hugetlb specific page flags. We
> should use hugetlb_page_subpool() to get the subpool pointer instead of
> page_private().
> 
> This 'could' prevent the migration of hugetlb pages. page_private(hpage)
> is now used for hugetlb page specific flags.  At migration time, the
> only flag which could be set is HPageVmemmapOptimized.  This flag will
> only be set if the new vmemmap reduction feature is enabled.  In
> addition, !page_mapping() implies an anonymous mapping.  So, this will
> prevent migration of hugetb pages in anonymous mappings if the vmemmap
> reduction feature is enabled.
> 
> In addition, that if statement checked for the rare race condition of a
> page being migrated while in the process of being freed.  Since that
> check is now wrong, we could leak hugetlb subpool usage counts.
> 
> The commit forgot to update it in the page migration routine. So fix it.
> 
> Fixes: d6995da31122 ("hugetlb: use page.private for hugetlb specific page flags")
> Reported-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

The patch looks good to me. I hope Fixes tag will not confuse stable
backporters as this is a non issue for any released kernel.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
> Changeslog in v2:
>  1. Update commit log suggested by Mike and Andrew.
>  2. Fix compiler error when !CONFIG_HUGETLB_PAGE reported by Randy.
> 
>  Thanks.
> 
>  include/linux/hugetlb.h | 5 +++++
>  mm/migrate.c            | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 9f192ce44088..e11a2e9aa514 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -915,6 +915,11 @@ static inline bool is_hugetlb_free_vmemmap_enabled(void)
>  #else	/* CONFIG_HUGETLB_PAGE */
>  struct hstate {};
>  
> +static inline struct hugepage_subpool *hugetlb_page_subpool(struct page *hpage)
> +{
> +	return NULL;
> +}
> +
>  static inline int isolate_or_dissolve_huge_page(struct page *page,
>  						struct list_head *list)
>  {
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 066b1e4e8bde..0692fc2638aa 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1292,7 +1292,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
>  	 * page_mapping() set, hugetlbfs specific move page routine will not
>  	 * be called and we could leak usage counts for subpools.
>  	 */
> -	if (page_private(hpage) && !page_mapping(hpage)) {
> +	if (hugetlb_page_subpool(hpage) && !page_mapping(hpage)) {
>  		rc = -EBUSY;
>  		goto out_unlock;
>  	}
> -- 
> 2.11.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2] mm: migrate: fix missing update page_private to hugetlb_page_subpool

[Anshuman Khandual]

On 5/21/21 7:57 AM, Muchun Song wrote:
> Since commit d6995da31122 ("hugetlb: use page.private for hugetlb specific
> page flags") converts page.private for hugetlb specific page flags. We
> should use hugetlb_page_subpool() to get the subpool pointer instead of
> page_private().
> 
> This 'could' prevent the migration of hugetlb pages. page_private(hpage)
> is now used for hugetlb page specific flags.  At migration time, the
> only flag which could be set is HPageVmemmapOptimized.  This flag will
> only be set if the new vmemmap reduction feature is enabled.  In
> addition, !page_mapping() implies an anonymous mapping.  So, this will
> prevent migration of hugetb pages in anonymous mappings if the vmemmap
> reduction feature is enabled.
> 
> In addition, that if statement checked for the rare race condition of a
> page being migrated while in the process of being freed.  Since that
> check is now wrong, we could leak hugetlb subpool usage counts.
> 
> The commit forgot to update it in the page migration routine. So fix it.
> 
> Fixes: d6995da31122 ("hugetlb: use page.private for hugetlb specific page flags")
> Reported-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

Tested on all page size configs and HugeTLB migrations work as expected.

Tested-by: Anshuman Khandual <anshuman.khandual@arm.com> #arm64