Re: [PATCH] mm: migrate: Simplify the refcount validation when migrating hugetlb mapping

[Mike Kravetz]

On 4/11/22 04:34, Baolin Wang wrote:
> There is no need to validate the hugetlb page's refcount before trying
> to freeze the hugetlb page's expected refcount, instead we can just
> rely on the page_ref_freeze() to simplify the validation.
> 
> Moreover we are always under the page lock when migrating the hugetlb
> page mapping, which means nowhere else can remove it from the page cache,
> so we can remove the xas_load() validation under the i_pages lock.

I agree with the reasoning here.

> 
> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> ---
>  mm/migrate.c | 5 -----
>  1 file changed, 5 deletions(-)

Matthew added that extra xas_load() and check both here and in
migrate_page_move_mapping.  However, migrate_page_move_mapping has been
converted to folio and does not appear to have the same check.

Ideally we will convert hugetlbfs to folio someday and rewrite
migrate_huge_page_move_mapping as well.

Would be good to get Matthew's opinion.

Acked-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
-- 
Mike Kravetz

> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a3d8c2b..b267827 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -477,11 +477,6 @@ int migrate_huge_page_move_mapping(struct address_space *mapping,
>  
>  	xas_lock_irq(&xas);
>  	expected_count = 2 + page_has_private(page);
> -	if (page_count(page) != expected_count || xas_load(&xas) != page) {
> -		xas_unlock_irq(&xas);
> -		return -EAGAIN;
> -	}
> -
>  	if (!page_ref_freeze(page, expected_count)) {
>  		xas_unlock_irq(&xas);
>  		return -EAGAIN;