[PATCH] ramfs: ignore tmpfs options when we emulate it

[PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
another option might be to restore the previous behavior where ramfs simply
ignored all unknown mount options ...

 fs/ramfs/inode.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index 3a6b193..57a797c 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -203,6 +203,16 @@ static int ramfs_parse_options(char *data, struct ramfs_mount_opts *opts)
 			opts->mode = option & S_IALLUGO;
 			break;
 		default:
+#ifndef CONFIG_SHMEM
+			/* If tmpfs is using us to emulate it, ignore its options */
+			if (!strncmp(p, "gid=", 4) ||
+			    !strncmp(p, "mpol=", 5) ||
+			    !strncmp(p, "nr_blocks=", 10) ||
+			    !strncmp(p, "nr_inodes=", 10) ||
+			    !strncmp(p, "size=", 5) ||
+			    !strncmp(p, "uid=", 4))
+				continue;
+#endif
 			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
 			return -EINVAL;
 		}
-- 
1.6.3.1

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Hugh Dickins]

On Sat, 13 Jun 2009, Mike Frysinger wrote:

> On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> fail when tmpfs options are used.  This is because tmpfs creates a small
> wrapper around ramfs which rejects unknown options, and ramfs itself only
> supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> to use the same userspace systems across different configuration systems.
> As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> wrapper around ramfs.

Yes, indeed, thanks a lot for reporting this.

But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
(perhaps that's silly: certainly tmpfs behaviour differs with it),
and uneasy with coding a list of options we need to remember to keep
in synch with mm/shmem.c.  It's easier to justify ignoring all options,
than rejecting some while ignoring others yet not respecting them.

> 
> This used to work before commit c3b1b1cbf0 as previously, ramfs would
> ignore all options.  But now, we get:
> ramfs: bad mount option: size=10M
> mount: mounting mdev on /dev failed: Invalid argument

I rather think the correct response to bugzilla #12843 should have
been to say, either use chmod 1777 yourself, or use CONFIG_SHMEM=y.
I fear we'll now get a line of requests for support of uid, gid, ...
in ramfs; whereas ramfs is about blind simplicity, not feature bloat.
However, that mode= feature is now in, so I guess we ride with it.

> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> another option might be to restore the previous behavior where ramfs simply
> ignored all unknown mount options ...

Yes, that would be my preference, return to the blind simplicity, with
that one exception for mode=.  Alternative patch suggested at the bottom,
let's see if Cc's added feel strongly about it one way or another.

Thanks,
Hugh

> 
>  fs/ramfs/inode.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index 3a6b193..57a797c 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -203,6 +203,16 @@ static int ramfs_parse_options(char *data, struct ramfs_mount_opts *opts)
>  			opts->mode = option & S_IALLUGO;
>  			break;
>  		default:
> +#ifndef CONFIG_SHMEM
> +			/* If tmpfs is using us to emulate it, ignore its options */
> +			if (!strncmp(p, "gid=", 4) ||
> +			    !strncmp(p, "mpol=", 5) ||
> +			    !strncmp(p, "nr_blocks=", 10) ||
> +			    !strncmp(p, "nr_inodes=", 10) ||
> +			    !strncmp(p, "size=", 5) ||
> +			    !strncmp(p, "uid=", 4))
> +				continue;
> +#endif
>  			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
>  			return -EINVAL;
>  		}
> -- 
> 1.6.3.1

[PATCH] ramfs: ignore unknown mount options

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- 2.6.30/fs/ramfs/inode.c	2009-06-10 04:05:27.000000000 +0100
+++ linux/fs/ramfs/inode.c	2009-06-13 14:45:33.000000000 +0100
@@ -202,9 +202,12 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
-		default:
-			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
-			return -EINVAL;
+		/*
+		 * We might like to report bad mount options here;
+		 * but traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, better continue to ignore other mount options.
+		 */
 		}
 	}
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On Sat, Jun 13, 2009 at 10:15, Hugh Dickins wrote:
> On Sat, 13 Jun 2009, Mike Frysinger wrote:
>> On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
>> fail when tmpfs options are used.  This is because tmpfs creates a small
>> wrapper around ramfs which rejects unknown options, and ramfs itself only
>> supports a tiny subset of what tmpfs supports.  This makes it pretty hard
>> to use the same userspace systems across different configuration systems.
>> As such, ramfs should ignore the tmpfs options when tmpfs is merely a
>> wrapper around ramfs.
>
> Yes, indeed, thanks a lot for reporting this.
>
> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> (perhaps that's silly: certainly tmpfs behaviour differs with it),
> and uneasy with coding a list of options we need to remember to keep
> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> than rejecting some while ignoring others yet not respecting them.

agreed

>> This used to work before commit c3b1b1cbf0 as previously, ramfs would
>> ignore all options.  But now, we get:
>> ramfs: bad mount option: size=10M
>> mount: mounting mdev on /dev failed: Invalid argument
>
> I rather think the correct response to bugzilla #12843 should have
> been to say, either use chmod 1777 yourself, or use CONFIG_SHMEM=y.
> I fear we'll now get a line of requests for support of uid, gid, ...
> in ramfs; whereas ramfs is about blind simplicity, not feature bloat.
> However, that mode= feature is now in, so I guess we ride with it.

i thought the bug report a bit odd in more than just this regard.
glad to see i'm not the only one ;).

>> another option might be to restore the previous behavior where ramfs simply
>> ignored all unknown mount options ...
>
> Yes, that would be my preference, return to the blind simplicity, with
> that one exception for mode=.  Alternative patch suggested at the bottom,
> let's see if Cc's added feel strongly about it one way or another.

i'm OK with either approach, thanks !
-mike

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Matt Mackall]

On Sat, 2009-06-13 at 15:15 +0100, Hugh Dickins wrote:
> On Sat, 13 Jun 2009, Mike Frysinger wrote:
> 
> > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > fail when tmpfs options are used.  This is because tmpfs creates a small
> > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > to use the same userspace systems across different configuration systems.
> > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > wrapper around ramfs.
> 
> Yes, indeed, thanks a lot for reporting this.
> 
> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> (perhaps that's silly: certainly tmpfs behaviour differs with it),
> and uneasy with coding a list of options we need to remember to keep
> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> than rejecting some while ignoring others yet not respecting them.
> 
> > 
> > This used to work before commit c3b1b1cbf0 as previously, ramfs would
> > ignore all options.  But now, we get:
> > ramfs: bad mount option: size=10M
> > mount: mounting mdev on /dev failed: Invalid argument
> 
> I rather think the correct response to bugzilla #12843 should have
> been to say, either use chmod 1777 yourself, or use CONFIG_SHMEM=y.
> I fear we'll now get a line of requests for support of uid, gid, ...
> in ramfs; whereas ramfs is about blind simplicity, not feature bloat.
> However, that mode= feature is now in, so I guess we ride with it.

Ugh, hadn't noticed that go by.

> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > another option might be to restore the previous behavior where ramfs simply
> > ignored all unknown mount options ...
> 
> Yes, that would be my preference, return to the blind simplicity, with
> that one exception for mode=.  Alternative patch suggested at the bottom,
> let's see if Cc's added feel strongly about it one way or another.



> Thanks,
> Hugh
> 
> > 
> >  fs/ramfs/inode.c |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> > index 3a6b193..57a797c 100644
> > --- a/fs/ramfs/inode.c
> > +++ b/fs/ramfs/inode.c
> > @@ -203,6 +203,16 @@ static int ramfs_parse_options(char *data, struct ramfs_mount_opts *opts)
> >  			opts->mode = option & S_IALLUGO;
> >  			break;
> >  		default:
> > +#ifndef CONFIG_SHMEM
> > +			/* If tmpfs is using us to emulate it, ignore its options */
> > +			if (!strncmp(p, "gid=", 4) ||
> > +			    !strncmp(p, "mpol=", 5) ||
> > +			    !strncmp(p, "nr_blocks=", 10) ||
> > +			    !strncmp(p, "nr_inodes=", 10) ||
> > +			    !strncmp(p, "size=", 5) ||
> > +			    !strncmp(p, "uid=", 4))
> > +				continue;
> > +#endif
> >  			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> >  			return -EINVAL;
> >  		}
> > -- 
> > 1.6.3.1
> 
> [PATCH] ramfs: ignore unknown mount options
> 
> From: Mike Frysinger <vapier@gentoo.org>
> 
> On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> fail when tmpfs options are used.  This is because tmpfs creates a small
> wrapper around ramfs which rejects unknown options, and ramfs itself only
> supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> to use the same userspace systems across different configuration systems.
> As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> wrapper around ramfs.
> 
> This used to work before commit c3b1b1cbf0 as previously, ramfs would
> ignore all options.  But now, we get:
> ramfs: bad mount option: size=10M
> mount: mounting mdev on /dev failed: Invalid argument
> 
> Another option might be to restore the previous behavior, where ramfs
> simply ignored all unknown mount options ... which is what Hugh prefers.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Acked-by: Matt Mackall <mpm@selenic.com>

-- 
http://selenic.com : development and support for Mercurial and Linux

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> On Sat, 13 Jun 2009, Mike Frysinger wrote:
> 
> > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > fail when tmpfs options are used.  This is because tmpfs creates a small
> > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > to use the same userspace systems across different configuration systems.
> > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > wrapper around ramfs.
> 
> Yes, indeed, thanks a lot for reporting this.
> 
> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> (perhaps that's silly: certainly tmpfs behaviour differs with it),
> and uneasy with coding a list of options we need to remember to keep
> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> than rejecting some while ignoring others yet not respecting them.

We can avoid the burden of syncing a list of options between
ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
ramfs behave like other filesystems when used standalone.

Thanks,
Fengguang

---
[PATCH] ramfs: ignore unknown mount options

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Acked-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- linux.orig/fs/ramfs/inode.c
+++ linux/fs/ramfs/inode.c
@@ -202,9 +202,17 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
+#ifndef	CONFIG_SHMEM
+		/*
+		 * We might like to report bad mount options here;
+		 * but traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, better continue to ignore other mount options.
+		 */
 		default:
 			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
 			return -EINVAL;
+#endif
 		}
 	}
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On Sun, Jun 14, 2009 at 06:01, Wu Fengguang wrote:
> On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
>> On Sat, 13 Jun 2009, Mike Frysinger wrote:
>> > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
>> > fail when tmpfs options are used.  This is because tmpfs creates a small
>> > wrapper around ramfs which rejects unknown options, and ramfs itself only
>> > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
>> > to use the same userspace systems across different configuration systems.
>> > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
>> > wrapper around ramfs.
>>
>> Yes, indeed, thanks a lot for reporting this.
>>
>> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
>> (perhaps that's silly: certainly tmpfs behaviour differs with it),
>> and uneasy with coding a list of options we need to remember to keep
>> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
>> than rejecting some while ignoring others yet not respecting them.
>
> We can avoid the burden of syncing a list of options between
> ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> ramfs behave like other filesystems when used standalone.

i think Hugh's suggestion to change the behavior of ramfs back to the
way it has always been (ignore unknown options) is the way to go
rather than making it change behavior based on configuration
-mike

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 06:20:11PM +0800, Mike Frysinger wrote:
> On Sun, Jun 14, 2009 at 06:01, Wu Fengguang wrote:
> > On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> >> On Sat, 13 Jun 2009, Mike Frysinger wrote:
> >> > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> >> > fail when tmpfs options are used.  This is because tmpfs creates a small
> >> > wrapper around ramfs which rejects unknown options, and ramfs itself only
> >> > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> >> > to use the same userspace systems across different configuration systems.
> >> > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> >> > wrapper around ramfs.
> >>
> >> Yes, indeed, thanks a lot for reporting this.
> >>
> >> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> >> (perhaps that's silly: certainly tmpfs behaviour differs with it),
> >> and uneasy with coding a list of options we need to remember to keep
> >> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> >> than rejecting some while ignoring others yet not respecting them.
> >
> > We can avoid the burden of syncing a list of options between
> > ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> > ramfs behave like other filesystems when used standalone.
> 
> i think Hugh's suggestion to change the behavior of ramfs back to the
> way it has always been (ignore unknown options) is the way to go
> rather than making it change behavior based on configuration

Right. I've just posted a new patch. Does that make sense to you?

Thanks,
Fengguang

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Hugh Dickins]

On Sun, 14 Jun 2009, Wu Fengguang wrote:
> On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> > On Sat, 13 Jun 2009, Mike Frysinger wrote:
> > 
> > > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > > fail when tmpfs options are used.  This is because tmpfs creates a small
> > > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > > to use the same userspace systems across different configuration systems.
> > > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > > wrapper around ramfs.
> > 
> > Yes, indeed, thanks a lot for reporting this.
> > 
> > But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> > (perhaps that's silly: certainly tmpfs behaviour differs with it),
> > and uneasy with coding a list of options we need to remember to keep
> > in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> > than rejecting some while ignoring others yet not respecting them.
> 
> We can avoid the burden of syncing a list of options between
> ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> ramfs behave like other filesystems when used standalone.

We could do; but I'm still preferring not.  How about you, Matt?
You decide, I think Andrew has chosen a different race track from
"The Merge Window" this weekend.

Either of our patches (or Mike's orginal) fixes Mike's actual problem:
but I'd rather keep ramfs as close as we can to its original behaviour,
and as simple as possible; not making it behave differently in the
CONFIG_SHMEM=y and CONFIG_SHMEM=n cases (you can still "mount -t ramfs"
when ramfs is also being used to serve "mount -t tmpfs").

> 
> Thanks,
> Fengguang
> 
> ---
> [PATCH] ramfs: ignore unknown mount options
> 
> From: Mike Frysinger <vapier@gentoo.org>
> 
> On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> fail when tmpfs options are used.  This is because tmpfs creates a small
> wrapper around ramfs which rejects unknown options, and ramfs itself only
> supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> to use the same userspace systems across different configuration systems.
> As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> wrapper around ramfs.
> 
> This used to work before commit c3b1b1cbf0 as previously, ramfs would
> ignore all options.  But now, we get:
> ramfs: bad mount option: size=10M
> mount: mounting mdev on /dev failed: Invalid argument
> 
> Another option might be to restore the previous behavior, where ramfs
> simply ignored all unknown mount options ... which is what Hugh prefers.
> 
> Acked-by: Matt Mackall <mpm@selenic.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Sorry, no, this one is not yet Signed-off-by me (nor Acked yet by Matt).
Though I admit we're arguing over a trifle!

Hugh

> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> Cc: stable@kernel.org
> ---
> 
>  fs/ramfs/inode.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> --- linux.orig/fs/ramfs/inode.c
> +++ linux/fs/ramfs/inode.c
> @@ -202,9 +202,17 @@ static int ramfs_parse_options(char *dat
>  				return -EINVAL;
>  			opts->mode = option & S_IALLUGO;
>  			break;
> +#ifndef	CONFIG_SHMEM
> +		/*
> +		 * We might like to report bad mount options here;
> +		 * but traditionally ramfs has ignored all mount options,
> +		 * and as it is used as a !CONFIG_SHMEM simple substitute
> +		 * for tmpfs, better continue to ignore other mount options.
> +		 */
>  		default:
>  			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
>  			return -EINVAL;
> +#endif
>  		}
>  	}

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 06:39:56PM +0800, Hugh Dickins wrote:
> On Sun, 14 Jun 2009, Wu Fengguang wrote:
> > On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> > > On Sat, 13 Jun 2009, Mike Frysinger wrote:
> > > 
> > > > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > > > fail when tmpfs options are used.  This is because tmpfs creates a small
> > > > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > > > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > > > to use the same userspace systems across different configuration systems.
> > > > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > > > wrapper around ramfs.
> > > 
> > > Yes, indeed, thanks a lot for reporting this.
> > > 
> > > But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> > > (perhaps that's silly: certainly tmpfs behaviour differs with it),
> > > and uneasy with coding a list of options we need to remember to keep
> > > in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> > > than rejecting some while ignoring others yet not respecting them.
> > 
> > We can avoid the burden of syncing a list of options between
> > ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> > ramfs behave like other filesystems when used standalone.
> 
> We could do; but I'm still preferring not.  How about you, Matt?
> You decide, I think Andrew has chosen a different race track from
> "The Merge Window" this weekend.
> 
> Either of our patches (or Mike's orginal) fixes Mike's actual problem:
> but I'd rather keep ramfs as close as we can to its original behaviour,
> and as simple as possible; not making it behave differently in the
> CONFIG_SHMEM=y and CONFIG_SHMEM=n cases (you can still "mount -t ramfs"
> when ramfs is also being used to serve "mount -t tmpfs").

Right. Compatibility and simplicity makes sense. Do you agree to emit
a warning message though? This updated patch changes KERN_ERR to KERN_WARNING.

Thanks,
Fengguang

---
[PATCH] ramfs: ignore unknown mount options

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Acked-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--- linux.orig/fs/ramfs/inode.c
+++ linux/fs/ramfs/inode.c
@@ -202,9 +202,14 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
+		/*
+		 * Traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, ignore other mount options with a warning.
+		 */
 		default:
-			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
-			return -EINVAL;
+			printk(KERN_WARNING "ramfs: bad mount option: %s\n", p);
+			break;
 		}
 	}
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Matt Mackall]

On Sun, 2009-06-14 at 11:39 +0100, Hugh Dickins wrote:
> On Sun, 14 Jun 2009, Wu Fengguang wrote:
> > On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> > > On Sat, 13 Jun 2009, Mike Frysinger wrote:
> > > 
> > > > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > > > fail when tmpfs options are used.  This is because tmpfs creates a small
> > > > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > > > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > > > to use the same userspace systems across different configuration systems.
> > > > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > > > wrapper around ramfs.
> > > 
> > > Yes, indeed, thanks a lot for reporting this.
> > > 
> > > But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> > > (perhaps that's silly: certainly tmpfs behaviour differs with it),
> > > and uneasy with coding a list of options we need to remember to keep
> > > in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> > > than rejecting some while ignoring others yet not respecting them.
> > 
> > We can avoid the burden of syncing a list of options between
> > ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> > ramfs behave like other filesystems when used standalone.
> 
> We could do; but I'm still preferring not.  How about you, Matt?
> You decide, I think Andrew has chosen a different race track from
> "The Merge Window" this weekend.

I prefer the 'silently ignore' approach.

The only other approach that I think is reasonably clean is to
'subclass' ramfs by wrapping its init function with one that discards
mount args. Unfortunately that adds code and data in the 'I don't give a
damn, just keep it tiny' case, so that's a non-starter.

-- 
http://selenic.com : development and support for Mercurial and Linux

[PATCH] ramfs: ignore unknown mount options

[Hugh Dickins]

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Acked-by: Matt Mackall <mpm@selenic.com>
Acked-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- 2.6.30/fs/ramfs/inode.c	2009-06-10 04:05:27.000000000 +0100
+++ linux/fs/ramfs/inode.c	2009-06-13 14:45:33.000000000 +0100
@@ -202,9 +202,12 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
-		default:
-			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
-			return -EINVAL;
+		/*
+		 * We might like to report bad mount options here;
+		 * but traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, better continue to ignore other mount options.
+		 */
 		}
 	}
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
> On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> > On Sat, 13 Jun 2009, Mike Frysinger wrote:
> > 
> > > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > > fail when tmpfs options are used.  This is because tmpfs creates a small
> > > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > > to use the same userspace systems across different configuration systems.
> > > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > > wrapper around ramfs.
> > 
> > Yes, indeed, thanks a lot for reporting this.
> > 
> > But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> > (perhaps that's silly: certainly tmpfs behaviour differs with it),
> > and uneasy with coding a list of options we need to remember to keep
> > in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> > than rejecting some while ignoring others yet not respecting them.
> 
> We can avoid the burden of syncing a list of options between
> ramfs<>tmpfs by a slightly differently patch. Hopefully this makes
> ramfs behave like other filesystems when used standalone.

Sorry I take back the previous patch. It makes sense to not break
existing user space tools, but a warning message looks OK to remind
people of possibly unexpected behavior.

Thanks,
Fengguang
 
---
[PATCH] ramfs: ignore unknown mount options

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Acked-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- linux.orig/fs/ramfs/inode.c
+++ linux/fs/ramfs/inode.c
@@ -202,9 +202,14 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
+		/*
+		 * Traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, ignore other mount options with a warning.
+		 */
 		default:
 			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
-			return -EINVAL;
+			break;
 		}
 	}
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
> On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
> Sorry I take back the previous patch. It makes sense to not break
> existing user space tools, but a warning message looks OK to remind
> people of possibly unexpected behavior.
>
>                default:
>                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> -                       return -EINVAL;
> +                       break;

hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
this.  otherwise we end up with warnings that can (should) be ignored
when tmpfs is being emulated with ramfs.
-mike

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 06:46:24PM +0800, Mike Frysinger wrote:
> On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
> > On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
> > Sorry I take back the previous patch. It makes sense to not break
> > existing user space tools, but a warning message looks OK to remind
> > people of possibly unexpected behavior.
> >
> >                default:
> >                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> > -                       return -EINVAL;
> > +                       break;
> 
> hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
> this.  otherwise we end up with warnings that can (should) be ignored
> when tmpfs is being emulated with ramfs.

We may change the "ramfs:" accordingly. But *silently* ignoring
options is bad anyway?

Does this message look better?

+			printk(KERN_WARNING "%s: ignoring mount option: %s\n",
+			       sb->s_id, p);

Thanks,
Fengguang

---
[PATCH] ramfs: ignore unknown mount options

From: Mike Frysinger <vapier@gentoo.org>

On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
fail when tmpfs options are used.  This is because tmpfs creates a small
wrapper around ramfs which rejects unknown options, and ramfs itself only
supports a tiny subset of what tmpfs supports.  This makes it pretty hard
to use the same userspace systems across different configuration systems.
As such, ramfs should ignore the tmpfs options when tmpfs is merely a
wrapper around ramfs.

This used to work before commit c3b1b1cbf0 as previously, ramfs would
ignore all options.  But now, we get:
ramfs: bad mount option: size=10M
mount: mounting mdev on /dev failed: Invalid argument

Another option might be to restore the previous behavior, where ramfs
simply ignored all unknown mount options ... which is what Hugh prefers.

Acked-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: stable@kernel.org
---

 fs/ramfs/inode.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

--- linux.orig/fs/ramfs/inode.c
+++ linux/fs/ramfs/inode.c
@@ -182,8 +182,9 @@ struct ramfs_fs_info {
 	struct ramfs_mount_opts mount_opts;
 };
 
-static int ramfs_parse_options(char *data, struct ramfs_mount_opts *opts)
+static int ramfs_parse_options(struct super_block *sb, char *data)
 {
+	struct ramfs_mount_opts *opts = sb->s_fs_info;
 	substring_t args[MAX_OPT_ARGS];
 	int option;
 	int token;
@@ -202,9 +203,15 @@ static int ramfs_parse_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
+		/*
+		 * Traditionally ramfs has ignored all mount options,
+		 * and as it is used as a !CONFIG_SHMEM simple substitute
+		 * for tmpfs, ignore other mount options with a warning.
+		 */
 		default:
-			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
-			return -EINVAL;
+			printk(KERN_WARNING "%s: ignoring mount option: %s\n",
+			       sb->s_id, p);
+			break;
 		}
 	}
 
@@ -227,7 +234,7 @@ static int ramfs_fill_super(struct super
 		goto fail;
 	}
 
-	err = ramfs_parse_options(data, &fsi->mount_opts);
+	err = ramfs_parse_options(sb, data);
 	if (err)
 		goto fail;
 

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On Sun, Jun 14, 2009 at 07:14, Wu Fengguang wrote:
> On Sun, Jun 14, 2009 at 06:46:24PM +0800, Mike Frysinger wrote:
>> On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
>> > On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
>> > Sorry I take back the previous patch. It makes sense to not break
>> > existing user space tools, but a warning message looks OK to remind
>> > people of possibly unexpected behavior.
>> >
>> >                default:
>> >                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
>> > -                       return -EINVAL;
>> > +                       break;
>>
>> hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
>> this.  otherwise we end up with warnings that can (should) be ignored
>> when tmpfs is being emulated with ramfs.
>
> We may change the "ramfs:" accordingly. But *silently* ignoring
> options is bad anyway?

i really hate nitpicking such minor shit, but reality is that output
displayed in the kernel log that is incorrect is going to cause me
grief via customer support, updating documentation, adding FAQs,
etc... and i doubt i'm the only one here.

my requirement is simple: valid tmpfs options should be silently
consumed (i.e. ignored) when tmpfs is being emulated by ramfs (i.e.
CONFIG_SHMEM=n).

so how about:
default:
    if (!strcmp(sb->s_id, "ramfs"))
        printk(KERN_WARNING "%s: ignoring mount option: %s\n", sb->s_id, p);
    break;
-mike

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 07:26:37PM +0800, Mike Frysinger wrote:
> On Sun, Jun 14, 2009 at 07:14, Wu Fengguang wrote:
> > On Sun, Jun 14, 2009 at 06:46:24PM +0800, Mike Frysinger wrote:
> >> On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
> >> > On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
> >> > Sorry I take back the previous patch. It makes sense to not break
> >> > existing user space tools, but a warning message looks OK to remind
> >> > people of possibly unexpected behavior.
> >> >
> >> >                default:
> >> >                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> >> > -                       return -EINVAL;
> >> > +                       break;
> >>
> >> hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
> >> this.  otherwise we end up with warnings that can (should) be ignored
> >> when tmpfs is being emulated with ramfs.
> >
> > We may change the "ramfs:" accordingly. But *silently* ignoring
> > options is bad anyway?
> 
> i really hate nitpicking such minor shit, but reality is that output
> displayed in the kernel log that is incorrect is going to cause me
> grief via customer support, updating documentation, adding FAQs,
> etc... and i doubt i'm the only one here.

I don't think the message is "incorrect" - it is reminding user the fact.

But I didn't know that ignorant users will ask you for customer
support on the "new" warning. Sorry.

> my requirement is simple: valid tmpfs options should be silently
> consumed (i.e. ignored) when tmpfs is being emulated by ramfs (i.e.
> CONFIG_SHMEM=n).
> 
> so how about:
> default:
>     if (!strcmp(sb->s_id, "ramfs"))
>         printk(KERN_WARNING "%s: ignoring mount option: %s\n", sb->s_id, p);
>     break;

This is going overly complex, maybe we just revert to Hugh's original
patch for *complete* compatibility? Sorry for making a fuss.

Thanks,
Fengguang

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Mike Frysinger]

On Sun, Jun 14, 2009 at 07:49, Wu Fengguang wrote:
> On Sun, Jun 14, 2009 at 07:26:37PM +0800, Mike Frysinger wrote:
>> On Sun, Jun 14, 2009 at 07:14, Wu Fengguang wrote:
>> > On Sun, Jun 14, 2009 at 06:46:24PM +0800, Mike Frysinger wrote:
>> >> On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
>> >> > On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
>> >> > Sorry I take back the previous patch. It makes sense to not break
>> >> > existing user space tools, but a warning message looks OK to remind
>> >> > people of possibly unexpected behavior.
>> >> >
>> >> >                default:
>> >> >                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
>> >> > -                       return -EINVAL;
>> >> > +                       break;
>> >>
>> >> hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
>> >> this.  otherwise we end up with warnings that can (should) be ignored
>> >> when tmpfs is being emulated with ramfs.
>> >
>> > We may change the "ramfs:" accordingly. But *silently* ignoring
>> > options is bad anyway?
>>
>> i really hate nitpicking such minor shit, but reality is that output
>> displayed in the kernel log that is incorrect is going to cause me
>> grief via customer support, updating documentation, adding FAQs,
>> etc... and i doubt i'm the only one here.
>
> I don't think the message is "incorrect" - it is reminding user the fact.

when talking about ramfs, the message is correct -- the option is
wrong.  when talking about tmpfs emulated by ramfs, that may be a
matter of opinion.  i can understand why you still prefer a warning,
but there is a significant body of people out there (myself including)
that views warnings generally as something that should be addressed.
ignoring that, people who see warnings and dont understand what's
going on will ask/complain/whatever to someone somewhere.  including
an explanatory message along side the warning will make that number go
down, but it wont go away, and it sucks to have to do that.  ive seen
people ask questions where they copy & paste error messages that
already included explanatory text in it telling them how to
fix/resolve/research the issue.  i'm sure you have too :).

>> my requirement is simple: valid tmpfs options should be silently
>> consumed (i.e. ignored) when tmpfs is being emulated by ramfs (i.e.
>> CONFIG_SHMEM=n).
>>
>> so how about:
>> default:
>>     if (!strcmp(sb->s_id, "ramfs"))
>>         printk(KERN_WARNING "%s: ignoring mount option: %s\n", sb->s_id, p);
>>     break;
>
> This is going overly complex, maybe we just revert to Hugh's original
> patch for *complete* compatibility?

if my basic requirement is met, i dont care much about the details
beyond that :).
-mike

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sun, Jun 14, 2009 at 07:58:29PM +0800, Mike Frysinger wrote:
> On Sun, Jun 14, 2009 at 07:49, Wu Fengguang wrote:
> > On Sun, Jun 14, 2009 at 07:26:37PM +0800, Mike Frysinger wrote:
> >> On Sun, Jun 14, 2009 at 07:14, Wu Fengguang wrote:
> >> > On Sun, Jun 14, 2009 at 06:46:24PM +0800, Mike Frysinger wrote:
> >> >> On Sun, Jun 14, 2009 at 06:42, Wu Fengguang wrote:
> >> >> > On Sun, Jun 14, 2009 at 06:01:10PM +0800, Wu Fengguang wrote:
> >> >> > Sorry I take back the previous patch. It makes sense to not break
> >> >> > existing user space tools, but a warning message looks OK to remind
> >> >> > people of possibly unexpected behavior.
> >> >> >
> >> >> >                default:
> >> >> >                        printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> >> >> > -                       return -EINVAL;
> >> >> > +                       break;
> >> >>
> >> >> hmm, if the warning was wrapped in #ifdef CONFIG_SHMEM, i'd be ok with
> >> >> this.  otherwise we end up with warnings that can (should) be ignored
> >> >> when tmpfs is being emulated with ramfs.
> >> >
> >> > We may change the "ramfs:" accordingly. But *silently* ignoring
> >> > options is bad anyway?
> >>
> >> i really hate nitpicking such minor shit, but reality is that output
> >> displayed in the kernel log that is incorrect is going to cause me
> >> grief via customer support, updating documentation, adding FAQs,
> >> etc... and i doubt i'm the only one here.
> >
> > I don't think the message is "incorrect" - it is reminding user the fact.
> 
> when talking about ramfs, the message is correct -- the option is
> wrong.  when talking about tmpfs emulated by ramfs, that may be a
> matter of opinion.  i can understand why you still prefer a warning,
> but there is a significant body of people out there (myself including)
> that views warnings generally as something that should be addressed.

Right. It will upset me, too. It's kind of this situation: "I knew it
(that the option takes no effect), but please shut up!" ;-)

> ignoring that, people who see warnings and dont understand what's
> going on will ask/complain/whatever to someone somewhere.  including
> an explanatory message along side the warning will make that number go
> down, but it wont go away, and it sucks to have to do that.  ive seen

Yes that's truth. People are often ignoring.

> people ask questions where they copy & paste error messages that
> already included explanatory text in it telling them how to
> fix/resolve/research the issue.  i'm sure you have too :).

Too bad this happened to me countless times..

> >> my requirement is simple: valid tmpfs options should be silently
> >> consumed (i.e. ignored) when tmpfs is being emulated by ramfs (i.e.
> >> CONFIG_SHMEM=n).
> >>
> >> so how about:
> >> default:
> >>     if (!strcmp(sb->s_id, "ramfs"))
> >>         printk(KERN_WARNING "%s: ignoring mount option: %s\n", sb->s_id, p);
> >>     break;
> >
> > This is going overly complex, maybe we just revert to Hugh's original
> > patch for *complete* compatibility?
> 
> if my basic requirement is met, i dont care much about the details
> beyond that :).

OK. Let's do it the Hugh way. Thanks for the explanations!

Thanks,
Fengguang

Re: [PATCH] ramfs: ignore tmpfs options when we emulate it

[Wu Fengguang]

On Sat, Jun 13, 2009 at 10:15:51PM +0800, Hugh Dickins wrote:
> On Sat, 13 Jun 2009, Mike Frysinger wrote:
> 
> > On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> > fail when tmpfs options are used.  This is because tmpfs creates a small
> > wrapper around ramfs which rejects unknown options, and ramfs itself only
> > supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> > to use the same userspace systems across different configuration systems.
> > As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> > wrapper around ramfs.
> 
> Yes, indeed, thanks a lot for reporting this.
> 
> But I'm uneasy with making ramfs behaviour differ with CONFIG_SHMEM
> (perhaps that's silly: certainly tmpfs behaviour differs with it),
> and uneasy with coding a list of options we need to remember to keep
> in synch with mm/shmem.c.  It's easier to justify ignoring all options,
> than rejecting some while ignoring others yet not respecting them.
> 
[snip] 
> [PATCH] ramfs: ignore unknown mount options
> 
> From: Mike Frysinger <vapier@gentoo.org>
> 
> On systems where CONFIG_SHMEM is disabled, mounting tmpfs filesystems can
> fail when tmpfs options are used.  This is because tmpfs creates a small
> wrapper around ramfs which rejects unknown options, and ramfs itself only
> supports a tiny subset of what tmpfs supports.  This makes it pretty hard
> to use the same userspace systems across different configuration systems.
> As such, ramfs should ignore the tmpfs options when tmpfs is merely a
> wrapper around ramfs.
> 
> This used to work before commit c3b1b1cbf0 as previously, ramfs would
> ignore all options.  But now, we get:
> ramfs: bad mount option: size=10M
> mount: mounting mdev on /dev failed: Invalid argument
> 
> Another option might be to restore the previous behavior, where ramfs
> simply ignored all unknown mount options ... which is what Hugh prefers.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Cc: stable@kernel.org
> ---
> 
>  fs/ramfs/inode.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> --- 2.6.30/fs/ramfs/inode.c	2009-06-10 04:05:27.000000000 +0100
> +++ linux/fs/ramfs/inode.c	2009-06-13 14:45:33.000000000 +0100
> @@ -202,9 +202,12 @@ static int ramfs_parse_options(char *dat
>  				return -EINVAL;
>  			opts->mode = option & S_IALLUGO;
>  			break;
> -		default:
> -			printk(KERN_ERR "ramfs: bad mount option: %s\n", p);
> -			return -EINVAL;
> +		/*
> +		 * We might like to report bad mount options here;
> +		 * but traditionally ramfs has ignored all mount options,
> +		 * and as it is used as a !CONFIG_SHMEM simple substitute
> +		 * for tmpfs, better continue to ignore other mount options.
> +		 */
>  		}
>  	}
>  

Acked-by: Wu Fengguang <fengguang.wu@intel.com>