linux-kernel - Re: [PATCH] mm/hugetlb: Remove obsolete comment in vma_has

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8f2f3bac-5b33-a560-9b84-b50972739636@oracle.com>
Date:   Thu, 4 Feb 2021 13:32:54 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/hugetlb: Remove obsolete comment in vma_has_reserves()

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@...wei.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
>