2007-02-21 18:07:44

by akuster

[permalink] [raw]
Subject: [patch 1/1] MM: detach_vmas_to_be_unmapped fix



---

mm/mmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN mm/mmap.c~Avoiding-mmap-fragmentation_fixup mm/mmap.c
--- linux-2.6_clean/mm/mmap.c~Avoiding-mmap-fragmentation_fixup 2007-02-21 09:49:32.000000000 -0800
+++ linux-2.6_clean-akuster/mm/mmap.c 2007-02-21 09:51:26.000000000 -0800
@@ -1720,9 +1720,9 @@ detach_vmas_to_be_unmapped(struct mm_str
*insertion_point = vma;
tail_vma->vm_next = NULL;
if (mm->unmap_area == arch_unmap_area)
- addr = prev ? prev->vm_end : mm->mmap_base;
+ addr = prev ? prev->vm_start : mm->mmap_base;
else
- addr = vma ? vma->vm_start : mm->mmap_base;
+ addr = vma ? vma->vm_end : mm->mmap_base;
mm->unmap_area(mm, addr);
mm->mmap_cache = NULL; /* Kill the cache. */
}
_


2007-02-21 21:05:58

by Ingo Oeser

[permalink] [raw]
Subject: Re: [patch 1/1] MM: detach_vmas_to_be_unmapped fix

Hi,

On Wednesday, 21. February 2007, [email protected] wrote:
>
> ---
>
> mm/mmap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff -puN mm/mmap.c~Avoiding-mmap-fragmentation_fixup mm/mmap.c
> --- linux-2.6_clean/mm/mmap.c~Avoiding-mmap-fragmentation_fixup 2007-02-21 09:49:32.000000000 -0800
> +++ linux-2.6_clean-akuster/mm/mmap.c 2007-02-21 09:51:26.000000000 -0800
> @@ -1720,9 +1720,9 @@ detach_vmas_to_be_unmapped(struct mm_str
> *insertion_point = vma;
> tail_vma->vm_next = NULL;
> if (mm->unmap_area == arch_unmap_area)
> - addr = prev ? prev->vm_end : mm->mmap_base;
> + addr = prev ? prev->vm_start : mm->mmap_base;
> else
> - addr = vma ? vma->vm_start : mm->mmap_base;
> + addr = vma ? vma->vm_end : mm->mmap_base;
> mm->unmap_area(mm, addr);
> mm->mmap_cache = NULL; /* Kill the cache. */
> }

Please comment, why you think this is necessary.


Thanks & Regards

Ingo Oeser

2007-02-23 16:28:35

by akuster

[permalink] [raw]
Subject: Re: [patch 1/1] MM: detach_vmas_to_be_unmapped fix



Ingo Oeser wrote:
> Hi,
>
> On Wednesday, 21. February 2007, [email protected] wrote:
>> ---
>>
>> mm/mmap.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff -puN mm/mmap.c~Avoiding-mmap-fragmentation_fixup mm/mmap.c
>> --- linux-2.6_clean/mm/mmap.c~Avoiding-mmap-fragmentation_fixup 2007-02-21 09:49:32.000000000 -0800
>> +++ linux-2.6_clean-akuster/mm/mmap.c 2007-02-21 09:51:26.000000000 -0800
>> @@ -1720,9 +1720,9 @@ detach_vmas_to_be_unmapped(struct mm_str
>> *insertion_point = vma;
>> tail_vma->vm_next = NULL;
>> if (mm->unmap_area == arch_unmap_area)
>> - addr = prev ? prev->vm_end : mm->mmap_base;
>> + addr = prev ? prev->vm_start : mm->mmap_base;
>> else
>> - addr = vma ? vma->vm_start : mm->mmap_base;
>> + addr = vma ? vma->vm_end : mm->mmap_base;
>> mm->unmap_area(mm, addr);
>> mm->mmap_cache = NULL; /* Kill the cache. */
>> }
>
> Please comment, why you think this is necessary.

Yes that would help.

On Feb. 16th I asked a question and got no response. Here is what should
have been included with the above patch.

Wolfgang Wander submitted a fix to address a mmap fragmentation issue.
The git patch ( 1363c3cd8603a913a27e2995dccbd70d5312d8e6 ) is somewhat
different and yields different results when running Wolfgang's test case
leakme.c.

IMHO, the vm start and end address are swapped in arch_unmap_area and
arch_unmap_area_topdown functions.

Prior to this patch arch_unmap_area() used area->vm_start and
arch_unmap_area_topdown used area->vm_end in the git patch the following
change showed up.

if (mm->unmap_area == arch_unmap_area)
addr = prev ? prev->vm_start : mm->mmap_base;
else
addr = vma ? vma->vm_end : mm->mmap_base;

Using Wolfgang Wander's leakme.c test, I get the same results seen with
his original "Avoiding mmap fragmentation" patch as I do after swapping
the start & end address in the above code segment. The patch I
submitted addresses this typo issue.

TIA,
Armin

>
>
> Thanks & Regards
>
> Ingo Oeser

2007-02-27 20:27:56

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] MM: detach_vmas_to_be_unmapped fix

> On Fri, 23 Feb 2007 08:27:00 -0800 akuster <[email protected]> wrote:
> Wolfgang Wander submitted a fix to address a mmap fragmentation issue.
> The git patch ( 1363c3cd8603a913a27e2995dccbd70d5312d8e6 ) is somewhat
> different and yields different results when running Wolfgang's test case
> leakme.c.
>
> IMHO, the vm start and end address are swapped in arch_unmap_area and
> arch_unmap_area_topdown functions.
>
> Prior to this patch arch_unmap_area() used area->vm_start and
> arch_unmap_area_topdown used area->vm_end in the git patch the following
> change showed up.
>
> if (mm->unmap_area == arch_unmap_area)
> addr = prev ? prev->vm_start : mm->mmap_base;
> else
> addr = vma ? vma->vm_end : mm->mmap_base;
>
> Using Wolfgang Wander's leakme.c test, I get the same results seen with
> his original "Avoiding mmap fragmentation" patch as I do after swapping
> the start & end address in the above code segment. The patch I
> submitted addresses this typo issue.

That's still not an adequate or terribly useful changlog, sorry.

Please tell us exactly what this patch does, without referring to some other
patch which I cannot remember nor presently access.

I particular, before-and-after testing results would be useful so we can
see what the effect of this change is.

Also, please send a Signed-of-by: for this work, thanks.