LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: DavidRientjes <rientjes@google.com>, Paul Jackson <pj@sgi.com>,
	Christoph Lameter <clameter@sgi.com>, Andi Kleen <ak@suse.de>,
	linux-kernel@vger.kernel.org, linux-mm <linux-mm@kvack.org>,
	Eric Whitney <eric.whitney@hp.com>
Subject: [PATCH] Mempolicy:  fix parsing of tmpfs mpol mount option
Date: Wed, 12 Mar 2008 15:33:15 -0400	[thread overview]
Message-ID: <1205350396.6000.56.camel@localhost> (raw)
In-Reply-To: <alpine.DEB.1.00.0803071341090.26765@chino.kir.corp.google.com>

Mempolicy:  fix parsing of tmpfs mpol= mount option.

Against:  2.6.25-rc5-mm1 atop:
+ mempolicy-disallow-static-or-relative-flags-for-local-preferred-mode
+ mempolicy-support-optional-mode-flags-fix [Hugh]

Parsing of new mode flags in the tmpfs mpol mount option is
slightly broken:

Setting a valid flag works OK:
	#mount -o remount,mpol=bind=static:1-2 /dev/shm
	#mount
	...
	tmpfs on /dev/shm type tmpfs (rw,mpol=bind=static:1-2)
	...

However, we can't remove them or change them, once we've
set a valid flag:

	#mount -o remount,mpol=bind:1-2 /dev/shm
	#mount
	...
	tmpfs on /dev/shm type tmpfs (rw,mpol=bind:1-2)
	...

It SAYS it removed it, but that's just a copy of the input
string.  If we now try to set it to a different flag, we
get:

	#mount -o remount,mpol=bind=relative:1-2 /dev/shm
	mount: /dev/shm not mounted already, or bad option

And on the console, we see:
	tmpfs: Bad value 'bind' for mount option 'mpol'
	                      ^ lost remainder of string

Furthermore, bogus flags are accepted with out error.
Granted, they are a no-op:

	#mount -o remount,mpol=interleave=foo:0-3 /dev/shm
	#mount
	...
	tmpfs on /dev/shm type tmpfs (rw,mpol=interleave=foo:0-3)

Again, that's just a copy of the input string shown by the
mount command.

This patch fixes the behavior by pre-zeroing the flags so that
only one of the mutually exclusive flags can be set at one time.
It also reports an error when an unrecognized flag is specified.

The check for both flags being set is removed because it can't
happen with this implementation.  If we ever want to support
multiple non-exclusive flags, this area will need rework and we
will need to check that any mutually exclusive flags aren't
specified.

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

 mm/shmem.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Index: linux-2.6.25-rc5-mm1/mm/shmem.c
===================================================================
--- linux-2.6.25-rc5-mm1.orig/mm/shmem.c	2008-03-12 14:15:27.000000000 -0400
+++ linux-2.6.25-rc5-mm1/mm/shmem.c	2008-03-12 14:18:09.000000000 -0400
@@ -1128,20 +1128,26 @@ static int shmem_parse_mpol(char *value,
 			*policy_nodes = node_states[N_HIGH_MEMORY];
 		err = 0;
 	}
+
+	*mode_flags = 0;
 	if (flags) {
+		/*
+		 * Currently, we only support two mutually exclusive
+		 * mode flags.
+		 */
 		if (!strcmp(flags, "static"))
 			*mode_flags |= MPOL_F_STATIC_NODES;
-		if (!strcmp(flags, "relative"))
+		else if (!strcmp(flags, "relative"))
 			*mode_flags |= MPOL_F_RELATIVE_NODES;
-
-		if ((*mode_flags & MPOL_F_STATIC_NODES) &&
-		    (*mode_flags & MPOL_F_RELATIVE_NODES))
-			err = 1;
+		else
+			err = 1;	/* unrecognized flag */
 	}
 out:
 	/* Restore string for error message */
 	if (nodelist)
 		*--nodelist = ':';
+	if (flags)
+		*--flags = '=';
 	return err;
 }
 



      parent reply	other threads:[~2008-03-12 19:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-06 20:05 [patch -mm 1/4] mempolicy: move rebind functions David Rientjes
2008-03-06 20:05 ` [patch -mm 2/4] mempolicy: create mempolicy_operations structure David Rientjes
2008-03-06 20:05   ` [patch -mm 3/4] mempolicy: small header file cleanup David Rientjes
2008-03-06 20:05     ` [patch -mm 4/4] mempolicy: remove includes for duplicate headers David Rientjes
2008-03-06 20:19       ` Paul Jackson
2008-03-06 20:51         ` David Rientjes
2008-03-06 21:01           ` Paul Jackson
2008-03-07  8:45             ` Andrew Morton
2008-03-07 20:44   ` Regression: Re: [patch -mm 2/4] mempolicy: create mempolicy_operations structure Lee Schermerhorn
2008-03-07 21:48     ` David Rientjes
2008-03-07 21:57       ` Paul Jackson
2008-03-08 18:49       ` Lee Schermerhorn
2008-03-08 22:09         ` David Rientjes
2008-03-10 14:58           ` Lee Schermerhorn
2008-03-12 19:33       ` Lee Schermerhorn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1205350396.6000.56.camel@localhost \
    --to=lee.schermerhorn@hp.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=clameter@sgi.com \
    --cc=eric.whitney@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pj@sgi.com \
    --cc=rientjes@google.com \
    --subject='Re: [PATCH] Mempolicy:  fix parsing of tmpfs mpol mount option' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).