Re: + tmpfs-fix-oops-on-remounts-with-mpol=default.patch added to -mm tree

Re: + tmpfs-fix-oops-on-remounts-with-mpol=default.patch added to -mm tree

[KOSAKI Motohiro]

> ------------------------------------------------------
> Subject: tmpfs: fix oops on remounts with mpol=default
> From: Ravikiran G Thirumalai <kiran@scalex86.org>
> 
> Fix an 'oops' when a tmpfs mount point is remounted with the 'default'
> mempolicy.
> 
> Upon remounting a tmpfs mount point with 'mpol=default' option, the
> remount code crashed with a null pointer dereference.  The initial problem
> report was on 2.6.27, but the problem exists in mainline 2.6.34-rc as
> well.  On examining the code, we see that mpol_new returns NULL if default
> mempolicy was requested.  This 'NULL' mempolicy is accessed to store the
> node mask resulting in oops.
> 
> The following patch fixes the oops by avoiding dereferencing NULL if the
> new mempolicy is NULL.  The patch also sets 'err' to 0 if MPOL_DEFAULT is
> passed (err is initialized to 1 initially at mpol_parse_str())

Hi Ravikiran,

I'm glad to your contribution. Unfortunately I've found various related
issue in mpol_parse_str() while reviewing your patch.

So, I'll post updated patches.

- kosaki


> 
> Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  mm/mempolicy.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff -puN mm/mempolicy.c~tmpfs-fix-oops-on-remounts-with-mpol=default mm/mempolicy.c
> --- a/mm/mempolicy.c~tmpfs-fix-oops-on-remounts-with-mpol=default
> +++ a/mm/mempolicy.c
> @@ -2213,10 +2213,14 @@ int mpol_parse_str(char *str, struct mem
>  			goto out;
>  		mode = MPOL_PREFERRED;
>  		break;
> -
> +	case MPOL_DEFAULT:
> +		/*
> +		 * mpol_new() enforces empty nodemask, ignores flags.
> +		 */
> +		err = 0;
> +		break;
>  	/*
>  	 * case MPOL_BIND:    mpol_new() enforces non-empty nodemask.
> -	 * case MPOL_DEFAULT: mpol_new() enforces empty nodemask, ignores flags.
>  	 */
>  	}
>  
> @@ -2250,7 +2254,7 @@ int mpol_parse_str(char *str, struct mem
>  		if (ret) {
>  			err = 1;
>  			mpol_put(new);
> -		} else if (no_context) {
> +		} else if (no_context && new) {
>  			/* save for contextualization */
>  			new->w.user_nodemask = nodes;
>  		}
> _
> 
> Patches currently in -mm which might be from kiran@scalex86.org are
> 
> tmpfs-fix-oops-on-remounts-with-mpol=default.patch
> slab-leaks3-default-y.patch
> 



--
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 1/5] tmpfs: fix oops on mounts with mpol=default

[KOSAKI Motohiro]

ChangeLog from Ravikiran's original one
  - Fix the patch description. the problem is in mount, not only remount.
  - Skip mpol_new() simply, instead adding NULL check.


=========================
From: Ravikiran G Thirumalai <kiran@scalex86.org>

Fix an 'oops' when a tmpfs mount point is mounted with the mpol=default
mempolicy.

Upon remounting a tmpfs mount point with 'mpol=default' option, the
mount code crashed with a null pointer dereference.  The initial
problem report was on 2.6.27, but the problem exists in mainline
2.6.34-rc as well.  On examining the code, we see that mpol_new returns
NULL if default mempolicy was requested.  This 'NULL' mempolicy is
accessed to store the node mask resulting in oops.

The following patch fixes it.

Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: <stable@kernel.org>
---
 mm/mempolicy.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index bda230e..25a0c0f 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2213,10 +2213,15 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 			goto out;
 		mode = MPOL_PREFERRED;
 		break;
-
+	case MPOL_DEFAULT:
+		/*
+		 * Insist on a empty nodelist
+		 */
+		if (!nodelist)
+			err = 0;
+		goto out;
 	/*
 	 * case MPOL_BIND:    mpol_new() enforces non-empty nodemask.
-	 * case MPOL_DEFAULT: mpol_new() enforces empty nodemask, ignores flags.
 	 */
 	}
 
-- 
1.6.5.2



--
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/5] tmpfs: fix oops on mounts with mpol=default

[Lee Schermerhorn]

On Tue, 2010-03-16 at 14:49 +0900, KOSAKI Motohiro wrote:
> ChangeLog from Ravikiran's original one
>   - Fix the patch description. the problem is in mount, not only remount.
>   - Skip mpol_new() simply, instead adding NULL check.
> 
> 
> =========================
> From: Ravikiran G Thirumalai <kiran@scalex86.org>
> 
> Fix an 'oops' when a tmpfs mount point is mounted with the mpol=default
> mempolicy.
> 
> Upon remounting a tmpfs mount point with 'mpol=default' option, the
> mount code crashed with a null pointer dereference.  The initial
> problem report was on 2.6.27, but the problem exists in mainline
> 2.6.34-rc as well.  On examining the code, we see that mpol_new returns
> NULL if default mempolicy was requested.  This 'NULL' mempolicy is
> accessed to store the node mask resulting in oops.
> 
> The following patch fixes it.
> 
> Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>

Acked-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

> ---
>  mm/mempolicy.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index bda230e..25a0c0f 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2213,10 +2213,15 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  			goto out;
>  		mode = MPOL_PREFERRED;
>  		break;
> -
> +	case MPOL_DEFAULT:
> +		/*
> +		 * Insist on a empty nodelist
> +		 */
> +		if (!nodelist)
> +			err = 0;
> +		goto out;
>  	/*
>  	 * case MPOL_BIND:    mpol_new() enforces non-empty nodemask.
> -	 * case MPOL_DEFAULT: mpol_new() enforces empty nodemask, ignores flags.
>  	 */
>  	}
>  

--
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/5] tmpfs: mpol=bind:0 don't cause mount error.

[KOSAKI Motohiro]

Currently, following mount operation cause mount error.

% mount -t tmpfs -ompol=bind:0 none /tmp

Because commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
shmem_sb_info) corrupted MPOL_BIND parse code.

This patch restore the needed one.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ravikiran Thirumalai <kiran@scalex86.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: <stable@kernel.org>
---
 mm/mempolicy.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 25a0c0f..3f77062 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2220,9 +2220,13 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		if (!nodelist)
 			err = 0;
 		goto out;
-	/*
-	 * case MPOL_BIND:    mpol_new() enforces non-empty nodemask.
-	 */
+	case MPOL_BIND:
+		/* 
+		 * Insist on a nodelist
+		 */
+		if (!nodelist)
+			goto out;
+		err = 0;
 	}
 
 	mode_flags = 0;
-- 
1.6.5.2



--
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/5] tmpfs: mpol=bind:0 don't cause mount error.

[Lee Schermerhorn]

On Tue, 2010-03-16 at 14:50 +0900, KOSAKI Motohiro wrote:
> Currently, following mount operation cause mount error.
> 
> % mount -t tmpfs -ompol=bind:0 none /tmp
> 
> Because commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> shmem_sb_info) corrupted MPOL_BIND parse code.
> 
> This patch restore the needed one.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>

There's a trailing space in the patch, but except for that:

Acked-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

> ---
>  mm/mempolicy.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 25a0c0f..3f77062 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2220,9 +2220,13 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		if (!nodelist)
>  			err = 0;
>  		goto out;
> -	/*
> -	 * case MPOL_BIND:    mpol_new() enforces non-empty nodemask.
> -	 */
> +	case MPOL_BIND:
> +		/* 
trailing space      ^

> +		 * Insist on a nodelist
> +		 */
> +		if (!nodelist)
> +			goto out;
> +		err = 0;
>  	}
>  
>  	mode_flags = 0;

--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[KOSAKI Motohiro]

commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
shmem_sb_info) added mpol=local mount option. but its feature is
broken since it was born. because such code always return 1 (i.e.
mount failure).

This patch fixes it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ravikiran Thirumalai <kiran@scalex86.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: <stable@kernel.org>
---
 mm/mempolicy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 3f77062..5c197d5 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2212,6 +2212,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		if (nodelist)
 			goto out;
 		mode = MPOL_PREFERRED;
+		err = 0;
 		break;
 	case MPOL_DEFAULT:
 		/*
-- 
1.6.5.2



--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[Lee Schermerhorn]

On Tue, 2010-03-16 at 14:51 +0900, KOSAKI Motohiro wrote:
> commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> shmem_sb_info) added mpol=local mount option. but its feature is
> broken since it was born. because such code always return 1 (i.e.
> mount failure).
> 
> This patch fixes it.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>

Acked-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

> ---
>  mm/mempolicy.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 3f77062..5c197d5 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2212,6 +2212,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		if (nodelist)
>  			goto out;
>  		mode = MPOL_PREFERRED;
> +		err = 0;
>  		break;
>  	case MPOL_DEFAULT:
>  		/*

--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[Hugh Dickins]

On Tue, 16 Mar 2010, KOSAKI Motohiro wrote:

> commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> shmem_sb_info) added mpol=local mount option. but its feature is
> broken since it was born. because such code always return 1 (i.e.
> mount failure).
> 
> This patch fixes it.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Ravikiran Thirumalai <kiran@scalex86.org>

Thank you both for finding and fixing these mpol embarrassments.

But if this "mpol=local" feature was never documented (not even in the
commit log), has been broken since birth 20 months ago, and nobody has
noticed: wouldn't it be better to save a little bloat and just rip it out?

Hugh

--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[KOSAKI Motohiro]

> On Tue, 16 Mar 2010, KOSAKI Motohiro wrote:
> 
> > commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> > shmem_sb_info) added mpol=local mount option. but its feature is
> > broken since it was born. because such code always return 1 (i.e.
> > mount failure).
> > 
> > This patch fixes it.
> > 
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> 
> Thank you both for finding and fixing these mpol embarrassments.
> 
> But if this "mpol=local" feature was never documented (not even in the
> commit log), has been broken since birth 20 months ago, and nobody has
> noticed: wouldn't it be better to save a little bloat and just rip it out?

I have no objection if lee agreed, lee?
Of cource, if we agree it, we can make the new patch soon :)



--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[Lee Schermerhorn]

On Thu, 2010-03-18 at 08:52 +0900, KOSAKI Motohiro wrote:
> > On Tue, 16 Mar 2010, KOSAKI Motohiro wrote:
> > 
> > > commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> > > shmem_sb_info) added mpol=local mount option. but its feature is
> > > broken since it was born. because such code always return 1 (i.e.
> > > mount failure).
> > > 
> > > This patch fixes it.
> > > 
> > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > > Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> > 
> > Thank you both for finding and fixing these mpol embarrassments.
> > 
> > But if this "mpol=local" feature was never documented (not even in the
> > commit log), has been broken since birth 20 months ago, and nobody has
> > noticed: wouldn't it be better to save a little bloat and just rip it out?
> 
> I have no objection if lee agreed, lee?
> Of cource, if we agree it, we can make the new patch soon :)
> 

Well, given the other issues with mpol_parse_str(), I suspect the entire
tmpfs mpol option is not used all that often in the mainline kernel.  I
recall that this feature was introduced by SGI for some of their
customers who may depend on it.  There have been cases where I could
have used it were it supported for the SYSV shm and MAP_ANON_MAP_SHARED
internal tmpfs mount.  Further, note that the addition of "mpol=local"
occurred between when the major "enterprise distributions" selected a
new mainline kernel.  Production users of those distros, who are the
likely users of this feature, tend not to live on the bleeding edge.
So, maybe we shouldn't read too much into it not being discovered until
now.

That being said, I suppose I wouldn't be all that opposed to deprecating
the entire tmpfs mpol option, and see who yells.   If the mpol mount
options stays, I'd like to see the 'local' option stay.  It's a
legitimate behavior that one can specify via the system calls and see
via numa_maps, so I think the mpol mount option, if it exists, should
support a way to specify it.  

As for bloat, the additional code on the mount option side to support it
is:

        case MPOL_LOCAL:
                /*
                 * Don't allow a nodelist;  mpol_new() checks flags
                 */
                if (nodelist)
                        goto out;
                mode = MPOL_PREFERRED;
                break;

Lee



--
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/5] tmpfs: handle MPOL_LOCAL mount option properly

[Hugh Dickins]

On Wed, 17 Mar 2010, Lee Schermerhorn wrote:
> On Thu, 2010-03-18 at 08:52 +0900, KOSAKI Motohiro wrote:
> > > On Tue, 16 Mar 2010, KOSAKI Motohiro wrote:
> > > 
> > > > commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> > > > shmem_sb_info) added mpol=local mount option. but its feature is
> > > > broken since it was born. because such code always return 1 (i.e.
> > > > mount failure).
> > > > 
> > > > This patch fixes it.
> > > > 
> > > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > > > Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> > > 
> > > Thank you both for finding and fixing these mpol embarrassments.
> > > 
> > > But if this "mpol=local" feature was never documented (not even in the
> > > commit log),

Sorry, I was absolutely wrong to say that: I seem to have been looking at
the wrong commit, but 3f226aa1 does document mpol=local, and does explain
why mpol=default differed; and KOSAKI-san's 5/5 corrects the mpol=default
description in tmpfs.txt, as well as adding mpol=local description.


> > > has been broken since birth 20 months ago, and nobody has
> > > noticed: wouldn't it be better to save a little bloat and just rip it out?
> > 
> > I have no objection if lee agreed, lee?
> > Of cource, if we agree it, we can make the new patch soon :)
> > 
> 
> Well, given the other issues with mpol_parse_str(), I suspect the entire
> tmpfs mpol option is not used all that often in the mainline kernel.  I
> recall that this feature was introduced by SGI for some of their
> customers who may depend on it.  There have been cases where I could
> have used it were it supported for the SYSV shm and MAP_ANON_MAP_SHARED
> internal tmpfs mount.  Further, note that the addition of "mpol=local"
> occurred between when the major "enterprise distributions" selected a
> new mainline kernel.  Production users of those distros, who are the
> likely users of this feature, tend not to live on the bleeding edge.
> So, maybe we shouldn't read too much into it not being discovered until
> now.

True.

> 
> That being said, I suppose I wouldn't be all that opposed to deprecating
> the entire tmpfs mpol option, and see who yells.   If the mpol mount
> options stays, I'd like to see the 'local' option stay.  It's a
> legitimate behavior that one can specify via the system calls and see
> via numa_maps, so I think the mpol mount option, if it exists, should
> support a way to specify it.  
> 
> As for bloat, the additional code on the mount option side to support it
> is:
> 
>         case MPOL_LOCAL:
>                 /*
>                  * Don't allow a nodelist;  mpol_new() checks flags
>                  */
>                 if (nodelist)
>                         goto out;
>                 mode = MPOL_PREFERRED;
>                 break;

Shocking :)

Yes, I withdraw my objection.  I'd been puzzled by why you would have
added an option which nobody was interested in, including yourself;
but as I see it now, you'd noticed a bug in mpol=default, and felt
that the right way to provide the missing functionality was to add
mpol=local: fair enough, let's keep it.

Hugh

--
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 4/5] tmpfs: cleanup mpol_parse_str()

[KOSAKI Motohiro]

mpol_parse_str() made lots 'err' variable related bug. because
it is ugly and reviewing unfriendly.

This patch makes simplify it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ravikiran Thirumalai <kiran@scalex86.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: <stable@kernel.org>
---
 mm/mempolicy.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 5c197d5..816419d 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2193,8 +2193,8 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 			char *rest = nodelist;
 			while (isdigit(*rest))
 				rest++;
-			if (!*rest)
-				err = 0;
+			if (*rest)
+				goto out;
 		}
 		break;
 	case MPOL_INTERLEAVE:
@@ -2203,7 +2203,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		 */
 		if (!nodelist)
 			nodes = node_states[N_HIGH_MEMORY];
-		err = 0;
 		break;
 	case MPOL_LOCAL:
 		/*
@@ -2212,7 +2211,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		if (nodelist)
 			goto out;
 		mode = MPOL_PREFERRED;
-		err = 0;
 		break;
 	case MPOL_DEFAULT:
 		/*
@@ -2227,7 +2225,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		 */
 		if (!nodelist)
 			goto out;
-		err = 0;
 	}
 
 	mode_flags = 0;
@@ -2241,13 +2238,14 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 		else if (!strcmp(flags, "relative"))
 			mode_flags |= MPOL_F_RELATIVE_NODES;
 		else
-			err = 1;
+			goto out;
 	}
 
 	new = mpol_new(mode, mode_flags, &nodes);
 	if (IS_ERR(new))
-		err = 1;
-	else {
+		goto out;
+
+	{
 		int ret;
 		NODEMASK_SCRATCH(scratch);
 		if (scratch) {
@@ -2258,13 +2256,15 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
 			ret = -ENOMEM;
 		NODEMASK_SCRATCH_FREE(scratch);
 		if (ret) {
-			err = 1;
 			mpol_put(new);
-		} else if (no_context) {
-			/* save for contextualization */
-			new->w.user_nodemask = nodes;
+			goto out;
 		}
 	}
+	err = 0;
+	if (no_context) {
+		/* save for contextualization */
+		new->w.user_nodemask = nodes;
+	}
 
 out:
 	/* Restore string for error message */
-- 
1.6.5.2



--
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 4/5] tmpfs: cleanup mpol_parse_str()

[Lee Schermerhorn]

On Tue, 2010-03-16 at 14:52 +0900, KOSAKI Motohiro wrote:
> mpol_parse_str() made lots 'err' variable related bug. because
> it is ugly and reviewing unfriendly.
> 
> This patch makes simplify it.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>

Nice cleanup.

Acked-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

> ---
>  mm/mempolicy.c |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 5c197d5..816419d 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2193,8 +2193,8 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  			char *rest = nodelist;
>  			while (isdigit(*rest))
>  				rest++;
> -			if (!*rest)
> -				err = 0;
> +			if (*rest)
> +				goto out;
>  		}
>  		break;
>  	case MPOL_INTERLEAVE:
> @@ -2203,7 +2203,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		 */
>  		if (!nodelist)
>  			nodes = node_states[N_HIGH_MEMORY];
> -		err = 0;
>  		break;
>  	case MPOL_LOCAL:
>  		/*
> @@ -2212,7 +2211,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		if (nodelist)
>  			goto out;
>  		mode = MPOL_PREFERRED;
> -		err = 0;
>  		break;
>  	case MPOL_DEFAULT:
>  		/*
> @@ -2227,7 +2225,6 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		 */
>  		if (!nodelist)
>  			goto out;
> -		err = 0;
>  	}
>  
>  	mode_flags = 0;
> @@ -2241,13 +2238,14 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  		else if (!strcmp(flags, "relative"))
>  			mode_flags |= MPOL_F_RELATIVE_NODES;
>  		else
> -			err = 1;
> +			goto out;
>  	}
>  
>  	new = mpol_new(mode, mode_flags, &nodes);
>  	if (IS_ERR(new))
> -		err = 1;
> -	else {
> +		goto out;
> +
> +	{
>  		int ret;
>  		NODEMASK_SCRATCH(scratch);
>  		if (scratch) {
> @@ -2258,13 +2256,15 @@ int mpol_parse_str(char *str, struct mempolicy **mpol, int no_context)
>  			ret = -ENOMEM;
>  		NODEMASK_SCRATCH_FREE(scratch);
>  		if (ret) {
> -			err = 1;
>  			mpol_put(new);
> -		} else if (no_context) {
> -			/* save for contextualization */
> -			new->w.user_nodemask = nodes;
> +			goto out;
>  		}
>  	}
> +	err = 0;
> +	if (no_context) {
> +		/* save for contextualization */
> +		new->w.user_nodemask = nodes;
> +	}
>  
>  out:
>  	/* Restore string for error message */

--
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 5/5] doc: add the documentation for mpol=local

[KOSAKI Motohiro]

commit 3f226aa1c (mempolicy: support mpol=local tmpfs mount option)
added new mpol=local mount option. but it didn't add a documentation.

This patch does it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ravikiran Thirumalai <kiran@scalex86.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: <stable@kernel.org>
---
 Documentation/filesystems/tmpfs.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/filesystems/tmpfs.txt b/Documentation/filesystems/tmpfs.txt
index 3015da0..fe09a2c 100644
--- a/Documentation/filesystems/tmpfs.txt
+++ b/Documentation/filesystems/tmpfs.txt
@@ -82,11 +82,13 @@ tmpfs has a mount option to set the NUMA memory allocation policy for
 all files in that instance (if CONFIG_NUMA is enabled) - which can be
 adjusted on the fly via 'mount -o remount ...'
 
-mpol=default             prefers to allocate memory from the local node
+mpol=default             use the process allocation policy
+                         (see set_mempolicy(2))
 mpol=prefer:Node         prefers to allocate memory from the given Node
 mpol=bind:NodeList       allocates memory only from nodes in NodeList
 mpol=interleave          prefers to allocate from each node in turn
 mpol=interleave:NodeList allocates from each node of NodeList in turn
+mpol=local		 prefers to allocate memory from the local node
 
 NodeList format is a comma-separated list of decimal numbers and ranges,
 a range being two hyphen-separated decimal numbers, the smallest and
@@ -134,3 +136,5 @@ Author:
    Christoph Rohland <cr@sap.com>, 1.12.01
 Updated:
    Hugh Dickins, 4 June 2007
+Updated:
+   KOSAKI Motohiro, 16 Mar 2010
-- 
1.6.5.2



--
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 5/5] doc: add the documentation for mpol=local

[Lee Schermerhorn]

On Tue, 2010-03-16 at 14:53 +0900, KOSAKI Motohiro wrote:
> commit 3f226aa1c (mempolicy: support mpol=local tmpfs mount option)
> added new mpol=local mount option. but it didn't add a documentation.
> 
> This patch does it.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Ravikiran Thirumalai <kiran@scalex86.org>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
> Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: <stable@kernel.org>

Acked-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

[Note:  looking at this patch in context, it appears that a few more
updates are in order.  E.g., "contextualization" of the specified
nodelists based on mems_allowed when one allocates a tmpfs file and what
happens when the mount option nodelist is disjoint from a task's
mems_allowed.  I'll test the behavior to make sure it matches my
expectations and update the doc accordingly in a subsequent patch.]

> ---
>  Documentation/filesystems/tmpfs.txt |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/filesystems/tmpfs.txt b/Documentation/filesystems/tmpfs.txt
> index 3015da0..fe09a2c 100644
> --- a/Documentation/filesystems/tmpfs.txt
> +++ b/Documentation/filesystems/tmpfs.txt
> @@ -82,11 +82,13 @@ tmpfs has a mount option to set the NUMA memory allocation policy for
>  all files in that instance (if CONFIG_NUMA is enabled) - which can be
>  adjusted on the fly via 'mount -o remount ...'
>  
> -mpol=default             prefers to allocate memory from the local node
> +mpol=default             use the process allocation policy
> +                         (see set_mempolicy(2))
>  mpol=prefer:Node         prefers to allocate memory from the given Node
>  mpol=bind:NodeList       allocates memory only from nodes in NodeList
>  mpol=interleave          prefers to allocate from each node in turn
>  mpol=interleave:NodeList allocates from each node of NodeList in turn
> +mpol=local		 prefers to allocate memory from the local node
>  
>  NodeList format is a comma-separated list of decimal numbers and ranges,
>  a range being two hyphen-separated decimal numbers, the smallest and
> @@ -134,3 +136,5 @@ Author:
>     Christoph Rohland <cr@sap.com>, 1.12.01
>  Updated:
>     Hugh Dickins, 4 June 2007
> +Updated:
> +   KOSAKI Motohiro, 16 Mar 2010

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