Re: [PATCH 4/5] mm/hugetlb: simplify the code when alloc_huge_page() failed in hugetlb_no_page()

[Mike Kravetz <mike.kravetz@oracle.com>]

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@huawei.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@oracle.com>

-- 
Mike Kravetz

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