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@xxxxxxx>
> Reported-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> Reviewed-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
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@xxxxxxxx>
> ---
> 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
