[PATCH 1/3] mm: move inc_zone_page_state(NR_ISOLATED) to just isolated place

[PATCH 1/3] mm: move inc_zone_page_state(NR_ISOLATED) to just isolated place

[KOSAKI Motohiro]

This patch series is trivial cleanup and fix of page migration.


==========================================================

Christoph pointed out inc_zone_page_state(NR_ISOLATED) should be placed
in right after isolate_page().

This patch does it.

Cc: Christoph Lameter <cl@linux-foundation.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/memory_hotplug.c |    4 ++++
 mm/mempolicy.c      |    3 +++
 mm/migrate.c        |   12 ++++--------
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 821dee5..653bf1e 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -26,6 +26,7 @@
 #include <linux/migrate.h>
 #include <linux/page-isolation.h>
 #include <linux/pfn.h>
+#include <linux/mm_inline.h>
 
 #include <asm/tlbflush.h>
 
@@ -663,6 +664,9 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 		if (!ret) { /* Success */
 			list_add_tail(&page->lru, &source);
 			move_pages--;
+			inc_zone_page_state(page, NR_ISOLATED_ANON +
+					    page_is_file_cache(page));
+
 		} else {
 			/* Becasue we don't have big zone->lock. we should
 			   check this again here. */
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 7dd9d9f..473f888 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -89,6 +89,7 @@
 #include <linux/security.h>
 #include <linux/syscalls.h>
 #include <linux/ctype.h>
+#include <linux/mm_inline.h>
 
 #include <asm/tlbflush.h>
 #include <asm/uaccess.h>
@@ -809,6 +810,8 @@ static void migrate_page_add(struct page *page, struct list_head *pagelist,
 	if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(page) == 1) {
 		if (!isolate_lru_page(page)) {
 			list_add_tail(&page->lru, pagelist);
+			inc_zone_page_state(page, NR_ISOLATED_ANON +
+					    page_is_file_cache(page));
 		}
 	}
 }
diff --git a/mm/migrate.c b/mm/migrate.c
index 1a4bf48..0f66803 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -746,13 +746,6 @@ int migrate_pages(struct list_head *from,
 	struct page *page2;
 	int swapwrite = current->flags & PF_SWAPWRITE;
 	int rc;
-	unsigned long flags;
-
-	local_irq_save(flags);
-	list_for_each_entry(page, from, lru)
-		__inc_zone_page_state(page, NR_ISOLATED_ANON +
-				page_is_file_cache(page));
-	local_irq_restore(flags);
 
 	if (!swapwrite)
 		current->flags |= PF_SWAPWRITE;
@@ -878,8 +871,11 @@ static int do_move_page_to_node_array(struct mm_struct *mm,
 			goto put_and_set;
 
 		err = isolate_lru_page(page);
-		if (!err)
+		if (!err) {
 			list_add_tail(&page->lru, &pagelist);
+			inc_zone_page_state(page, NR_ISOLATED_ANON +
+					    page_is_file_cache(page));
+		}
 put_and_set:
 		/*
 		 * Either remove the duplicate refcount from
-- 
1.6.0.GIT



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] Fix memory leak of never putback pages in mbind()

[KOSAKI Motohiro]

if mbind() receive invalid address, do_mbind makes leaked page.
following test program detect its leak.

This patch fixes it.


migrate_efault.c
============================================
 #include <numaif.h>
 #include <numa.h>
 #include <sys/mman.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>

static unsigned long pagesize;

static void* make_hole_mapping(void)
{

	void* addr;

	addr = mmap(NULL, pagesize*3, PROT_READ|PROT_WRITE,
		    MAP_ANON|MAP_PRIVATE, 0, 0);
	if (addr == MAP_FAILED)
		return NULL;

	/* make page populate */
	memset(addr, 0, pagesize*3);

	/* make memory hole */
	munmap(addr+pagesize, pagesize);

	return addr;
}

int main(int argc, char** argv)
{
	void* addr;
	int ch;
	int node;
	struct bitmask *nmask = numa_allocate_nodemask();
	int err;
	int node_set = 0;

	while ((ch = getopt(argc, argv, "n:")) != -1){
		switch (ch){
		case 'n':
			node = strtol(optarg, NULL, 0);
			numa_bitmask_setbit(nmask, node);
			node_set = 1;
			break;
		default:
			;
		}
	}
	argc -= optind;
	argv += optind;

	if (!node_set)
		numa_bitmask_setbit(nmask, 0);

	pagesize = getpagesize();

	addr = make_hole_mapping();

	err = mbind(addr, pagesize*3, MPOL_BIND, nmask->maskp, nmask->size, MPOL_MF_MOVE_ALL);
	if (err)
		perror("mbind ");

	return 0;
}
============================================

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/mempolicy.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 473f888..824abf3 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1061,6 +1061,8 @@ static long do_mbind(unsigned long start, unsigned long len,
 
 		if (!err && nr_failed && (flags & MPOL_MF_STRICT))
 			err = -EIO;
+	} else {
+		putback_lru_pages(&pagelist);
 	}
 
 	up_write(&mm->mmap_sem);
-- 
1.6.0.GIT



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] Fix memory leak of do_mbind()

[KOSAKI Motohiro]

If migrate_prep is failed, new variable is leaked.
This patch fixes it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/mempolicy.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 824abf3..38ce2a7 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1027,7 +1027,7 @@ static long do_mbind(unsigned long start, unsigned long len,
 
 		err = migrate_prep();
 		if (err)
-			return err;
+			goto mpol_out;
 	}
 	{
 		NODEMASK_SCRATCH(scratch);
@@ -1042,10 +1042,9 @@ static long do_mbind(unsigned long start, unsigned long len,
 			err = -ENOMEM;
 		NODEMASK_SCRATCH_FREE(scratch);
 	}
-	if (err) {
-		mpol_put(new);
-		return err;
-	}
+	if (err)
+		goto mpol_out;
+
 	vma = check_range(mm, start, end, nmask,
 			  flags | MPOL_MF_INVERT, &pagelist);
 
@@ -1066,6 +1065,7 @@ static long do_mbind(unsigned long start, unsigned long len,
 	}
 
 	up_write(&mm->mmap_sem);
+ mpol_out:
 	mpol_put(new);
 	return err;
 }
-- 
1.6.0.GIT



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] Fix memory leak of never putback pages in mbind()

[KOSAKI Motohiro]

> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> ---
>  mm/mempolicy.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 473f888..824abf3 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1061,6 +1061,8 @@ static long do_mbind(unsigned long start, unsigned long len,
>  
>  		if (!err && nr_failed && (flags & MPOL_MF_STRICT))
>  			err = -EIO;
> +	} else {
> +		putback_lru_pages(&pagelist);
>  	}
>  
>  	up_write(&mm->mmap_sem);


Oops, I forgot to remove unnecessary brace.
updated patch is here.

================================================================
Subject: [PATCH] Fix memory leak of never putback pages in mbind()

if mbind() receive invalid address, do_mbind makes leaked page.
following test program detect its leak.

This patch fixes it.

migrate_efault.c
=======================================
 #include <numaif.h>
 #include <numa.h>
 #include <sys/mman.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>

static unsigned long pagesize;

static void* make_hole_mapping(void)
{

	void* addr;

	addr = mmap(NULL, pagesize*3, PROT_READ|PROT_WRITE,
		    MAP_ANON|MAP_PRIVATE, 0, 0);
	if (addr == MAP_FAILED)
		return NULL;

	/* make page populate */
	memset(addr, 0, pagesize*3);

	/* make memory hole */
	munmap(addr+pagesize, pagesize);

	return addr;
}

int main(int argc, char** argv)
{
	void* addr;
	int ch;
	int node;
	struct bitmask *nmask = numa_allocate_nodemask();
	int err;
	int node_set = 0;

	while ((ch = getopt(argc, argv, "n:")) != -1){
		switch (ch){
		case 'n':
			node = strtol(optarg, NULL, 0);
			numa_bitmask_setbit(nmask, node);
			node_set = 1;
			break;
		default:
			;
		}
	}
	argc -= optind;
	argv += optind;

	if (!node_set)
		numa_bitmask_setbit(nmask, 0);

	pagesize = getpagesize();

	addr = make_hole_mapping();

	err = mbind(addr, pagesize*3, MPOL_BIND, nmask->maskp, nmask->size, MPOL_MF_MOVE_ALL);
	if (err)
		perror("mbind ");

	return 0;
}
=======================================


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 mm/mempolicy.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/mm/mempolicy.c
===================================================================
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1061,7 +1061,8 @@ static long do_mbind(unsigned long start
 
 		if (!err && nr_failed && (flags & MPOL_MF_STRICT))
 			err = -EIO;
-	}
+	} else
+		putback_lru_pages(&pagelist);
 
 	up_write(&mm->mmap_sem);
 	mpol_put(new);



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm: move inc_zone_page_state(NR_ISOLATED) to just isolated place

[Christoph Lameter]

Reviewed-by: Christoph Lameter <cl@linux-foundation.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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] Fix memory leak of never putback pages in mbind()

[Christoph Lameter]

On Fri, 9 Oct 2009, KOSAKI Motohiro wrote:

> if mbind() receive invalid address, do_mbind makes leaked page.
> following test program detect its leak.

Acked-by: Christoph Lameter <cl@linux-foundation.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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] Fix memory leak of do_mbind()

[Christoph Lameter]

On Fri, 9 Oct 2009, KOSAKI Motohiro wrote:

> If migrate_prep is failed, new variable is leaked.
> This patch fixes it.

Acked-by: Christoph Lameter <cl@linux-foundation.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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] Fix memory leak of never putback pages in mbind()

[Christoph Lameter]

On Fri, 9 Oct 2009, KOSAKI Motohiro wrote:

> Oops, I forgot to remove unnecessary brace.
> updated patch is here.

Thats a style issue. There are other weird things in do_mbind as well
like starting a new block in the middle of another.

Having

}
{

in a program is a bit confusing. So could you do a cleanup patch for
mpol_bind? Preferably it should make it easy to read to and bring some
order to the confusing error handling.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/3] Fix memory leak of never putback pages in mbind()

[KOSAKI Motohiro]

>> Oops, I forgot to remove unnecessary brace.
>> updated patch is here.
>
> Thats a style issue. There are other weird things in do_mbind as well
> like starting a new block in the middle of another.
>
> Having
>
> }
> {
>
> in a program is a bit confusing. So could you do a cleanup patch for
> mpol_bind? Preferably it should make it easy to read to and bring some
> order to the confusing error handling.

Yes, I'll do.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm: move inc_zone_page_state(NR_ISOLATED) to just isolated place

[Andrew Morton]

On Fri,  9 Oct 2009 10:06:58 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> This patch series is trivial cleanup and fix of page migration.
> 
> 
> ==========================================================
> 
> Christoph pointed out inc_zone_page_state(NR_ISOLATED) should be placed
> in right after isolate_page().

The bugfixes are appropriate for 2.6.32 and should be backported into
-stable too, I think.  I haven't checked to see how long those bugs
have been present.

The cleanup is more appropriate for 2.6.33 so I had to switch the order
of these patches.  Hopefully the bugfixes were not dependent on the
cleanup.  

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] mm: move inc_zone_page_state(NR_ISOLATED) to just isolated place

[KOSAKI Motohiro]

> On Fri,  9 Oct 2009 10:06:58 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > This patch series is trivial cleanup and fix of page migration.
> > 
> > 
> > ==========================================================
> > 
> > Christoph pointed out inc_zone_page_state(NR_ISOLATED) should be placed
> > in right after isolate_page().
> 
> The bugfixes are appropriate for 2.6.32 and should be backported into
> -stable too, I think.  I haven't checked to see how long those bugs
> have been present.
> 
> The cleanup is more appropriate for 2.6.33 so I had to switch the order
> of these patches.  Hopefully the bugfixes were not dependent on the
> cleanup.  

Yes, each patches are independent.
[1/3] is cleanup.
[2/3] and [3/3] are bugfixes.

I'm sorry for lack of prudence of patch order.



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>