[PATCH]

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

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/

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]

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

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