[Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount

cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
       [not found] <20140313073936.GA14663@infradead.org>
@ 2014-03-13 14:20 ` Theodore Ts'o
  2014-03-13 16:23   ` Jan Kara
  2014-03-14  0:33   ` Steve French
  0 siblings, 2 replies; 7+ messages in thread
From: Theodore Ts'o @ 2014-03-13 14:20 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Previously, the no-op "mount -o mount /dev/xxx" operation when the
file system is already mounted read-write causes an implied,
unconditional syncfs().  This seems pretty stupid, and it's certainly
documented or guaraunteed to do this, nor is it particularly useful,
except in the case where the file system was mounted rw and is getting
remounted read-only.

However, it's possible that there might be some file systems that are
actually depending on this behavior.  In most file systems, it's
probably fine to only call sync_filesystem() when transitioning from
read-write to read-only, and there are some file systems where this is
not needed at all (for example, for a pseudo-filesystem or something
like romfs).

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-fsdevel at vger.kernel.org
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Evgeniy Dushistov <dushistov@mail.ru>
Cc: Jan Kara <jack@suse.cz>
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Anders Larsen <al@alarsen.net>
Cc: Phillip Lougher <phillip@squashfs.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: Petr Vandrovec <petr@vandrovec.name>
Cc: xfs at oss.sgi.com
Cc: linux-btrfs at vger.kernel.org
Cc: linux-cifs at vger.kernel.org
Cc: samba-technical at lists.samba.org
Cc: codalist at coda.cs.cmu.edu
Cc: linux-ext4 at vger.kernel.org
Cc: linux-f2fs-devel at lists.sourceforge.net
Cc: fuse-devel at lists.sourceforge.net
Cc: cluster-devel at redhat.com
Cc: linux-mtd at lists.infradead.org
Cc: jfs-discussion at lists.sourceforge.net
Cc: linux-nfs at vger.kernel.org
Cc: linux-nilfs at vger.kernel.org
Cc: linux-ntfs-dev at lists.sourceforge.net
Cc: ocfs2-devel at oss.oracle.com
Cc: reiserfs-devel at vger.kernel.org
---
 fs/adfs/super.c          | 1 +
 fs/affs/super.c          | 1 +
 fs/befs/linuxvfs.c       | 1 +
 fs/btrfs/super.c         | 1 +
 fs/cifs/cifsfs.c         | 1 +
 fs/coda/inode.c          | 1 +
 fs/cramfs/inode.c        | 1 +
 fs/debugfs/inode.c       | 1 +
 fs/devpts/inode.c        | 1 +
 fs/efs/super.c           | 1 +
 fs/ext2/super.c          | 1 +
 fs/ext3/super.c          | 2 ++
 fs/ext4/super.c          | 2 ++
 fs/f2fs/super.c          | 2 ++
 fs/fat/inode.c           | 2 ++
 fs/freevxfs/vxfs_super.c | 1 +
 fs/fuse/inode.c          | 1 +
 fs/gfs2/super.c          | 2 ++
 fs/hfs/super.c           | 1 +
 fs/hfsplus/super.c       | 1 +
 fs/hpfs/super.c          | 2 ++
 fs/isofs/inode.c         | 1 +
 fs/jffs2/super.c         | 1 +
 fs/jfs/super.c           | 1 +
 fs/minix/inode.c         | 1 +
 fs/ncpfs/inode.c         | 1 +
 fs/nfs/super.c           | 2 ++
 fs/nilfs2/super.c        | 1 +
 fs/ntfs/super.c          | 2 ++
 fs/ocfs2/super.c         | 2 ++
 fs/openpromfs/inode.c    | 1 +
 fs/proc/root.c           | 2 ++
 fs/pstore/inode.c        | 1 +
 fs/qnx4/inode.c          | 1 +
 fs/qnx6/inode.c          | 1 +
 fs/reiserfs/super.c      | 1 +
 fs/romfs/super.c         | 1 +
 fs/squashfs/super.c      | 1 +
 fs/super.c               | 2 --
 fs/sysv/inode.c          | 1 +
 fs/ubifs/super.c         | 1 +
 fs/udf/super.c           | 1 +
 fs/ufs/super.c           | 1 +
 fs/xfs/xfs_super.c       | 1 +
 44 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/fs/adfs/super.c b/fs/adfs/super.c
index 7b3003c..952aeb0 100644
--- a/fs/adfs/super.c
+++ b/fs/adfs/super.c
@@ -212,6 +212,7 @@ static int parse_options(struct super_block *sb, char *options)
 
 static int adfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NODIRATIME;
 	return parse_options(sb, data);
 }
diff --git a/fs/affs/super.c b/fs/affs/super.c
index d098731..3074530 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -530,6 +530,7 @@ affs_remount(struct super_block *sb, int *flags, char *data)
 
 	pr_debug("AFFS: remount(flags=0x%x,opts=\"%s\")\n",*flags,data);
 
+	sync_filesystem(sb);
 	*flags |= MS_NODIRATIME;
 
 	memcpy(volume, sbi->s_volume, 32);
diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 845d2d6..56d70c8 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -913,6 +913,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
 static int
 befs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	if (!(*flags & MS_RDONLY))
 		return -EINVAL;
 	return 0;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 97cc241..00cd0c5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1381,6 +1381,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 	unsigned int old_metadata_ratio = fs_info->metadata_ratio;
 	int ret;
 
+	sync_filesystem(sb);
 	btrfs_remount_prepare(fs_info);
 
 	ret = btrfs_parse_options(root, data);
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 849f613..4942c94 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -541,6 +541,7 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
 
 static int cifs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NODIRATIME;
 	return 0;
 }
diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index 506de34..3f48000 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -96,6 +96,7 @@ void coda_destroy_inodecache(void)
 
 static int coda_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NOATIME;
 	return 0;
 }
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 06610cf..a275911 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -244,6 +244,7 @@ static void cramfs_kill_sb(struct super_block *sb)
 
 static int cramfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 9c0444c..02928a9 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -218,6 +218,7 @@ static int debugfs_remount(struct super_block *sb, int *flags, char *data)
 	int err;
 	struct debugfs_fs_info *fsi = sb->s_fs_info;
 
+	sync_filesystem(sb);
 	err = debugfs_parse_options(data, &fsi->mount_opts);
 	if (err)
 		goto fail;
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index a726b9f..c710380 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -313,6 +313,7 @@ static int devpts_remount(struct super_block *sb, int *flags, char *data)
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
 	struct pts_mount_opts *opts = &fsi->mount_opts;
 
+	sync_filesystem(sb);
 	err = parse_mount_options(data, PARSE_REMOUNT, opts);
 
 	/*
diff --git a/fs/efs/super.c b/fs/efs/super.c
index 50215bb..103bbd8 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -114,6 +114,7 @@ static void destroy_inodecache(void)
 
 static int efs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 20d6697..d260115 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1254,6 +1254,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
 	unsigned long old_sb_flags;
 	int err;
 
+	sync_filesystem(sb);
 	spin_lock(&sbi->s_lock);
 
 	/* Store the old options */
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 37fd31e..95c6c5a 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -2649,6 +2649,8 @@ static int ext3_remount (struct super_block * sb, int * flags, char * data)
 	int i;
 #endif
 
+	sync_filesystem(sb);
+
 	/* Store the original options */
 	old_sb_flags = sb->s_flags;
 	old_opts.s_mount_opt = sbi->s_mount_opt;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f5c13b8..aa3842f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4767,6 +4767,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 #endif
 	char *orig_data = kstrdup(data, GFP_KERNEL);
 
+	sync_filesystem(sb);
+
 	/* Store the original options */
 	old_sb_flags = sb->s_flags;
 	old_opts.s_mount_opt = sbi->s_mount_opt;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1a85f83..856bdf9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -568,6 +568,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
 	struct f2fs_mount_info org_mount_opt;
 	int err, active_logs;
 
+	sync_filesystem(sb);
+
 	/*
 	 * Save the old mount options in case we
 	 * need to restore them.
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 854b578..343e477 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -635,6 +635,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
 
+	sync_filesystem(sb);
+
 	/* make sure we update state on remount. */
 	new_rdonly = *flags & MS_RDONLY;
 	if (new_rdonly != (sb->s_flags & MS_RDONLY)) {
diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
index e37eb27..7ca8c75 100644
--- a/fs/freevxfs/vxfs_super.c
+++ b/fs/freevxfs/vxfs_super.c
@@ -124,6 +124,7 @@ vxfs_statfs(struct dentry *dentry, struct kstatfs *bufp)
 
 static int vxfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d468643..ecdb255d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -135,6 +135,7 @@ static void fuse_evict_inode(struct inode *inode)
 
 static int fuse_remount_fs(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	if (*flags & MS_MANDLOCK)
 		return -EINVAL;
 
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 60f60f6..4c6dd50 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1175,6 +1175,8 @@ static int gfs2_remount_fs(struct super_block *sb, int *flags, char *data)
 	struct gfs2_tune *gt = &sdp->sd_tune;
 	int error;
 
+	sync_filesystem(sb);
+
 	spin_lock(&gt->gt_spin);
 	args.ar_commit = gt->gt_logd_secs;
 	args.ar_quota_quantum = gt->gt_quota_quantum;
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 2d2039e..eee7206 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -112,6 +112,7 @@ static int hfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 static int hfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NODIRATIME;
 	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
 		return 0;
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 80875aa..8eb787b 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -323,6 +323,7 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
 		return 0;
 	if (!(*flags & MS_RDONLY)) {
diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
index 4534ff6..fe3463a 100644
--- a/fs/hpfs/super.c
+++ b/fs/hpfs/super.c
@@ -421,6 +421,8 @@ static int hpfs_remount_fs(struct super_block *s, int *flags, char *data)
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
 	char *new_opts = kstrdup(data, GFP_KERNEL);
 	
+	sync_filesystem(s);
+
 	*flags |= MS_NOATIME;
 	
 	hpfs_lock(s);
diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 4a9e10e..6af66ee 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -117,6 +117,7 @@ static void destroy_inodecache(void)
 
 static int isofs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	if (!(*flags & MS_RDONLY))
 		return -EROFS;
 	return 0;
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 0defb1c..0918f0e 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -243,6 +243,7 @@ static int jffs2_remount_fs(struct super_block *sb, int *flags, char *data)
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
 	int err;
 
+	sync_filesystem(sb);
 	err = jffs2_parse_options(c, data);
 	if (err)
 		return -EINVAL;
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index e2b7483..97f7fda 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -418,6 +418,7 @@ static int jfs_remount(struct super_block *sb, int *flags, char *data)
 	int flag = JFS_SBI(sb)->flag;
 	int ret;
 
+	sync_filesystem(sb);
 	if (!parse_options(data, sb, &newLVSize, &flag)) {
 		return -EINVAL;
 	}
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 0332109..dcdc298 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -123,6 +123,7 @@ static int minix_remount (struct super_block * sb, int * flags, char * data)
 	struct minix_sb_info * sbi = minix_sb(sb);
 	struct minix_super_block * ms;
 
+	sync_filesystem(sb);
 	ms = sbi->s_ms;
 	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
 		return 0;
diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c
index 2cf2ebe..5f86e80 100644
--- a/fs/ncpfs/inode.c
+++ b/fs/ncpfs/inode.c
@@ -99,6 +99,7 @@ static void destroy_inodecache(void)
 
 static int ncp_remount(struct super_block *sb, int *flags, char* data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NODIRATIME;
 	return 0;
 }
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 910ed90..2cb5694 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2215,6 +2215,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
 	struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
 	u32 nfsvers = nfss->nfs_client->rpc_ops->version;
 
+	sync_filesystem(sb);
+
 	/*
 	 * Userspace mount programs that send binary options generally send
 	 * them populated with default values. We have no way to know which
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 7ac2a12..8c532b2 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1129,6 +1129,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 	unsigned long old_mount_opt;
 	int err;
 
+	sync_filesystem(sb);
 	old_sb_flags = sb->s_flags;
 	old_mount_opt = nilfs->ns_mount_opt;
 
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 82650d5..bd5610d 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -468,6 +468,8 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
 
 	ntfs_debug("Entering with remount options string: %s", opt);
 
+	sync_filesystem(sb);
+
 #ifndef NTFS_RW
 	/* For read-only compiled driver, enforce read-only flag. */
 	*flags |= MS_RDONLY;
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 49d84f8..5f9bf8f 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -631,6 +631,8 @@ static int ocfs2_remount(struct super_block *sb, int *flags, char *data)
 	struct ocfs2_super *osb = OCFS2_SB(sb);
 	u32 tmp;
 
+	sync_filesystem(sb);
+
 	if (!ocfs2_parse_options(sb, data, &parsed_options, 1) ||
 	    !ocfs2_check_set_options(sb, &parsed_options)) {
 		ret = -EINVAL;
diff --git a/fs/openpromfs/inode.c b/fs/openpromfs/inode.c
index 8c0ceb8..15e4500 100644
--- a/fs/openpromfs/inode.c
+++ b/fs/openpromfs/inode.c
@@ -368,6 +368,7 @@ static struct inode *openprom_iget(struct super_block *sb, ino_t ino)
 
 static int openprom_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_NOATIME;
 	return 0;
 }
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 87dbcbe..ac823a8 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -92,6 +92,8 @@ static int proc_parse_options(char *options, struct pid_namespace *pid)
 int proc_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct pid_namespace *pid = sb->s_fs_info;
+
+	sync_filesystem(sb);
 	return !proc_parse_options(data, pid);
 }
 
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1282384..192297b 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -249,6 +249,7 @@ static void parse_options(char *options)
 
 static int pstore_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	parse_options(data);
 
 	return 0;
diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 8955881..c4bcb77 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -44,6 +44,7 @@ static int qnx4_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct qnx4_sb_info *qs;
 
+	sync_filesystem(sb);
 	qs = qnx4_sb(sb);
 	qs->Version = QNX4_VERSION;
 	*flags |= MS_RDONLY;
diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 8d941ed..65cdaab 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -55,6 +55,7 @@ static int qnx6_show_options(struct seq_file *seq, struct dentry *root)
 
 static int qnx6_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 2c80335..abf2b76 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1319,6 +1319,7 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg)
 	int i;
 #endif
 
+	sync_filesystem(s);
 	reiserfs_write_lock(s);
 
 #ifdef CONFIG_QUOTA
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index d841878..ef90e8b 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -432,6 +432,7 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
  */
 static int romfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 202df63..031c8d67 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -371,6 +371,7 @@ static int squashfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 static int squashfs_remount(struct super_block *sb, int *flags, char *data)
 {
+	sync_filesystem(sb);
 	*flags |= MS_RDONLY;
 	return 0;
 }
diff --git a/fs/super.c b/fs/super.c
index 80d5cf2..e9dc3c3 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -719,8 +719,6 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
 		}
 	}
 
-	sync_filesystem(sb);
-
 	if (sb->s_op->remount_fs) {
 		retval = sb->s_op->remount_fs(sb, &flags, data);
 		if (retval) {
diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
index c327d4e..4742e58 100644
--- a/fs/sysv/inode.c
+++ b/fs/sysv/inode.c
@@ -60,6 +60,7 @@ static int sysv_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct sysv_sb_info *sbi = SYSV_SB(sb);
 
+	sync_filesystem(sb);
 	if (sbi->s_forced_ro)
 		*flags |= MS_RDONLY;
 	return 0;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 5ded849..e1598ab 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1827,6 +1827,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
 	int err;
 	struct ubifs_info *c = sb->s_fs_info;
 
+	sync_filesystem(sb);
 	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
 
 	err = ubifs_parse_options(c, data, 1);
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 3306b9f..64f2b73 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -646,6 +646,7 @@ static int udf_remount_fs(struct super_block *sb, int *flags, char *options)
 	int error = 0;
 	struct logicalVolIntegrityDescImpUse *lvidiu = udf_sb_lvidiu(sb);
 
+	sync_filesystem(sb);
 	if (lvidiu) {
 		int write_rev = le16_to_cpu(lvidiu->minUDFWriteRev);
 		if (write_rev > UDF_MAX_WRITE_VERSION && !(*flags & MS_RDONLY))
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 329f2f5..b8c6791 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -1280,6 +1280,7 @@ static int ufs_remount (struct super_block *sb, int *mount_flags, char *data)
 	unsigned new_mount_opt, ufstype;
 	unsigned flags;
 
+	sync_filesystem(sb);
 	lock_ufs(sb);
 	mutex_lock(&UFS_SB(sb)->s_lock);
 	uspi = UFS_SB(sb)->s_uspi;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f317488..aaa3eca 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1197,6 +1197,7 @@ xfs_fs_remount(
 	char			*p;
 	int			error;
 
+	sync_filesystem(sb);
 	while ((p = strsep(&options, ",")) != NULL) {
 		int token;
 
-- 
1.9.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-13 14:20 ` [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs() Theodore Ts'o
@ 2014-03-13 16:23   ` Jan Kara
  2014-03-13 16:28     ` Steven Whitehouse
  2014-03-14  0:33   ` Steve French
  1 sibling, 1 reply; 7+ messages in thread
From: Jan Kara @ 2014-03-13 16:23 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu 13-03-14 10:20:56, Ted Tso wrote:
> Previously, the no-op "mount -o mount /dev/xxx" operation when the
                                  ^^remount

> file system is already mounted read-write causes an implied,
> unconditional syncfs().  This seems pretty stupid, and it's certainly
> documented or guaraunteed to do this, nor is it particularly useful,
> except in the case where the file system was mounted rw and is getting
> remounted read-only.
> 
> However, it's possible that there might be some file systems that are
> actually depending on this behavior.  In most file systems, it's
> probably fine to only call sync_filesystem() when transitioning from
> read-write to read-only, and there are some file systems where this is
> not needed at all (for example, for a pseudo-filesystem or something
> like romfs).
  Hum, I'd avoid this excercise at least for filesystem where
sync_filesystem() is obviously useless - proc, debugfs, pstore, devpts,
also always read-only filesystems such as isofs, qnx4, qnx6, befs, cramfs,
efs, freevxfs, romfs, squashfs. I think you can find a couple more which
clearly don't care about sync_filesystem() if you look a bit closer.

								Honza
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: linux-fsdevel at vger.kernel.org
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Evgeniy Dushistov <dushistov@mail.ru>
> Cc: Jan Kara <jack@suse.cz>
> Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Cc: Anders Larsen <al@alarsen.net>
> Cc: Phillip Lougher <phillip@squashfs.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> Cc: Petr Vandrovec <petr@vandrovec.name>
> Cc: xfs at oss.sgi.com
> Cc: linux-btrfs at vger.kernel.org
> Cc: linux-cifs at vger.kernel.org
> Cc: samba-technical at lists.samba.org
> Cc: codalist at coda.cs.cmu.edu
> Cc: linux-ext4 at vger.kernel.org
> Cc: linux-f2fs-devel at lists.sourceforge.net
> Cc: fuse-devel at lists.sourceforge.net
> Cc: cluster-devel at redhat.com
> Cc: linux-mtd at lists.infradead.org
> Cc: jfs-discussion at lists.sourceforge.net
> Cc: linux-nfs at vger.kernel.org
> Cc: linux-nilfs at vger.kernel.org
> Cc: linux-ntfs-dev at lists.sourceforge.net
> Cc: ocfs2-devel at oss.oracle.com
> Cc: reiserfs-devel at vger.kernel.org
> ---
>  fs/adfs/super.c          | 1 +
>  fs/affs/super.c          | 1 +
>  fs/befs/linuxvfs.c       | 1 +
>  fs/btrfs/super.c         | 1 +
>  fs/cifs/cifsfs.c         | 1 +
>  fs/coda/inode.c          | 1 +
>  fs/cramfs/inode.c        | 1 +
>  fs/debugfs/inode.c       | 1 +
>  fs/devpts/inode.c        | 1 +
>  fs/efs/super.c           | 1 +
>  fs/ext2/super.c          | 1 +
>  fs/ext3/super.c          | 2 ++
>  fs/ext4/super.c          | 2 ++
>  fs/f2fs/super.c          | 2 ++
>  fs/fat/inode.c           | 2 ++
>  fs/freevxfs/vxfs_super.c | 1 +
>  fs/fuse/inode.c          | 1 +
>  fs/gfs2/super.c          | 2 ++
>  fs/hfs/super.c           | 1 +
>  fs/hfsplus/super.c       | 1 +
>  fs/hpfs/super.c          | 2 ++
>  fs/isofs/inode.c         | 1 +
>  fs/jffs2/super.c         | 1 +
>  fs/jfs/super.c           | 1 +
>  fs/minix/inode.c         | 1 +
>  fs/ncpfs/inode.c         | 1 +
>  fs/nfs/super.c           | 2 ++
>  fs/nilfs2/super.c        | 1 +
>  fs/ntfs/super.c          | 2 ++
>  fs/ocfs2/super.c         | 2 ++
>  fs/openpromfs/inode.c    | 1 +
>  fs/proc/root.c           | 2 ++
>  fs/pstore/inode.c        | 1 +
>  fs/qnx4/inode.c          | 1 +
>  fs/qnx6/inode.c          | 1 +
>  fs/reiserfs/super.c      | 1 +
>  fs/romfs/super.c         | 1 +
>  fs/squashfs/super.c      | 1 +
>  fs/super.c               | 2 --
>  fs/sysv/inode.c          | 1 +
>  fs/ubifs/super.c         | 1 +
>  fs/udf/super.c           | 1 +
>  fs/ufs/super.c           | 1 +
>  fs/xfs/xfs_super.c       | 1 +
>  44 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/adfs/super.c b/fs/adfs/super.c
> index 7b3003c..952aeb0 100644
> --- a/fs/adfs/super.c
> +++ b/fs/adfs/super.c
> @@ -212,6 +212,7 @@ static int parse_options(struct super_block *sb, char *options)
>  
>  static int adfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NODIRATIME;
>  	return parse_options(sb, data);
>  }
> diff --git a/fs/affs/super.c b/fs/affs/super.c
> index d098731..3074530 100644
> --- a/fs/affs/super.c
> +++ b/fs/affs/super.c
> @@ -530,6 +530,7 @@ affs_remount(struct super_block *sb, int *flags, char *data)
>  
>  	pr_debug("AFFS: remount(flags=0x%x,opts=\"%s\")\n",*flags,data);
>  
> +	sync_filesystem(sb);
>  	*flags |= MS_NODIRATIME;
>  
>  	memcpy(volume, sbi->s_volume, 32);
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 845d2d6..56d70c8 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -913,6 +913,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  static int
>  befs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	if (!(*flags & MS_RDONLY))
>  		return -EINVAL;
>  	return 0;
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 97cc241..00cd0c5 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1381,6 +1381,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>  	unsigned int old_metadata_ratio = fs_info->metadata_ratio;
>  	int ret;
>  
> +	sync_filesystem(sb);
>  	btrfs_remount_prepare(fs_info);
>  
>  	ret = btrfs_parse_options(root, data);
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 849f613..4942c94 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -541,6 +541,7 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
>  
>  static int cifs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NODIRATIME;
>  	return 0;
>  }
> diff --git a/fs/coda/inode.c b/fs/coda/inode.c
> index 506de34..3f48000 100644
> --- a/fs/coda/inode.c
> +++ b/fs/coda/inode.c
> @@ -96,6 +96,7 @@ void coda_destroy_inodecache(void)
>  
>  static int coda_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NOATIME;
>  	return 0;
>  }
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 06610cf..a275911 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -244,6 +244,7 @@ static void cramfs_kill_sb(struct super_block *sb)
>  
>  static int cramfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index 9c0444c..02928a9 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -218,6 +218,7 @@ static int debugfs_remount(struct super_block *sb, int *flags, char *data)
>  	int err;
>  	struct debugfs_fs_info *fsi = sb->s_fs_info;
>  
> +	sync_filesystem(sb);
>  	err = debugfs_parse_options(data, &fsi->mount_opts);
>  	if (err)
>  		goto fail;
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index a726b9f..c710380 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -313,6 +313,7 @@ static int devpts_remount(struct super_block *sb, int *flags, char *data)
>  	struct pts_fs_info *fsi = DEVPTS_SB(sb);
>  	struct pts_mount_opts *opts = &fsi->mount_opts;
>  
> +	sync_filesystem(sb);
>  	err = parse_mount_options(data, PARSE_REMOUNT, opts);
>  
>  	/*
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index 50215bb..103bbd8 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -114,6 +114,7 @@ static void destroy_inodecache(void)
>  
>  static int efs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 20d6697..d260115 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1254,6 +1254,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
>  	unsigned long old_sb_flags;
>  	int err;
>  
> +	sync_filesystem(sb);
>  	spin_lock(&sbi->s_lock);
>  
>  	/* Store the old options */
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 37fd31e..95c6c5a 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -2649,6 +2649,8 @@ static int ext3_remount (struct super_block * sb, int * flags, char * data)
>  	int i;
>  #endif
>  
> +	sync_filesystem(sb);
> +
>  	/* Store the original options */
>  	old_sb_flags = sb->s_flags;
>  	old_opts.s_mount_opt = sbi->s_mount_opt;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5c13b8..aa3842f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4767,6 +4767,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>  #endif
>  	char *orig_data = kstrdup(data, GFP_KERNEL);
>  
> +	sync_filesystem(sb);
> +
>  	/* Store the original options */
>  	old_sb_flags = sb->s_flags;
>  	old_opts.s_mount_opt = sbi->s_mount_opt;
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 1a85f83..856bdf9 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -568,6 +568,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>  	struct f2fs_mount_info org_mount_opt;
>  	int err, active_logs;
>  
> +	sync_filesystem(sb);
> +
>  	/*
>  	 * Save the old mount options in case we
>  	 * need to restore them.
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index 854b578..343e477 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -635,6 +635,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
>  
> +	sync_filesystem(sb);
> +
>  	/* make sure we update state on remount. */
>  	new_rdonly = *flags & MS_RDONLY;
>  	if (new_rdonly != (sb->s_flags & MS_RDONLY)) {
> diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
> index e37eb27..7ca8c75 100644
> --- a/fs/freevxfs/vxfs_super.c
> +++ b/fs/freevxfs/vxfs_super.c
> @@ -124,6 +124,7 @@ vxfs_statfs(struct dentry *dentry, struct kstatfs *bufp)
>  
>  static int vxfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index d468643..ecdb255d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -135,6 +135,7 @@ static void fuse_evict_inode(struct inode *inode)
>  
>  static int fuse_remount_fs(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	if (*flags & MS_MANDLOCK)
>  		return -EINVAL;
>  
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 60f60f6..4c6dd50 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1175,6 +1175,8 @@ static int gfs2_remount_fs(struct super_block *sb, int *flags, char *data)
>  	struct gfs2_tune *gt = &sdp->sd_tune;
>  	int error;
>  
> +	sync_filesystem(sb);
> +
>  	spin_lock(&gt->gt_spin);
>  	args.ar_commit = gt->gt_logd_secs;
>  	args.ar_quota_quantum = gt->gt_quota_quantum;
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index 2d2039e..eee7206 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -112,6 +112,7 @@ static int hfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  static int hfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NODIRATIME;
>  	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>  		return 0;
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 80875aa..8eb787b 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -323,6 +323,7 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>  		return 0;
>  	if (!(*flags & MS_RDONLY)) {
> diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
> index 4534ff6..fe3463a 100644
> --- a/fs/hpfs/super.c
> +++ b/fs/hpfs/super.c
> @@ -421,6 +421,8 @@ static int hpfs_remount_fs(struct super_block *s, int *flags, char *data)
>  	struct hpfs_sb_info *sbi = hpfs_sb(s);
>  	char *new_opts = kstrdup(data, GFP_KERNEL);
>  	
> +	sync_filesystem(s);
> +
>  	*flags |= MS_NOATIME;
>  	
>  	hpfs_lock(s);
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 4a9e10e..6af66ee 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -117,6 +117,7 @@ static void destroy_inodecache(void)
>  
>  static int isofs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	if (!(*flags & MS_RDONLY))
>  		return -EROFS;
>  	return 0;
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 0defb1c..0918f0e 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -243,6 +243,7 @@ static int jffs2_remount_fs(struct super_block *sb, int *flags, char *data)
>  	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
>  	int err;
>  
> +	sync_filesystem(sb);
>  	err = jffs2_parse_options(c, data);
>  	if (err)
>  		return -EINVAL;
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index e2b7483..97f7fda 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -418,6 +418,7 @@ static int jfs_remount(struct super_block *sb, int *flags, char *data)
>  	int flag = JFS_SBI(sb)->flag;
>  	int ret;
>  
> +	sync_filesystem(sb);
>  	if (!parse_options(data, sb, &newLVSize, &flag)) {
>  		return -EINVAL;
>  	}
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index 0332109..dcdc298 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -123,6 +123,7 @@ static int minix_remount (struct super_block * sb, int * flags, char * data)
>  	struct minix_sb_info * sbi = minix_sb(sb);
>  	struct minix_super_block * ms;
>  
> +	sync_filesystem(sb);
>  	ms = sbi->s_ms;
>  	if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>  		return 0;
> diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c
> index 2cf2ebe..5f86e80 100644
> --- a/fs/ncpfs/inode.c
> +++ b/fs/ncpfs/inode.c
> @@ -99,6 +99,7 @@ static void destroy_inodecache(void)
>  
>  static int ncp_remount(struct super_block *sb, int *flags, char* data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NODIRATIME;
>  	return 0;
>  }
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 910ed90..2cb5694 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2215,6 +2215,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
>  	struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
>  	u32 nfsvers = nfss->nfs_client->rpc_ops->version;
>  
> +	sync_filesystem(sb);
> +
>  	/*
>  	 * Userspace mount programs that send binary options generally send
>  	 * them populated with default values. We have no way to know which
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 7ac2a12..8c532b2 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -1129,6 +1129,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
>  	unsigned long old_mount_opt;
>  	int err;
>  
> +	sync_filesystem(sb);
>  	old_sb_flags = sb->s_flags;
>  	old_mount_opt = nilfs->ns_mount_opt;
>  
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 82650d5..bd5610d 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -468,6 +468,8 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
>  
>  	ntfs_debug("Entering with remount options string: %s", opt);
>  
> +	sync_filesystem(sb);
> +
>  #ifndef NTFS_RW
>  	/* For read-only compiled driver, enforce read-only flag. */
>  	*flags |= MS_RDONLY;
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 49d84f8..5f9bf8f 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -631,6 +631,8 @@ static int ocfs2_remount(struct super_block *sb, int *flags, char *data)
>  	struct ocfs2_super *osb = OCFS2_SB(sb);
>  	u32 tmp;
>  
> +	sync_filesystem(sb);
> +
>  	if (!ocfs2_parse_options(sb, data, &parsed_options, 1) ||
>  	    !ocfs2_check_set_options(sb, &parsed_options)) {
>  		ret = -EINVAL;
> diff --git a/fs/openpromfs/inode.c b/fs/openpromfs/inode.c
> index 8c0ceb8..15e4500 100644
> --- a/fs/openpromfs/inode.c
> +++ b/fs/openpromfs/inode.c
> @@ -368,6 +368,7 @@ static struct inode *openprom_iget(struct super_block *sb, ino_t ino)
>  
>  static int openprom_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_NOATIME;
>  	return 0;
>  }
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 87dbcbe..ac823a8 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -92,6 +92,8 @@ static int proc_parse_options(char *options, struct pid_namespace *pid)
>  int proc_remount(struct super_block *sb, int *flags, char *data)
>  {
>  	struct pid_namespace *pid = sb->s_fs_info;
> +
> +	sync_filesystem(sb);
>  	return !proc_parse_options(data, pid);
>  }
>  
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 1282384..192297b 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -249,6 +249,7 @@ static void parse_options(char *options)
>  
>  static int pstore_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	parse_options(data);
>  
>  	return 0;
> diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
> index 8955881..c4bcb77 100644
> --- a/fs/qnx4/inode.c
> +++ b/fs/qnx4/inode.c
> @@ -44,6 +44,7 @@ static int qnx4_remount(struct super_block *sb, int *flags, char *data)
>  {
>  	struct qnx4_sb_info *qs;
>  
> +	sync_filesystem(sb);
>  	qs = qnx4_sb(sb);
>  	qs->Version = QNX4_VERSION;
>  	*flags |= MS_RDONLY;
> diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
> index 8d941ed..65cdaab 100644
> --- a/fs/qnx6/inode.c
> +++ b/fs/qnx6/inode.c
> @@ -55,6 +55,7 @@ static int qnx6_show_options(struct seq_file *seq, struct dentry *root)
>  
>  static int qnx6_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index 2c80335..abf2b76 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -1319,6 +1319,7 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg)
>  	int i;
>  #endif
>  
> +	sync_filesystem(s);
>  	reiserfs_write_lock(s);
>  
>  #ifdef CONFIG_QUOTA
> diff --git a/fs/romfs/super.c b/fs/romfs/super.c
> index d841878..ef90e8b 100644
> --- a/fs/romfs/super.c
> +++ b/fs/romfs/super.c
> @@ -432,6 +432,7 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   */
>  static int romfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
> index 202df63..031c8d67 100644
> --- a/fs/squashfs/super.c
> +++ b/fs/squashfs/super.c
> @@ -371,6 +371,7 @@ static int squashfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  static int squashfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +	sync_filesystem(sb);
>  	*flags |= MS_RDONLY;
>  	return 0;
>  }
> diff --git a/fs/super.c b/fs/super.c
> index 80d5cf2..e9dc3c3 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -719,8 +719,6 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
>  		}
>  	}
>  
> -	sync_filesystem(sb);
> -
>  	if (sb->s_op->remount_fs) {
>  		retval = sb->s_op->remount_fs(sb, &flags, data);
>  		if (retval) {
> diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
> index c327d4e..4742e58 100644
> --- a/fs/sysv/inode.c
> +++ b/fs/sysv/inode.c
> @@ -60,6 +60,7 @@ static int sysv_remount(struct super_block *sb, int *flags, char *data)
>  {
>  	struct sysv_sb_info *sbi = SYSV_SB(sb);
>  
> +	sync_filesystem(sb);
>  	if (sbi->s_forced_ro)
>  		*flags |= MS_RDONLY;
>  	return 0;
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 5ded849..e1598ab 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1827,6 +1827,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
>  	int err;
>  	struct ubifs_info *c = sb->s_fs_info;
>  
> +	sync_filesystem(sb);
>  	dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
>  
>  	err = ubifs_parse_options(c, data, 1);
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 3306b9f..64f2b73 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -646,6 +646,7 @@ static int udf_remount_fs(struct super_block *sb, int *flags, char *options)
>  	int error = 0;
>  	struct logicalVolIntegrityDescImpUse *lvidiu = udf_sb_lvidiu(sb);
>  
> +	sync_filesystem(sb);
>  	if (lvidiu) {
>  		int write_rev = le16_to_cpu(lvidiu->minUDFWriteRev);
>  		if (write_rev > UDF_MAX_WRITE_VERSION && !(*flags & MS_RDONLY))
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 329f2f5..b8c6791 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -1280,6 +1280,7 @@ static int ufs_remount (struct super_block *sb, int *mount_flags, char *data)
>  	unsigned new_mount_opt, ufstype;
>  	unsigned flags;
>  
> +	sync_filesystem(sb);
>  	lock_ufs(sb);
>  	mutex_lock(&UFS_SB(sb)->s_lock);
>  	uspi = UFS_SB(sb)->s_uspi;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f317488..aaa3eca 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1197,6 +1197,7 @@ xfs_fs_remount(
>  	char			*p;
>  	int			error;
>  
> +	sync_filesystem(sb);
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int token;
>  
> -- 
> 1.9.0
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-13 16:23   ` Jan Kara
@ 2014-03-13 16:28     ` Steven Whitehouse
  2014-03-13 23:15       ` Theodore Ts'o
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Whitehouse @ 2014-03-13 16:28 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

On Thu, 2014-03-13 at 17:23 +0100, Jan Kara wrote:
> On Thu 13-03-14 10:20:56, Ted Tso wrote:
> > Previously, the no-op "mount -o mount /dev/xxx" operation when the
>                                   ^^remount
> 
> > file system is already mounted read-write causes an implied,
> > unconditional syncfs().  This seems pretty stupid, and it's certainly
> > documented or guaraunteed to do this, nor is it particularly useful,
> > except in the case where the file system was mounted rw and is getting
> > remounted read-only.
> > 
> > However, it's possible that there might be some file systems that are
> > actually depending on this behavior.  In most file systems, it's
> > probably fine to only call sync_filesystem() when transitioning from
> > read-write to read-only, and there are some file systems where this is
> > not needed at all (for example, for a pseudo-filesystem or something
> > like romfs).
>   Hum, I'd avoid this excercise at least for filesystem where
> sync_filesystem() is obviously useless - proc, debugfs, pstore, devpts,
> also always read-only filesystems such as isofs, qnx4, qnx6, befs, cramfs,
> efs, freevxfs, romfs, squashfs. I think you can find a couple more which
> clearly don't care about sync_filesystem() if you look a bit closer.
> 
>
> 								Honza

I guess the same is true for other file systems which are mounted ro
too. So maybe a check for MS_RDONLY before doing the sync in those
cases?

Steve.




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-13 16:28     ` Steven Whitehouse
@ 2014-03-13 23:15       ` Theodore Ts'o
  2014-03-14 12:13         ` Jan Kara
  0 siblings, 1 reply; 7+ messages in thread
From: Theodore Ts'o @ 2014-03-13 23:15 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu, Mar 13, 2014 at 04:28:23PM +0000, Steven Whitehouse wrote:
> 
> I guess the same is true for other file systems which are mounted ro
> too. So maybe a check for MS_RDONLY before doing the sync in those
> cases?

My original patch moved the sync_filesystem into the check for
MS_RDONLY in the core VFS code.  The objection was raised that there
might be some file system out there that might depend on this
behaviour.  I can't imagine why, but I suppose it's at least
theoretically possible.

So the idea is that this particular patch is *guaranteed* not to make
any difference.  That way there can be no question about the patch'es
correctness.

I'm going to follow up with a patch for ext4 that does exactly that,
but the idea is to allow each file system maintainer to do that for
their own file system.

I could do that as well for file systems that are "obviously"
read-only, but then I'll find out that there's some wierd case where
the file system can be used in a read-write fashion.  (Example: UDF is
normally used for DVD's, but at least in theory it can be used
read/write --- I'm told that Windows supports read-write UDF file
systems on USB sticks, and at least in theory it could be used as a
inter-OS exchange format in situations where VFAT and exFAT might not
be appropriate for various reasons.)

Cheers,

						- Ted

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-13 23:15       ` Theodore Ts'o
@ 2014-03-14 12:13         ` Jan Kara
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Kara @ 2014-03-14 12:13 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu 13-03-14 19:15:06, Ted Tso wrote:
> On Thu, Mar 13, 2014 at 04:28:23PM +0000, Steven Whitehouse wrote:
> > 
> > I guess the same is true for other file systems which are mounted ro
> > too. So maybe a check for MS_RDONLY before doing the sync in those
> > cases?
> 
> My original patch moved the sync_filesystem into the check for
> MS_RDONLY in the core VFS code.  The objection was raised that there
> might be some file system out there that might depend on this
> behaviour.  I can't imagine why, but I suppose it's at least
> theoretically possible.
  Well, syncfs() in RO->RW transition clearly isn't needed. In RW->RO
transition basically everybody needs it. What is disputable is the case of
RW->RW remounts and I could imagine someone relying on syncfs() there
although 99% of filesystems don't need it. So I agree in principle with
moving the responsibility for syncfs() to ->remount so that each filesystem
can decide.

> So the idea is that this particular patch is *guaranteed* not to make
> any difference.  That way there can be no question about the patch'es
> correctness.
> 
> I'm going to follow up with a patch for ext4 that does exactly that,
> but the idea is to allow each file system maintainer to do that for
> their own file system.
> 
> I could do that as well for file systems that are "obviously"
> read-only, but then I'll find out that there's some wierd case where
> the file system can be used in a read-write fashion.  (Example: UDF is
> normally used for DVD's, but at least in theory it can be used
> read/write --- I'm told that Windows supports read-write UDF file
> systems on USB sticks, and at least in theory it could be used as a
> inter-OS exchange format in situations where VFAT and exFAT might not
> be appropriate for various reasons.)
  Yes, some people do use UDF for USB sticks - Linux supports it in
read-write mode as well (although with somewhat limited set of features).
But the filesystem's I've named are those which clearly either bail with
error if RW mount is attempted or silently change it to RO mount. In these
cases, I'd just patch away the unnecessary sync_fs().

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-13 14:20 ` [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs() Theodore Ts'o
  2014-03-13 16:23   ` Jan Kara
@ 2014-03-14  0:33   ` Steve French
  2014-03-14  1:23     ` Theodore Ts'o
  1 sibling, 1 reply; 7+ messages in thread
From: Steve French @ 2014-03-14  0:33 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu, Mar 13, 2014 at 9:20 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> Previously, the no-op "mount -o mount /dev/xxx" operation when the
> file system is already mounted read-write causes an implied,
> unconditional syncfs().  This seems pretty stupid, and it's certainly
> documented or guaraunteed to do this, nor is it particularly useful,
> except in the case where the file system was mounted rw and is getting
> remounted read-only.

Is there a case where a file system, not mounted read-only,
would want to skip the syncfs on remount?  I don't know
of any particular reason to do a syncfs on remount unless
caching behavior is changing (or moving to read-only mount),
but if as you say it is documented and guaranteed...


> However, it's possible that there might be some file systems that are
> actually depending on this behavior.  In most file systems, it's
> probably fine to only call sync_filesystem() when transitioning from
> read-write to read-only, and there are some file systems where this is
> not needed at all (for example, for a pseudo-filesystem or something
> like romfs).
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: linux-fsdevel at vger.kernel.org
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Evgeniy Dushistov <dushistov@mail.ru>
> Cc: Jan Kara <jack@suse.cz>
> Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Cc: Anders Larsen <al@alarsen.net>
> Cc: Phillip Lougher <phillip@squashfs.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> Cc: Petr Vandrovec <petr@vandrovec.name>
> Cc: xfs at oss.sgi.com
> Cc: linux-btrfs at vger.kernel.org
> Cc: linux-cifs at vger.kernel.org
> Cc: samba-technical at lists.samba.org
> Cc: codalist at coda.cs.cmu.edu
> Cc: linux-ext4 at vger.kernel.org
> Cc: linux-f2fs-devel at lists.sourceforge.net
> Cc: fuse-devel at lists.sourceforge.net
> Cc: cluster-devel at redhat.com
> Cc: linux-mtd at lists.infradead.org
> Cc: jfs-discussion at lists.sourceforge.net
> Cc: linux-nfs at vger.kernel.org
> Cc: linux-nilfs at vger.kernel.org
> Cc: linux-ntfs-dev at lists.sourceforge.net
> Cc: ocfs2-devel at oss.oracle.com
> Cc: reiserfs-devel at vger.kernel.org
> ---
>  fs/adfs/super.c          | 1 +
>  fs/affs/super.c          | 1 +
>  fs/befs/linuxvfs.c       | 1 +
>  fs/btrfs/super.c         | 1 +
>  fs/cifs/cifsfs.c         | 1 +
>  fs/coda/inode.c          | 1 +
>  fs/cramfs/inode.c        | 1 +
>  fs/debugfs/inode.c       | 1 +
>  fs/devpts/inode.c        | 1 +
>  fs/efs/super.c           | 1 +
>  fs/ext2/super.c          | 1 +
>  fs/ext3/super.c          | 2 ++
>  fs/ext4/super.c          | 2 ++
>  fs/f2fs/super.c          | 2 ++
>  fs/fat/inode.c           | 2 ++
>  fs/freevxfs/vxfs_super.c | 1 +
>  fs/fuse/inode.c          | 1 +
>  fs/gfs2/super.c          | 2 ++
>  fs/hfs/super.c           | 1 +
>  fs/hfsplus/super.c       | 1 +
>  fs/hpfs/super.c          | 2 ++
>  fs/isofs/inode.c         | 1 +
>  fs/jffs2/super.c         | 1 +
>  fs/jfs/super.c           | 1 +
>  fs/minix/inode.c         | 1 +
>  fs/ncpfs/inode.c         | 1 +
>  fs/nfs/super.c           | 2 ++
>  fs/nilfs2/super.c        | 1 +
>  fs/ntfs/super.c          | 2 ++
>  fs/ocfs2/super.c         | 2 ++
>  fs/openpromfs/inode.c    | 1 +
>  fs/proc/root.c           | 2 ++
>  fs/pstore/inode.c        | 1 +
>  fs/qnx4/inode.c          | 1 +
>  fs/qnx6/inode.c          | 1 +
>  fs/reiserfs/super.c      | 1 +
>  fs/romfs/super.c         | 1 +
>  fs/squashfs/super.c      | 1 +
>  fs/super.c               | 2 --
>  fs/sysv/inode.c          | 1 +
>  fs/ubifs/super.c         | 1 +
>  fs/udf/super.c           | 1 +
>  fs/ufs/super.c           | 1 +
>  fs/xfs/xfs_super.c       | 1 +
>  44 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/fs/adfs/super.c b/fs/adfs/super.c
> index 7b3003c..952aeb0 100644
> --- a/fs/adfs/super.c
> +++ b/fs/adfs/super.c
> @@ -212,6 +212,7 @@ static int parse_options(struct super_block *sb, char *options)
>
>  static int adfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NODIRATIME;
>         return parse_options(sb, data);
>  }
> diff --git a/fs/affs/super.c b/fs/affs/super.c
> index d098731..3074530 100644
> --- a/fs/affs/super.c
> +++ b/fs/affs/super.c
> @@ -530,6 +530,7 @@ affs_remount(struct super_block *sb, int *flags, char *data)
>
>         pr_debug("AFFS: remount(flags=0x%x,opts=\"%s\")\n",*flags,data);
>
> +       sync_filesystem(sb);
>         *flags |= MS_NODIRATIME;
>
>         memcpy(volume, sbi->s_volume, 32);
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 845d2d6..56d70c8 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -913,6 +913,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  static int
>  befs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         if (!(*flags & MS_RDONLY))
>                 return -EINVAL;
>         return 0;
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 97cc241..00cd0c5 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1381,6 +1381,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>         unsigned int old_metadata_ratio = fs_info->metadata_ratio;
>         int ret;
>
> +       sync_filesystem(sb);
>         btrfs_remount_prepare(fs_info);
>
>         ret = btrfs_parse_options(root, data);
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 849f613..4942c94 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -541,6 +541,7 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
>
>  static int cifs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NODIRATIME;
>         return 0;
>  }
> diff --git a/fs/coda/inode.c b/fs/coda/inode.c
> index 506de34..3f48000 100644
> --- a/fs/coda/inode.c
> +++ b/fs/coda/inode.c
> @@ -96,6 +96,7 @@ void coda_destroy_inodecache(void)
>
>  static int coda_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NOATIME;
>         return 0;
>  }
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 06610cf..a275911 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -244,6 +244,7 @@ static void cramfs_kill_sb(struct super_block *sb)
>
>  static int cramfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index 9c0444c..02928a9 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -218,6 +218,7 @@ static int debugfs_remount(struct super_block *sb, int *flags, char *data)
>         int err;
>         struct debugfs_fs_info *fsi = sb->s_fs_info;
>
> +       sync_filesystem(sb);
>         err = debugfs_parse_options(data, &fsi->mount_opts);
>         if (err)
>                 goto fail;
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index a726b9f..c710380 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -313,6 +313,7 @@ static int devpts_remount(struct super_block *sb, int *flags, char *data)
>         struct pts_fs_info *fsi = DEVPTS_SB(sb);
>         struct pts_mount_opts *opts = &fsi->mount_opts;
>
> +       sync_filesystem(sb);
>         err = parse_mount_options(data, PARSE_REMOUNT, opts);
>
>         /*
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index 50215bb..103bbd8 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -114,6 +114,7 @@ static void destroy_inodecache(void)
>
>  static int efs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 20d6697..d260115 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1254,6 +1254,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
>         unsigned long old_sb_flags;
>         int err;
>
> +       sync_filesystem(sb);
>         spin_lock(&sbi->s_lock);
>
>         /* Store the old options */
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 37fd31e..95c6c5a 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -2649,6 +2649,8 @@ static int ext3_remount (struct super_block * sb, int * flags, char * data)
>         int i;
>  #endif
>
> +       sync_filesystem(sb);
> +
>         /* Store the original options */
>         old_sb_flags = sb->s_flags;
>         old_opts.s_mount_opt = sbi->s_mount_opt;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5c13b8..aa3842f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4767,6 +4767,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>  #endif
>         char *orig_data = kstrdup(data, GFP_KERNEL);
>
> +       sync_filesystem(sb);
> +
>         /* Store the original options */
>         old_sb_flags = sb->s_flags;
>         old_opts.s_mount_opt = sbi->s_mount_opt;
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 1a85f83..856bdf9 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -568,6 +568,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>         struct f2fs_mount_info org_mount_opt;
>         int err, active_logs;
>
> +       sync_filesystem(sb);
> +
>         /*
>          * Save the old mount options in case we
>          * need to restore them.
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index 854b578..343e477 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -635,6 +635,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
>         struct msdos_sb_info *sbi = MSDOS_SB(sb);
>         *flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
>
> +       sync_filesystem(sb);
> +
>         /* make sure we update state on remount. */
>         new_rdonly = *flags & MS_RDONLY;
>         if (new_rdonly != (sb->s_flags & MS_RDONLY)) {
> diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
> index e37eb27..7ca8c75 100644
> --- a/fs/freevxfs/vxfs_super.c
> +++ b/fs/freevxfs/vxfs_super.c
> @@ -124,6 +124,7 @@ vxfs_statfs(struct dentry *dentry, struct kstatfs *bufp)
>
>  static int vxfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index d468643..ecdb255d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -135,6 +135,7 @@ static void fuse_evict_inode(struct inode *inode)
>
>  static int fuse_remount_fs(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         if (*flags & MS_MANDLOCK)
>                 return -EINVAL;
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 60f60f6..4c6dd50 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1175,6 +1175,8 @@ static int gfs2_remount_fs(struct super_block *sb, int *flags, char *data)
>         struct gfs2_tune *gt = &sdp->sd_tune;
>         int error;
>
> +       sync_filesystem(sb);
> +
>         spin_lock(&gt->gt_spin);
>         args.ar_commit = gt->gt_logd_secs;
>         args.ar_quota_quantum = gt->gt_quota_quantum;
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index 2d2039e..eee7206 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -112,6 +112,7 @@ static int hfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>
>  static int hfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NODIRATIME;
>         if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>                 return 0;
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 80875aa..8eb787b 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -323,6 +323,7 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
>
>  static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>                 return 0;
>         if (!(*flags & MS_RDONLY)) {
> diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
> index 4534ff6..fe3463a 100644
> --- a/fs/hpfs/super.c
> +++ b/fs/hpfs/super.c
> @@ -421,6 +421,8 @@ static int hpfs_remount_fs(struct super_block *s, int *flags, char *data)
>         struct hpfs_sb_info *sbi = hpfs_sb(s);
>         char *new_opts = kstrdup(data, GFP_KERNEL);
>
> +       sync_filesystem(s);
> +
>         *flags |= MS_NOATIME;
>
>         hpfs_lock(s);
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 4a9e10e..6af66ee 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -117,6 +117,7 @@ static void destroy_inodecache(void)
>
>  static int isofs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         if (!(*flags & MS_RDONLY))
>                 return -EROFS;
>         return 0;
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 0defb1c..0918f0e 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -243,6 +243,7 @@ static int jffs2_remount_fs(struct super_block *sb, int *flags, char *data)
>         struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
>         int err;
>
> +       sync_filesystem(sb);
>         err = jffs2_parse_options(c, data);
>         if (err)
>                 return -EINVAL;
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index e2b7483..97f7fda 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -418,6 +418,7 @@ static int jfs_remount(struct super_block *sb, int *flags, char *data)
>         int flag = JFS_SBI(sb)->flag;
>         int ret;
>
> +       sync_filesystem(sb);
>         if (!parse_options(data, sb, &newLVSize, &flag)) {
>                 return -EINVAL;
>         }
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index 0332109..dcdc298 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -123,6 +123,7 @@ static int minix_remount (struct super_block * sb, int * flags, char * data)
>         struct minix_sb_info * sbi = minix_sb(sb);
>         struct minix_super_block * ms;
>
> +       sync_filesystem(sb);
>         ms = sbi->s_ms;
>         if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
>                 return 0;
> diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c
> index 2cf2ebe..5f86e80 100644
> --- a/fs/ncpfs/inode.c
> +++ b/fs/ncpfs/inode.c
> @@ -99,6 +99,7 @@ static void destroy_inodecache(void)
>
>  static int ncp_remount(struct super_block *sb, int *flags, char* data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NODIRATIME;
>         return 0;
>  }
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 910ed90..2cb5694 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2215,6 +2215,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
>         struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
>         u32 nfsvers = nfss->nfs_client->rpc_ops->version;
>
> +       sync_filesystem(sb);
> +
>         /*
>          * Userspace mount programs that send binary options generally send
>          * them populated with default values. We have no way to know which
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 7ac2a12..8c532b2 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -1129,6 +1129,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
>         unsigned long old_mount_opt;
>         int err;
>
> +       sync_filesystem(sb);
>         old_sb_flags = sb->s_flags;
>         old_mount_opt = nilfs->ns_mount_opt;
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 82650d5..bd5610d 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -468,6 +468,8 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
>
>         ntfs_debug("Entering with remount options string: %s", opt);
>
> +       sync_filesystem(sb);
> +
>  #ifndef NTFS_RW
>         /* For read-only compiled driver, enforce read-only flag. */
>         *flags |= MS_RDONLY;
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 49d84f8..5f9bf8f 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -631,6 +631,8 @@ static int ocfs2_remount(struct super_block *sb, int *flags, char *data)
>         struct ocfs2_super *osb = OCFS2_SB(sb);
>         u32 tmp;
>
> +       sync_filesystem(sb);
> +
>         if (!ocfs2_parse_options(sb, data, &parsed_options, 1) ||
>             !ocfs2_check_set_options(sb, &parsed_options)) {
>                 ret = -EINVAL;
> diff --git a/fs/openpromfs/inode.c b/fs/openpromfs/inode.c
> index 8c0ceb8..15e4500 100644
> --- a/fs/openpromfs/inode.c
> +++ b/fs/openpromfs/inode.c
> @@ -368,6 +368,7 @@ static struct inode *openprom_iget(struct super_block *sb, ino_t ino)
>
>  static int openprom_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_NOATIME;
>         return 0;
>  }
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 87dbcbe..ac823a8 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -92,6 +92,8 @@ static int proc_parse_options(char *options, struct pid_namespace *pid)
>  int proc_remount(struct super_block *sb, int *flags, char *data)
>  {
>         struct pid_namespace *pid = sb->s_fs_info;
> +
> +       sync_filesystem(sb);
>         return !proc_parse_options(data, pid);
>  }
>
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 1282384..192297b 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -249,6 +249,7 @@ static void parse_options(char *options)
>
>  static int pstore_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         parse_options(data);
>
>         return 0;
> diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
> index 8955881..c4bcb77 100644
> --- a/fs/qnx4/inode.c
> +++ b/fs/qnx4/inode.c
> @@ -44,6 +44,7 @@ static int qnx4_remount(struct super_block *sb, int *flags, char *data)
>  {
>         struct qnx4_sb_info *qs;
>
> +       sync_filesystem(sb);
>         qs = qnx4_sb(sb);
>         qs->Version = QNX4_VERSION;
>         *flags |= MS_RDONLY;
> diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
> index 8d941ed..65cdaab 100644
> --- a/fs/qnx6/inode.c
> +++ b/fs/qnx6/inode.c
> @@ -55,6 +55,7 @@ static int qnx6_show_options(struct seq_file *seq, struct dentry *root)
>
>  static int qnx6_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index 2c80335..abf2b76 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -1319,6 +1319,7 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg)
>         int i;
>  #endif
>
> +       sync_filesystem(s);
>         reiserfs_write_lock(s);
>
>  #ifdef CONFIG_QUOTA
> diff --git a/fs/romfs/super.c b/fs/romfs/super.c
> index d841878..ef90e8b 100644
> --- a/fs/romfs/super.c
> +++ b/fs/romfs/super.c
> @@ -432,6 +432,7 @@ static int romfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   */
>  static int romfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
> index 202df63..031c8d67 100644
> --- a/fs/squashfs/super.c
> +++ b/fs/squashfs/super.c
> @@ -371,6 +371,7 @@ static int squashfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>
>  static int squashfs_remount(struct super_block *sb, int *flags, char *data)
>  {
> +       sync_filesystem(sb);
>         *flags |= MS_RDONLY;
>         return 0;
>  }
> diff --git a/fs/super.c b/fs/super.c
> index 80d5cf2..e9dc3c3 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -719,8 +719,6 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
>                 }
>         }
>
> -       sync_filesystem(sb);
> -
>         if (sb->s_op->remount_fs) {
>                 retval = sb->s_op->remount_fs(sb, &flags, data);
>                 if (retval) {
> diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
> index c327d4e..4742e58 100644
> --- a/fs/sysv/inode.c
> +++ b/fs/sysv/inode.c
> @@ -60,6 +60,7 @@ static int sysv_remount(struct super_block *sb, int *flags, char *data)
>  {
>         struct sysv_sb_info *sbi = SYSV_SB(sb);
>
> +       sync_filesystem(sb);
>         if (sbi->s_forced_ro)
>                 *flags |= MS_RDONLY;
>         return 0;
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 5ded849..e1598ab 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1827,6 +1827,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
>         int err;
>         struct ubifs_info *c = sb->s_fs_info;
>
> +       sync_filesystem(sb);
>         dbg_gen("old flags %#lx, new flags %#x", sb->s_flags, *flags);
>
>         err = ubifs_parse_options(c, data, 1);
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 3306b9f..64f2b73 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -646,6 +646,7 @@ static int udf_remount_fs(struct super_block *sb, int *flags, char *options)
>         int error = 0;
>         struct logicalVolIntegrityDescImpUse *lvidiu = udf_sb_lvidiu(sb);
>
> +       sync_filesystem(sb);
>         if (lvidiu) {
>                 int write_rev = le16_to_cpu(lvidiu->minUDFWriteRev);
>                 if (write_rev > UDF_MAX_WRITE_VERSION && !(*flags & MS_RDONLY))
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 329f2f5..b8c6791 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -1280,6 +1280,7 @@ static int ufs_remount (struct super_block *sb, int *mount_flags, char *data)
>         unsigned new_mount_opt, ufstype;
>         unsigned flags;
>
> +       sync_filesystem(sb);
>         lock_ufs(sb);
>         mutex_lock(&UFS_SB(sb)->s_lock);
>         uspi = UFS_SB(sb)->s_uspi;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f317488..aaa3eca 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1197,6 +1197,7 @@ xfs_fs_remount(
>         char                    *p;
>         int                     error;
>
> +       sync_filesystem(sb);
>         while ((p = strsep(&options, ",")) != NULL) {
>                 int token;
>
> --
> 1.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs()
  2014-03-14  0:33   ` Steve French
@ 2014-03-14  1:23     ` Theodore Ts'o
  0 siblings, 0 replies; 7+ messages in thread
From: Theodore Ts'o @ 2014-03-14  1:23 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu, Mar 13, 2014 at 07:33:02PM -0500, Steve French wrote:
> On Thu, Mar 13, 2014 at 9:20 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> > Previously, the no-op "mount -o mount /dev/xxx" operation when the
> > file system is already mounted read-write causes an implied,
> > unconditional syncfs().  This seems pretty stupid, and it's certainly
> > documented or guaraunteed to do this, nor is it particularly useful,
> > except in the case where the file system was mounted rw and is getting
> > remounted read-only.
> 
> Is there a case where a file system, not mounted read-only,
> would want to skip the syncfs on remount?  I don't know
> of any particular reason to do a syncfs on remount unless
> caching behavior is changing (or moving to read-only mount),
> but if as you say it is documented and guaranteed...

If the file system is mounted read-write, and it is transitioning to
read-only, i.e. "mount -o remount,ro /" then you do want to write out
all dirty data before you transition it to be read-only (otherwise you
would lose data).

It is my belief that this is the _only_ data integrity issue which is
implied by remount (and this is more about not losing work done by
previous system calls).

The background reason for this commit is that a user complained on the
ext4 list that he is using containers in a virtualization environment,
and due to the init scripts which the user doesn't want to change, it
is causing gazillions of no-op remounts, and this is causing ext4 (and
xfs) to do send CACHE FLUSH commands because it is required by the
syncfs(2) system call, which also calls sync_filesystem.  These CACHE
FLUSH commands are unneeded for anything, especially for these no-op
remounts, and so I want them to go away for remounts, but they should
still be there in response to syncfs(2) requests.

Cheers,

					- Ted

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-03-14 12:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20140313073936.GA14663@infradead.org>
2014-03-13 14:20 ` [Cluster-devel] [PATCH] fs: push sync_filesystem() down to the file system's remount_fs() Theodore Ts'o
2014-03-13 16:23   ` Jan Kara
2014-03-13 16:28     ` Steven Whitehouse
2014-03-13 23:15       ` Theodore Ts'o
2014-03-14 12:13         ` Jan Kara
2014-03-14  0:33   ` Steve French
2014-03-14  1:23     ` Theodore Ts'o

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