[patch 00/26] mount options: fix filesystem's ->show_options

[patch 00/26] mount options: fix filesystem's ->show_options

[Miklos Szeredi]

Andrew,

Would you please consider these patches for -mm?  They should be
relatively uncontroversial and straightforward fixes.

They touch a lot of filesystems though, so not sure about the
logistics...

For the description, see first patch's header.

Thanks,
Miklos

--

[patch 01/26] mount options: add documentation

[Miklos Szeredi]

[-- Attachment #1: fs_options_doc.patch --]
[-- Type: text/plain, Size: 7091 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

This series addresses the problem of showing mount options in
/proc/mounts.

Several filesystems which use mount options, have not implemented a
.show_options superblock operation.  Several others have implemented
this callback, but have not kept it fully up to date with the parsed
options.

Q: Why do we need correct option showing in /proc/mounts?
A: We want /proc/mounts to fully replace /etc/mtab.  The reasons for
   this are:
    - unprivileged mounters won't be able to update /etc/mtab
    - /etc/mtab doesn't work with private mount namespaces
    - /etc/mtab can become out-of-sync with reality

Q: Can't this be done, so that filesystems need not bother with
   implementing a .show_mounts callback, and keeping it up to date?
A: Only in some cases.  Certain filesystems allow modification of a
   subset of options in their remount_fs method.  It is not possible
   to take this into account without knowing exactly how the
   filesystem handles options.

For the simple case (no remount or remount resets all options) the
patchset introduces two helpers:

  generic_show_options()
  save_mount_options()

These can also be used to emulate the old /etc/mtab behavior, until
proper support is added.  Even if this is not 100% correct, it's still
better than showing no options at all.

The following patches fix up most in-tree filesystems, they have been
compile tested only.  I would like to ask maintainers (CC-d on
respective patches) to please review, test and ACK these changes.

The following filesystems still need fixing: CIFS, NFS, XFS, Unionfs,
Reiser4.  For CIFS, NFS and XFS I wasn't able to understand how some
of the options are used.  The last two are not yet in mainline, so I
leave fixing those to their respective maintainers out of pure
laziness.

Table displaying status of all in-kernel filesystems:
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
legend:

  none - fs has options, but doesn't define ->show_options()
  some - fs defines ->show_options(), but some only options are shown
  most - fs defines ->show_options(), and shows most of them
  good - fs shows all options
  noopt - fs does not have options
  patch - a patch will be posted

9p          good
adfs        patch
affs        patch
afs         patch
autofs      patch
autofs4     patch
befs        patch
bfs         noopt
cifs        some
coda        noopt
configfs    noopt
cramfs      noopt
debugfs     noopt
devpts      patch
ecryptfs    good
efs         noopt
ext2        patch
ext3        good
ext4        patch
fat         patch
freevxfs    noopt
fuse        patch
fusectl	    noopt
gfs2        good
gfs2meta    noopt
hfs         good
hfsplus     good
hostfs      patch
hpfs        patch
hppfs       noopt
hugetlbfs   patch
isofs       patch
jffs2       noopt
jfs         patch
minix       noopt
msdos       ->fat
ncpfs       patch
nfs         patch,most
nfsd        noopt
ntfs        good
ocfs2       good
ocfs2/dlmfs noopt
openpromfs  noopt
proc        noopt
qnx4        noopt
ramfs       noopt
reiserfs    patch
romfs       noopt
smbfs       good
sysfs       noopt
sysv        noopt
udf         patch
ufs         good
vfat        ->fat
xfs         most

mm/shmem.c                                    patch
drivers/oprofile/oprofilefs.c                 noopt
drivers/infiniband/hw/ipath/ipath_fs.c        noopt
drivers/misc/ibmasm/ibmasmfs.c                noopt
drivers/usb/core (usbfs)                      patch
drivers/usb/gadget (gadgetfs)                 noopt
drivers/isdn/capi/capifs.c                    patch
kernel/cpuset.c                               noopt
fs/binfmt_misc.c                              noopt
net/sunrpc/rpc_pipe.c                         noopt
arch/powerpc/platforms/cell/spufs             patch
arch/s390/hypfs                               good
ipc/mqueue.c                                  noopt
security (securityfs)                         noopt
security/selinux/selinuxfs.c                  noopt
kernel/cgroup.c                               good
security/smack/smackfs.c                      noopt

in -mm:

reiser4     some
unionfs     none
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

This patch:

Document the rules for handling mount options in the .show_options
super operation.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/Documentation/filesystems/vfs.txt
===================================================================
--- linux.orig/Documentation/filesystems/vfs.txt	2008-01-24 11:42:48.000000000 +0100
+++ linux/Documentation/filesystems/vfs.txt	2008-01-24 17:12:25.000000000 +0100
@@ -151,7 +151,7 @@ The get_sb() method has the following ar
   const char *dev_name: the device name we are mounting.
 
   void *data: arbitrary mount options, usually comes as an ASCII
-	string
+	string (see "Mount Options" section)
 
   struct vfsmount *mnt: a vfs-internal representation of a mount point
 
@@ -182,7 +182,7 @@ A fill_super() method implementation has
   	must initialize this properly.
 
   void *data: arbitrary mount options, usually comes as an ASCII
-	string
+	string (see "Mount Options" section)
 
   int silent: whether or not to be silent on error
 
@@ -291,7 +291,8 @@ or bottom half).
 
   umount_begin: called when the VFS is unmounting a filesystem.
 
-  show_options: called by the VFS to show mount options for /proc/<pid>/mounts.
+  show_options: called by the VFS to show mount options for
+	/proc/<pid>/mounts.  (see "Mount Options" section)
 
   quota_read: called by the VFS to read from filesystem quota file.
 
@@ -969,6 +970,45 @@ manipulate dentries:
 For further information on dentry locking, please refer to the document
 Documentation/filesystems/dentry-locking.txt.
 
+Mount Options
+=============
+
+Parsing options
+---------------
+
+On mount and remount the filesystem is passed a string containing a
+comma separated list of mount options.  The options can have either of
+these forms:
+
+  option
+  option=value
+
+The <linux/parser.h> header defines an API that helps parse these
+options.  There are plenty of examples on how to use it in existing
+filesystems.
+
+Showing options
+---------------
+
+If a filesystem accepts mount options, it must define show_options()
+to show all the currently active options.  The rules are:
+
+  - options MUST be shown which are not default or their values differ
+    from the default
+
+  - options MAY be shown which are enabled by default or have their
+    default value
+
+Options used only internally between a mount helper and the kernel
+(such as file descriptors), or which only have an effect during the
+mounting (such as ones controlling the creation of a journal) are exempt
+from the above rules.
+
+A simple method of saving options at mount/remount time and showing
+them is provided with the save_mount_options() and
+generic_show_options() helper functions.  Please note, that using
+these may have drawbacks.  For more info see header comments for these
+functions in fs/namespace.c.
 
 Resources
 =========

--

[patch 02/26] mount options: add generic_show_options()

[Miklos Szeredi]

[-- Attachment #1: generic_show_options.patch --]
[-- Type: text/plain, Size: 3688 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a new s_options field to struct super_block.  Filesystems can save
mount options passed to them in mount or remount.  It is automatically
freed when the superblock is destroyed.

A new helper function, generic_show_options() is introduced, which uses
this field to display the mount options in /proc/mounts.

Another helper function, save_mount_options() may be used by
filesystems to save the options in the super block.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c	2008-01-24 17:07:46.000000000 +0100
+++ linux/fs/namespace.c	2008-01-24 17:34:50.000000000 +0100
@@ -575,6 +575,50 @@ void mnt_unpin(struct vfsmount *mnt)
 
 EXPORT_SYMBOL(mnt_unpin);
 
+static inline void mangle(struct seq_file *m, const char *s)
+{
+	seq_escape(m, s, " \t\n\\");
+}
+
+/*
+ * Simple .show_options callback for filesystems which don't want to
+ * implement more complex mount option showing.
+ *
+ * See also save_mount_options().
+ */
+int generic_show_options(struct seq_file *m, struct vfsmount *mnt)
+{
+	const char *options = mnt->mnt_sb->s_options;
+
+	if (options != NULL && options[0]) {
+		seq_putc(m, ',');
+		mangle(m, options);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(generic_show_options);
+
+/*
+ * If filesystem uses generic_show_options(), this function should be
+ * called from the fill_super() callback.
+ *
+ * The .remount_fs callback usually needs to be handled in a special
+ * way, to make sure, that previous options are not overwritten if the
+ * remount fails.
+ *
+ * Also note, that if the filesystem's .remount_fs function doesn't
+ * reset all options to their default value, but changes only newly
+ * given options, then the displayed options will not reflect reality
+ * any more.
+ */
+void save_mount_options(struct super_block *sb, char *options)
+{
+	kfree(sb->s_options);
+	sb->s_options = kstrdup(options, GFP_KERNEL);
+}
+EXPORT_SYMBOL(save_mount_options);
+
 /* iterator */
 static void *m_start(struct seq_file *m, loff_t *pos)
 {
@@ -596,11 +640,6 @@ static void m_stop(struct seq_file *m, v
 	up_read(&namespace_sem);
 }
 
-static inline void mangle(struct seq_file *m, const char *s)
-{
-	seq_escape(m, s, " \t\n\\");
-}
-
 static int show_vfsmnt(struct seq_file *m, void *v)
 {
 	struct vfsmount *mnt = list_entry(v, struct vfsmount, mnt_list);
Index: linux/fs/super.c
===================================================================
--- linux.orig/fs/super.c	2008-01-24 17:07:46.000000000 +0100
+++ linux/fs/super.c	2008-01-24 17:12:33.000000000 +0100
@@ -105,6 +105,7 @@ static inline void destroy_super(struct 
 {
 	security_sb_free(s);
 	kfree(s->s_subtype);
+	kfree(s->s_options);
 	kfree(s);
 }
 
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h	2008-01-24 17:07:46.000000000 +0100
+++ linux/include/linux/fs.h	2008-01-24 17:12:33.000000000 +0100
@@ -1042,6 +1042,12 @@ struct super_block {
 	 * in /proc/mounts will be "type.subtype"
 	 */
 	char *s_subtype;
+
+	/*
+	 * Saved mount options for lazy filesystems using
+	 * generic_show_options()
+	 */
+	char *s_options;
 };
 
 extern struct timespec current_fs_time(struct super_block *sb);
@@ -1992,6 +1998,9 @@ extern int __must_check inode_setattr(st
 
 extern void file_update_time(struct file *file);
 
+extern int generic_show_options(struct seq_file *m, struct vfsmount *mnt);
+extern void save_mount_options(struct super_block *sb, char *options);
+
 static inline ino_t parent_ino(struct dentry *dentry)
 {
 	ino_t res;

--

[patch 03/26] mount options: fix adfs

[Miklos Szeredi]

[-- Attachment #1: adfs_opts.patch --]
[-- Type: text/plain, Size: 2153 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to adfs.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/adfs/super.c
===================================================================
--- linux.orig/fs/adfs/super.c	2008-01-24 13:48:43.000000000 +0100
+++ linux/fs/adfs/super.c	2008-01-24 15:55:26.000000000 +0100
@@ -20,6 +20,8 @@
 #include <linux/vfs.h>
 #include <linux/parser.h>
 #include <linux/bitops.h>
+#include <linux/mount.h>
+#include <linux/seq_file.h>
 
 #include <asm/uaccess.h>
 #include <asm/system.h>
@@ -30,6 +32,9 @@
 #include "dir_f.h"
 #include "dir_fplus.h"
 
+#define ADFS_DEFAULT_OWNER_MASK S_IRWXU
+#define ADFS_DEFAULT_OTHER_MASK (S_IRWXG | S_IRWXO)
+
 void __adfs_error(struct super_block *sb, const char *function, const char *fmt, ...)
 {
 	char error_buf[128];
@@ -134,6 +139,22 @@ static void adfs_put_super(struct super_
 	sb->s_fs_info = NULL;
 }
 
+static int adfs_show_options(struct seq_file *seq, struct vfsmount *mnt)
+{
+	struct adfs_sb_info *asb = ADFS_SB(mnt->mnt_sb);
+
+	if (asb->s_uid != 0)
+		seq_printf(seq, ",uid=%u", asb->s_uid);
+	if (asb->s_gid != 0)
+		seq_printf(seq, ",gid=%u", asb->s_gid);
+	if (asb->s_owner_mask != ADFS_DEFAULT_OWNER_MASK)
+		seq_printf(seq, ",ownmask=%o", asb->s_owner_mask);
+	if (asb->s_other_mask != ADFS_DEFAULT_OTHER_MASK)
+		seq_printf(seq, ",othmask=%o", asb->s_other_mask);
+
+	return 0;
+}
+
 enum {Opt_uid, Opt_gid, Opt_ownmask, Opt_othmask, Opt_err};
 
 static match_table_t tokens = {
@@ -259,6 +280,7 @@ static const struct super_operations adf
 	.put_super	= adfs_put_super,
 	.statfs		= adfs_statfs,
 	.remount_fs	= adfs_remount,
+	.show_options	= adfs_show_options,
 };
 
 static struct adfs_discmap *adfs_read_map(struct super_block *sb, struct adfs_discrecord *dr)
@@ -344,8 +366,8 @@ static int adfs_fill_super(struct super_
 	/* set default options */
 	asb->s_uid = 0;
 	asb->s_gid = 0;
-	asb->s_owner_mask = S_IRWXU;
-	asb->s_other_mask = S_IRWXG | S_IRWXO;
+	asb->s_owner_mask = ADFS_DEFAULT_OWNER_MASK;
+	asb->s_other_mask = ADFS_DEFAULT_OTHER_MASK;
 
 	if (parse_options(sb, data))
 		goto error;

--

[patch 04/26] mount options: fix affs

[Miklos Szeredi]

[-- Attachment #1: affs_opts.patch --]
[-- Type: text/plain, Size: 1669 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to affs.

Use generic_show_options() and save the complete option string in
affs_fill_super() and affs_remount().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/affs/super.c
===================================================================
--- linux.orig/fs/affs/super.c	2008-01-24 18:57:19.000000000 +0100
+++ linux/fs/affs/super.c	2008-01-24 19:01:21.000000000 +0100
@@ -122,6 +122,7 @@ static const struct super_operations aff
 	.write_super	= affs_write_super,
 	.statfs		= affs_statfs,
 	.remount_fs	= affs_remount,
+	.show_options	= generic_show_options,
 };
 
 enum {
@@ -272,6 +273,8 @@ static int affs_fill_super(struct super_
 	u8			 sig[4];
 	int			 ret = -EINVAL;
 
+	save_mount_options(sb, data);
+
 	pr_debug("AFFS: read_super(%s)\n",data ? (const char *)data : "no options");
 
 	sb->s_magic             = AFFS_SUPER_MAGIC;
@@ -487,14 +490,21 @@ affs_remount(struct super_block *sb, int
 	int			 root_block;
 	unsigned long		 mount_flags;
 	int			 res = 0;
+	char			*new_opts = kstrdup(data, GFP_KERNEL);
 
 	pr_debug("AFFS: remount(flags=0x%x,opts=\"%s\")\n",*flags,data);
 
 	*flags |= MS_NODIRATIME;
 
-	if (!parse_options(data,&uid,&gid,&mode,&reserved,&root_block,
-	    &blocksize,&sbi->s_prefix,sbi->s_volume,&mount_flags))
+	if (!parse_options(data, &uid, &gid, &mode, &reserved, &root_block,
+			   &blocksize, &sbi->s_prefix, sbi->s_volume,
+			   &mount_flags)) {
+		kfree(new_opts);
 		return -EINVAL;
+	}
+	kfree(sb->s_options);
+	sb->s_options = new_opts;
+
 	sbi->s_flags = mount_flags;
 	sbi->s_mode  = mode;
 	sbi->s_uid   = uid;

--

[patch 05/26] mount options: fix afs

[Miklos Szeredi]

[-- Attachment #1: afs_opts.patch --]
[-- Type: text/plain, Size: 1369 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to afs.

Use generic_show_options() and save the complete option string in
afs_get_sb().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/afs/super.c
===================================================================
--- linux.orig/fs/afs/super.c	2008-01-24 11:42:44.000000000 +0100
+++ linux/fs/afs/super.c	2008-01-24 12:05:50.000000000 +0100
@@ -52,6 +52,7 @@ static const struct super_operations afs
 	.clear_inode	= afs_clear_inode,
 	.umount_begin	= afs_umount_begin,
 	.put_super	= afs_put_super,
+	.show_options	= generic_show_options,
 };
 
 static struct kmem_cache *afs_inode_cachep;
@@ -357,6 +358,7 @@ static int afs_get_sb(struct file_system
 	struct super_block *sb;
 	struct afs_volume *vol;
 	struct key *key;
+	char *new_opts = kstrdup(options, GFP_KERNEL);
 	int ret;
 
 	_enter(",,%s,%p", dev_name, options);
@@ -408,9 +410,11 @@ static int afs_get_sb(struct file_system
 			deactivate_super(sb);
 			goto error;
 		}
+		sb->s_options = new_opts;
 		sb->s_flags |= MS_ACTIVE;
 	} else {
 		_debug("reuse");
+		kfree(new_opts);
 		ASSERTCMP(sb->s_flags, &, MS_ACTIVE);
 	}
 
@@ -424,6 +428,7 @@ error:
 	afs_put_volume(params.volume);
 	afs_put_cell(params.cell);
 	key_put(params.key);
+	kfree(new_opts);
 	_leave(" = %d", ret);
 	return ret;
 }

--

[patch 06/26] mount options: fix autofs4

[Miklos Szeredi]

[-- Attachment #1: autofs4_opts.patch --]
[-- Type: text/plain, Size: 1002 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add uid= and gid= options to /proc/mounts for autofs4 filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/autofs4/inode.c
===================================================================
--- linux.orig/fs/autofs4/inode.c	2008-01-22 15:52:42.000000000 +0100
+++ linux/fs/autofs4/inode.c	2008-01-22 23:36:02.000000000 +0100
@@ -188,11 +188,16 @@ out_kill_sb:
 static int autofs4_show_options(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(mnt->mnt_sb);
+	struct inode *root_inode = mnt->mnt_sb->s_root->d_inode;
 
 	if (!sbi)
 		return 0;
 
 	seq_printf(m, ",fd=%d", sbi->pipefd);
+	if (root_inode->i_uid != 0)
+		seq_printf(m, ",uid=%u", root_inode->i_uid);
+	if (root_inode->i_gid != 0)
+		seq_printf(m, ",gid=%u", root_inode->i_gid);
 	seq_printf(m, ",pgrp=%d", sbi->oz_pgrp);
 	seq_printf(m, ",timeout=%lu", sbi->exp_timeout/HZ);
 	seq_printf(m, ",minproto=%d", sbi->min_proto);

--

[patch 07/26] mount options: fix autofs

[Miklos Szeredi]

[-- Attachment #1: autofs_opts.patch --]
[-- Type: text/plain, Size: 921 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to autofs.

Use generic_show_options() and save the complete option string in
autofs_fill_super().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/autofs/inode.c
===================================================================
--- linux.orig/fs/autofs/inode.c	2008-01-17 19:00:54.000000000 +0100
+++ linux/fs/autofs/inode.c	2008-01-24 11:16:30.000000000 +0100
@@ -54,6 +54,7 @@ out_kill_sb:
 
 static const struct super_operations autofs_sops = {
 	.statfs		= simple_statfs,
+	.show_options	= generic_show_options,
 };
 
 enum {Opt_err, Opt_fd, Opt_uid, Opt_gid, Opt_pgrp, Opt_minproto, Opt_maxproto};
@@ -140,6 +141,8 @@ int autofs_fill_super(struct super_block
 	int minproto, maxproto;
 	pid_t pgid;
 
+	save_mount_options(s, data);
+
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
 		goto fail_unlock;

--

[patch 08/26] mount options: fix befs

[Miklos Szeredi]

[-- Attachment #1: befs_opts.patch --]
[-- Type: text/plain, Size: 1086 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to befs.

Use generic_show_options() and save the complete option string in
befs_fill_super().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/befs/linuxvfs.c
===================================================================
--- linux.orig/fs/befs/linuxvfs.c	2008-01-17 19:00:54.000000000 +0100
+++ linux/fs/befs/linuxvfs.c	2008-01-22 21:40:05.000000000 +0100
@@ -57,6 +57,7 @@ static const struct super_operations bef
 	.put_super	= befs_put_super,	/* uninit super */
 	.statfs		= befs_statfs,	/* statfs */
 	.remount_fs	= befs_remount,
+	.show_options	= generic_show_options,
 };
 
 /* slab cache for befs_inode_info objects */
@@ -759,10 +760,11 @@ befs_fill_super(struct super_block *sb, 
 	befs_super_block *disk_sb;
 	struct inode *root;
 	long ret = -EINVAL;
-
 	const unsigned long sb_block = 0;
 	const off_t x86_sb_off = 512;
 
+	save_mount_options(sb, data);
+
 	sb->s_fs_info = kmalloc(sizeof (*befs_sb), GFP_KERNEL);
 	if (sb->s_fs_info == NULL) {
 		printk(KERN_ERR

--

[patch 09/26] mount options: fix capifs

[Miklos Szeredi]

[-- Attachment #1: capifs_opts.patch --]
[-- Type: text/plain, Size: 1188 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to capifs.

Use generic_show_options() and save the complete option string in
capifs_remount().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/drivers/isdn/capi/capifs.c
===================================================================
--- linux.orig/drivers/isdn/capi/capifs.c	2007-10-09 22:31:38.000000000 +0200
+++ linux/drivers/isdn/capi/capifs.c	2008-01-24 11:37:42.000000000 +0100
@@ -52,6 +52,7 @@ static int capifs_remount(struct super_b
 	gid_t gid = 0;
 	umode_t mode = 0600;
 	char *this_char;
+	char *new_opt = kstrdup(data, GFP_KERNEL);
 
 	this_char = NULL;
 	while ((this_char = strsep(&data, ",")) != NULL) {
@@ -72,11 +73,16 @@ static int capifs_remount(struct super_b
 			return -EINVAL;
 		}
 	}
+
+	kfree(s->s_options);
+	s->s_options = new_opt;
+
 	config.setuid  = setuid;
 	config.setgid  = setgid;
 	config.uid     = uid;
 	config.gid     = gid;
 	config.mode    = mode;
+
 	return 0;
 }
 
@@ -84,6 +90,7 @@ static struct super_operations capifs_so
 {
 	.statfs		= simple_statfs,
 	.remount_fs	= capifs_remount,
+	.show_options	= generic_show_options,
 };
 
 

--

[patch 10/26] mount options: fix devpts

[Miklos Szeredi]

[-- Attachment #1: devpts_opts.patch --]
[-- Type: text/plain, Size: 2119 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to devpts.

Also add minor fix: when parsing the "mode" option, mask with
S_IALLUGO instead of ~S_IFMT, which could leave unsed bits in the
mask.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/devpts/inode.c
===================================================================
--- linux.orig/fs/devpts/inode.c	2008-01-22 23:43:12.000000000 +0100
+++ linux/fs/devpts/inode.c	2008-01-23 13:01:05.000000000 +0100
@@ -20,9 +20,12 @@
 #include <linux/devpts_fs.h>
 #include <linux/parser.h>
 #include <linux/fsnotify.h>
+#include <linux/seq_file.h>
 
 #define DEVPTS_SUPER_MAGIC 0x1cd1
 
+#define DEVPTS_DEFAULT_MODE 0600
+
 static struct vfsmount *devpts_mnt;
 static struct dentry *devpts_root;
 
@@ -32,7 +35,7 @@ static struct {
 	uid_t   uid;
 	gid_t   gid;
 	umode_t mode;
-} config = {.mode = 0600};
+} config = {.mode = DEVPTS_DEFAULT_MODE};
 
 enum {
 	Opt_uid, Opt_gid, Opt_mode,
@@ -54,7 +57,7 @@ static int devpts_remount(struct super_b
 	config.setgid  = 0;
 	config.uid     = 0;
 	config.gid     = 0;
-	config.mode    = 0600;
+	config.mode    = DEVPTS_DEFAULT_MODE;
 
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -81,7 +84,7 @@ static int devpts_remount(struct super_b
 		case Opt_mode:
 			if (match_octal(&args[0], &option))
 				return -EINVAL;
-			config.mode = option & ~S_IFMT;
+			config.mode = option & S_IALLUGO;
 			break;
 		default:
 			printk(KERN_ERR "devpts: called with bogus options\n");
@@ -92,9 +95,22 @@ static int devpts_remount(struct super_b
 	return 0;
 }
 
+static int devpts_show_options(struct seq_file *seq, struct vfsmount *vfs)
+{
+	if (config.setuid)
+		seq_printf(seq, ",uid=%u", config.uid);
+	if (config.setgid)
+		seq_printf(seq, ",gid=%u", config.gid);
+	if (config.mode != DEVPTS_DEFAULT_MODE)
+		seq_printf(seq, ",mode=%03o", config.mode);
+
+	return 0;
+}
+
 static const struct super_operations devpts_sops = {
 	.statfs		= simple_statfs,
 	.remount_fs	= devpts_remount,
+	.show_options	= devpts_show_options,
 };
 
 static int

--

[patch 11/26] mount options: fix ext2

[Miklos Szeredi]

[-- Attachment #1: ext2_opts.patch --]
[-- Type: text/plain, Size: 575 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add noreservation option to /proc/mounts for ext2 filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/ext2/super.c
===================================================================
--- linux.orig/fs/ext2/super.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/ext2/super.c	2008-01-23 21:38:08.000000000 +0100
@@ -285,6 +285,9 @@ static int ext2_show_options(struct seq_
 		seq_puts(seq, ",xip");
 #endif
 
+	if (!test_opt(sb, RESERVATION))
+		seq_puts(seq, ",noreservation");
+
 	return 0;
 }
 

--

[patch 12/26] mount options: fix ext4

[Miklos Szeredi]

[-- Attachment #1: ext4_opts.patch --]
[-- Type: text/plain, Size: 668 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add stripe= option to /proc/mounts for ext4 filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/ext4/super.c
===================================================================
--- linux.orig/fs/ext4/super.c	2008-01-23 12:57:07.000000000 +0100
+++ linux/fs/ext4/super.c	2008-01-23 21:43:51.000000000 +0100
@@ -742,7 +742,8 @@ static int ext4_show_options(struct seq_
 		seq_puts(seq, ",nomballoc");
 	if (!test_opt(sb, DELALLOC))
 		seq_puts(seq, ",nodelalloc");
-
+	if (sbi->s_stripe)
+		seq_printf(seq, ",stripe=%lu", sbi->s_stripe);
 
 	/*
 	 * journal mode get enabled in different ways

--

[patch 13/26] mount options: fix fat

[Miklos Szeredi]

[-- Attachment #1: fat_opts.patch --]
[-- Type: text/plain, Size: 576 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add flush option to /proc/mounts for msdos and vfat filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/fat/inode.c
===================================================================
--- linux.orig/fs/fat/inode.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/fat/inode.c	2008-01-21 19:00:40.000000000 +0100
@@ -837,6 +837,8 @@ static int fat_show_options(struct seq_f
 		if (!opts->numtail)
 			seq_puts(m, ",nonumtail");
 	}
+	if (sbi->options.flush)
+		seq_puts(m, ",flush");
 
 	return 0;
 }

--

[patch 14/26] mount options: fix fuse

[Miklos Szeredi]

[-- Attachment #1: fuse_opts.patch --]
[-- Type: text/plain, Size: 1151 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add blksize= option to /proc/mounts for fuseblk filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/fuse/inode.c
===================================================================
--- linux.orig/fs/fuse/inode.c	2008-01-19 11:56:34.000000000 +0100
+++ linux/fs/fuse/inode.c	2008-01-21 17:53:06.000000000 +0100
@@ -29,6 +29,8 @@ DEFINE_MUTEX(fuse_mutex);
 
 #define FUSE_SUPER_MAGIC 0x65735546
 
+#define FUSE_DEFAULT_BLKSIZE 512
+
 struct fuse_mount_data {
 	int fd;
 	unsigned rootmode;
@@ -355,7 +357,7 @@ static int parse_fuse_opt(char *opt, str
 	char *p;
 	memset(d, 0, sizeof(struct fuse_mount_data));
 	d->max_read = ~0;
-	d->blksize = 512;
+	d->blksize = FUSE_DEFAULT_BLKSIZE;
 
 	while ((p = strsep(&opt, ",")) != NULL) {
 		int token;
@@ -440,6 +442,9 @@ static int fuse_show_options(struct seq_
 		seq_puts(m, ",allow_other");
 	if (fc->max_read != ~0)
 		seq_printf(m, ",max_read=%u", fc->max_read);
+	if (mnt->mnt_sb->s_bdev &&
+	    mnt->mnt_sb->s_blocksize != FUSE_DEFAULT_BLKSIZE)
+		seq_printf(m, ",blksize=%lu", mnt->mnt_sb->s_blocksize);
 	return 0;
 }
 

--

[patch 15/26] mount options: fix hostfs

[Miklos Szeredi]

[-- Attachment #1: hostfs_opts.patch --]
[-- Type: text/plain, Size: 1505 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add the "host path" option to /proc/mounts for UML hostfs filesystems.

The mount source (mnt_devname) should really be used for this, but not
easy to change now in a backward compatible way.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/hostfs/hostfs_kern.c
===================================================================
--- linux.orig/fs/hostfs/hostfs_kern.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/hostfs/hostfs_kern.c	2008-01-21 19:19:55.000000000 +0100
@@ -11,6 +11,7 @@
 #include <linux/mm.h>
 #include <linux/pagemap.h>
 #include <linux/statfs.h>
+#include <linux/seq_file.h>
 #include "hostfs.h"
 #include "init.h"
 #include "kern.h"
@@ -322,12 +323,25 @@ static void hostfs_destroy_inode(struct 
 	kfree(HOSTFS_I(inode));
 }
 
+static int hostfs_show_options(struct seq_file *seq, struct vfsmount *vfs)
+{
+	struct inode *root = vfs->mnt_sb->s_root->d_inode;
+	const char *root_path = HOSTFS_I(root)->host_filename;
+	size_t offset = strlen(root_ino) + 1;
+
+	if (strlen(root_path) > offset)
+		seq_printf(seq, ",%s", root_path + offset);
+
+	return 0;
+}
+
 static const struct super_operations hostfs_sbops = {
 	.alloc_inode	= hostfs_alloc_inode,
 	.drop_inode	= generic_delete_inode,
 	.delete_inode   = hostfs_delete_inode,
 	.destroy_inode	= hostfs_destroy_inode,
 	.statfs		= hostfs_statfs,
+	.show_options	= hostfs_show_options,
 };
 
 int hostfs_readdir(struct file *file, void *ent, filldir_t filldir)

--

[patch 16/26] mount options: fix hpfs

[Miklos Szeredi]

[-- Attachment #1: hpfs_opts.patch --]
[-- Type: text/plain, Size: 2036 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to hpfs.

Use generic_show_options() and save the complete option string in
hpfs_fill_super() and hpfs_remount_fs().

Also add a small fix: hpfs_remount_fs() should return -EINVAL on
error, instead of 1, which is not an error value.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/hpfs/super.c
===================================================================
--- linux.orig/fs/hpfs/super.c	2008-01-17 19:00:14.000000000 +0100
+++ linux/fs/hpfs/super.c	2008-01-23 23:36:53.000000000 +0100
@@ -386,6 +386,7 @@ static int hpfs_remount_fs(struct super_
 	int lowercase, conv, eas, chk, errs, chkdsk, timeshift;
 	int o;
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
+	char *new_opts = kstrdup(data, GFP_KERNEL);
 	
 	*flags |= MS_NOATIME;
 	
@@ -398,15 +399,15 @@ static int hpfs_remount_fs(struct super_
 	if (!(o = parse_opts(data, &uid, &gid, &umask, &lowercase, &conv,
 	    &eas, &chk, &errs, &chkdsk, &timeshift))) {
 		printk("HPFS: bad mount options.\n");
-	    	return 1;
+		goto out_err;
 	}
 	if (o == 2) {
 		hpfs_help();
-		return 1;
+		goto out_err;
 	}
 	if (timeshift != sbi->sb_timeshift) {
 		printk("HPFS: timeshift can't be changed using remount.\n");
-		return 1;
+		goto out_err;
 	}
 
 	unmark_dirty(s);
@@ -419,7 +420,14 @@ static int hpfs_remount_fs(struct super_
 
 	if (!(*flags & MS_RDONLY)) mark_dirty(s);
 
+	kfree(s->s_options);
+	s->s_options = new_opts;
+
 	return 0;
+
+out_err:
+	kfree(new_opts);
+	return -EINVAL;
 }
 
 /* Super operations */
@@ -432,6 +440,7 @@ static const struct super_operations hpf
 	.put_super	= hpfs_put_super,
 	.statfs		= hpfs_statfs,
 	.remount_fs	= hpfs_remount_fs,
+	.show_options	= generic_show_options,
 };
 
 static int hpfs_fill_super(struct super_block *s, void *options, int silent)
@@ -454,6 +463,8 @@ static int hpfs_fill_super(struct super_
 
 	int o;
 
+	save_mount_options(s, options);
+
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
 		return -ENOMEM;

--

[patch 17/26] mount options: fix hugetlbfs

[Miklos Szeredi]

[-- Attachment #1: hugetlbfs_opts.patch --]
[-- Type: text/plain, Size: 1045 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to hugetlbfs.

Use generic_show_options() and save the complete option string in
hugetlbfs_fill_super().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/hugetlbfs/inode.c
===================================================================
--- linux.orig/fs/hugetlbfs/inode.c	2008-01-22 21:31:53.000000000 +0100
+++ linux/fs/hugetlbfs/inode.c	2008-01-22 21:32:20.000000000 +0100
@@ -734,6 +734,7 @@ static const struct super_operations hug
 	.delete_inode	= hugetlbfs_delete_inode,
 	.drop_inode	= hugetlbfs_drop_inode,
 	.put_super	= hugetlbfs_put_super,
+	.show_options	= generic_show_options,
 };
 
 static int
@@ -817,6 +818,8 @@ hugetlbfs_fill_super(struct super_block 
 	struct hugetlbfs_config config;
 	struct hugetlbfs_sb_info *sbinfo;
 
+	save_mount_options(sb, data);
+
 	config.nr_blocks = -1; /* No limit on size by default */
 	config.nr_inodes = -1; /* No limit on number of inodes by default */
 	config.uid = current->fsuid;

--

[patch 18/26] mount options: fix isofs

[Miklos Szeredi]

[-- Attachment #1: isofs_opts.patch --]
[-- Type: text/plain, Size: 892 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to isofs.

Use generic_show_options() and save the complete option string in
isofs_fill_super().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/isofs/inode.c
===================================================================
--- linux.orig/fs/isofs/inode.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/isofs/inode.c	2008-01-23 22:07:51.000000000 +0100
@@ -110,6 +110,7 @@ static const struct super_operations iso
 	.put_super	= isofs_put_super,
 	.statfs		= isofs_statfs,
 	.remount_fs	= isofs_remount,
+	.show_options	= generic_show_options,
 };
 
 
@@ -554,6 +555,8 @@ static int isofs_fill_super(struct super
 	int table, error = -EINVAL;
 	unsigned int vol_desc_start;
 
+	save_mount_options(s, data);
+
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
 		return -ENOMEM;

--

[patch 19/26] mount options: fix jfs

[Miklos Szeredi]

[-- Attachment #1: jfs_opts.patch --]
[-- Type: text/plain, Size: 854 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add iocharset= and errors= options to /proc/mounts for jfs
filesystems.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/jfs/super.c
===================================================================
--- linux.orig/fs/jfs/super.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/jfs/super.c	2008-01-21 19:39:30.000000000 +0100
@@ -602,6 +602,12 @@ static int jfs_show_options(struct seq_f
 		seq_printf(seq, ",umask=%03o", sbi->umask);
 	if (sbi->flag & JFS_NOINTEGRITY)
 		seq_puts(seq, ",nointegrity");
+	if (sbi->nls_tab)
+		seq_printf(seq, ",iocharset=%s", sbi->nls_tab->charset);
+	if (sbi->flag & JFS_ERR_CONTINUE)
+		seq_printf(seq, ",errors=continue");
+	if (sbi->flag & JFS_ERR_PANIC)
+		seq_printf(seq, ",errors=panic");
 
 #ifdef CONFIG_QUOTA
 	if (sbi->flag & JFS_USRQUOTA)

--

[patch 20/26] mount options: fix ncpfs

[Miklos Szeredi]

[-- Attachment #1: ncpfs_opts.patch --]
[-- Type: text/plain, Size: 3398 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to ncpfs.

Small fix: add FS_BINARY_MOUNTDATA to the filesystem type flags, since
it can take binary data, as well as text (similarly to NFS).

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/ncpfs/inode.c
===================================================================
--- linux.orig/fs/ncpfs/inode.c	2008-01-24 13:48:37.000000000 +0100
+++ linux/fs/ncpfs/inode.c	2008-01-24 15:57:17.000000000 +0100
@@ -28,6 +28,8 @@
 #include <linux/init.h>
 #include <linux/smp_lock.h>
 #include <linux/vfs.h>
+#include <linux/mount.h>
+#include <linux/seq_file.h>
 
 #include <linux/ncp_fs.h>
 
@@ -36,9 +38,15 @@
 #include "ncplib_kernel.h"
 #include "getopt.h"
 
+#define NCP_DEFAULT_FILE_MODE 0600
+#define NCP_DEFAULT_DIR_MODE 0700
+#define NCP_DEFAULT_TIME_OUT 10
+#define NCP_DEFAULT_RETRY_COUNT 20
+
 static void ncp_delete_inode(struct inode *);
 static void ncp_put_super(struct super_block *);
 static int  ncp_statfs(struct dentry *, struct kstatfs *);
+static int  ncp_show_options(struct seq_file *, struct vfsmount *);
 
 static struct kmem_cache * ncp_inode_cachep;
 
@@ -96,6 +104,7 @@ static const struct super_operations ncp
 	.put_super	= ncp_put_super,
 	.statfs		= ncp_statfs,
 	.remount_fs	= ncp_remount,
+	.show_options	= ncp_show_options,
 };
 
 extern struct dentry_operations ncp_root_dentry_operations;
@@ -304,6 +313,37 @@ static void ncp_stop_tasks(struct ncp_se
 	flush_scheduled_work();
 }
 
+static int  ncp_show_options(struct seq_file *seq, struct vfsmount *mnt)
+{
+	struct ncp_server *server = NCP_SBP(mnt->mnt_sb);
+	unsigned int tmp;
+
+	if (server->m.uid != 0)
+		seq_printf(seq, ",uid=%u", server->m.uid);
+	if (server->m.gid != 0)
+		seq_printf(seq, ",gid=%u", server->m.gid);
+	if (server->m.mounted_uid != 0)
+		seq_printf(seq, ",owner=%u", server->m.mounted_uid);
+	tmp = server->m.file_mode & S_IALLUGO;
+	if (tmp != NCP_DEFAULT_FILE_MODE)
+		seq_printf(seq, ",mode=0%o", tmp);
+	tmp = server->m.dir_mode & S_IALLUGO;
+	if (tmp != NCP_DEFAULT_DIR_MODE)
+		seq_printf(seq, ",dirmode=0%o", tmp);
+	if (server->m.time_out != NCP_DEFAULT_TIME_OUT * HZ / 100) {
+		tmp = server->m.time_out * 100 / HZ;
+		seq_printf(seq, ",timeout=%u", tmp);
+	}
+	if (server->m.retry_count != NCP_DEFAULT_RETRY_COUNT)
+		seq_printf(seq, ",retry=%u", server->m.retry_count);
+	if (server->m.flags != 0)
+		seq_printf(seq, ",flags=%lu", server->m.flags);
+	if (server->m.wdog_pid != NULL)
+		seq_printf(seq, ",wdogpid=%u", pid_vnr(server->m.wdog_pid));
+
+	return 0;
+}
+
 static const struct ncp_option ncp_opts[] = {
 	{ "uid",	OPT_INT,	'u' },
 	{ "gid",	OPT_INT,	'g' },
@@ -331,12 +371,12 @@ static int ncp_parse_options(struct ncp_
 	data->mounted_uid = 0;
 	data->wdog_pid = NULL;
 	data->ncp_fd = ~0;
-	data->time_out = 10;
-	data->retry_count = 20;
+	data->time_out = NCP_DEFAULT_TIME_OUT;
+	data->retry_count = NCP_DEFAULT_RETRY_COUNT;
 	data->uid = 0;
 	data->gid = 0;
-	data->file_mode = 0600;
-	data->dir_mode = 0700;
+	data->file_mode = NCP_DEFAULT_FILE_MODE;
+	data->dir_mode = NCP_DEFAULT_DIR_MODE;
 	data->info_fd = -1;
 	data->mounted_vol[0] = 0;
 	
@@ -982,6 +1022,7 @@ static struct file_system_type ncp_fs_ty
 	.name		= "ncpfs",
 	.get_sb		= ncp_get_sb,
 	.kill_sb	= kill_anon_super,
+	.fs_flags	= FS_BINARY_MOUNTDATA,
 };
 
 static int __init init_ncp_fs(void)

--

[patch 21/26] mount options: partially fix nfs

[Miklos Szeredi]

[-- Attachment #1: nfs_opts.patch --]
[-- Type: text/plain, Size: 1975 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add posix, bsize=, namelen= options to /proc/mounts for nfs
filesystems.

Document several other options that are still missing.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/nfs/super.c
===================================================================
--- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
+++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
@@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
 	} nfs_info[] = {
 		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
 		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
+		{ NFS_MOUNT_POSIX, ",posix", "" },
 		{ NFS_MOUNT_NOCTO, ",nocto", "" },
 		{ NFS_MOUNT_NOAC, ",noac", "" },
 		{ NFS_MOUNT_NONLM, ",nolock", "" },
@@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
 	};
 	const struct proc_nfs_info *nfs_infop;
 	struct nfs_client *clp = nfss->nfs_client;
+	unsigned int default_namelen =
+		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
+		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
 
 	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
 	seq_printf(m, ",rsize=%d", nfss->rsize);
 	seq_printf(m, ",wsize=%d", nfss->wsize);
+	if (nfss->bsize != 0)
+		seq_printf(m, ",bsize=%d", nfss->bsize);
+	if (nfss->namelen != default_namelen)
+		seq_printf(m, ",namelen=%d", nfss->namelen);
 	if (nfss->acregmin != 3*HZ || showdefaults)
 		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
 	if (nfss->acregmax != 60*HZ || showdefaults)
@@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
 	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
 	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
 	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
+
+	/*
+	 * Missing options:
+	 * port=
+	 * mountport=
+	 * mountvers=
+	 * mountproto=
+	 * addr=
+	 * clientaddr=
+	 * mounthost=
+	 * mountaddr=
+	 */
 }
 
 /*

--

[patch 22/26] mount options: fix reiserfs

[Miklos Szeredi]

[-- Attachment #1: reiserfs_opts.patch --]
[-- Type: text/plain, Size: 3773 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to reiserfs.

Use generic_show_options() and save the complete option string in
reiserfs_fill_super() and reiserfs_remount().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/reiserfs/super.c
===================================================================
--- linux.orig/fs/reiserfs/super.c	2008-01-17 19:00:55.000000000 +0100
+++ linux/fs/reiserfs/super.c	2008-01-22 21:20:33.000000000 +0100
@@ -617,6 +617,7 @@ static const struct super_operations rei
 	.unlockfs = reiserfs_unlockfs,
 	.statfs = reiserfs_statfs,
 	.remount_fs = reiserfs_remount,
+	.show_options = generic_show_options,
 #ifdef CONFIG_QUOTA
 	.quota_read = reiserfs_quota_read,
 	.quota_write = reiserfs_quota_write,
@@ -1138,6 +1139,7 @@ static int reiserfs_remount(struct super
 	unsigned long safe_mask = 0;
 	unsigned int commit_max_age = (unsigned int)-1;
 	struct reiserfs_journal *journal = SB_JOURNAL(s);
+	char *new_opts = kstrdup(arg, GFP_KERNEL);
 	int err;
 #ifdef CONFIG_QUOTA
 	int i;
@@ -1153,7 +1155,8 @@ static int reiserfs_remount(struct super
 			REISERFS_SB(s)->s_qf_names[i] = NULL;
 		}
 #endif
-		return -EINVAL;
+		err = -EINVAL;
+		goto out_err;
 	}
 
 	handle_attrs(s);
@@ -1191,9 +1194,9 @@ static int reiserfs_remount(struct super
 	}
 
 	if (blocks) {
-		int rc = reiserfs_resize(s, blocks);
-		if (rc != 0)
-			return rc;
+		err = reiserfs_resize(s, blocks);
+		if (err != 0)
+			goto out_err;
 	}
 
 	if (*mount_flags & MS_RDONLY) {
@@ -1201,16 +1204,16 @@ static int reiserfs_remount(struct super
 		/* remount read-only */
 		if (s->s_flags & MS_RDONLY)
 			/* it is read-only already */
-			return 0;
+			goto out_ok;
 		/* try to remount file system with read-only permissions */
 		if (sb_umount_state(rs) == REISERFS_VALID_FS
 		    || REISERFS_SB(s)->s_mount_state != REISERFS_VALID_FS) {
-			return 0;
+			goto out_ok;
 		}
 
 		err = journal_begin(&th, s, 10);
 		if (err)
-			return err;
+			goto out_err;
 
 		/* Mounting a rw partition read-only. */
 		reiserfs_prepare_for_journal(s, SB_BUFFER_WITH_SB(s), 1);
@@ -1220,11 +1223,13 @@ static int reiserfs_remount(struct super
 		/* remount read-write */
 		if (!(s->s_flags & MS_RDONLY)) {
 			reiserfs_xattr_init(s, *mount_flags);
-			return 0;	/* We are read-write already */
+			goto out_ok;	/* We are read-write already */
 		}
 
-		if (reiserfs_is_journal_aborted(journal))
-			return journal->j_errno;
+		if (reiserfs_is_journal_aborted(journal)) {
+			err = journal->j_errno;
+			goto out_err;
+		}
 
 		handle_data_mode(s, mount_options);
 		handle_barrier_mode(s, mount_options);
@@ -1232,7 +1237,7 @@ static int reiserfs_remount(struct super
 		s->s_flags &= ~MS_RDONLY;	/* now it is safe to call journal_begin */
 		err = journal_begin(&th, s, 10);
 		if (err)
-			return err;
+			goto out_err;
 
 		/* Mount a partition which is read-only, read-write */
 		reiserfs_prepare_for_journal(s, SB_BUFFER_WITH_SB(s), 1);
@@ -1247,7 +1252,7 @@ static int reiserfs_remount(struct super
 	SB_JOURNAL(s)->j_must_wait = 1;
 	err = journal_end(&th, s, 10);
 	if (err)
-		return err;
+		goto out_err;
 	s->s_dirt = 0;
 
 	if (!(*mount_flags & MS_RDONLY)) {
@@ -1255,7 +1260,14 @@ static int reiserfs_remount(struct super
 		reiserfs_xattr_init(s, *mount_flags);
 	}
 
+out_ok:
+	kfree(s->s_options);
+	s->s_options = new_opts;
 	return 0;
+
+out_err:
+	kfree(new_opts);
+	return err;
 }
 
 static int read_super_block(struct super_block *s, int offset)
@@ -1559,6 +1571,8 @@ static int reiserfs_fill_super(struct su
 	struct reiserfs_sb_info *sbi;
 	int errval = -EINVAL;
 
+	save_mount_options(s, data);
+
 	sbi = kzalloc(sizeof(struct reiserfs_sb_info), GFP_KERNEL);
 	if (!sbi) {
 		errval = -ENOMEM;

--

[patch 23/26] mount options: fix spufs

[Miklos Szeredi]

[-- Attachment #1: spufs_opts.patch --]
[-- Type: text/plain, Size: 904 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to spufs.

Use generic_show_options() and save the complete option string in
spufs_fill_super().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/arch/powerpc/platforms/cell/spufs/inode.c
===================================================================
--- linux.orig/arch/powerpc/platforms/cell/spufs/inode.c	2008-01-17 19:00:52.000000000 +0100
+++ linux/arch/powerpc/platforms/cell/spufs/inode.c	2008-01-23 23:44:36.000000000 +0100
@@ -744,8 +744,11 @@ spufs_fill_super(struct super_block *sb,
 		.statfs = simple_statfs,
 		.delete_inode = spufs_delete_inode,
 		.drop_inode = generic_delete_inode,
+		.show_options = generic_show_options,
 	};
 
+	save_mount_options(sb, data);
+
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_blocksize = PAGE_CACHE_SIZE;
 	sb->s_blocksize_bits = PAGE_CACHE_SHIFT;

--

[patch 24/26] mount options: fix tmpfs

[Miklos Szeredi]

[-- Attachment #1: tmpfs_opts.patch --]
[-- Type: text/plain, Size: 12811 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add .show_options super operation to tmpfs.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/mm/shmem.c
===================================================================
--- linux.orig/mm/shmem.c	2008-01-21 21:20:04.000000000 +0100
+++ linux/mm/shmem.c	2008-01-21 21:30:04.000000000 +0100
@@ -49,6 +49,7 @@
 #include <linux/ctype.h>
 #include <linux/migrate.h>
 #include <linux/highmem.h>
+#include <linux/seq_file.h>
 
 #include <asm/uaccess.h>
 #include <asm/div64.h>
@@ -198,7 +199,7 @@ static DEFINE_MUTEX(shmem_swaplist_mutex
 static void shmem_free_blocks(struct inode *inode, long pages)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
-	if (sbinfo->max_blocks) {
+	if (sbinfo->config.max_blocks) {
 		spin_lock(&sbinfo->stat_lock);
 		sbinfo->free_blocks += pages;
 		inode->i_blocks -= pages*BLOCKS_PER_PAGE;
@@ -209,7 +210,7 @@ static void shmem_free_blocks(struct ino
 static int shmem_reserve_inode(struct super_block *sb)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
-	if (sbinfo->max_inodes) {
+	if (sbinfo->config.max_inodes) {
 		spin_lock(&sbinfo->stat_lock);
 		if (!sbinfo->free_inodes) {
 			spin_unlock(&sbinfo->stat_lock);
@@ -224,7 +225,7 @@ static int shmem_reserve_inode(struct su
 static void shmem_free_inode(struct super_block *sb)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
-	if (sbinfo->max_inodes) {
+	if (sbinfo->config.max_inodes) {
 		spin_lock(&sbinfo->stat_lock);
 		sbinfo->free_inodes++;
 		spin_unlock(&sbinfo->stat_lock);
@@ -388,7 +389,7 @@ static swp_entry_t *shmem_swp_alloc(stru
 		 * page (and perhaps indirect index pages) yet to allocate:
 		 * a waste to allocate index if we cannot allocate data.
 		 */
-		if (sbinfo->max_blocks) {
+		if (sbinfo->config.max_blocks) {
 			spin_lock(&sbinfo->stat_lock);
 			if (sbinfo->free_blocks <= 1) {
 				spin_unlock(&sbinfo->stat_lock);
@@ -1338,7 +1339,7 @@ repeat:
 	} else {
 		shmem_swp_unmap(entry);
 		sbinfo = SHMEM_SB(inode->i_sb);
-		if (sbinfo->max_blocks) {
+		if (sbinfo->config.max_blocks) {
 			spin_lock(&sbinfo->stat_lock);
 			if (sbinfo->free_blocks == 0 ||
 			    shmem_acct_block(info->flags)) {
@@ -1519,8 +1520,9 @@ shmem_get_inode(struct super_block *sb, 
 		case S_IFREG:
 			inode->i_op = &shmem_inode_operations;
 			inode->i_fop = &shmem_file_operations;
-			mpol_shared_policy_init(&info->policy, sbinfo->policy,
-							&sbinfo->policy_nodes);
+			mpol_shared_policy_init(&info->policy,
+						sbinfo->config.policy,
+						&sbinfo->config.policy_nodes);
 			break;
 		case S_IFDIR:
 			inc_nlink(inode);
@@ -1720,12 +1722,12 @@ static int shmem_statfs(struct dentry *d
 	buf->f_bsize = PAGE_CACHE_SIZE;
 	buf->f_namelen = NAME_MAX;
 	spin_lock(&sbinfo->stat_lock);
-	if (sbinfo->max_blocks) {
-		buf->f_blocks = sbinfo->max_blocks;
+	if (sbinfo->config.max_blocks) {
+		buf->f_blocks = sbinfo->config.max_blocks;
 		buf->f_bavail = buf->f_bfree = sbinfo->free_blocks;
 	}
-	if (sbinfo->max_inodes) {
-		buf->f_files = sbinfo->max_inodes;
+	if (sbinfo->config.max_inodes) {
+		buf->f_files = sbinfo->config.max_inodes;
 		buf->f_ffree = sbinfo->free_inodes;
 	}
 	/* else leave those fields 0 like simple_statfs */
@@ -2077,9 +2079,8 @@ static const struct export_operations sh
 	.fh_to_dentry	= shmem_fh_to_dentry,
 };
 
-static int shmem_parse_options(char *options, int *mode, uid_t *uid,
-	gid_t *gid, unsigned long *blocks, unsigned long *inodes,
-	int *policy, nodemask_t *policy_nodes)
+static int shmem_parse_options(char *options, struct shmem_config *config,
+			       bool remount)
 {
 	char *this_char, *value, *rest;
 
@@ -2122,35 +2123,43 @@ static int shmem_parse_options(char *opt
 			}
 			if (*rest)
 				goto bad_val;
-			*blocks = DIV_ROUND_UP(size, PAGE_CACHE_SIZE);
+			config->max_blocks =
+				DIV_ROUND_UP(size, PAGE_CACHE_SIZE);
+			config->max_blocks_changed = 1;
 		} else if (!strcmp(this_char,"nr_blocks")) {
-			*blocks = memparse(value,&rest);
+			config->max_blocks = memparse(value, &rest);
+			config->max_blocks_changed = 1;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"nr_inodes")) {
-			*inodes = memparse(value,&rest);
+			config->max_inodes = memparse(value, &rest);
+			config->max_inodes_changed = 1;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"mode")) {
-			if (!mode)
+			if (remount)
 				continue;
-			*mode = simple_strtoul(value,&rest,8);
+			config->mode = simple_strtoul(value, &rest, 8) & 07777;
+			config->mode_changed = 1;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"uid")) {
-			if (!uid)
+			if (remount)
 				continue;
-			*uid = simple_strtoul(value,&rest,0);
+			config->uid = simple_strtoul(value, &rest, 0);
+			config->uid_changed = 1;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"gid")) {
-			if (!gid)
+			if (remount)
 				continue;
-			*gid = simple_strtoul(value,&rest,0);
+			config->gid = simple_strtoul(value, &rest, 0);
+			config->gid_changed = 1;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"mpol")) {
-			if (shmem_parse_mpol(value,policy,policy_nodes))
+			if (shmem_parse_mpol(value, &config->policy,
+					     &config->policy_nodes))
 				goto bad_val;
 		} else {
 			printk(KERN_ERR "tmpfs: Bad mount option %s\n",
@@ -2170,24 +2179,20 @@ bad_val:
 static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
-	unsigned long max_blocks = sbinfo->max_blocks;
-	unsigned long max_inodes = sbinfo->max_inodes;
-	int policy = sbinfo->policy;
-	nodemask_t policy_nodes = sbinfo->policy_nodes;
+	struct shmem_config config = sbinfo->config;
 	unsigned long blocks;
 	unsigned long inodes;
 	int error = -EINVAL;
 
-	if (shmem_parse_options(data, NULL, NULL, NULL, &max_blocks,
-				&max_inodes, &policy, &policy_nodes))
+	if (shmem_parse_options(data, &config, true))
 		return error;
 
 	spin_lock(&sbinfo->stat_lock);
-	blocks = sbinfo->max_blocks - sbinfo->free_blocks;
-	inodes = sbinfo->max_inodes - sbinfo->free_inodes;
-	if (max_blocks < blocks)
+	blocks = sbinfo->config.max_blocks - sbinfo->free_blocks;
+	inodes = sbinfo->config.max_inodes - sbinfo->free_inodes;
+	if (config.max_blocks < blocks)
 		goto out;
-	if (max_inodes < inodes)
+	if (config.max_inodes < inodes)
 		goto out;
 	/*
 	 * Those tests also disallow limited->unlimited while any are in
@@ -2195,24 +2200,85 @@ static int shmem_remount_fs(struct super
 	 * but we must separately disallow unlimited->limited, because
 	 * in that case we have no record of how much is already in use.
 	 */
-	if (max_blocks && !sbinfo->max_blocks)
+	if (config.max_blocks && !sbinfo->config.max_blocks)
 		goto out;
-	if (max_inodes && !sbinfo->max_inodes)
+	if (config.max_inodes && !sbinfo->config.max_inodes)
 		goto out;
 
 	error = 0;
-	sbinfo->max_blocks  = max_blocks;
-	sbinfo->free_blocks = max_blocks - blocks;
-	sbinfo->max_inodes  = max_inodes;
-	sbinfo->free_inodes = max_inodes - inodes;
-	sbinfo->policy = policy;
-	sbinfo->policy_nodes = policy_nodes;
+	sbinfo->config = config;
+	sbinfo->free_blocks = config.max_blocks - blocks;
+	sbinfo->free_inodes = config.max_inodes - inodes;
 out:
 	spin_unlock(&sbinfo->stat_lock);
 	return error;
 }
+
+#ifdef CONFIG_NUMA
+static void shmem_show_mpol(struct seq_file *seq, int policy,
+			    const nodemask_t policy_nodes)
+{
+	seq_puts(seq, ",mpol=");
+	switch (policy) {
+	case MPOL_PREFERRED:
+		seq_puts(seq, "default");
+		break;
+	case MPOL_BIND:
+		seq_puts(seq, "bind");
+		break;
+	case MPOL_INTERLEAVE:
+		seq_puts(seq, "interleave");
+		break;
+	default:
+		seq_puts(seq, "default");
+	}
+
+	if (policy == MPOL_PREFERRED || policy == MPOL_BIND ||
+	    (policy == MPOL_INTERLEAVE &&
+	     !nodes_equal(policy_nodes, node_online_map))) {
+		char buffer[64];
+		int len;
+
+		len = nodelist_scnprintf(buffer, sizeof(buffer), policy_nodes);
+		if (len < sizeof(buffer))
+			seq_printf(seq, ":%s", buffer);
+		else
+			seq_printf(seq, ":?");
+	}
+}
+#else
+static void shmem_show_mpol(struct seq_file *seq, int policy,
+			    const nodemask_t policy_nodes)
+{
+}
 #endif
 
+static int shmem_show_options(struct seq_file *seq, struct vfsmount *vfs)
+{
+	struct super_block *sb = vfs->mnt_sb;
+	struct shmem_config *config = &SHMEM_SB(sb)->config;
+
+	if (config->max_blocks_changed) {
+		unsigned long sizek;
+
+		sizek = config->max_blocks << (PAGE_CACHE_SHIFT - 10);
+		seq_printf(seq, ",size=%luk", sizek);
+	}
+	if (config->max_inodes_changed)
+		seq_printf(seq, ",nr_inodes=%lu", config->max_inodes);
+	if (config->mode_changed)
+		seq_printf(seq, ",mode=%03o", config->mode);
+	if (config->uid_changed)
+		seq_printf(seq, ",uid=%u", config->uid);
+	if (config->gid_changed)
+		seq_printf(seq, ",gid=%u", config->gid);
+	if (config->policy != MPOL_DEFAULT)
+		shmem_show_mpol(seq, config->policy, config->policy_nodes);
+
+	return 0;
+}
+#endif /* CONFIG_TMPFS */
+
 static void shmem_put_super(struct super_block *sb)
 {
 	kfree(sb->s_fs_info);
@@ -2224,15 +2290,17 @@ static int shmem_fill_super(struct super
 {
 	struct inode *inode;
 	struct dentry *root;
-	int mode   = S_IRWXUGO | S_ISVTX;
-	uid_t uid = current->fsuid;
-	gid_t gid = current->fsgid;
 	int err = -ENOMEM;
 	struct shmem_sb_info *sbinfo;
-	unsigned long blocks = 0;
-	unsigned long inodes = 0;
-	int policy = MPOL_DEFAULT;
-	nodemask_t policy_nodes = node_states[N_HIGH_MEMORY];
+	struct shmem_config config = {
+		.mode = S_IRWXUGO | S_ISVTX,
+		.uid = current->fsuid,
+		.gid = current->fsgid,
+		.max_blocks = 0,
+		.max_inodes = 0,
+		.policy = MPOL_DEFAULT,
+		.policy_nodes = node_states[N_HIGH_MEMORY],
+	};
 
 #ifdef CONFIG_TMPFS
 	/*
@@ -2241,12 +2309,11 @@ static int shmem_fill_super(struct super
 	 * but the internal instance is left unlimited.
 	 */
 	if (!(sb->s_flags & MS_NOUSER)) {
-		blocks = totalram_pages / 2;
-		inodes = totalram_pages - totalhigh_pages;
-		if (inodes > blocks)
-			inodes = blocks;
-		if (shmem_parse_options(data, &mode, &uid, &gid, &blocks,
-					&inodes, &policy, &policy_nodes))
+		config.max_blocks = totalram_pages / 2;
+		config.max_inodes = totalram_pages - totalhigh_pages;
+		if (config.max_inodes > config.max_blocks)
+			config.max_inodes = config.max_blocks;
+		if (shmem_parse_options(data, &config, false))
 			return -EINVAL;
 	}
 	sb->s_export_op = &shmem_export_ops;
@@ -2261,12 +2328,9 @@ static int shmem_fill_super(struct super
 		return -ENOMEM;
 
 	spin_lock_init(&sbinfo->stat_lock);
-	sbinfo->max_blocks = blocks;
-	sbinfo->free_blocks = blocks;
-	sbinfo->max_inodes = inodes;
-	sbinfo->free_inodes = inodes;
-	sbinfo->policy = policy;
-	sbinfo->policy_nodes = policy_nodes;
+	sbinfo->config = config;
+	sbinfo->free_blocks = config.max_blocks;
+	sbinfo->free_inodes = config.max_inodes;
 
 	sb->s_fs_info = sbinfo;
 	sb->s_maxbytes = SHMEM_MAX_BYTES;
@@ -2280,11 +2344,11 @@ static int shmem_fill_super(struct super
 	sb->s_flags |= MS_POSIXACL;
 #endif
 
-	inode = shmem_get_inode(sb, S_IFDIR | mode, 0);
+	inode = shmem_get_inode(sb, S_IFDIR | config.mode, 0);
 	if (!inode)
 		goto failed;
-	inode->i_uid = uid;
-	inode->i_gid = gid;
+	inode->i_uid = config.uid;
+	inode->i_gid = config.gid;
 	root = d_alloc_root(inode);
 	if (!root)
 		goto failed_iput;
@@ -2420,6 +2484,7 @@ static const struct super_operations shm
 #ifdef CONFIG_TMPFS
 	.statfs		= shmem_statfs,
 	.remount_fs	= shmem_remount_fs,
+	.show_options	= shmem_show_options,
 #endif
 	.delete_inode	= shmem_delete_inode,
 	.drop_inode	= generic_delete_inode,
Index: linux/include/linux/shmem_fs.h
===================================================================
--- linux.orig/include/linux/shmem_fs.h	2008-01-21 21:20:04.000000000 +0100
+++ linux/include/linux/shmem_fs.h	2008-01-21 21:21:08.000000000 +0100
@@ -25,14 +25,27 @@ struct shmem_inode_info {
 #endif
 };
 
-struct shmem_sb_info {
+struct shmem_config {
+	mode_t mode;
+	uid_t uid;
+	gid_t gid;
 	unsigned long max_blocks;   /* How many blocks are allowed */
-	unsigned long free_blocks;  /* How many are left for allocation */
 	unsigned long max_inodes;   /* How many inodes are allowed */
-	unsigned long free_inodes;  /* How many are left for allocation */
 	int policy;		    /* Default NUMA memory alloc policy */
 	nodemask_t policy_nodes;    /* nodemask for preferred and bind */
+
+	unsigned mode_changed : 1;
+	unsigned uid_changed : 1;
+	unsigned gid_changed : 1;
+	unsigned max_blocks_changed : 1;
+	unsigned max_inodes_changed : 1;
+};
+
+struct shmem_sb_info {
+	unsigned long free_blocks;  /* How many are left for allocation */
+	unsigned long free_inodes;  /* How many are left for allocation */
 	spinlock_t    stat_lock;
+	struct shmem_config config;
 };
 
 static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)

--

[patch 25/26] mount options: fix udf

[Miklos Szeredi]

[-- Attachment #1: udf_opts.patch --]
[-- Type: text/plain, Size: 5232 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to udf.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/udf/super.c
===================================================================
--- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
+++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
@@ -53,6 +53,8 @@
 #include <linux/vfs.h>
 #include <linux/vmalloc.h>
 #include <linux/errno.h>
+#include <linux/mount.h>
+#include <linux/seq_file.h>
 #include <asm/byteorder.h>
 
 #include <linux/udf_fs.h>
@@ -71,6 +73,8 @@
 #define VDS_POS_TERMINATING_DESC	6
 #define VDS_POS_LENGTH			7
 
+#define UDF_DEFAULT_BLOCKSIZE 2048
+
 static char error_buf[1024];
 
 /* These are the "meat" - everything else is stuffing */
@@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
 static void udf_close_lvid(struct super_block *);
 static unsigned int udf_count_free(struct super_block *);
 static int udf_statfs(struct dentry *, struct kstatfs *);
+static int udf_show_options(struct seq_file *, struct vfsmount *);
 
 struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
 {
@@ -181,6 +186,7 @@ static const struct super_operations udf
 	.write_super	= udf_write_super,
 	.statfs		= udf_statfs,
 	.remount_fs	= udf_remount_fs,
+	.show_options	= udf_show_options,
 };
 
 struct udf_options {
@@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
 	return 0;
 }
 
+static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
+{
+	struct super_block *sb = mnt->mnt_sb;
+	struct udf_sb_info *sbi = UDF_SB(sb);
+
+	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
+		seq_puts(seq, ",nostrict");
+	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
+		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
+		seq_puts(seq, ",unhide");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
+		seq_puts(seq, ",undelete");
+	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_USE_AD_IN_ICB))
+		seq_puts(seq, ",noadinicb");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_USE_SHORT_AD))
+		seq_puts(seq, ",shortad");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_FORGET))
+		seq_puts(seq, ",uid=forget");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_IGNORE))
+		seq_puts(seq, ",uid=ignore");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_FORGET))
+		seq_puts(seq, ",gid=forget");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_IGNORE))
+		seq_puts(seq, ",gid=ignore");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_SET))
+		seq_printf(seq, ",uid=%u", sbi->s_uid);
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_SET))
+		seq_printf(seq, ",gid=%u", sbi->s_gid);
+	if (sbi->s_umask != 0)
+		seq_printf(seq, ",umask=%o", sbi->s_umask);
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
+		seq_printf(seq, ",session=%u", sbi->s_session);
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
+		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
+	/* is this correct? */
+	if (sbi->s_anchor[2] != 0)
+		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
+	/*
+	 * volume, partition, fileset and rootdir seem to be ignored
+	 * currently
+	 */
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8))
+		seq_puts(seq, ",utf8");
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP) && sbi->s_nls_map)
+		seq_printf(seq, ",iocharset=%s", sbi->s_nls_map->charset);
+
+	return 0;
+}
+
 /*
  * udf_parse_options
  *
@@ -339,13 +395,14 @@ static match_table_t tokens = {
 	{Opt_err,	NULL}
 };
 
-static int udf_parse_options(char *options, struct udf_options *uopt)
+static int udf_parse_options(char *options, struct udf_options *uopt,
+			     bool remount)
 {
 	char *p;
 	int option;
 
 	uopt->novrs = 0;
-	uopt->blocksize = 2048;
+	uopt->blocksize = UDF_DEFAULT_BLOCKSIZE;
 	uopt->partition = 0xFFFF;
 	uopt->session = 0xFFFFFFFF;
 	uopt->lastblock = 0;
@@ -415,11 +472,15 @@ static int udf_parse_options(char *optio
 			if (match_int(args, &option))
 				return 0;
 			uopt->session = option;
+			if (!remount)
+				uopt->flags |= (1 << UDF_FLAG_SESSION_SET);
 			break;
 		case Opt_lastblock:
 			if (match_int(args, &option))
 				return 0;
 			uopt->lastblock = option;
+			if (!remount)
+				uopt->flags |= (1 << UDF_FLAG_LASTBLOCK_SET);
 			break;
 		case Opt_anchor:
 			if (match_int(args, &option))
@@ -497,7 +558,7 @@ static int udf_remount_fs(struct super_b
 	uopt.gid   = sbi->s_gid;
 	uopt.umask = sbi->s_umask;
 
-	if (!udf_parse_options(options, &uopt))
+	if (!udf_parse_options(options, &uopt, true))
 		return -EINVAL;
 
 	sbi->s_flags = uopt.flags;
@@ -1679,7 +1740,7 @@ static int udf_fill_super(struct super_b
 
 	mutex_init(&sbi->s_alloc_mutex);
 
-	if (!udf_parse_options((char *)options, &uopt))
+	if (!udf_parse_options((char *)options, &uopt, false))
 		goto error_out;
 
 	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
Index: linux/fs/udf/udf_sb.h
===================================================================
--- linux.orig/fs/udf/udf_sb.h	2008-01-24 13:48:37.000000000 +0100
+++ linux/fs/udf/udf_sb.h	2008-01-24 13:51:08.000000000 +0100
@@ -26,6 +26,8 @@
 #define UDF_FLAG_GID_IGNORE     14
 #define UDF_FLAG_UID_SET	15
 #define UDF_FLAG_GID_SET	16
+#define UDF_FLAG_SESSION_SET	17
+#define UDF_FLAG_LASTBLOCK_SET	18
 
 #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
 #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002

--

[patch 26/26] mount options: fix usbfs

[Miklos Szeredi]

[-- Attachment #1: usbfs_opts.patch --]
[-- Type: text/plain, Size: 2735 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a .show_options super operation to usbfs.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/drivers/usb/core/inode.c
===================================================================
--- linux.orig/drivers/usb/core/inode.c	2008-01-24 13:48:37.000000000 +0100
+++ linux/drivers/usb/core/inode.c	2008-01-24 16:00:03.000000000 +0100
@@ -38,10 +38,15 @@
 #include <linux/usbdevice_fs.h>
 #include <linux/parser.h>
 #include <linux/notifier.h>
+#include <linux/seq_file.h>
 #include <asm/byteorder.h>
 #include "usb.h"
 #include "hcd.h"
 
+#define USBFS_DEFAULT_DEVMODE (S_IWUSR | S_IRUGO)
+#define USBFS_DEFAULT_BUSMODE (S_IXUGO | S_IRUGO)
+#define USBFS_DEFAULT_LISTMODE S_IRUGO
+
 static struct super_operations usbfs_ops;
 static const struct file_operations default_file_operations;
 static struct vfsmount *usbfs_mount;
@@ -57,9 +62,33 @@ static uid_t listuid;	/* = 0 */
 static gid_t devgid;	/* = 0 */
 static gid_t busgid;	/* = 0 */
 static gid_t listgid;	/* = 0 */
-static umode_t devmode = S_IWUSR | S_IRUGO;
-static umode_t busmode = S_IXUGO | S_IRUGO;
-static umode_t listmode = S_IRUGO;
+static umode_t devmode = USBFS_DEFAULT_DEVMODE;
+static umode_t busmode = USBFS_DEFAULT_BUSMODE;
+static umode_t listmode = USBFS_DEFAULT_LISTMODE;
+
+static int usbfs_show_options(struct seq_file *seq, struct vfsmount *mnt)
+{
+	if (devuid != 0)
+		seq_printf(seq, ",devuid=%u", devuid);
+	if (devgid != 0)
+		seq_printf(seq, ",devgid=%u", devgid);
+	if (devmode != USBFS_DEFAULT_DEVMODE)
+		seq_printf(seq, ",devmode=%o", devmode);
+	if (busuid != 0)
+		seq_printf(seq, ",busuid=%u", busuid);
+	if (busgid != 0)
+		seq_printf(seq, ",busgid=%u", busgid);
+	if (busmode != USBFS_DEFAULT_BUSMODE)
+		seq_printf(seq, ",busmode=%o", busmode);
+	if (listuid != 0)
+		seq_printf(seq, ",listuid=%u", listuid);
+	if (listgid != 0)
+		seq_printf(seq, ",listgid=%u", listgid);
+	if (listmode != USBFS_DEFAULT_LISTMODE)
+		seq_printf(seq, ",listmode=%o", listmode);
+
+	return 0;
+}
 
 enum {
 	Opt_devuid, Opt_devgid, Opt_devmode,
@@ -93,9 +122,9 @@ static int parse_options(struct super_bl
 	devgid = 0;
 	busgid = 0;
 	listgid = 0;
-	devmode = S_IWUSR | S_IRUGO;
-	busmode = S_IXUGO | S_IRUGO;
-	listmode = S_IRUGO;
+	devmode = USBFS_DEFAULT_DEVMODE;
+	busmode = USBFS_DEFAULT_BUSMODE;
+	listmode = USBFS_DEFAULT_LISTMODE;
 
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -418,6 +447,7 @@ static struct super_operations usbfs_ops
 	.statfs =	simple_statfs,
 	.drop_inode =	generic_delete_inode,
 	.remount_fs =	remount,
+	.show_options = usbfs_show_options,
 };
 
 static int usbfs_fill_super(struct super_block *sb, void *data, int silent)

--

Re: [patch 10/26] mount options: fix devpts

[H. Peter Anvin]

Miklos Szeredi wrote:
> Also add minor fix: when parsing the "mode" option, mask with
> S_IALLUGO instead of ~S_IFMT, which could leave unsed bits in the
> mask.

umode_t is 16 bits, so it doesn't.  The change is still good, of course.

> +	if (config.mode != DEVPTS_DEFAULT_MODE)
> +		seq_printf(seq, ",mode=%03o", config.mode);

I would rather this be unconditional, than that it be conditional on 
something other than the user having specified it in the first place.

Other than that,

Acked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa

Re: [patch 07/26] mount options: fix autofs

[H. Peter Anvin]

Miklos Szeredi wrote:
[autofs patch]

Acked-by: H. Peter Anvin <hpa@zytor.com>

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +
|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
| +		seq_puts(seq, ",unhide");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
| +		seq_puts(seq, ",undelete");
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_USE_AD_IN_ICB))
| +		seq_puts(seq, ",noadinicb");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_USE_SHORT_AD))
| +		seq_puts(seq, ",shortad");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_FORGET))
| +		seq_puts(seq, ",uid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_IGNORE))
| +		seq_puts(seq, ",uid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_FORGET))
| +		seq_puts(seq, ",gid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_IGNORE))
| +		seq_puts(seq, ",gid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_SET))
| +		seq_printf(seq, ",uid=%u", sbi->s_uid);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_SET))
| +		seq_printf(seq, ",gid=%u", sbi->s_gid);
| +	if (sbi->s_umask != 0)
| +		seq_printf(seq, ",umask=%o", sbi->s_umask);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
| +		seq_printf(seq, ",session=%u", sbi->s_session);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
| +		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
| +	/* is this correct? */
| +	if (sbi->s_anchor[2] != 0)
| +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
| +	/*
| +	 * volume, partition, fileset and rootdir seem to be ignored
| +	 * currently
| +	 */
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8))
| +		seq_puts(seq, ",utf8");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP) && sbi->s_nls_map)
| +		seq_printf(seq, ",iocharset=%s", sbi->s_nls_map->charset);
| +
| +	return 0;
| +}
| +
|  /*
|   * udf_parse_options
|   *
| @@ -339,13 +395,14 @@ static match_table_t tokens = {
|  	{Opt_err,	NULL}
|  };
|  
| -static int udf_parse_options(char *options, struct udf_options *uopt)
| +static int udf_parse_options(char *options, struct udf_options *uopt,
| +			     bool remount)
|  {
|  	char *p;
|  	int option;
|  
|  	uopt->novrs = 0;
| -	uopt->blocksize = 2048;
| +	uopt->blocksize = UDF_DEFAULT_BLOCKSIZE;
|  	uopt->partition = 0xFFFF;
|  	uopt->session = 0xFFFFFFFF;
|  	uopt->lastblock = 0;
| @@ -415,11 +472,15 @@ static int udf_parse_options(char *optio
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->session = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_SESSION_SET);
|  			break;
|  		case Opt_lastblock:
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->lastblock = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_LASTBLOCK_SET);
|  			break;
|  		case Opt_anchor:
|  			if (match_int(args, &option))
| @@ -497,7 +558,7 @@ static int udf_remount_fs(struct super_b
|  	uopt.gid   = sbi->s_gid;
|  	uopt.umask = sbi->s_umask;
|  
| -	if (!udf_parse_options(options, &uopt))
| +	if (!udf_parse_options(options, &uopt, true))
|  		return -EINVAL;
|  
|  	sbi->s_flags = uopt.flags;
| @@ -1679,7 +1740,7 @@ static int udf_fill_super(struct super_b
|  
|  	mutex_init(&sbi->s_alloc_mutex);
|  
| -	if (!udf_parse_options((char *)options, &uopt))
| +	if (!udf_parse_options((char *)options, &uopt, false))
|  		goto error_out;
|  
|  	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
| Index: linux/fs/udf/udf_sb.h
| ===================================================================
| --- linux.orig/fs/udf/udf_sb.h	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/udf_sb.h	2008-01-24 13:51:08.000000000 +0100
| @@ -26,6 +26,8 @@
|  #define UDF_FLAG_GID_IGNORE     14
|  #define UDF_FLAG_UID_SET	15
|  #define UDF_FLAG_GID_SET	16
| +#define UDF_FLAG_SESSION_SET	17
| +#define UDF_FLAG_LASTBLOCK_SET	18
|  
|  #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
|  #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
| 
| --
| 

Jan Kara CC'ed

		- Cyrill -

Re: [patch 03/26] mount options: fix adfs

[Russell King]

On Thu, Jan 24, 2008 at 08:33:44PM +0100, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add a .show_options super operation to adfs.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Thanks.

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +

thanks, that is good cleanup

|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
| +		seq_puts(seq, ",unhide");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
| +		seq_puts(seq, ",undelete");
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_USE_AD_IN_ICB))
| +		seq_puts(seq, ",noadinicb");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_USE_SHORT_AD))
| +		seq_puts(seq, ",shortad");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_FORGET))
| +		seq_puts(seq, ",uid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_IGNORE))
| +		seq_puts(seq, ",uid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_FORGET))
| +		seq_puts(seq, ",gid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_IGNORE))
| +		seq_puts(seq, ",gid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_SET))
| +		seq_printf(seq, ",uid=%u", sbi->s_uid);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_SET))
| +		seq_printf(seq, ",gid=%u", sbi->s_gid);
| +	if (sbi->s_umask != 0)
| +		seq_printf(seq, ",umask=%o", sbi->s_umask);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
| +		seq_printf(seq, ",session=%u", sbi->s_session);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
| +		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
| +	/* is this correct? */
| +	if (sbi->s_anchor[2] != 0)
| +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);

you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
in sake of style unification but we should wait for Jan's
decision (i'm not the expert in this area ;)

| +	/*
| +	 * volume, partition, fileset and rootdir seem to be ignored
| +	 * currently
| +	 */
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8))
| +		seq_puts(seq, ",utf8");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP) && sbi->s_nls_map)
| +		seq_printf(seq, ",iocharset=%s", sbi->s_nls_map->charset);
| +
| +	return 0;
| +}
| +
|  /*
|   * udf_parse_options
|   *
| @@ -339,13 +395,14 @@ static match_table_t tokens = {
|  	{Opt_err,	NULL}
|  };
|  
| -static int udf_parse_options(char *options, struct udf_options *uopt)
| +static int udf_parse_options(char *options, struct udf_options *uopt,
| +			     bool remount)
|  {
|  	char *p;
|  	int option;
|  
|  	uopt->novrs = 0;
| -	uopt->blocksize = 2048;
| +	uopt->blocksize = UDF_DEFAULT_BLOCKSIZE;
|  	uopt->partition = 0xFFFF;
|  	uopt->session = 0xFFFFFFFF;
|  	uopt->lastblock = 0;
| @@ -415,11 +472,15 @@ static int udf_parse_options(char *optio
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->session = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_SESSION_SET);
|  			break;
|  		case Opt_lastblock:
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->lastblock = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_LASTBLOCK_SET);
|  			break;
|  		case Opt_anchor:
|  			if (match_int(args, &option))
| @@ -497,7 +558,7 @@ static int udf_remount_fs(struct super_b
|  	uopt.gid   = sbi->s_gid;
|  	uopt.umask = sbi->s_umask;
|  
| -	if (!udf_parse_options(options, &uopt))
| +	if (!udf_parse_options(options, &uopt, true))
|  		return -EINVAL;
|  
|  	sbi->s_flags = uopt.flags;
| @@ -1679,7 +1740,7 @@ static int udf_fill_super(struct super_b
|  
|  	mutex_init(&sbi->s_alloc_mutex);
|  
| -	if (!udf_parse_options((char *)options, &uopt))
| +	if (!udf_parse_options((char *)options, &uopt, false))
|  		goto error_out;
|  
|  	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
| Index: linux/fs/udf/udf_sb.h
| ===================================================================
| --- linux.orig/fs/udf/udf_sb.h	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/udf_sb.h	2008-01-24 13:51:08.000000000 +0100
| @@ -26,6 +26,8 @@
|  #define UDF_FLAG_GID_IGNORE     14
|  #define UDF_FLAG_UID_SET	15
|  #define UDF_FLAG_GID_SET	16
| +#define UDF_FLAG_SESSION_SET	17
| +#define UDF_FLAG_LASTBLOCK_SET	18
|  
|  #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
|  #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
| 
| --
| 

Other then that it looks ok for me (feel free to Ack by me
if you need it ;)

		- Cyrill -

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

[Miklos Szeredi - Thu, Jan 24, 2008 at 08:34:06PM +0100]
| From: Miklos Szeredi <mszeredi@suse.cz>
| 
| Add a .show_options super operation to udf.
| 
| Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
| ---
| 
| Index: linux/fs/udf/super.c
| ===================================================================
| --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
| @@ -53,6 +53,8 @@
|  #include <linux/vfs.h>
|  #include <linux/vmalloc.h>
|  #include <linux/errno.h>
| +#include <linux/mount.h>
| +#include <linux/seq_file.h>
|  #include <asm/byteorder.h>
|  
|  #include <linux/udf_fs.h>
| @@ -71,6 +73,8 @@
|  #define VDS_POS_TERMINATING_DESC	6
|  #define VDS_POS_LENGTH			7
|  
| +#define UDF_DEFAULT_BLOCKSIZE 2048
| +
|  static char error_buf[1024];
|  
|  /* These are the "meat" - everything else is stuffing */
| @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
|  static void udf_close_lvid(struct super_block *);
|  static unsigned int udf_count_free(struct super_block *);
|  static int udf_statfs(struct dentry *, struct kstatfs *);
| +static int udf_show_options(struct seq_file *, struct vfsmount *);
|  
|  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
|  {
| @@ -181,6 +186,7 @@ static const struct super_operations udf
|  	.write_super	= udf_write_super,
|  	.statfs		= udf_statfs,
|  	.remount_fs	= udf_remount_fs,
| +	.show_options	= udf_show_options,
|  };
|  
|  struct udf_options {
| @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
|  	return 0;
|  }
|  
| +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
| +{
| +	struct super_block *sb = mnt->mnt_sb;
| +	struct udf_sb_info *sbi = UDF_SB(sb);
| +
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
| +		seq_puts(seq, ",nostrict");
| +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
| +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
| +		seq_puts(seq, ",unhide");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
| +		seq_puts(seq, ",undelete");
| +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_USE_AD_IN_ICB))
| +		seq_puts(seq, ",noadinicb");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_USE_SHORT_AD))
| +		seq_puts(seq, ",shortad");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_FORGET))
| +		seq_puts(seq, ",uid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_IGNORE))
| +		seq_puts(seq, ",uid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_FORGET))
| +		seq_puts(seq, ",gid=forget");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_IGNORE))
| +		seq_puts(seq, ",gid=ignore");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_SET))
| +		seq_printf(seq, ",uid=%u", sbi->s_uid);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_SET))
| +		seq_printf(seq, ",gid=%u", sbi->s_gid);
| +	if (sbi->s_umask != 0)
| +		seq_printf(seq, ",umask=%o", sbi->s_umask);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
| +		seq_printf(seq, ",session=%u", sbi->s_session);
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
| +		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
| +	/* is this correct? */
| +	if (sbi->s_anchor[2] != 0)
| +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);

btw, Miklos, i think you may strip off != and use just if (val)
form, like "if (sbi->s_umask)"

| +	/*
| +	 * volume, partition, fileset and rootdir seem to be ignored
| +	 * currently
| +	 */
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8))
| +		seq_puts(seq, ",utf8");
| +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP) && sbi->s_nls_map)
| +		seq_printf(seq, ",iocharset=%s", sbi->s_nls_map->charset);
| +
| +	return 0;
| +}
| +
|  /*
|   * udf_parse_options
|   *
| @@ -339,13 +395,14 @@ static match_table_t tokens = {
|  	{Opt_err,	NULL}
|  };
|  
| -static int udf_parse_options(char *options, struct udf_options *uopt)
| +static int udf_parse_options(char *options, struct udf_options *uopt,
| +			     bool remount)
|  {
|  	char *p;
|  	int option;
|  
|  	uopt->novrs = 0;
| -	uopt->blocksize = 2048;
| +	uopt->blocksize = UDF_DEFAULT_BLOCKSIZE;
|  	uopt->partition = 0xFFFF;
|  	uopt->session = 0xFFFFFFFF;
|  	uopt->lastblock = 0;
| @@ -415,11 +472,15 @@ static int udf_parse_options(char *optio
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->session = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_SESSION_SET);
|  			break;
|  		case Opt_lastblock:
|  			if (match_int(args, &option))
|  				return 0;
|  			uopt->lastblock = option;
| +			if (!remount)
| +				uopt->flags |= (1 << UDF_FLAG_LASTBLOCK_SET);
|  			break;
|  		case Opt_anchor:
|  			if (match_int(args, &option))
| @@ -497,7 +558,7 @@ static int udf_remount_fs(struct super_b
|  	uopt.gid   = sbi->s_gid;
|  	uopt.umask = sbi->s_umask;
|  
| -	if (!udf_parse_options(options, &uopt))
| +	if (!udf_parse_options(options, &uopt, true))
|  		return -EINVAL;
|  
|  	sbi->s_flags = uopt.flags;
| @@ -1679,7 +1740,7 @@ static int udf_fill_super(struct super_b
|  
|  	mutex_init(&sbi->s_alloc_mutex);
|  
| -	if (!udf_parse_options((char *)options, &uopt))
| +	if (!udf_parse_options((char *)options, &uopt, false))
|  		goto error_out;
|  
|  	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
| Index: linux/fs/udf/udf_sb.h
| ===================================================================
| --- linux.orig/fs/udf/udf_sb.h	2008-01-24 13:48:37.000000000 +0100
| +++ linux/fs/udf/udf_sb.h	2008-01-24 13:51:08.000000000 +0100
| @@ -26,6 +26,8 @@
|  #define UDF_FLAG_GID_IGNORE     14
|  #define UDF_FLAG_UID_SET	15
|  #define UDF_FLAG_GID_SET	16
| +#define UDF_FLAG_SESSION_SET	17
| +#define UDF_FLAG_LASTBLOCK_SET	18
|  
|  #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
|  #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
| 
| --
| 
		- Cyrill -

Re: [patch 21/26] mount options: partially fix nfs

[Chuck Lever]

[-- Attachment #1: Type: text/plain, Size: 2983 bytes --]

Hi Miklos-

Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add posix, bsize=, namelen= options to /proc/mounts for nfs
> filesystems.
> 
> Document several other options that are still missing.

NFS lists only some options in /proc/mounts on purpose: only the 
essential options are mentioned there to keep clutter down.  The three 
you've added here are for all intents and purposes deprecated, which is 
why they are not supported.

NFS lists a more complete set of mount options for a mount point in 
/proc/self/mountstats.  See nfs_show_stats().

Since your cover letter does not explain why you are changing this code, 
can you refer me to a description of why you are doing this?

More below.

> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/nfs/super.c
> ===================================================================
> --- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
> +++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
> @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
>  	} nfs_info[] = {
>  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
>  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
> +		{ NFS_MOUNT_POSIX, ",posix", "" },
>  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
>  		{ NFS_MOUNT_NOAC, ",noac", "" },
>  		{ NFS_MOUNT_NONLM, ",nolock", "" },
> @@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
>  	};
>  	const struct proc_nfs_info *nfs_infop;
>  	struct nfs_client *clp = nfss->nfs_client;
> +	unsigned int default_namelen =
> +		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
> +		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
>  
>  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
>  	seq_printf(m, ",rsize=%d", nfss->rsize);
>  	seq_printf(m, ",wsize=%d", nfss->wsize);
> +	if (nfss->bsize != 0)
> +		seq_printf(m, ",bsize=%d", nfss->bsize);
> +	if (nfss->namelen != default_namelen)
> +		seq_printf(m, ",namelen=%d", nfss->namelen);
>  	if (nfss->acregmin != 3*HZ || showdefaults)
>  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
>  	if (nfss->acregmax != 60*HZ || showdefaults)
> @@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
>  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
>  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
>  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
> +
> +	/*
> +	 * Missing options:
> +	 * port=

Probably should be supported.

> +	 * addr=

This one is already supported; see nfs_show_options().

> +	 * clientaddr=

This one isn't, and should be... would be useful for tracking down 
certain NFSv4 problems.

> +	 * mounthost=
> +	 * mountaddr=
 > +	 * mountport=
 > +	 * mountvers=
 > +	 * mountproto=

And these mount* options are for the kernel's new mount protocol client. 
  They aren't really useful for understanding steady-state NFS client 
behavior, they only effect mount-time behavior.

[-- Attachment #2: chuck_lever.vcf --]
[-- Type: text/x-vcard, Size: 315 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
email;internet:chuck dot lever at nospam oracle dot com
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: [patch 21/26] mount options: partially fix nfs

[Trond Myklebust]

On Thu, 2008-01-24 at 20:34 +0100, Miklos Szeredi wrote:
> plain text document attachment (nfs_opts.patch)
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add posix, bsize=, namelen= options to /proc/mounts for nfs
> filesystems.
> 
> Document several other options that are still missing.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/nfs/super.c
> ===================================================================
> --- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
> +++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
> @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
>  	} nfs_info[] = {
>  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
>  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
> +		{ NFS_MOUNT_POSIX, ",posix", "" },
>  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
>  		{ NFS_MOUNT_NOAC, ",noac", "" },
>  		{ NFS_MOUNT_NONLM, ",nolock", "" },
> @@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
>  	};
>  	const struct proc_nfs_info *nfs_infop;
>  	struct nfs_client *clp = nfss->nfs_client;
> +	unsigned int default_namelen =
> +		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
> +		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
>  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
>  	seq_printf(m, ",rsize=%d", nfss->rsize);
>  	seq_printf(m, ",wsize=%d", nfss->wsize);
> +	if (nfss->bsize != 0)
> +		seq_printf(m, ",bsize=%d", nfss->bsize);
> +	if (nfss->namelen != default_namelen)
> +		seq_printf(m, ",namelen=%d", nfss->namelen);

You really just want to look at the value of nfss->namelen. It should
always be set.

>  	if (nfss->acregmin != 3*HZ || showdefaults)
>  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
>  	if (nfss->acregmax != 60*HZ || showdefaults)
> @@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
>  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
>  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
>  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
> +
> +	/*
> +	 * Missing options:
> +	 * port=
> +	 * mountport=
> +	 * mountvers=
> +	 * mountproto=
> +	 * addr=
> +	 * clientaddr=
> +	 * mounthost=
> +	 * mountaddr=
> +	 */

The new text mount interface actually does allow us to store these
values if we really do need to. That should be a separate patch,
however.

Trond

Re: [patch 19/26] mount options: fix jfs

[Dave Kleikamp]

On Thu, 2008-01-24 at 20:34 +0100, Miklos Szeredi wrote:
> plain text document attachment (jfs_opts.patch)
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add iocharset= and errors= options to /proc/mounts for jfs
> filesystems.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Acked-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

Andrew,
Would you like me to add this to the jfs git tree, or would you like to
handle these patches as a set?

Thanks,
Shaggy

> ---
> 
> Index: linux/fs/jfs/super.c
> ===================================================================
> --- linux.orig/fs/jfs/super.c	2008-01-17 19:00:55.000000000 +0100
> +++ linux/fs/jfs/super.c	2008-01-21 19:39:30.000000000 +0100
> @@ -602,6 +602,12 @@ static int jfs_show_options(struct seq_f
>  		seq_printf(seq, ",umask=%03o", sbi->umask);
>  	if (sbi->flag & JFS_NOINTEGRITY)
>  		seq_puts(seq, ",nointegrity");
> +	if (sbi->nls_tab)
> +		seq_printf(seq, ",iocharset=%s", sbi->nls_tab->charset);
> +	if (sbi->flag & JFS_ERR_CONTINUE)
> +		seq_printf(seq, ",errors=continue");
> +	if (sbi->flag & JFS_ERR_PANIC)
> +		seq_printf(seq, ",errors=panic");
>  
>  #ifdef CONFIG_QUOTA
>  	if (sbi->flag & JFS_USRQUOTA)
> 
> --
-- 
David Kleikamp
IBM Linux Technology Center

Re: [patch 19/26] mount options: fix jfs

[Andrew Morton]

> On Thu, 24 Jan 2008 15:15:01 -0600 Dave Kleikamp <shaggy@linux.vnet.ibm.com> wrote:
> On Thu, 2008-01-24 at 20:34 +0100, Miklos Szeredi wrote:
> > plain text document attachment (jfs_opts.patch)
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Add iocharset= and errors= options to /proc/mounts for jfs
> > filesystems.
> > 
> > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> 
> Acked-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> 
> Andrew,
> Would you like me to add this to the jfs git tree, or would you like to
> handle these patches as a set?
> 

My usual algorithm here is to

1: queue all the patches and send the ones which have a maintainer to
   that maintainer until he merges it.

2: If the patches have a dependency upon (say) a VFS patch then I'll
   merge the VFS patch and will then goto 1.

I don't think this particular patch has a VFS depencency so sure, merge
away.  You'll probably see that I merged it anyway, but I'll drop it again
when I see it turn up in your tree (I used to resync with the git trees
at least daily, but I now do this far less frequently because it is such
torture because everyone is paddling in everyone else's puddle).

Re: [patch 26/26] mount options: fix usbfs

[Greg KH]

On Thu, Jan 24, 2008 at 08:34:07PM +0100, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add a .show_options super operation to usbfs.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Looks good to me.  Do you want to take this through your tree, as it is
dependant on other changes, or do you want me to take this through the
USB tree?  Whatever is easier for you is fine for me.

thanks,

greg k-h

Re: [patch 19/26] mount options: fix jfs

[Dave Kleikamp]

On Thu, 2008-01-24 at 13:57 -0800, Andrew Morton wrote:

> My usual algorithm here is to
> 
> 1: queue all the patches and send the ones which have a maintainer to
>    that maintainer until he merges it.
> 
> 2: If the patches have a dependency upon (say) a VFS patch then I'll
>    merge the VFS patch and will then goto 1.
> 
> I don't think this particular patch has a VFS depencency so sure, merge
> away.  You'll probably see that I merged it anyway, but I'll drop it again
> when I see it turn up in your tree (I used to resync with the git trees
> at least daily, but I now do this far less frequently because it is such
> torture because everyone is paddling in everyone else's puddle).

Merged.  Thanks.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [patch 01/26] mount options: add documentation

[Erez Zadok]

In message <20080124193416.379218079@szeredi.hu>, Miklos Szeredi writes:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> This series addresses the problem of showing mount options in
> /proc/mounts.
> 
> Several filesystems which use mount options, have not implemented a
> .show_options superblock operation.  Several others have implemented
> this callback, but have not kept it fully up to date with the parsed
> options.
[...]

> The following filesystems still need fixing: CIFS, NFS, XFS, Unionfs,
> Reiser4.  For CIFS, NFS and XFS I wasn't able to understand how some
> of the options are used.  The last two are not yet in mainline, so I
> leave fixing those to their respective maintainers out of pure
> laziness.
> 
> Table displaying status of all in-kernel filesystems:
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> legend:
> 
>   none - fs has options, but doesn't define ->show_options()
>   some - fs defines ->show_options(), but some only options are shown
>   most - fs defines ->show_options(), and shows most of them
>   good - fs shows all options
>   noopt - fs does not have options
>   patch - a patch will be posted
[...]

> in -mm:
> 
> reiser4     some
> unionfs     none

Hi Miklos,

Where did you check for the existence of a ->show_options method for
unionfs?  Unionfs does implement ->show_options and supports all of the
mount/remount options.  See:

<http://git.kernel.org/?p=linux/kernel/git/ezk/unionfs.git;a=blob;f=fs/unionfs/super.c;h=986c980261a5b171147d66ac05bf08423e2fd6b6;hb=HEAD#l963>

The unionfs ->remount code supports branch-management options which can
add/del/change a branch, but we don't show those directly in ->show_options;
it makes more sense to show the final (and thus most current) branch
configuration.

Could you update your records please?

BTW, I should be able to use your save_mount_options().

Cheers,
Erez.

Re: [patch 06/26] mount options: fix autofs4

[Ian Kent]

On Thu, 2008-01-24 at 20:33 +0100, Miklos Szeredi wrote:
> plain text document attachment (autofs4_opts.patch)
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add uid= and gid= options to /proc/mounts for autofs4 filesystems.

Apologies, I did say I would do this but have been quite busy.

> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by Ian Kent <raven@themaw.net>

I haven't tested this yet but it is fairly straight forward.
I will check it out as soon as I get back to some work that I'm doing on
autofs4 (next few days).

> ---
> 
> Index: linux/fs/autofs4/inode.c
> ===================================================================
> --- linux.orig/fs/autofs4/inode.c	2008-01-22 15:52:42.000000000 +0100
> +++ linux/fs/autofs4/inode.c	2008-01-22 23:36:02.000000000 +0100
> @@ -188,11 +188,16 @@ out_kill_sb:
>  static int autofs4_show_options(struct seq_file *m, struct vfsmount *mnt)
>  {
>  	struct autofs_sb_info *sbi = autofs4_sbi(mnt->mnt_sb);
> +	struct inode *root_inode = mnt->mnt_sb->s_root->d_inode;
>  
>  	if (!sbi)
>  		return 0;
>  
>  	seq_printf(m, ",fd=%d", sbi->pipefd);
> +	if (root_inode->i_uid != 0)
> +		seq_printf(m, ",uid=%u", root_inode->i_uid);
> +	if (root_inode->i_gid != 0)
> +		seq_printf(m, ",gid=%u", root_inode->i_gid);
>  	seq_printf(m, ",pgrp=%d", sbi->oz_pgrp);
>  	seq_printf(m, ",timeout=%lu", sbi->exp_timeout/HZ);
>  	seq_printf(m, ",minproto=%d", sbi->min_proto);
> 
> --

Re: [patch 12/26] mount options: fix ext4

[Andreas Dilger]

On Jan 24, 2008  20:33 +0100, Miklos Szeredi wrote:
> Add stripe= option to /proc/mounts for ext4 filesystems.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Acked-by: Andreas Dilger <adilger@sun.com>

> Index: linux/fs/ext4/super.c
> ===================================================================
> --- linux.orig/fs/ext4/super.c	2008-01-23 12:57:07.000000000 +0100
> +++ linux/fs/ext4/super.c	2008-01-23 21:43:51.000000000 +0100
> @@ -742,7 +742,8 @@ static int ext4_show_options(struct seq_
>  		seq_puts(seq, ",nomballoc");
>  	if (!test_opt(sb, DELALLOC))
>  		seq_puts(seq, ",nodelalloc");
> -
> +	if (sbi->s_stripe)
> +		seq_printf(seq, ",stripe=%lu", sbi->s_stripe);
>  
>  	/*
>  	 * journal mode get enabled in different ways

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [patch 01/26] mount options: add documentation

[David Chinner]

> In message <20080124193416.379218079@szeredi.hu>, Miklos Szeredi writes:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > This series addresses the problem of showing mount options in
> > /proc/mounts.
[...]
> > The following filesystems still need fixing: CIFS, NFS, XFS, Unionfs,
> > Reiser4.  For CIFS, NFS and XFS I wasn't able to understand how some
> > of the options are used.  The last two are not yet in mainline, so I
> > leave fixing those to their respective maintainers out of pure
> > laziness.

XFS has already been updated. The fix is in the XFs git tree that
Andrew picks up for -mm releases and will be merged in the 2.6.25-rc1
window. Commit is here:

http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c33fb6ca99aa17373bd3d5a507ac0eaefb7abb4

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [patch 10/26] mount options: fix devpts

[Miklos Szeredi]

> > Also add minor fix: when parsing the "mode" option, mask with
> > S_IALLUGO instead of ~S_IFMT, which could leave unsed bits in the
> > mask.
> 
> umode_t is 16 bits, so it doesn't.  The change is still good, of course.

We still use 16 bit types?  Strange ;)

> 
> > +	if (config.mode != DEVPTS_DEFAULT_MODE)
> > +		seq_printf(seq, ",mode=%03o", config.mode);
> 
> I would rather this be unconditional, than that it be conditional on 
> something other than the user having specified it in the first place.

Yeah, it's a matter of taste.  I'll update the patch.

Actually, a lot of filesystems share the options 'uid=X', 'gid=X',
'mode=X' (or 'umask=X').  This could be handled by the VFS, saving
some code, and making things more consistent.  One day maybe...

Thanks,
Miklos

Re: [patch 25/26] mount options: fix udf

[Miklos Szeredi]

> | +	/* is this correct? */
> | +	if (sbi->s_anchor[2] != 0)
> | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> 
> you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> in sake of style unification but we should wait for Jan's
> decision (i'm not the expert in this area ;)

I think UDF_SB_ANCHOR macro was removed by some patch in -mm.

I'm more interested if the second element of the s_anchor array really
does always have the value of the 'anchor=N' mount option.  I haven't
been able to verify that fully.  Do you have some insight into that?

Thanks,
Miklos

Re: [patch 21/26] mount options: partially fix nfs

[Miklos Szeredi]

> Miklos Szeredi wrote:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Add posix, bsize=, namelen= options to /proc/mounts for nfs
> > filesystems.
> > 
> > Document several other options that are still missing.
> 
> NFS lists only some options in /proc/mounts on purpose: only the 
> essential options are mentioned there to keep clutter down.  The three 
> you've added here are for all intents and purposes deprecated, which is 
> why they are not supported.
> 
> NFS lists a more complete set of mount options for a mount point in 
> /proc/self/mountstats.  See nfs_show_stats().
> 
> Since your cover letter does not explain why you are changing this code, 
> can you refer me to a description of why you are doing this?

Descritption is in the 01/26 patch.

> More below.
> 
> > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> > ---
> > 
> > Index: linux/fs/nfs/super.c
> > ===================================================================
> > --- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
> > +++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
> > @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
> >  	} nfs_info[] = {
> >  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
> >  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
> > +		{ NFS_MOUNT_POSIX, ",posix", "" },
> >  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
> >  		{ NFS_MOUNT_NOAC, ",noac", "" },
> >  		{ NFS_MOUNT_NONLM, ",nolock", "" },
> > @@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
> >  	};
> >  	const struct proc_nfs_info *nfs_infop;
> >  	struct nfs_client *clp = nfss->nfs_client;
> > +	unsigned int default_namelen =
> > +		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
> > +		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
> >  
> >  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
> >  	seq_printf(m, ",rsize=%d", nfss->rsize);
> >  	seq_printf(m, ",wsize=%d", nfss->wsize);
> > +	if (nfss->bsize != 0)
> > +		seq_printf(m, ",bsize=%d", nfss->bsize);
> > +	if (nfss->namelen != default_namelen)
> > +		seq_printf(m, ",namelen=%d", nfss->namelen);
> >  	if (nfss->acregmin != 3*HZ || showdefaults)
> >  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
> >  	if (nfss->acregmax != 60*HZ || showdefaults)
> > @@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
> >  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
> >  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
> >  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
> > +
> > +	/*
> > +	 * Missing options:
> > +	 * port=
> 
> Probably should be supported.
> 
> > +	 * addr=
> 
> This one is already supported; see nfs_show_options().

Right, thanks.

> 
> > +	 * clientaddr=
> 
> This one isn't, and should be... would be useful for tracking down 
> certain NFSv4 problems.
> 
> > +	 * mounthost=
> > +	 * mountaddr=
>  > +	 * mountport=
>  > +	 * mountvers=
>  > +	 * mountproto=
> 
> And these mount* options are for the kernel's new mount protocol client. 
>   They aren't really useful for understanding steady-state NFS client 
> behavior, they only effect mount-time behavior.

All mount options should be shown, which are needed to reconstruct a
previous mount.

For example, if you copy options out from /proc/mount, umount the
filesystem, and then create a new mount with the copied options, you
should get the same mount.

So not only those options are interesting which are useful for
understanding steady state behavior.

The only options, which should not be shown, are those which have a
permanent effect at mount time, like journal creation, etc.  And those
which are meaningless across different mounts, like communication file
descriptors.

Thanks,
Miklos

Re: [patch 21/26] mount options: partially fix nfs

[Miklos Szeredi]

> On Thu, 2008-01-24 at 20:34 +0100, Miklos Szeredi wrote:
> > plain text document attachment (nfs_opts.patch)
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Add posix, bsize=, namelen= options to /proc/mounts for nfs
> > filesystems.
> > 
> > Document several other options that are still missing.
> > 
> > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> > ---
> > 
> > Index: linux/fs/nfs/super.c
> > ===================================================================
> > --- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
> > +++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
> > @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
> >  	} nfs_info[] = {
> >  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
> >  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
> > +		{ NFS_MOUNT_POSIX, ",posix", "" },
> >  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
> >  		{ NFS_MOUNT_NOAC, ",noac", "" },
> >  		{ NFS_MOUNT_NONLM, ",nolock", "" },
> > @@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
> >  	};
> >  	const struct proc_nfs_info *nfs_infop;
> >  	struct nfs_client *clp = nfss->nfs_client;
> > +	unsigned int default_namelen =
> > +		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
> > +		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
> >  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
> >  	seq_printf(m, ",rsize=%d", nfss->rsize);
> >  	seq_printf(m, ",wsize=%d", nfss->wsize);
> > +	if (nfss->bsize != 0)
> > +		seq_printf(m, ",bsize=%d", nfss->bsize);
> > +	if (nfss->namelen != default_namelen)
> > +		seq_printf(m, ",namelen=%d", nfss->namelen);
> 
> You really just want to look at the value of nfss->namelen. It should
> always be set.

OK, I usually add the condition for (value != default_value) to avoid
unnecessary clutter.  But sure, there's no problem with showing the
option unconditionally.

> 
> >  	if (nfss->acregmin != 3*HZ || showdefaults)
> >  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
> >  	if (nfss->acregmax != 60*HZ || showdefaults)
> > @@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
> >  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
> >  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
> >  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
> > +
> > +	/*
> > +	 * Missing options:
> > +	 * port=
> > +	 * mountport=
> > +	 * mountvers=
> > +	 * mountproto=
> > +	 * addr=
> > +	 * clientaddr=
> > +	 * mounthost=
> > +	 * mountaddr=
> > +	 */
> 
> The new text mount interface actually does allow us to store these
> values if we really do need to. That should be a separate patch,
> however.

OK.

Thanks,
Miklos

Re: [patch 26/26] mount options: fix usbfs

[Miklos Szeredi]

> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Add a .show_options super operation to usbfs.
> > 
> > Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> 
> Looks good to me.  Do you want to take this through your tree, as it is
> dependant on other changes, or do you want me to take this through the
> USB tree?  Whatever is easier for you is fine for me.

Please take it, it should be independent of the other changes.

Thanks,
Miklos

Re: [patch 01/26] mount options: add documentation

[Miklos Szeredi]

> Where did you check for the existence of a ->show_options method for
> unionfs?  Unionfs does implement ->show_options and supports all of the
> mount/remount options.  See:
> 
> <http://git.kernel.org/?p=linux/kernel/git/ezk/unionfs.git;a=blob;f=fs/unionfs/super.c;h=986c980261a5b171147d66ac05bf08423e2fd6b6;hb=HEAD#l963>
> 
> The unionfs ->remount code supports branch-management options which can
> add/del/change a branch, but we don't show those directly in ->show_options;
> it makes more sense to show the final (and thus most current) branch
> configuration.
> 
> Could you update your records please?

Sure.  Sorry about that, I did actually look at unionfs, and it was
just an administration error and bad memory (in my head).

> BTW, I should be able to use your save_mount_options().

It is probably better not to use save_mount_options().  Especially,
since unionfs implemets a remount, that changes the tree only
partially AFAICS.

Miklos

Re: [patch 09/26] mount options: fix capifs

[Karsten Keil]

On Thu, Jan 24, 2008 at 08:33:50PM +0100, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add a .show_options super operation to capifs.
> 
> Use generic_show_options() and save the complete option string in
> capifs_remount().
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by: Karsten Keil <kkeil@suse.de>
> ---
> 
> Index: linux/drivers/isdn/capi/capifs.c
> ===================================================================
> --- linux.orig/drivers/isdn/capi/capifs.c	2007-10-09 22:31:38.000000000 +0200
> +++ linux/drivers/isdn/capi/capifs.c	2008-01-24 11:37:42.000000000 +0100
> @@ -52,6 +52,7 @@ static int capifs_remount(struct super_b
>  	gid_t gid = 0;
>  	umode_t mode = 0600;
>  	char *this_char;
> +	char *new_opt = kstrdup(data, GFP_KERNEL);
>  
>  	this_char = NULL;
>  	while ((this_char = strsep(&data, ",")) != NULL) {
> @@ -72,11 +73,16 @@ static int capifs_remount(struct super_b
>  			return -EINVAL;
>  		}
>  	}
> +
> +	kfree(s->s_options);
> +	s->s_options = new_opt;
> +
>  	config.setuid  = setuid;
>  	config.setgid  = setgid;
>  	config.uid     = uid;
>  	config.gid     = gid;
>  	config.mode    = mode;
> +
>  	return 0;
>  }
>  
> @@ -84,6 +90,7 @@ static struct super_operations capifs_so
>  {
>  	.statfs		= simple_statfs,
>  	.remount_fs	= capifs_remount,
> +	.show_options	= generic_show_options,
>  };
>  
>  
> 
> --

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

On Jan 25, 2008 12:29 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > | +   /* is this correct? */
> > | +   if (sbi->s_anchor[2] != 0)
> > | +           seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> >
> > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> > in sake of style unification but we should wait for Jan's
> > decision (i'm not the expert in this area ;)
>
> I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
>
> I'm more interested if the second element of the s_anchor array really
> does always have the value of the 'anchor=N' mount option.  I haven't
> been able to verify that fully.  Do you have some insight into that?
>
> Thanks,
> Miklos
>

Miklos,

I'll check this today evening (a bit busy now).

- Cyrill -

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

[Miklos Szeredi - Fri, Jan 25, 2008 at 10:29:21AM +0100]
| > | +	/* is this correct? */
| > | +	if (sbi->s_anchor[2] != 0)
| > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
| > 
| > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
| > in sake of style unification but we should wait for Jan's
| > decision (i'm not the expert in this area ;)
| 
| I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
| 
| I'm more interested if the second element of the s_anchor array really
| does always have the value of the 'anchor=N' mount option.  I haven't
| been able to verify that fully.  Do you have some insight into that?
| 
| Thanks,
| Miklos
| 

Hello Miklos,

well, actually - no. anchor entities can be set to 0 if we have been failed
to read them in udf_find_anchor(). So it seems you've to use some
additional flag to store it.

Btw, Miklos the patch is over -mm tree?

		- Cyrill -

Re: [patch 11/26] mount options: fix ext2

[Jan Kara]

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add noreservation option to /proc/mounts for ext2 filesystems.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/ext2/super.c
> ===================================================================
> --- linux.orig/fs/ext2/super.c	2008-01-17 19:00:55.000000000 +0100
> +++ linux/fs/ext2/super.c	2008-01-23 21:38:08.000000000 +0100
> @@ -285,6 +285,9 @@ static int ext2_show_options(struct seq_
>  		seq_puts(seq, ",xip");
>  #endif
>  
> +	if (!test_opt(sb, RESERVATION))
> +		seq_puts(seq, ",noreservation");
> +
>  	return 0;
>  }
  Looks fine. Acked-by: Jan Kara <jack@suse.cz>

									Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [patch 12/26] mount options: fix ext4

[Jan Kara]

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add stripe= option to /proc/mounts for ext4 filesystems.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/ext4/super.c
> ===================================================================
> --- linux.orig/fs/ext4/super.c	2008-01-23 12:57:07.000000000 +0100
> +++ linux/fs/ext4/super.c	2008-01-23 21:43:51.000000000 +0100
> @@ -742,7 +742,8 @@ static int ext4_show_options(struct seq_
>  		seq_puts(seq, ",nomballoc");
>  	if (!test_opt(sb, DELALLOC))
>  		seq_puts(seq, ",nodelalloc");
> -
> +	if (sbi->s_stripe)
> +		seq_printf(seq, ",stripe=%lu", sbi->s_stripe);
  I think this should go via ext4 patch queue... Besides that the patch
is fine. Mingming, will you pickup the patch?

									Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [patch 18/26] mount options: fix isofs

[Jan Kara]

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add a .show_options super operation to isofs.
> 
> Use generic_show_options() and save the complete option string in
> isofs_fill_super().
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/isofs/inode.c
> ===================================================================
> --- linux.orig/fs/isofs/inode.c	2008-01-17 19:00:55.000000000 +0100
> +++ linux/fs/isofs/inode.c	2008-01-23 22:07:51.000000000 +0100
> @@ -110,6 +110,7 @@ static const struct super_operations iso
>  	.put_super	= isofs_put_super,
>  	.statfs		= isofs_statfs,
>  	.remount_fs	= isofs_remount,
> +	.show_options	= generic_show_options,
>  };
>  
>  
> @@ -554,6 +555,8 @@ static int isofs_fill_super(struct super
>  	int table, error = -EINVAL;
>  	unsigned int vol_desc_start;
>  
> +	save_mount_options(s, data);
> +
>  	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
>  	if (!sbi)
>  		return -ENOMEM;
  Looks, fine.
  Acked-by: Jan Kara <jack@suse.cz>

										Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [patch 25/26] mount options: fix udf

[Jan Kara]

> > | +	/* is this correct? */
> > | +	if (sbi->s_anchor[2] != 0)
> > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> > 
> > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> > in sake of style unification but we should wait for Jan's
> > decision (i'm not the expert in this area ;)
> 
> I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
  Yes, it's going to be removed so don't use it. Actually, basing this
patch on top of -mm is a good idea because there are quite some changes
in Andrew's queue.

> I'm more interested if the second element of the s_anchor array really
> does always have the value of the 'anchor=N' mount option.  I haven't
> been able to verify that fully.  Do you have some insight into that?
  As Cyrill wrote, it could be zeroed out in case there is no anchor in
the specified block. So I guess you have to store the passed value
somewhere else..

									Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [patch 25/26] mount options: fix udf

[Jan Kara]

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add a .show_options super operation to udf.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/udf/super.c
> ===================================================================
> --- linux.orig/fs/udf/super.c	2008-01-24 13:48:37.000000000 +0100
> +++ linux/fs/udf/super.c	2008-01-24 15:58:21.000000000 +0100
> @@ -53,6 +53,8 @@
>  #include <linux/vfs.h>
>  #include <linux/vmalloc.h>
>  #include <linux/errno.h>
> +#include <linux/mount.h>
> +#include <linux/seq_file.h>
>  #include <asm/byteorder.h>
>  
>  #include <linux/udf_fs.h>
> @@ -71,6 +73,8 @@
>  #define VDS_POS_TERMINATING_DESC	6
>  #define VDS_POS_LENGTH			7
>  
> +#define UDF_DEFAULT_BLOCKSIZE 2048
> +
>  static char error_buf[1024];
>  
>  /* These are the "meat" - everything else is stuffing */
> @@ -95,6 +99,7 @@ static void udf_open_lvid(struct super_b
>  static void udf_close_lvid(struct super_block *);
>  static unsigned int udf_count_free(struct super_block *);
>  static int udf_statfs(struct dentry *, struct kstatfs *);
> +static int udf_show_options(struct seq_file *, struct vfsmount *);
>  
>  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
>  {
> @@ -181,6 +186,7 @@ static const struct super_operations udf
>  	.write_super	= udf_write_super,
>  	.statfs		= udf_statfs,
>  	.remount_fs	= udf_remount_fs,
> +	.show_options	= udf_show_options,
>  };
>  
>  struct udf_options {
> @@ -247,6 +253,56 @@ static int udf_sb_alloc_partition_maps(s
>  	return 0;
>  }
>  
> +static int udf_show_options(struct seq_file *seq, struct vfsmount *mnt)
> +{
> +	struct super_block *sb = mnt->mnt_sb;
> +	struct udf_sb_info *sbi = UDF_SB(sb);
> +
> +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT))
> +		seq_puts(seq, ",nostrict");
> +	if (sb->s_blocksize != UDF_DEFAULT_BLOCKSIZE)
> +		seq_printf(seq, ",bs=%lu", sb->s_blocksize);
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNHIDE))
> +		seq_puts(seq, ",unhide");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UNDELETE))
> +		seq_puts(seq, ",undelete");
> +	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_USE_AD_IN_ICB))
> +		seq_puts(seq, ",noadinicb");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_USE_SHORT_AD))
> +		seq_puts(seq, ",shortad");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_FORGET))
> +		seq_puts(seq, ",uid=forget");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_IGNORE))
> +		seq_puts(seq, ",uid=ignore");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_FORGET))
> +		seq_puts(seq, ",gid=forget");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_IGNORE))
> +		seq_puts(seq, ",gid=ignore");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UID_SET))
> +		seq_printf(seq, ",uid=%u", sbi->s_uid);
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_GID_SET))
> +		seq_printf(seq, ",gid=%u", sbi->s_gid);
> +	if (sbi->s_umask != 0)
> +		seq_printf(seq, ",umask=%o", sbi->s_umask);
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
> +		seq_printf(seq, ",session=%u", sbi->s_session);
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
> +		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
> +	/* is this correct? */
> +	if (sbi->s_anchor[2] != 0)
> +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> +	/*
> +	 * volume, partition, fileset and rootdir seem to be ignored
> +	 * currently
> +	 */
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8))
> +		seq_puts(seq, ",utf8");
> +	if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP) && sbi->s_nls_map)
> +		seq_printf(seq, ",iocharset=%s", sbi->s_nls_map->charset);
> +
> +	return 0;
> +}
> +
>  /*
>   * udf_parse_options
>   *
> @@ -339,13 +395,14 @@ static match_table_t tokens = {
>  	{Opt_err,	NULL}
>  };
>  
> -static int udf_parse_options(char *options, struct udf_options *uopt)
> +static int udf_parse_options(char *options, struct udf_options *uopt,
> +			     bool remount)
>  {
  Please use just 'int' for 'remount' option. We are slowly trying to
get rid of these strange things in UDF code so adding new ones isn't
desirable.

>  	char *p;
>  	int option;
>  
>  	uopt->novrs = 0;
> -	uopt->blocksize = 2048;
> +	uopt->blocksize = UDF_DEFAULT_BLOCKSIZE;
>  	uopt->partition = 0xFFFF;
>  	uopt->session = 0xFFFFFFFF;
>  	uopt->lastblock = 0;
> @@ -415,11 +472,15 @@ static int udf_parse_options(char *optio
>  			if (match_int(args, &option))
>  				return 0;
>  			uopt->session = option;
> +			if (!remount)
> +				uopt->flags |= (1 << UDF_FLAG_SESSION_SET);
>  			break;
>  		case Opt_lastblock:
>  			if (match_int(args, &option))
>  				return 0;
>  			uopt->lastblock = option;
> +			if (!remount)
> +				uopt->flags |= (1 << UDF_FLAG_LASTBLOCK_SET);
>  			break;
>  		case Opt_anchor:
>  			if (match_int(args, &option))
> @@ -497,7 +558,7 @@ static int udf_remount_fs(struct super_b
>  	uopt.gid   = sbi->s_gid;
>  	uopt.umask = sbi->s_umask;
>  
> -	if (!udf_parse_options(options, &uopt))
> +	if (!udf_parse_options(options, &uopt, true))
>  		return -EINVAL;
>  
>  	sbi->s_flags = uopt.flags;
> @@ -1679,7 +1740,7 @@ static int udf_fill_super(struct super_b
>  
>  	mutex_init(&sbi->s_alloc_mutex);
>  
> -	if (!udf_parse_options((char *)options, &uopt))
> +	if (!udf_parse_options((char *)options, &uopt, false))
>  		goto error_out;
>  
>  	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
> Index: linux/fs/udf/udf_sb.h
> ===================================================================
> --- linux.orig/fs/udf/udf_sb.h	2008-01-24 13:48:37.000000000 +0100
> +++ linux/fs/udf/udf_sb.h	2008-01-24 13:51:08.000000000 +0100
> @@ -26,6 +26,8 @@
>  #define UDF_FLAG_GID_IGNORE     14
>  #define UDF_FLAG_UID_SET	15
>  #define UDF_FLAG_GID_SET	16
> +#define UDF_FLAG_SESSION_SET	17
> +#define UDF_FLAG_LASTBLOCK_SET	18
>  
>  #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
>  #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
  Otherwise (apart from comments I wrote in the other email) the patch
is fine.

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

Re: [patch 25/26] mount options: fix udf

[Miklos Szeredi]

> > > | +	/* is this correct? */
> > > | +	if (sbi->s_anchor[2] != 0)
> > > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> > > 
> > > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> > > in sake of style unification but we should wait for Jan's
> > > decision (i'm not the expert in this area ;)
> > 
> > I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
>   Yes, it's going to be removed so don't use it. Actually, basing this
> patch on top of -mm is a good idea because there are quite some changes
> in Andrew's queue.
> 
> > I'm more interested if the second element of the s_anchor array really
> > does always have the value of the 'anchor=N' mount option.  I haven't
> > been able to verify that fully.  Do you have some insight into that?
>   As Cyrill wrote, it could be zeroed out in case there is no anchor in
> the specified block. So I guess you have to store the passed value
> somewhere else..

But in that case, would the value of the anchor= option matter?

This is actually a somewhat philosophical question about what the
mount options in /proc/mounts mean:

 1) Options _given_ by the user for the mount
 2) Options which are _effective_ for the mount

If we take interpretation 2) and there was no anchor (whatever that
means), then the anchor=N option wasn't effective, and not giving it
would have had the same effect.

This could be confusing to the user, though...

Thanks,
Miklos

Re: [patch 25/26] mount options: fix udf

[Miklos Szeredi]

>   Please use just 'int' for 'remount' option. We are slowly trying to
> get rid of these strange things in UDF code so adding new ones isn't
> desirable.

What's wrong with bool?

I'm not advocating mass replacements, but all new code _should_ use
it, because it's a very useful and good type.

We are just not very much used to it yet, but don't tell me it's
confusing to see a type like this ;)

Miklos

Re: [patch 25/26] mount options: fix udf

[Cyrill Gorcunov]

[Miklos Szeredi - Fri, Jan 25, 2008 at 04:50:15PM +0100]
| > > > | +	/* is this correct? */
| > > > | +	if (sbi->s_anchor[2] != 0)
| > > > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
| > > > 
| > > > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
| > > > in sake of style unification but we should wait for Jan's
| > > > decision (i'm not the expert in this area ;)
| > > 
| > > I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
| >   Yes, it's going to be removed so don't use it. Actually, basing this
| > patch on top of -mm is a good idea because there are quite some changes
| > in Andrew's queue.
| > 
| > > I'm more interested if the second element of the s_anchor array really
| > > does always have the value of the 'anchor=N' mount option.  I haven't
| > > been able to verify that fully.  Do you have some insight into that?
| >   As Cyrill wrote, it could be zeroed out in case there is no anchor in
| > the specified block. So I guess you have to store the passed value
| > somewhere else..
| 
| But in that case, would the value of the anchor= option matter?
| 
| This is actually a somewhat philosophical question about what the
| mount options in /proc/mounts mean:
| 
|  1) Options _given_ by the user for the mount
|  2) Options which are _effective_ for the mount
| 
| If we take interpretation 2) and there was no anchor (whatever that
| means), then the anchor=N option wasn't effective, and not giving it
| would have had the same effect.
| 
| This could be confusing to the user, though...
| 
| Thanks,
| Miklos
| 

I think _effective_ options is much more important - they could
show you that something bad happened (and if this zeroing of anchor
has been happened udf print debug message) Anyway, Miklos, I think
the options _given_ by a user does not mean anything in that case
because it just doesn't reveal what is being used in _real_.

		- Cyrill -

Re: [patch 25/26] mount options: fix udf

[Jan Kara]

On Fri 25-01-08 16:56:13, Miklos Szeredi wrote:
> >   Please use just 'int' for 'remount' option. We are slowly trying to
> > get rid of these strange things in UDF code so adding new ones isn't
> > desirable.
> 
> What's wrong with bool?
> 
> I'm not advocating mass replacements, but all new code _should_ use
> it, because it's a very useful and good type.
> 
> We are just not very much used to it yet, but don't tell me it's
> confusing to see a type like this ;)
  It's not so confusing but one really isn't used to it ;) But OK, I don't
mind that much...
								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [patch 25/26] mount options: fix udf

[Jan Kara]

On Fri 25-01-08 16:50:15, Miklos Szeredi wrote:
> > > > | +	/* is this correct? */
> > > > | +	if (sbi->s_anchor[2] != 0)
> > > > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> > > > 
> > > > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> > > > in sake of style unification but we should wait for Jan's
> > > > decision (i'm not the expert in this area ;)
> > > 
> > > I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
> >   Yes, it's going to be removed so don't use it. Actually, basing this
> > patch on top of -mm is a good idea because there are quite some changes
> > in Andrew's queue.
> > 
> > > I'm more interested if the second element of the s_anchor array really
> > > does always have the value of the 'anchor=N' mount option.  I haven't
> > > been able to verify that fully.  Do you have some insight into that?
> >   As Cyrill wrote, it could be zeroed out in case there is no anchor in
> > the specified block. So I guess you have to store the passed value
> > somewhere else..
> 
> But in that case, would the value of the anchor= option matter?
  No, it would not.

> This is actually a somewhat philosophical question about what the
> mount options in /proc/mounts mean:
> 
>  1) Options _given_ by the user for the mount
>  2) Options which are _effective_ for the mount
> 
> If we take interpretation 2) and there was no anchor (whatever that
> means), then the anchor=N option wasn't effective, and not giving it
> would have had the same effect.
> 
> This could be confusing to the user, though...
  Hmm, given that options are modified by remount for some filesystems,
it's probably the best to display the effective state. So your code should
display the right thing as it is.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [patch 25/26] mount options: fix udf

[Miklos Szeredi]

> On Fri 25-01-08 16:50:15, Miklos Szeredi wrote:
> > > > > | +	/* is this correct? */
> > > > > | +	if (sbi->s_anchor[2] != 0)
> > > > > | +		seq_printf(seq, ",anchor=%u", sbi->s_anchor[2]);
> > > > > 
> > > > > you know, I would prefer to use form UDF_SB_ANCHOR(sb)[2]
> > > > > in sake of style unification but we should wait for Jan's
> > > > > decision (i'm not the expert in this area ;)
> > > > 
> > > > I think UDF_SB_ANCHOR macro was removed by some patch in -mm.
> > >   Yes, it's going to be removed so don't use it. Actually, basing this
> > > patch on top of -mm is a good idea because there are quite some changes
> > > in Andrew's queue.
> > > 
> > > > I'm more interested if the second element of the s_anchor array really
> > > > does always have the value of the 'anchor=N' mount option.  I haven't
> > > > been able to verify that fully.  Do you have some insight into that?
> > >   As Cyrill wrote, it could be zeroed out in case there is no anchor in
> > > the specified block. So I guess you have to store the passed value
> > > somewhere else..
> > 
> > But in that case, would the value of the anchor= option matter?
>   No, it would not.
> 
> > This is actually a somewhat philosophical question about what the
> > mount options in /proc/mounts mean:
> > 
> >  1) Options _given_ by the user for the mount
> >  2) Options which are _effective_ for the mount
> > 
> > If we take interpretation 2) and there was no anchor (whatever that
> > means), then the anchor=N option wasn't effective, and not giving it
> > would have had the same effect.
> > 
> > This could be confusing to the user, though...
>   Hmm, given that options are modified by remount for some filesystems,
> it's probably the best to display the effective state. So your code should
> display the right thing as it is.

OK.  Cyrill, Jan, thanks for the reviews.

Miklos

Re: [patch 21/26] mount options: partially fix nfs

[Chuck Lever]

On Jan 25, 2008, at 4:39 AM, Miklos Szeredi wrote:
>> Miklos Szeredi wrote:
>>> From: Miklos Szeredi <mszeredi@suse.cz>
>>>
>>> Add posix, bsize=, namelen= options to /proc/mounts for nfs
>>> filesystems.
>>>
>>> Document several other options that are still missing.
>>
>> NFS lists only some options in /proc/mounts on purpose: only the
>> essential options are mentioned there to keep clutter down.  The  
>> three
>> you've added here are for all intents and purposes deprecated,  
>> which is
>> why they are not supported.
>>
>> NFS lists a more complete set of mount options for a mount point in
>> /proc/self/mountstats.  See nfs_show_stats().
>>
>> Since your cover letter does not explain why you are changing this  
>> code,
>> can you refer me to a description of why you are doing this?
>
> Descritption is in the 01/26 patch.
>
>> More below.
>>
>>> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
>>> ---
>>>
>>> Index: linux/fs/nfs/super.c
>>> ===================================================================
>>> --- linux.orig/fs/nfs/super.c	2008-01-19 11:56:34.000000000 +0100
>>> +++ linux/fs/nfs/super.c	2008-01-21 20:41:30.000000000 +0100
>>> @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
>>>  	} nfs_info[] = {
>>>  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
>>>  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
>>> +		{ NFS_MOUNT_POSIX, ",posix", "" },
>>>  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
>>>  		{ NFS_MOUNT_NOAC, ",noac", "" },
>>>  		{ NFS_MOUNT_NONLM, ",nolock", "" },
>>> @@ -459,10 +460,17 @@ static void nfs_show_mount_options(struc
>>>  	};
>>>  	const struct proc_nfs_info *nfs_infop;
>>>  	struct nfs_client *clp = nfss->nfs_client;
>>> +	unsigned int default_namelen =
>>> +		clp->rpc_ops->version == 4 ? NFS4_MAXNAMLEN :
>>> +		clp->rpc_ops->version == 3 ? NFS3_MAXNAMLEN : NFS2_MAXNAMLEN;
>>>
>>>  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
>>>  	seq_printf(m, ",rsize=%d", nfss->rsize);
>>>  	seq_printf(m, ",wsize=%d", nfss->wsize);
>>> +	if (nfss->bsize != 0)
>>> +		seq_printf(m, ",bsize=%d", nfss->bsize);
>>> +	if (nfss->namelen != default_namelen)
>>> +		seq_printf(m, ",namelen=%d", nfss->namelen);
>>>  	if (nfss->acregmin != 3*HZ || showdefaults)
>>>  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
>>>  	if (nfss->acregmax != 60*HZ || showdefaults)
>>> @@ -482,6 +490,18 @@ static void nfs_show_mount_options(struc
>>>  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout- 
>>> >to_initval / HZ);
>>>  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout- 
>>> >to_retries);
>>>  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client- 
>>> >cl_auth->au_flavor));
>>> +
>>> +	/*
>>> +	 * Missing options:
>>> +	 * port=
>>
>> Probably should be supported.
>>
>>> +	 * addr=
>>
>> This one is already supported; see nfs_show_options().
>
> Right, thanks.
>
>>
>>> +	 * clientaddr=
>>
>> This one isn't, and should be... would be useful for tracking down
>> certain NFSv4 problems.
>>
>>> +	 * mounthost=
>>> +	 * mountaddr=
>>> +	 * mountport=
>>> +	 * mountvers=
>>> +	 * mountproto=
>>
>> And these mount* options are for the kernel's new mount protocol  
>> client.
>>   They aren't really useful for understanding steady-state NFS client
>> behavior, they only effect mount-time behavior.
>
> All mount options should be shown, which are needed to reconstruct a
> previous mount.

Ah, OK.

I'm happy to implement logic to display the all missing options.  I  
should have updated nfs_show_mount_options() when I wrote the NFS  
mount option parser.

Let me know your preference.

> For example, if you copy options out from /proc/mount, umount the
> filesystem, and then create a new mount with the copied options, you
> should get the same mount.

For NFS, umount also needs to read some of the options in order to  
determine how mountd is to connect to the server for the unmount.   
(That's why we have addr= in the first place).

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [patch 12/26] mount options: fix ext4

[Mingming Cao]

On Thu, 2008-01-24 at 20:33 +0100, Miklos Szeredi wrote:
> plain text document attachment (ext4_opts.patch)
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add stripe= option to /proc/mounts for ext4 filesystems.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> 
> Index: linux/fs/ext4/super.c
> ===================================================================
> --- linux.orig/fs/ext4/super.c	2008-01-23 12:57:07.000000000 +0100
> +++ linux/fs/ext4/super.c	2008-01-23 21:43:51.000000000 +0100
> @@ -742,7 +742,8 @@ static int ext4_show_options(struct seq_
>  		seq_puts(seq, ",nomballoc");
>  	if (!test_opt(sb, DELALLOC))
>  		seq_puts(seq, ",nodelalloc");
> -
> +	if (sbi->s_stripe)
> +		seq_printf(seq, ",stripe=%lu", sbi->s_stripe);
> 
>  	/*
>  	 * journal mode get enabled in different ways
> 

Added to ext4 patch queue. Thanks!
http://repo.or.cz/w/ext4-patch-queue.git

Mingming

Re: [patch 00/26] mount options: fix filesystem's ->show_options

[Andrew Morton]

> On Thu, 24 Jan 2008 20:33:41 +0100 Miklos Szeredi <miklos@szeredi.hu> wrote:
> Andrew,
> 
> Would you please consider these patches for -mm?

Sure, but I'm too lazy to pick through them and work out which ones need
updating, which ones got acked and which ones someone else merged, all on a
very bumpy plane flight ;)

Please resend when the dust has settled?

Re: [patch 24/26] mount options: fix tmpfs

[Hugh Dickins]

On Thu, 24 Jan 2008, Miklos Szeredi wrote:
>

Thanks Miklos, that's a welcome enhancement, nicely done.  I've only
noticed one thing wrong (MPOL_PREFERRED shown as "default"); but thought
shmem_config didn't add much value - I'd rather avoid those syntactic
changes to unchanged code; and several tmpfs defaults being relative
(e.g. to totalram_pages, or to mounter's fsuid), I ended up preferring
to do real tests in shmem_show_options.

Thus, for example, if memory is hotplugged in or out later, what started
out as an unspecified size option will then get shown as explicit size.
(I did think for a while that I wanted to show explicit size in all
cases; but it looked pretty silly on udev.)  I think that's the correct
behaviour, that otherwise would be misleading; but I may be looking at
this the wrong way round, what's your view?

If you agree with the version below, please take it into your collection
and insert your Signed-off-by.  I should admit, I've not yet tested how
the NUMA policies look: you'll hear from me again tomorrow morning if
those turn out to wrong.

Hugh

From: Miklos Szeredi <mszeredi@suse.cz>

Add .show_options super operation to tmpfs.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 include/linux/shmem_fs.h |    5 
 mm/shmem.c               |  189 ++++++++++++++++++++++++-------------
 2 files changed, 129 insertions(+), 65 deletions(-)

--- 2.6.24-rc8-mm1/include/linux/shmem_fs.h	2006-11-29 21:57:37.000000000 +0000
+++ linux/include/linux/shmem_fs.h	2008-01-27 22:42:52.000000000 +0000
@@ -30,9 +30,12 @@ struct shmem_sb_info {
 	unsigned long free_blocks;  /* How many are left for allocation */
 	unsigned long max_inodes;   /* How many inodes are allowed */
 	unsigned long free_inodes;  /* How many are left for allocation */
+	spinlock_t stat_lock;	    /* Serialize shmem_sb_info changes */
+	uid_t uid;		    /* Mount uid for root directory */
+	gid_t gid;		    /* Mount gid for root directory */
+	mode_t mode;		    /* Mount mode for root directory */
 	int policy;		    /* Default NUMA memory alloc policy */
 	nodemask_t policy_nodes;    /* nodemask for preferred and bind */
-	spinlock_t    stat_lock;
 };
 
 static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
--- 2.6.24-rc8-mm1/mm/shmem.c	2008-01-17 16:51:21.000000000 +0000
+++ linux/mm/shmem.c	2008-01-27 23:41:31.000000000 +0000
@@ -49,6 +49,7 @@
 #include <linux/ctype.h>
 #include <linux/migrate.h>
 #include <linux/highmem.h>
+#include <linux/seq_file.h>
 
 #include <asm/uaccess.h>
 #include <asm/div64.h>
@@ -1068,7 +1069,8 @@ redirty:
 }
 
 #ifdef CONFIG_NUMA
-static inline int shmem_parse_mpol(char *value, int *policy, nodemask_t *policy_nodes)
+#ifdef CONFIG_TMPFS
+static int shmem_parse_mpol(char *value, int *policy, nodemask_t *policy_nodes)
 {
 	char *nodelist = strchr(value, ':');
 	int err = 1;
@@ -1117,6 +1119,42 @@ out:
 	return err;
 }
 
+static void shmem_show_mpol(struct seq_file *seq, int policy,
+			    const nodemask_t policy_nodes)
+{
+	char *policy_string;
+
+	switch (policy) {
+	case MPOL_PREFERRED:
+		policy_string = "prefer";
+		break;
+	case MPOL_BIND:
+		policy_string = "bind";
+		break;
+	case MPOL_INTERLEAVE:
+		policy_string = "interleave";
+		break;
+	default:
+		/* MPOL_DEFAULT */
+		return;
+	}
+
+	seq_printf(seq, ",mpol=%s", policy_string);
+
+	if (policy != MPOL_INTERLEAVE ||
+	    !nodes_equal(policy_nodes, node_states[N_HIGH_MEMORY])) {
+		char buffer[64];
+		int len;
+
+		len = nodelist_scnprintf(buffer, sizeof(buffer), policy_nodes);
+		if (len < sizeof(buffer))
+			seq_printf(seq, ":%s", buffer);
+		else
+			seq_printf(seq, ":?");
+	}
+}
+#endif /* CONFIG_TMPFS */
+
 static struct page *shmem_swapin(swp_entry_t entry, gfp_t gfp,
 			struct shmem_inode_info *info, unsigned long idx)
 {
@@ -1148,13 +1186,20 @@ static struct page *shmem_alloc_page(gfp
 	mpol_free(pvma.vm_policy);
 	return page;
 }
-#else
+#else /* !CONFIG_NUMA */
+#ifdef CONFIG_TMPFS
 static inline int shmem_parse_mpol(char *value, int *policy,
 						nodemask_t *policy_nodes)
 {
 	return 1;
 }
 
+static inline void shmem_show_mpol(struct seq_file *seq, int policy,
+			    const nodemask_t policy_nodes)
+{
+}
+#endif /* CONFIG_TMPFS */
+
 static inline struct page *shmem_swapin(swp_entry_t entry, gfp_t gfp,
 			struct shmem_inode_info *info, unsigned long idx)
 {
@@ -1166,7 +1211,7 @@ static inline struct page *shmem_alloc_p
 {
 	return alloc_page(gfp);
 }
-#endif
+#endif /* CONFIG_NUMA */
 
 /*
  * shmem_getpage - either get the page from swap or allocate a new one
@@ -2077,9 +2122,8 @@ static const struct export_operations sh
 	.fh_to_dentry	= shmem_fh_to_dentry,
 };
 
-static int shmem_parse_options(char *options, int *mode, uid_t *uid,
-	gid_t *gid, unsigned long *blocks, unsigned long *inodes,
-	int *policy, nodemask_t *policy_nodes)
+static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
+			       bool remount)
 {
 	char *this_char, *value, *rest;
 
@@ -2122,35 +2166,37 @@ static int shmem_parse_options(char *opt
 			}
 			if (*rest)
 				goto bad_val;
-			*blocks = DIV_ROUND_UP(size, PAGE_CACHE_SIZE);
+			sbinfo->max_blocks =
+				DIV_ROUND_UP(size, PAGE_CACHE_SIZE);
 		} else if (!strcmp(this_char,"nr_blocks")) {
-			*blocks = memparse(value,&rest);
+			sbinfo->max_blocks = memparse(value, &rest);
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"nr_inodes")) {
-			*inodes = memparse(value,&rest);
+			sbinfo->max_inodes = memparse(value, &rest);
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"mode")) {
-			if (!mode)
+			if (remount)
 				continue;
-			*mode = simple_strtoul(value,&rest,8);
+			sbinfo->mode = simple_strtoul(value, &rest, 8) & 07777;
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"uid")) {
-			if (!uid)
+			if (remount)
 				continue;
-			*uid = simple_strtoul(value,&rest,0);
+			sbinfo->uid = simple_strtoul(value, &rest, 0);
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"gid")) {
-			if (!gid)
+			if (remount)
 				continue;
-			*gid = simple_strtoul(value,&rest,0);
+			sbinfo->gid = simple_strtoul(value, &rest, 0);
 			if (*rest)
 				goto bad_val;
 		} else if (!strcmp(this_char,"mpol")) {
-			if (shmem_parse_mpol(value,policy,policy_nodes))
+			if (shmem_parse_mpol(value, &sbinfo->policy,
+					     &sbinfo->policy_nodes))
 				goto bad_val;
 		} else {
 			printk(KERN_ERR "tmpfs: Bad mount option %s\n",
@@ -2170,24 +2216,20 @@ bad_val:
 static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
 {
 	struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
-	unsigned long max_blocks = sbinfo->max_blocks;
-	unsigned long max_inodes = sbinfo->max_inodes;
-	int policy = sbinfo->policy;
-	nodemask_t policy_nodes = sbinfo->policy_nodes;
+	struct shmem_sb_info config = *sbinfo;
 	unsigned long blocks;
 	unsigned long inodes;
 	int error = -EINVAL;
 
-	if (shmem_parse_options(data, NULL, NULL, NULL, &max_blocks,
-				&max_inodes, &policy, &policy_nodes))
+	if (shmem_parse_options(data, &config, true))
 		return error;
 
 	spin_lock(&sbinfo->stat_lock);
 	blocks = sbinfo->max_blocks - sbinfo->free_blocks;
 	inodes = sbinfo->max_inodes - sbinfo->free_inodes;
-	if (max_blocks < blocks)
+	if (config.max_blocks < blocks)
 		goto out;
-	if (max_inodes < inodes)
+	if (config.max_inodes < inodes)
 		goto out;
 	/*
 	 * Those tests also disallow limited->unlimited while any are in
@@ -2195,23 +2237,43 @@ static int shmem_remount_fs(struct super
 	 * but we must separately disallow unlimited->limited, because
 	 * in that case we have no record of how much is already in use.
 	 */
-	if (max_blocks && !sbinfo->max_blocks)
+	if (config.max_blocks && !sbinfo->max_blocks)
 		goto out;
-	if (max_inodes && !sbinfo->max_inodes)
+	if (config.max_inodes && !sbinfo->max_inodes)
 		goto out;
 
 	error = 0;
-	sbinfo->max_blocks  = max_blocks;
-	sbinfo->free_blocks = max_blocks - blocks;
-	sbinfo->max_inodes  = max_inodes;
-	sbinfo->free_inodes = max_inodes - inodes;
-	sbinfo->policy = policy;
-	sbinfo->policy_nodes = policy_nodes;
+	sbinfo->max_blocks  = config.max_blocks;
+	sbinfo->free_blocks = config.max_blocks - blocks;
+	sbinfo->max_inodes  = config.max_inodes;
+	sbinfo->free_inodes = config.max_inodes - inodes;
+	sbinfo->policy      = config.policy;
+	sbinfo->policy_nodes = config.policy_nodes;
 out:
 	spin_unlock(&sbinfo->stat_lock);
 	return error;
 }
-#endif
+
+static int shmem_show_options(struct seq_file *seq, struct vfsmount *vfs)
+{
+	struct shmem_sb_info *sbinfo = SHMEM_SB(vfs->mnt_sb);
+
+	if (sbinfo->max_blocks != totalram_pages / 2)
+		seq_printf(seq, ",size=%luk",
+			sbinfo->max_blocks << (PAGE_CACHE_SHIFT - 10));
+	if (sbinfo->max_inodes !=
+	    min(totalram_pages - totalhigh_pages, totalram_pages / 2))
+		seq_printf(seq, ",nr_inodes=%lu", sbinfo->max_inodes);
+	if (sbinfo->mode != (S_IRWXUGO | S_ISVTX))
+		seq_printf(seq, ",mode=%03o", sbinfo->mode);
+	if (sbinfo->uid != 0)
+		seq_printf(seq, ",uid=%u", sbinfo->uid);
+	if (sbinfo->gid != 0)
+		seq_printf(seq, ",gid=%u", sbinfo->gid);
+	shmem_show_mpol(seq, sbinfo->policy, sbinfo->policy_nodes);
+	return 0;
+}
+#endif /* CONFIG_TMPFS */
 
 static void shmem_put_super(struct super_block *sb)
 {
@@ -2224,15 +2286,23 @@ static int shmem_fill_super(struct super
 {
 	struct inode *inode;
 	struct dentry *root;
-	int mode   = S_IRWXUGO | S_ISVTX;
-	uid_t uid = current->fsuid;
-	gid_t gid = current->fsgid;
-	int err = -ENOMEM;
 	struct shmem_sb_info *sbinfo;
-	unsigned long blocks = 0;
-	unsigned long inodes = 0;
-	int policy = MPOL_DEFAULT;
-	nodemask_t policy_nodes = node_states[N_HIGH_MEMORY];
+	int err = -ENOMEM;
+
+	/* Round up to L1_CACHE_BYTES to resist false sharing */
+	sbinfo = kmalloc(max((int)sizeof(struct shmem_sb_info),
+				L1_CACHE_BYTES), GFP_KERNEL);
+	if (!sbinfo)
+		return -ENOMEM;
+
+	sbinfo->max_blocks = 0;
+	sbinfo->max_inodes = 0;
+	sbinfo->mode = S_IRWXUGO | S_ISVTX;
+	sbinfo->uid = current->fsuid;
+	sbinfo->gid = current->fsgid;
+	sbinfo->policy = MPOL_DEFAULT;
+	sbinfo->policy_nodes = node_states[N_HIGH_MEMORY];
+	sb->s_fs_info = sbinfo;
 
 #ifdef CONFIG_TMPFS
 	/*
@@ -2241,34 +2311,24 @@ static int shmem_fill_super(struct super
 	 * but the internal instance is left unlimited.
 	 */
 	if (!(sb->s_flags & MS_NOUSER)) {
-		blocks = totalram_pages / 2;
-		inodes = totalram_pages - totalhigh_pages;
-		if (inodes > blocks)
-			inodes = blocks;
-		if (shmem_parse_options(data, &mode, &uid, &gid, &blocks,
-					&inodes, &policy, &policy_nodes))
-			return -EINVAL;
+		sbinfo->max_blocks = totalram_pages / 2;
+		sbinfo->max_inodes = totalram_pages - totalhigh_pages;
+		if (sbinfo->max_inodes > sbinfo->max_blocks)
+			sbinfo->max_inodes = sbinfo->max_blocks;
+		if (shmem_parse_options(data, sbinfo, false)) {
+			err = -EINVAL;
+			goto failed;
+		}
 	}
 	sb->s_export_op = &shmem_export_ops;
 #else
 	sb->s_flags |= MS_NOUSER;
 #endif
 
-	/* Round up to L1_CACHE_BYTES to resist false sharing */
-	sbinfo = kmalloc(max((int)sizeof(struct shmem_sb_info),
-				L1_CACHE_BYTES), GFP_KERNEL);
-	if (!sbinfo)
-		return -ENOMEM;
-
 	spin_lock_init(&sbinfo->stat_lock);
-	sbinfo->max_blocks = blocks;
-	sbinfo->free_blocks = blocks;
-	sbinfo->max_inodes = inodes;
-	sbinfo->free_inodes = inodes;
-	sbinfo->policy = policy;
-	sbinfo->policy_nodes = policy_nodes;
+	sbinfo->free_blocks = sbinfo->max_blocks;
+	sbinfo->free_inodes = sbinfo->max_inodes;
 
-	sb->s_fs_info = sbinfo;
 	sb->s_maxbytes = SHMEM_MAX_BYTES;
 	sb->s_blocksize = PAGE_CACHE_SIZE;
 	sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
@@ -2280,11 +2340,11 @@ static int shmem_fill_super(struct super
 	sb->s_flags |= MS_POSIXACL;
 #endif
 
-	inode = shmem_get_inode(sb, S_IFDIR | mode, 0);
+	inode = shmem_get_inode(sb, S_IFDIR | sbinfo->mode, 0);
 	if (!inode)
 		goto failed;
-	inode->i_uid = uid;
-	inode->i_gid = gid;
+	inode->i_uid = sbinfo->uid;
+	inode->i_gid = sbinfo->gid;
 	root = d_alloc_root(inode);
 	if (!root)
 		goto failed_iput;
@@ -2420,6 +2480,7 @@ static const struct super_operations shm
 #ifdef CONFIG_TMPFS
 	.statfs		= shmem_statfs,
 	.remount_fs	= shmem_remount_fs,
+	.show_options	= shmem_show_options,
 #endif
 	.delete_inode	= shmem_delete_inode,
 	.drop_inode	= generic_delete_inode,

Re: [patch 21/26] mount options: partially fix nfs

[Miklos Szeredi]

> > All mount options should be shown, which are needed to reconstruct a
> > previous mount.
> 
> Ah, OK.
> 
> I'm happy to implement logic to display the all missing options.  I  
> should have updated nfs_show_mount_options() when I wrote the NFS  
> mount option parser.
> 
> Let me know your preference.

You are more familiar with NFS, so I think it would be better if you
updated nfs_show_mount_options().

Could you also queue my patch (updated) or incorporate it into a
combined fix?

Thanks,
Miklos
----

Subject: mount options: partially fix nfs

From: Miklos Szeredi <mszeredi@suse.cz>

Add posix, bsize=, namelen= options to /proc/mounts for nfs
filesystems.

Document several other options that are still missing.

Changes:

 - display namelen= unconditionally
 - addr= isn't missing after all

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Trond Myklebust <Trond.Myklebust@netapp.com>
---

Index: linux/fs/nfs/super.c
===================================================================
--- linux.orig/fs/nfs/super.c	2008-01-25 15:44:56.000000000 +0100
+++ linux/fs/nfs/super.c	2008-01-25 15:57:32.000000000 +0100
@@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
 	} nfs_info[] = {
 		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
 		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
+		{ NFS_MOUNT_POSIX, ",posix", "" },
 		{ NFS_MOUNT_NOCTO, ",nocto", "" },
 		{ NFS_MOUNT_NOAC, ",noac", "" },
 		{ NFS_MOUNT_NONLM, ",nolock", "" },
@@ -463,6 +464,9 @@ static void nfs_show_mount_options(struc
 	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
 	seq_printf(m, ",rsize=%d", nfss->rsize);
 	seq_printf(m, ",wsize=%d", nfss->wsize);
+	seq_printf(m, ",namelen=%d", nfss->namelen);
+	if (nfss->bsize != 0)
+		seq_printf(m, ",bsize=%d", nfss->bsize);
 	if (nfss->acregmin != 3*HZ || showdefaults)
 		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
 	if (nfss->acregmax != 60*HZ || showdefaults)
@@ -482,6 +486,17 @@ static void nfs_show_mount_options(struc
 	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
 	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
 	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
+
+	/*
+	 * Missing options:
+	 * port=
+	 * mountport=
+	 * mountvers=
+	 * mountproto=
+	 * clientaddr=
+	 * mounthost=
+	 * mountaddr=
+	 */
 }
 
 /*

Re: [patch 00/26] mount options: fix filesystem's ->show_options

[Miklos Szeredi]

> > On Thu, 24 Jan 2008 20:33:41 +0100 Miklos Szeredi <miklos@szeredi.hu> wrote:
> > Andrew,
> > 
> > Would you please consider these patches for -mm?
> 
> Sure, but I'm too lazy to pick through them and work out which ones need
> updating, which ones got acked and which ones someone else merged, all on a
> very bumpy plane flight ;)
> 
> Please resend when the dust has settled?

Yes, I should have thought, it won't quite work in a single iteration :)

I'll resend them in a moment.

Thanks,
Miklos

Re: [patch 24/26] mount options: fix tmpfs

[Miklos Szeredi]

> 
> Thanks Miklos, that's a welcome enhancement, nicely done.  I've only
> noticed one thing wrong (MPOL_PREFERRED shown as "default"); but thought
> shmem_config didn't add much value - I'd rather avoid those syntactic
> changes to unchanged code; and several tmpfs defaults being relative
> (e.g. to totalram_pages, or to mounter's fsuid), I ended up preferring
> to do real tests in shmem_show_options.

I completely agree, this is much better than my version.

> Thus, for example, if memory is hotplugged in or out later, what started
> out as an unspecified size option will then get shown as explicit size.
> (I did think for a while that I wanted to show explicit size in all
> cases; but it looked pretty silly on udev.)  I think that's the correct
> behaviour, that otherwise would be misleading; but I may be looking at
> this the wrong way round, what's your view?

I agree, this is the correct way.

I'll add functions for calculating the default max values, so the
calculations won't accidentally become different for the
initialization and the option showing.

> If you agree with the version below, please take it into your collection
> and insert your Signed-off-by.  I should admit, I've not yet tested how
> the NUMA policies look: you'll hear from me again tomorrow morning if
> those turn out to wrong.

OK, I'll send this to Andrew.  Maybe I'll wait until tomorrow to hear
if it's working on NUMA.

Thanks,
Miklos

Re: [patch 21/26] mount options: partially fix nfs

[Chuck Lever]

On Jan 28, 2008, at 6:34 AM, Miklos Szeredi wrote:
>>> All mount options should be shown, which are needed to reconstruct a
>>> previous mount.
>>
>> Ah, OK.
>>
>> I'm happy to implement logic to display the all missing options.  I
>> should have updated nfs_show_mount_options() when I wrote the NFS
>> mount option parser.
>>
>> Let me know your preference.
>
> You are more familiar with NFS, so I think it would be better if you
> updated nfs_show_mount_options().
>
> Could you also queue my patch (updated) or incorporate it into a
> combined fix?

Yes.  I'll have time in a day or two to get this finished.

> Thanks,
> Miklos
> ----
>
> Subject: mount options: partially fix nfs
>
> From: Miklos Szeredi <mszeredi@suse.cz>
>
> Add posix, bsize=, namelen= options to /proc/mounts for nfs
> filesystems.
>
> Document several other options that are still missing.
>
> Changes:
>
>  - display namelen= unconditionally
>  - addr= isn't missing after all
>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> CC: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>
> Index: linux/fs/nfs/super.c
> ===================================================================
> --- linux.orig/fs/nfs/super.c	2008-01-25 15:44:56.000000000 +0100
> +++ linux/fs/nfs/super.c	2008-01-25 15:57:32.000000000 +0100
> @@ -449,6 +449,7 @@ static void nfs_show_mount_options(struc
>  	} nfs_info[] = {
>  		{ NFS_MOUNT_SOFT, ",soft", ",hard" },
>  		{ NFS_MOUNT_INTR, ",intr", ",nointr" },
> +		{ NFS_MOUNT_POSIX, ",posix", "" },
>  		{ NFS_MOUNT_NOCTO, ",nocto", "" },
>  		{ NFS_MOUNT_NOAC, ",noac", "" },
>  		{ NFS_MOUNT_NONLM, ",nolock", "" },
> @@ -463,6 +464,9 @@ static void nfs_show_mount_options(struc
>  	seq_printf(m, ",vers=%d", clp->rpc_ops->version);
>  	seq_printf(m, ",rsize=%d", nfss->rsize);
>  	seq_printf(m, ",wsize=%d", nfss->wsize);
> +	seq_printf(m, ",namelen=%d", nfss->namelen);
> +	if (nfss->bsize != 0)
> +		seq_printf(m, ",bsize=%d", nfss->bsize);
>  	if (nfss->acregmin != 3*HZ || showdefaults)
>  		seq_printf(m, ",acregmin=%d", nfss->acregmin/HZ);
>  	if (nfss->acregmax != 60*HZ || showdefaults)
> @@ -482,6 +486,17 @@ static void nfs_show_mount_options(struc
>  	seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout- 
> >to_initval / HZ);
>  	seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
>  	seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client- 
> >cl_auth->au_flavor));
> +
> +	/*
> +	 * Missing options:
> +	 * port=
> +	 * mountport=
> +	 * mountvers=
> +	 * mountproto=
> +	 * clientaddr=
> +	 * mounthost=
> +	 * mountaddr=
> +	 */
>  }
>
>  /*

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [patch 05/26] mount options: fix afs

[David Howells]

Miklos Szeredi <miklos@szeredi.hu> wrote:

> Add a .show_options super operation to afs.
> 
> Use generic_show_options() and save the complete option string in
> afs_get_sb().

Sounds reasonable, but I can't test it till I get back from LCA.

David

Re: [patch 24/26] mount options: fix tmpfs

[Hugh Dickins]

On Mon, 28 Jan 2008, Miklos Szeredi wrote:
> 
> I'll add functions for calculating the default max values, so the
> calculations won't accidentally become different for the
> initialization and the option showing.

Excellent.

> > If you agree with the version below, please take it into your collection
> > and insert your Signed-off-by.  I should admit, I've not yet tested how
> > the NUMA policies look: you'll hear from me again tomorrow morning if
> > those turn out to wrong.
> 
> OK, I'll send this to Andrew.  Maybe I'll wait until tomorrow to hear
> if it's working on NUMA.

I've just now checked: the NUMA options are showing fine.

Thanks,
Hugh

Re: [patch 01/26] mount options: add documentation

[Roman Zippel]

Hi,

On Thursday 24. January 2008, Miklos Szeredi wrote:

> Q: Why do we need correct option showing in /proc/mounts?
> A: We want /proc/mounts to fully replace /etc/mtab.  The reasons for
>    this are:
>     - unprivileged mounters won't be able to update /etc/mtab
>     - /etc/mtab doesn't work with private mount namespaces
>     - /etc/mtab can become out-of-sync with reality

I asked this before and I don't remember getting an answer:
How does this deal with certain special cases:
- chroot: how will mount/df only show the for chroot relevant mounts?
- loop: how is the connection between file and loop device maintained?

I don't quite see how you want to achieve a _full_ replacement.

Could also please explain why you want to go via user mounts. Other OS use a 
daemon for that, which e.g. can maintain access controls. How do you want to 
manage this?

bye, Roman

Re: [patch 01/26] mount options: add documentation

[Miklos Szeredi]

> > Q: Why do we need correct option showing in /proc/mounts?
> > A: We want /proc/mounts to fully replace /etc/mtab.  The reasons for
> >    this are:
> >     - unprivileged mounters won't be able to update /etc/mtab
> >     - /etc/mtab doesn't work with private mount namespaces
> >     - /etc/mtab can become out-of-sync with reality
> 
> I asked this before and I don't remember getting an answer:
> How does this deal with certain special cases:
> - chroot: how will mount/df only show the for chroot relevant mounts?

That is a very good question.  Andreas Gruenbacher had some patches
for fixing behavior of /proc/mounts under a chroot, but people are
paranoid about userspace ABI changes (unwarranted in this case, IMO).

  http://lkml.org/lkml/2007/4/20/147

Anyway, if we are going to have a new 'mountinfo' file, this could be
easily fixed as well.

> - loop: how is the connection between file and loop device maintained?

We also discussed this with Karel, maybe it didn't make it onto lkml.

The proposed solution was to store the "loop" flag separately in a
file under /var.  It could just be an empty file for each such loop
device:

  /var/lib/mount/loops/loop0

This file is created by mount(8) if the '-oloop' option is given.  And
umount(8) automatically tears down the loop device if it finds this
file.

> I don't quite see how you want to achieve a _full_ replacement.

I think a full replacement is perfectly feasible, just needs some more
work on the userspace side.

> Could also please explain why you want to go via user mounts. Other OS use a 
> daemon for that, which e.g. can maintain access controls. How do you want to 
> manage this?

The unprivileged mounts patches do contain a simple form of access
control.  I don't think anything more is needed, but of course, having
unprivileged mounts in the kernel does not prevent the use of a more
sophisticated access control daemon in userspace, if that becomes
necessary.

Miklos

Re: [patch 01/26] mount options: add documentation

[Karel Zak]

On Wed, Jan 30, 2008 at 10:09:03AM +0100, Miklos Szeredi wrote:
> > - loop: how is the connection between file and loop device maintained?
> 
> We also discussed this with Karel, maybe it didn't make it onto lkml.
> 
> The proposed solution was to store the "loop" flag separately in a
> file under /var.  It could just be an empty file for each such loop
> device:
> 
>   /var/lib/mount/loops/loop0
> 
> This file is created by mount(8) if the '-oloop' option is given.  And
> umount(8) automatically tears down the loop device if it finds this
> file.

 It seems we needn't this solution. There is loop auto-destruction
 patch in -mm.

 Kernel part:
    http://marc.info/?l=linux-kernel&m=119361296818388&w=2

 mount(8) part:
    http://marc.info/?l=util-linux-ng&m=119362955431694&w=2

 So, with this patch mount(8) needn't to maintain info about loops and
 umount(8) doesn't need to call LOOP_CLR_FD ioctl, because umount(2)
 is enough.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>

Re: [patch 01/26] mount options: add documentation

[Miklos Szeredi]

> > > - loop: how is the connection between file and loop device maintained?
> > 
> > We also discussed this with Karel, maybe it didn't make it onto lkml.
> > 
> > The proposed solution was to store the "loop" flag separately in a
> > file under /var.  It could just be an empty file for each such loop
> > device:
> > 
> >   /var/lib/mount/loops/loop0
> > 
> > This file is created by mount(8) if the '-oloop' option is given.  And
> > umount(8) automatically tears down the loop device if it finds this
> > file.
> 
>  It seems we needn't this solution. There is loop auto-destruction
>  patch in -mm.
> 
>  Kernel part:
>     http://marc.info/?l=linux-kernel&m=119361296818388&w=2
> 
>  mount(8) part:
>     http://marc.info/?l=util-linux-ng&m=119362955431694&w=2
> 
>  So, with this patch mount(8) needn't to maintain info about loops and
>  umount(8) doesn't need to call LOOP_CLR_FD ioctl, because umount(2)
>  is enough.

Excellent!  This is a very good example how moving a functionality
into the kernel can greatly simplify it.

Thanks,
Miklos

Re: [patch 01/26] mount options: add documentation

[Roman Zippel]

Hi,

On Wed, 30 Jan 2008, Miklos Szeredi wrote:

> > How does this deal with certain special cases:
> > - chroot: how will mount/df only show the for chroot relevant mounts?
> 
> That is a very good question.  Andreas Gruenbacher had some patches
> for fixing behavior of /proc/mounts under a chroot, but people are
> paranoid about userspace ABI changes (unwarranted in this case, IMO).
> 
>   http://lkml.org/lkml/2007/4/20/147
> 
> Anyway, if we are going to have a new 'mountinfo' file, this could be
> easily fixed as well.
> 
> > - loop: how is the connection between file and loop device maintained?
> 
> We also discussed this with Karel, maybe it didn't make it onto lkml.
> 
> The proposed solution was to store the "loop" flag separately in a
> file under /var.  It could just be an empty file for each such loop
> device:
> 
>   /var/lib/mount/loops/loop0
> 
> This file is created by mount(8) if the '-oloop' option is given.  And
> umount(8) automatically tears down the loop device if it finds this
> file.

My question was maybe a little short. I don't doubt that we can shove a 
lot into the kernel, the question is rather how much of this will be 
unnecessary information, which the kernel doesn't really need itself.

> > Could also please explain why you want to go via user mounts. Other OS use a 
> > daemon for that, which e.g. can maintain access controls. How do you want to 
> > manage this?
> 
> The unprivileged mounts patches do contain a simple form of access
> control.  I don't think anything more is needed, but of course, having
> unprivileged mounts in the kernel does not prevent the use of a more
> sophisticated access control daemon in userspace, if that becomes
> necessary.

A "I don't think anything more is needed" lets go off all sorts of warning 
lights. Most things start out simple, so IMO it's very worth it to check 
where it might go to to know the limits beforehand. The main question here 
is why should a kernel based solution be preferable over a daemon based 
solution?

If we look for example look at OS X, it has no need for user mounts but 
has a daemon instead, which also provides an interesting notification 
system for new devices, mounts or unmount requests. All this could also be 
done in the kernel, but where would be the advantage in doing so? The 
kernel implementation would be either rather limited or only bloat the 
kernel. What is the feature that would make user mounts more than just a 
cool kernel hack?

bye, Roman

Re: [patch 01/26] mount options: add documentation

[Miklos Szeredi]

> > > Could also please explain why you want to go via user
> > > mounts. Other OS use a daemon for that, which e.g. can maintain
> > > access controls. How do you want to manage this?
> > 
> > The unprivileged mounts patches do contain a simple form of access
> > control.  I don't think anything more is needed, but of course, having
> > unprivileged mounts in the kernel does not prevent the use of a more
> > sophisticated access control daemon in userspace, if that becomes
> > necessary.
> 
> A "I don't think anything more is needed" lets go off all sorts of warning 
> lights. Most things start out simple, so IMO it's very worth it to check 
> where it might go to to know the limits beforehand. The main question here 
> is why should a kernel based solution be preferable over a daemon based 
> solution?

A daemon based solution would work for the "normal" case, where we
have a single mount namespace and a single /etc/mtab file, and we hope
it doesn't get too much out of sync with what is actually in the
kernel (on remount the mount options do get out of sync, but hey, we
seem to be able to live with that).

However, once you start using multiple namespaces, the daemon based
solution quickly becomes unusable, because you would need a separate
daemon for each namespace, and it would have to somehow keep track of
mount propagations in userspace (which is basically impossible), etc,
etc...

Does that answer your question?

Thanks,
Miklos