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

[PATCH]

[Rik van Riel]

Hi Alan,

the following patch fixes reclaim_page() and page_launder() to
correctly reactivate a page based one page->count value.

Note that we shouldn't be hitting this code very much with the
current immediate reactivation in __find_page_nolock(), but I
guess it would be useful to have as a safety net against things
like the shmem code and other areas I don't about ;)

regards,

Rik
--
IA64: a worthy successor to i860.


--- linux-2.4.8-ac7/mm/vmscan.c.orig	Mon Aug 20 11:29:24 2001
+++ linux-2.4.8-ac7/mm/vmscan.c	Mon Aug 20 11:30:46 2001
@@ -456,7 +456,7 @@

 		/* Page is or was in use?  Move it to the active list. */
 		if (PageReferenced(page) || page->age > 0 ||
-				(!page->buffers && page_count(page) > 1)) {
+				page_count(page) > (1 + !!page->buffers)) {
 			del_page_from_inactive_clean_list(page);
 			add_page_to_active_list(page);
 			continue;
@@ -594,7 +594,7 @@

 		/* Page is or was in use?  Move it to the active list. */
 		if (PageReferenced(page) || page->age > 0 ||
-				(!page->buffers && page_count(page) > 1) ||
+				page_count(page) > (1 + !!page->buffers) ||
 				page_ramdisk(page)) {
 			del_page_from_inactive_dirty_list(page);
 			add_page_to_active_list(page);

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

[Marcelo Tosatti]

Rik,

Think about a thread blocked on ->writepage() called from page_launder(),
which has gotten an additional reference on the page.

Any other thread looping on page_launder() will move the given page being
written to the active list, even if we should just drop the page as soon
as its writeout is finished.

That is just _one_ case the patch will do the wrong thing (there may be
more). I suggest more investigation before actually merging the patch in
the -ac tree.

On Mon, 20 Aug 2001, Rik van Riel wrote:

> Hi Alan,
> 
> the following patch fixes reclaim_page() and page_launder() to
> correctly reactivate a page based one page->count value.
> 
> Note that we shouldn't be hitting this code very much with the
> current immediate reactivation in __find_page_nolock(), but I
> guess it would be useful to have as a safety net against things
> like the shmem code and other areas I don't about ;)
> 
> regards,
> 
> Rik
> --
> IA64: a worthy successor to i860.
> 
> 
> --- linux-2.4.8-ac7/mm/vmscan.c.orig	Mon Aug 20 11:29:24 2001
> +++ linux-2.4.8-ac7/mm/vmscan.c	Mon Aug 20 11:30:46 2001
> @@ -456,7 +456,7 @@
> 
>  		/* Page is or was in use?  Move it to the active list. */
>  		if (PageReferenced(page) || page->age > 0 ||
> -				(!page->buffers && page_count(page) > 1)) {
> +				page_count(page) > (1 + !!page->buffers)) {
>  			del_page_from_inactive_clean_list(page);
>  			add_page_to_active_list(page);
>  			continue;
> @@ -594,7 +594,7 @@
> 
>  		/* Page is or was in use?  Move it to the active list. */
>  		if (PageReferenced(page) || page->age > 0 ||
> -				(!page->buffers && page_count(page) > 1) ||
> +				page_count(page) > (1 + !!page->buffers) ||
>  				page_ramdisk(page)) {
>  			del_page_from_inactive_dirty_list(page);
>  			add_page_to_active_list(page);
> 
> --
> 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/
> 

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

[Rik van Riel]

On Mon, 20 Aug 2001, Marcelo Tosatti wrote:

> Think about a thread blocked on ->writepage() called from
> page_launder(), which has gotten an additional reference on the
> page.
>
> Any other thread looping on page_launder() will move the given
> page being written to the active list, even if we should just
> drop the page as soon as its writeout is finished.

You're right, we need to add one more check, for PageLocked(page).

If the page is locked, we should not reactivate it...

regards,

Rik
--
IA64: a worthy successor to the i860.

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

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