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

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

On 1/4/21 6:38 PM, Li Xinhai wrote:
> 
> 
> On 1/5/21 10:10 AM, Li Xinhai wrote:
>>
>>
>> On 1/5/21 2:59 AM, Mike Kravetz wrote:
>>> 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.
>>>
>>
>>
>> Thanks for checking. An example like this:
>> (vm_start, vm_end) = (2G - 6M, 2G+8M), so this vma is bigger than
>> PUD_SIZE, and the check for vma size bigger than PUD_SIZE will pass.
>>
>> With (start, end) = (2G-4M, 2G-2M), the previous patch will not adjust
>> start(because adjust it from (2G-4M) to 1G will exceeding vm_start), but
>> it will adjust end from (2G-2M) to 2G (because 2G still below vm_end).
>>
>> The adjustment of end is incorrect, because (2G-6M, 2G) range of vma
>> is not allowed for PMD sharing(i.e., that range do not fully occupy
>> PUD_SIZE). If we make the wrong adjustment, then we will unnecessarily
>> impact to range (2G-2M, 2G).
>>
>>
> sorry, the above example is not correct for vma size, update it as
> below:
> 
> (vm_start, vm_end) = (2G - 600M, 2G+800M), so this vma is bigger than
> PUD_SIZE, and the check for vma size bigger than PUD_SIZE will pass.
> 
> With (start, end) = (2G-4M, 2G-2M), the previous patch will not adjust
> start(because adjust it from (2G-4M) to 1G will exceeding vm_start), but
> it will adjust end from (2G-2M) to 2G (because 2G still below vm_end).
> 
> The adjustment of end is incorrect, because (2G-600M, 2G) range of vma
> is not allowed for PMD sharing(i.e., that range do not fully occupy
> PUD_SIZE). If we make the wrong adjustment, then we will unnecessarily
> impact to range (2G-2M, 2G).

Thanks you for the example!

This routine is conceptually simple, but for some reason there have been
multiple problems in the implementation.

-- 
Mike Kravetz