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

[Miaohe Lin]

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@xxxxxxxxxx>
>> ---
>>  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. :)