linux-kernel - Re: [PATCH 4/5] mm/hugetlb: simplify the code when alloc_huge_page() failed in hugetlb_no

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 for Android: free password hash cracker in your pocket

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

Message-ID: <987f50e0-5511-7f53-c4b9-d8878d840bb5@oracle.com>
Date:   Fri, 12 Mar 2021 11:58:12 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 4/5] mm/hugetlb: simplify the code when alloc_huge_page()
 failed in hugetlb_no_page()

On 3/8/21 3:28 AM, Miaohe Lin wrote:
> Rework the error handling code when alloc_huge_page() failed to remove some
> duplicated code and simplify the code slightly.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
>  mm/hugetlb.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 695603071f2c..69b8de866a24 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4337,13 +4337,10 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
>  			 * sure there really is no pte entry.
>  			 */
>  			ptl = huge_pte_lock(h, mm, ptep);
> -			if (!huge_pte_none(huge_ptep_get(ptep))) {
> -				ret = 0;
> -				spin_unlock(ptl);
> -				goto out;
> -			}
> +			ret = 0;
> +			if (huge_pte_none(huge_ptep_get(ptep)))
> +				ret = vmf_error(PTR_ERR(page));

This new code is simpler.

The !huge_pte_none() catches an unlikely race.  IMO, the existing code
made that very clear.  Would have been even more clear with an unlikely
modifier.  In any case, the lengthy comment above this code makes it
clear why the check is there.  Code changes are fine.

Reviewed-by: Mike Kravetz <mike.kravetz@...cle.com>

-- 
Mike Kravetz

>  			spin_unlock(ptl);
> -			ret = vmf_error(PTR_ERR(page));
>  			goto out;
>  		}
>  		clear_huge_page(page, address, pages_per_huge_page(h));
>