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

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

[Miaohe Lin]

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 */
 	if (vma->vm_flags & VM_MAYSHARE) {
 		/*
 		 * We know VM_NORESERVE is not set.  Therefore, there SHOULD
-- 
2.19.1

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

[Mike Kravetz]

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
> 

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

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