Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()

From: Anshuman Khandual
Date: Tue Jul 11 2017 - 00:30:46 EST


On 07/10/2017 07:19 PM, Michal Hocko wrote:
> On Mon 10-07-17 16:40:59, Anshuman Khandual wrote:
>> As 'delta' is an unsigned long, 'end' (vma->vm_end + delta) cannot
>> be less than 'vma->vm_end'.
>
> This just doesn't make any sense. This is exactly what the overflow
> check is for. Maybe vm_end + delta can never overflow because of
> (old_len == vma->vm_end - addr) and guarantee old_len < new_len
> in mremap but I haven't checked that too deeply.

Irrespective of that, just looking at the variables inside this
particular function where delta is an 'unsigned long', 'end' cannot
be less than vma->vm_end. Is not that true ?

>
>> Checking for availability of virtual
>> address range at the end of the VMA for the incremental size is
>> also reduntant at this point. Hence drop them both.
>
> OK, this seems to be the case due the above (comment says "old_len
> exactly to the end of the area..").

yeah but is the check necessary ?

>
> But I am wondering what led you to the patch because you do not say so

As can be seen in the test program, was trying to measure the speed
of VMA expansion and contraction inside an address space and then
figured out that dropping this check improves the speed prima facie.


> here. This is hardly something that would save many cycles in a
> relatively cold path.

Though I have not done any detailed instruction level measurement,
there is a reduction in real and system amount of time to execute
the test with and without the patch.

Without the patch

real 0m2.100s
user 0m0.162s
sys 0m1.937s

With this patch

real 0m0.928s
user 0m0.161s
sys 0m0.756s