[PATCH]

[PATCH]

[Ben LaHaise]

Hello,

The patch below enables vma merging for a couple of additional cases with
anon mmaps as glibc has a habit of passing in differing flags for some
cases (ie memory remapping, extending specific malloc blocks, etc).  This
is to help Mozilla which ends up with thousands of vma's that are
sequential and anonymous, but unmerged.  There may still be issues with
mremap, but I think this is a step in the right direction.

		-ben

diff -urN /md0/kernels/2.4/v2.4.8-ac5/mm/mmap.c work-v2.4.8-ac5/mm/mmap.c
--- /md0/kernels/2.4/v2.4.8-ac5/mm/mmap.c	Wed Aug 15 12:57:40 2001
+++ work-v2.4.8-ac5/mm/mmap.c	Wed Aug 15 13:02:35 2001
@@ -309,7 +309,8 @@
 	if (addr && !file && !(vm_flags & VM_SHARED)) {
 		struct vm_area_struct * vma = find_vma(mm, addr-1);
 		if (vma && vma->vm_end == addr && !vma->vm_file &&
-		    vma->vm_flags == vm_flags) {
+		    (vma->vm_flags & ~(MAP_NORESERVE | MAP_FIXED)) ==
+		    (vm_flags & ~(MAP_NORESERVE | MAP_FIXED))) {
 			vma->vm_end = addr + len;
 			goto out;
 		}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH]

[Linus Torvalds]

On Wed, 15 Aug 2001, Ben LaHaise wrote:
>
> The patch below enables vma merging for a couple of additional cases with
> anon mmaps as glibc has a habit of passing in differing flags for some
> cases (ie memory remapping, extending specific malloc blocks, etc).  This
> is to help Mozilla which ends up with thousands of vma's that are
> sequential and anonymous, but unmerged.  There may still be issues with
> mremap, but I think this is a step in the right direction.

Good catch.

However, I really think we should just mask those bits out in general:
we've already used them up by this time, and they make no sense at all to
maintain in the VMA either, so it looks like it would be a cleaner (and
shorter) patch to just do

	/* get rid of mmap-time-only flags */
	vm_flags &= ~(MAP_NORESERVE | MAP_FIXED);

just after we've checked the MAP_NORESERVE bit, and just before we check
whether we can expand an old mapping. That way the (now meaningless) bits
don't end up as noise in the vma->vm_flags, AND we guarantee that merging
doesn't merge two fields that have different "noise" in their vm_flags.

Agreed?

		Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH]

[Ben LaHaise]

On Wed, 15 Aug 2001, Linus Torvalds wrote:

> Good catch.

Eeep, on re-reading it, I was wrong: vm_flags only has VM_* in the bits,
so this patch would introduce a bug (oops, I must need coffee).  So, it
must be in the mremap/mprotect related bits.

		-ben

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH]

[Daniel Phillips]

On August 15, 2001 07:40 pm, Linus Torvalds wrote:
> On Wed, 15 Aug 2001, Ben LaHaise wrote:
> >
> > The patch below enables vma merging for a couple of additional cases with
> > anon mmaps as glibc has a habit of passing in differing flags for some
> > cases (ie memory remapping, extending specific malloc blocks, etc).  This
> > is to help Mozilla which ends up with thousands of vma's that are
> > sequential and anonymous, but unmerged.  There may still be issues with
> > mremap, but I think this is a step in the right direction.
> 
> Good catch.
> 
> However, I really think we should just mask those bits out in general:
> we've already used them up by this time, and they make no sense at all to
> maintain in the VMA either, so it looks like it would be a cleaner (and
> shorter) patch to just do
> 
> 	/* get rid of mmap-time-only flags */
> 	vm_flags &= ~(MAP_NORESERVE | MAP_FIXED);
> 
> just after we've checked the MAP_NORESERVE bit, and just before we check
> whether we can expand an old mapping. That way the (now meaningless) bits
> don't end up as noise in the vma->vm_flags, AND we guarantee that merging
> doesn't merge two fields that have different "noise" in their vm_flags.

Another one in that category is BH_New which we use only immediately after
*_get_block, afterwards it dangles meaninglessly.  Not a bug, but
misleading.

--
Daniel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

[PATCH] mremap merging.

[Ben LaHaise]

This one should actually be correct -- it just looks big as it flattens
the code in do_mremap somewhat.  There's a test program at
http://www.kvack.org/~blah/mremap_test5.c that will trigger the "merged!"
printk and show the results from /proc/<pid>/maps.  Mozilla isn't
triggering it, though, so I'm looking at mprotect now.

Also, mmap/mremap are failing to merge some segments as the BSS of a
program is marked with the executable bit.  At least on x86, X on a page
does nothing, so I'm wondering if we should strip that out from vm_flags
as I was originally suspecting?  At least on my machine it seems to be hit
occasionally.

		-ben

diff -ur /md0/kernels/2.4/v2.4.8-ac5/mm/mremap.c work-v2.4.8-ac5/mm/mremap.c
--- /md0/kernels/2.4/v2.4.8-ac5/mm/mremap.c	Wed Aug 15 12:57:40 2001
+++ work-v2.4.8-ac5/mm/mremap.c	Wed Aug 15 14:59:02 2001
@@ -132,10 +132,23 @@
 	unsigned long new_addr)
 {
 	struct vm_area_struct * new_vma;
+	int allocated_vma = 0;

-	new_vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
-	if (new_vma) {
-		if (!move_page_tables(current->mm, new_addr, addr, old_len)) {
+	/* First, check if we can merge a mapping. -ben */
+	new_vma = find_vma(current->mm, new_addr-1);
+	if (new_vma && new_vma->vm_end == new_addr && !new_vma->vm_file &&
+		new_vma->vm_flags == vma->vm_flags) {
+		new_vma->vm_end = new_addr + new_len;
+printk("merged!\n");
+	} else {
+		new_vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
+		if (!new_vma)
+			goto no_mem;
+		allocated_vma = 1;
+	}
+
+	if (!move_page_tables(current->mm, new_addr, addr, old_len)) {
+		if (allocated_vma) {
 			*new_vma = *vma;
 			new_vma->vm_start = new_addr;
 			new_vma->vm_end = new_addr+new_len;
@@ -146,17 +159,20 @@
 			if (new_vma->vm_ops && new_vma->vm_ops->open)
 				new_vma->vm_ops->open(new_vma);
 			insert_vm_struct(current->mm, new_vma);
-			do_munmap(current->mm, addr, old_len);
-			current->mm->total_vm += new_len >> PAGE_SHIFT;
-			if (new_vma->vm_flags & VM_LOCKED) {
-				current->mm->locked_vm += new_len >> PAGE_SHIFT;
-				make_pages_present(new_vma->vm_start,
-						   new_vma->vm_end);
-			}
-			return new_addr;
 		}
-		kmem_cache_free(vm_area_cachep, new_vma);
+		do_munmap(current->mm, addr, old_len);
+		current->mm->total_vm += new_len >> PAGE_SHIFT;
+		if (new_vma->vm_flags & VM_LOCKED) {
+			current->mm->locked_vm += new_len >> PAGE_SHIFT;
+			make_pages_present(new_vma->vm_start,
+					   new_vma->vm_end);
+		}
+		return new_addr;
 	}
+	if (allocated_vma)
+		kmem_cache_free(vm_area_cachep, new_vma);
+
+no_mem:
 	return -ENOMEM;
 }


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

[PATCH] mmap tail merging

[Ben LaHaise]

Hello,

Here's a patch to mmap.c that performs tail merging on mmap.  In testing,
ld tends to hit it a few times during linking, and mozilla hit it a couple
of dozen times.  This probably comes from larger blocks of memory that
were malloc'd and later free'd with the following memory segment still in
use.  Alas, that doesn't solve all of the excess vma's in mozilla.

		-ben

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH] mmap tail merging

[Rik van Riel]

On Wed, 15 Aug 2001, Ben LaHaise wrote:

> Here's a patch to mmap.c that performs tail merging on mmap.

This patch sure is compact ;)

Rik
--
IA64: a worthy successor to i860.

http://www.surriel.com/		http://distro.conectiva.com/

Send all your spam to aardvark@nl.linux.org (spam digging piggy)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [PATCH] mmap tail merging

[Ben LaHaise]

On Wed, 15 Aug 2001, Rik van Riel wrote:

> On Wed, 15 Aug 2001, Ben LaHaise wrote:
>
> > Here's a patch to mmap.c that performs tail merging on mmap.
>
> This patch sure is compact ;)

Right.  Let's try this instead:

		-ben

diff -ur /md0/kernels/2.4/v2.4.8-ac5/mm/mmap.c work-v2.4.8-ac5/mm/mmap.c
--- /md0/kernels/2.4/v2.4.8-ac5/mm/mmap.c	Wed Aug 15 12:57:40 2001
+++ work-v2.4.8-ac5/mm/mmap.c	Wed Aug 15 18:24:58 2001
@@ -17,6 +17,10 @@
 #include <asm/uaccess.h>
 #include <asm/pgalloc.h>

+#define vm_avl_empty	(struct vm_area_struct *) NULL
+
+#include "mmap_avl.c"
+
 /* description of effects of mapping type and prot in current implementation.
  * this is due to the limited x86 page protection hardware.  The expected
  * behavior is in parens:
@@ -307,7 +311,7 @@

 	/* Can we just expand an old anonymous mapping? */
 	if (addr && !file && !(vm_flags & VM_SHARED)) {
-		struct vm_area_struct * vma = find_vma(mm, addr-1);
+		vma = find_vma(mm, addr-1);
 		if (vma && vma->vm_end == addr && !vma->vm_file &&
 		    vma->vm_flags == vm_flags) {
 			vma->vm_end = addr + len;
@@ -363,12 +367,30 @@
 	if (correct_wcount)
 		atomic_inc(&file->f_dentry->d_inode->i_writecount);

-out:
+out:
 	mm->total_vm += len >> PAGE_SHIFT;
 	if (vm_flags & VM_LOCKED) {
 		mm->locked_vm += len >> PAGE_SHIFT;
 		make_pages_present(addr, addr + len);
 	}
+
+	/* Can we merge this anonymous mapping with the one following it? */
+	if (!file && !(vm_flags & VM_SHARED)) {
+		struct vm_area_struct *next = vma->vm_next;
+		if (next && vma->vm_end == next->vm_start && !next->vm_file &&
+		    vma->vm_flags == next->vm_flags) {
+			spin_lock(&mm->page_table_lock);
+			vma->vm_next = next->vm_next;
+			if (mm->mmap_avl)
+				avl_remove(next, &mm->mmap_avl);
+			vma->vm_end = next->vm_end;
+			mm->mmap_cache = vma;	/* Kill the cache. */
+			spin_unlock(&mm->page_table_lock);
+
+			kmem_cache_free(vm_area_cachep, next);
+		}
+	}
+
 	return addr;

 unmap_and_free_vma:
@@ -443,10 +465,6 @@
 	return arch_get_unmapped_area(file, addr, len, pgoff, flags);
 }

-#define vm_avl_empty	(struct vm_area_struct *) NULL
-
-#include "mmap_avl.c"
-
 /* Look up the first VMA which satisfies  addr < vm_end,  NULL if none. */
 struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long addr)
 {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/