Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1684146 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-10 13:20 +0200
    Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-10 15:50 +0200
      Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 06:30 +0200
        Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-11 08:10 +0200
          Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Vlastimil Babka <vbabka@suse.cz> - 2017-07-11 08:30 +0200
            Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-11 09:00 +0200
              Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Vlastimil Babka <vbabka@suse.cz> - 2017-07-11 09:00 +0200
                Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-11 09:20 +0200
                  Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-11 09:30 +0200
                    Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 13:20 +0200
                  Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 13:20 +0200
                Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 13:10 +0200
                  Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Michal Hocko <mhocko@kernel.org> - 2017-07-11 13:30 +0200
              Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 13:10 +0200
            Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 11:50 +0200
      Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-11 06:40 +0200

#1684146 — [RFC] mm/mremap: Remove redundant checks inside vma_expandable()

As 'delta' is an unsigned long, 'end' (vma->vm_end + delta) cannot
be less than 'vma->vm_end'. 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.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---

The following test program achieves fatser execution time with
this change.

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/mman.h>
#include <sys/time.h>

#define ALLOC_SIZE 0x10000UL
#define MAX_COUNT 1024 * 1024

int main(int argc, char *argv[])
{
        unsigned long count;
        char *ptr;

        ptr = mmap(NULL, ALLOC_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE| MAP_ANONYMOUS, -1, 0);
        if (ptr == MAP_FAILED) {
                perror("map() failed");
                return -1;
        }
        memset(ptr, 0, ALLOC_SIZE);

        for (count = 1; count <= MAX_COUNT; count++) {
                ptr =  (char *) mremap(ptr, ALLOC_SIZE * count, ALLOC_SIZE * (count + 1), 1);
                if (ptr == MAP_FAILED) {
                        perror("mremap() failed");
                        printf("At %lu size", ALLOC_SIZE * (count + 1));
                        return -1;
                }
                /*
                memset(ptr, 0, ALLOC_SIZE * (count + 1));
                */
        }


        for (count = MAX_COUNT; count > 1; count--) {
                ptr =  (char *) mremap(ptr, ALLOC_SIZE * count, ALLOC_SIZE * (count - 1), 1);
                if (ptr == MAP_FAILED) {
                        perror("mremap() failed");
                        printf("At %lu size", ALLOC_SIZE * (count - 1));
                        return -1;
                }
                /*
                memset(ptr, 0, ALLOC_SIZE * (count - 1));
                */
        }
        return 0;
}


 mm/mremap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index cd8a1b1..b937c28 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -487,12 +487,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
 static int vma_expandable(struct vm_area_struct *vma, unsigned long delta)
 {
 	unsigned long end = vma->vm_end + delta;
-	if (end < vma->vm_end) /* overflow */
-		return 0;
-	if (vma->vm_next && vma->vm_next->vm_start < end) /* intersection */
-		return 0;
-	if (get_unmapped_area(NULL, vma->vm_start, end - vma->vm_start,
-			      0, MAP_FIXED) & ~PAGE_MASK)
+
+	/* Intersection with next VMA */
+	if (vma->vm_next && vma->vm_next->vm_start < end)
 		return 0;
 	return 1;
 }
-- 
1.8.5.2

[toc] | [next] | [standalone]

#1684251

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.

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

But I am wondering what led you to the patch because you do not say so
here. This is hardly something that would save many cycles in a
relatively cold path.

> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> 
> The following test program achieves fatser execution time with
> this change.
> 
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
> #include <errno.h>
> #include <sys/mman.h>
> #include <sys/time.h>
> 
> #define ALLOC_SIZE 0x10000UL
> #define MAX_COUNT 1024 * 1024
> 
> int main(int argc, char *argv[])
> {
>         unsigned long count;
>         char *ptr;
> 
>         ptr = mmap(NULL, ALLOC_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE| MAP_ANONYMOUS, -1, 0);
>         if (ptr == MAP_FAILED) {
>                 perror("map() failed");
>                 return -1;
>         }
>         memset(ptr, 0, ALLOC_SIZE);
> 
>         for (count = 1; count <= MAX_COUNT; count++) {
>                 ptr =  (char *) mremap(ptr, ALLOC_SIZE * count, ALLOC_SIZE * (count + 1), 1);
>                 if (ptr == MAP_FAILED) {
>                         perror("mremap() failed");
>                         printf("At %lu size", ALLOC_SIZE * (count + 1));
>                         return -1;
>                 }
>                 /*
>                 memset(ptr, 0, ALLOC_SIZE * (count + 1));
>                 */
>         }
> 
> 
>         for (count = MAX_COUNT; count > 1; count--) {
>                 ptr =  (char *) mremap(ptr, ALLOC_SIZE * count, ALLOC_SIZE * (count - 1), 1);
>                 if (ptr == MAP_FAILED) {
>                         perror("mremap() failed");
>                         printf("At %lu size", ALLOC_SIZE * (count - 1));
>                         return -1;
>                 }
>                 /*
>                 memset(ptr, 0, ALLOC_SIZE * (count - 1));
>                 */
>         }
>         return 0;
> }
> 
> 
>  mm/mremap.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index cd8a1b1..b937c28 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -487,12 +487,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>  static int vma_expandable(struct vm_area_struct *vma, unsigned long delta)
>  {
>  	unsigned long end = vma->vm_end + delta;
> -	if (end < vma->vm_end) /* overflow */
> -		return 0;
> -	if (vma->vm_next && vma->vm_next->vm_start < end) /* intersection */
> -		return 0;
> -	if (get_unmapped_area(NULL, vma->vm_start, end - vma->vm_start,
> -			      0, MAP_FIXED) & ~PAGE_MASK)
> +
> +	/* Intersection with next VMA */
> +	if (vma->vm_next && vma->vm_next->vm_start < end)
>  		return 0;
>  	return 1;
>  }
> -- 
> 1.8.5.2

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684768

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

[toc] | [prev] | [next] | [standalone]

#1684792

On Tue 11-07-17 09:58:42, Anshuman Khandual wrote:
> 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 ?

no. What happens when end is too large?

[...]

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

Are you telling me that two if conditions cause more than a second
difference? That sounds suspicious.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684798

On 07/11/2017 08:03 AM, Michal Hocko wrote:
> On Tue 11-07-17 09:58:42, Anshuman Khandual wrote:
>>> 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
> 
> Are you telling me that two if conditions cause more than a second
> difference? That sounds suspicious.

It's removing also a call to get_unmapped_area(), AFAICS. That means a
vma search?

[toc] | [prev] | [next] | [standalone]

#1684812

On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
> On 07/11/2017 08:03 AM, Michal Hocko wrote:
> > On Tue 11-07-17 09:58:42, Anshuman Khandual wrote:
> >>> 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
> > 
> > Are you telling me that two if conditions cause more than a second
> > difference? That sounds suspicious.
> 
> It's removing also a call to get_unmapped_area(), AFAICS. That means a
> vma search?

Ohh, right. I have somehow missed that. Is this removal intentional? The
changelog is silent about it.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684817

On 07/11/2017 08:50 AM, Michal Hocko wrote:
> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>>
>>> Are you telling me that two if conditions cause more than a second
>>> difference? That sounds suspicious.
>>
>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>> vma search?
> 
> Ohh, right. I have somehow missed that. Is this removal intentional?

I think it is: "Checking for availability of virtual address range at
the end of the VMA for the incremental size is also reduntant at this
point."

> The
> changelog is silent about it.

It doesn't explain why it's redundant, indeed. Unfortunately, the commit
f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
also doesn't explain why it's needed.

[toc] | [prev] | [next] | [standalone]

#1684826

On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
> On 07/11/2017 08:50 AM, Michal Hocko wrote:
> > On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
> >> On 07/11/2017 08:03 AM, Michal Hocko wrote:
> >>>
> >>> Are you telling me that two if conditions cause more than a second
> >>> difference? That sounds suspicious.
> >>
> >> It's removing also a call to get_unmapped_area(), AFAICS. That means a
> >> vma search?
> > 
> > Ohh, right. I have somehow missed that. Is this removal intentional?
> 
> I think it is: "Checking for availability of virtual address range at
> the end of the VMA for the incremental size is also reduntant at this
> point."

I though this referred to this check
	if (vma->vm_next && vma->vm_next->vm_start < end)

becuase get_unampped_area with MAP_FIXED doesn't really check
anything. It will simply return the given address. Btw. this also rules
out find_vma.
 
> > The
> > changelog is silent about it.
> 
> It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> also doesn't explain why it's needed.

Because it doesn't do anything AFAICS.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684832

On Tue 11-07-17 09:16:12, Michal Hocko wrote:
> On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
[...]
> > It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> > f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> > also doesn't explain why it's needed.
> 
> Because it doesn't do anything AFAICS.

Well, it does actually. I have missed security_mmap_addr hook.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684979

On 07/11/2017 12:52 PM, Michal Hocko wrote:
> On Tue 11-07-17 09:16:12, Michal Hocko wrote:
>> On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
> [...]
>>> It doesn't explain why it's redundant, indeed. Unfortunately, the commit
>>> f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
>>> also doesn't explain why it's needed.
>>
>> Because it doesn't do anything AFAICS.
> 
> Well, it does actually. I have missed security_mmap_addr hook.

But we any way call get_unmapped_area() down the line in the function,
it should be covered then. Does the proposed change look good and be
considered, or any changes required or can be dropped ?

[toc] | [prev] | [next] | [standalone]

#1684976

On 07/11/2017 12:46 PM, Michal Hocko wrote:
> On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
>> On 07/11/2017 08:50 AM, Michal Hocko wrote:
>>> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>>>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>>>>
>>>>> Are you telling me that two if conditions cause more than a second
>>>>> difference? That sounds suspicious.
>>>>
>>>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>>>> vma search?
>>>
>>> Ohh, right. I have somehow missed that. Is this removal intentional?
>>
>> I think it is: "Checking for availability of virtual address range at
>> the end of the VMA for the incremental size is also reduntant at this
>> point."
> 
> I though this referred to this check
> 	if (vma->vm_next && vma->vm_next->vm_start < end)

No, that check is still there in the code.

[toc] | [prev] | [next] | [standalone]

#1684970

On 07/11/2017 12:26 PM, Vlastimil Babka wrote:
> On 07/11/2017 08:50 AM, Michal Hocko wrote:
>> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>>>
>>>> Are you telling me that two if conditions cause more than a second
>>>> difference? That sounds suspicious.
>>>
>>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>>> vma search?
>>
>> Ohh, right. I have somehow missed that. Is this removal intentional?
> 
> I think it is: "Checking for availability of virtual address range at
> the end of the VMA for the incremental size is also reduntant at this
> point."
> 
>> The
>> changelog is silent about it.
> 
> It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> also doesn't explain why it's needed.

Its redundant because there are calls to get_unmapped_area() down the
line in the function whose failure will anyway fail the expansion of
the VMA.

[toc] | [prev] | [next] | [standalone]

#1684983

On Tue 11-07-17 16:38:46, Anshuman Khandual wrote:
> On 07/11/2017 12:26 PM, Vlastimil Babka wrote:
> > On 07/11/2017 08:50 AM, Michal Hocko wrote:
> >> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
> >>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
> >>>>
> >>>> Are you telling me that two if conditions cause more than a second
> >>>> difference? That sounds suspicious.
> >>>
> >>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
> >>> vma search?
> >>
> >> Ohh, right. I have somehow missed that. Is this removal intentional?
> > 
> > I think it is: "Checking for availability of virtual address range at
> > the end of the VMA for the incremental size is also reduntant at this
> > point."
> > 
> >> The
> >> changelog is silent about it.
> > 
> > It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> > f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> > also doesn't explain why it's needed.
> 
> Its redundant because there are calls to get_unmapped_area() down the
> line in the function whose failure will anyway fail the expansion of
> the VMA.

mremap code is quite complex and I am not sure you are right here.
Anyway, please make sure you document why you believe those checks are
not needed when resubmitting your patch.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684968

On 07/11/2017 12:20 PM, Michal Hocko wrote:
> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>> On Tue 11-07-17 09:58:42, Anshuman Khandual wrote:
>>>>> 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
>>> Are you telling me that two if conditions cause more than a second
>>> difference? That sounds suspicious.
>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>> vma search?
> Ohh, right. I have somehow missed that. Is this removal intentional? The
> changelog is silent about it.

Yeah it was.

[toc] | [prev] | [next] | [standalone]

#1684928

On 07/11/2017 11:56 AM, Vlastimil Babka wrote:
> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>> On Tue 11-07-17 09:58:42, Anshuman Khandual wrote:
>>>> 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
>> Are you telling me that two if conditions cause more than a second
>> difference? That sounds suspicious.
> It's removing also a call to get_unmapped_area(), AFAICS. That means a
> vma search?

I believe removing this function is responsible for the
increase in speed of the test execution.

[toc] | [prev] | [next] | [standalone]

#1684769

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web