Re: [PATCH] mm/hugetlb: Remove obsolete comment in vma_has_reserves()

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

On 2/4/21 3:15 AM, Miaohe Lin wrote:
> Shared mappings are allowed to be created without reservations since
> commit c37f9fb11c97 ("hugetlb: allow huge page mappings to be created
> without reservations"). Remove this obsolete comment which may cause
> confusing.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/hugetlb.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 9501ec6ad517..cf82629319ed 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -998,7 +998,6 @@ static bool vma_has_reserves(struct vm_area_struct *vma, long chg)
>  			return false;
>  	}
>  
> -	/* Shared mappings always use reserves */

I would not say the comment is entirely obsolete or does not apply here.

As mentioned in the commit message, commit c37f9fb11c97 allowed hugetlb
mappings to be created without reserves.  It does this by supporting the
MAP_NORESERVE flag which corresponds to the VM_NORESERVE vma flag.

Right before this comment, a check is made for VM_NORESERVE and the
routine will return.  Therefore, by the time we get to this comment
we know MAP_NORESERVE does not apply and there are reserves associated
the shared mapping.  In this case the comment is making a distinction
between shared mappings which will always have reserves, and private
mappings which may or may not have reserves depending on ownership.

I would suggest either leaving the comment as is, or modifying to include
the information above.  To me, the three distinct blocks of code handling
the NORESERVE, shared and private cases makes things fairly clear and
the comment does apply in that context.
-- 
Mike Kravetz

>  	if (vma->vm_flags & VM_MAYSHARE) {
>  		/*
>  		 * We know VM_NORESERVE is not set.  Therefore, there SHOULD
>