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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b30e9ae6-86b4-57f8-8484-d437996510bb@oracle.com>
Date:   Thu, 14 Apr 2022 15:07:16 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Baolin Wang <baolin.wang@...ux.alibaba.com>,
        akpm@...ux-foundation.org
Cc:     willy@...radead.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: migrate: Simplify the refcount validation when
 migrating hugetlb mapping

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@...ux.alibaba.com>
> Cc: Matthew Wilcox <willy@...radead.org>
> ---
>  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@...cle.com>
-- 
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;