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

Message-ID: <43b7a6e0-1940-ebe8-4005-d6d8021653c3@huawei.com>
Date:   Fri, 5 Feb 2021 11:08:12 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
CC:     <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm/hugetlb: Remove obsolete comment in vma_has_reserves()

Hi:
On 2021/2/5 5:32, Mike Kravetz wrote:
> 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.
> 

Yes. If I think about it this way, the comment is really making a distinction
between shared mappings and private mappings when not in VM_NORESERVE case.

> I would suggest either leaving the comment as is, or modifying to include

I'd like to leave the comment as is. Many thanks for detailed explanation.

> 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.
> 
Many thanks again. :)