Re: [PATCH] mm/hugetlb.c: fix unnecessary address expansion of pmd sharing

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

On 1/3/21 11:10 PM, Li Xinhai wrote:
> 
> 
> On 1/4/21 11:55 AM, Mike Kravetz wrote:
>> I believe the only case where your patch produced incorrect results is
>> when the range was within a vma that was smaller than PUD_SIZE.  Do you
>> agree?
>>
> Not exactly. We need to consider for vma which span at least one
> PUD_SIZE after align its vm_start and vm_end.
> 

I know that I provided an incorrect example again.  Sorry (again)!

Can you provide an example where adding the simple check for vma size less
than PUD_SIZE to your original patch will not work.  The logic in your V2
patch is correct.  However, I am having a hard time finding a problem with
this simpler approach.
-- 
Mike Kravetz


>> If that is the case, then how about just adding the following to your patch?
>> I think this is simpler and faster than the 'range_in_vma' checking I proposed.
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 49990c0a02a3..716d1e58a7ae 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -5261,7 +5261,9 @@ void adjust_range_if_pmd_sharing_possible(struct vm_area_struct *vma,
>>   {
>>       unsigned long a_start, a_end;
>>   -    if (!(vma->vm_flags & VM_MAYSHARE))
>> +    /* Quick check for vma capable of pmd sharing */
>> +    if (!(vma->vm_flags & VM_MAYSHARE) ||
>> +        (vma->vm_start - vma->vm_end) < PUD_SIZE)
>>           return;
>>         /* Extend the range to be PUD aligned for a worst case scenario */
> 
> Your suggestion is good, we are able to simplify the code a bit, with
> less comparison, and maybe easier to follow. I will send a new patch to
> review.