[PATCH] jfs: serialize remount option snapshots

Cen Zhang posted 1 patch 1 month, 1 week ago
fs/jfs/jfs_incore.h |  1 +
fs/jfs/super.c      | 53 +++++++++++++++++++++++++++++----------------
2 files changed, 35 insertions(+), 19 deletions(-)
[PATCH] jfs: serialize remount option snapshots
Posted by Cen Zhang 1 month, 1 week ago
JFS initializes a reconfigure fs_context by copying the current
superblock mount-option state before the VFS takes sb->s_umount for the
actual reconfigure operation.  Another remount can therefore update the
JFS option fields in jfs_reconfigure() while jfs_init_options() is
copying them.

The affected fields are copied as a group so that unspecified remount
options inherit the current state.  Reading them concurrently with the
publisher can create a mixed snapshot that never existed as a complete
JFS option state, for example combining a discard flag from one remount
with a minblks_trim value from another.

Add a small JFS-private mutex and use it only around the remount option
snapshot and publication of the scalar option fields.  The existing VFS
superblock lock continues to serialize the reconfigure operation itself;
this mutex covers the earlier context-initialization gap without taking
sb->s_umount recursively.

Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/jfs/jfs_incore.h |  1 +
 fs/jfs/super.c      | 53 +++++++++++++++++++++++++++++----------------
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/fs/jfs/jfs_incore.h b/fs/jfs/jfs_incore.h
index 5aaafed..b222468 100644
--- a/fs/jfs/jfs_incore.h
+++ b/fs/jfs/jfs_incore.h
@@ -189,6 +189,7 @@ struct jfs_sb_info {
 	/* Formerly in ipbmap */
 	struct bmap	*bmap;		/* incore bmap descriptor	*/
 	struct nls_table *nls_tab;	/* current codepage		*/
+	struct mutex	remount_mutex;	/* serializes scalar option copy/update */
 	struct inode *direct_inode;	/* metadata inode */
 	uint		state;		/* mount/recovery state	*/
 	unsigned long	flag;		/* mount time flags */
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 61575f7..2e4177f 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -247,6 +247,13 @@ struct jfs_context {
 	s64	newLVSize;
 };
 
+static void jfs_update_remount_flag(struct jfs_sb_info *sbi, int flag)
+{
+	mutex_lock(&sbi->remount_mutex);
+	sbi->flag = flag;
+	mutex_unlock(&sbi->remount_mutex);
+}
+
 static int jfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
 	struct jfs_context *ctx = fc->fs_private;
@@ -362,6 +369,7 @@ static int jfs_reconfigure(struct fs_context *fc)
 {
 	struct jfs_context *ctx = fc->fs_private;
 	struct super_block *sb = fc->root->d_sb;
+	struct jfs_sb_info *sbi = JFS_SBI(sb);
 	int readonly = fc->sb_flags & SB_RDONLY;
 	int rc = 0;
 	int flag = ctx->flag;
@@ -369,15 +377,17 @@ static int jfs_reconfigure(struct fs_context *fc)
 
 	sync_filesystem(sb);
 
-	/* Transfer results of parsing to the sbi */
-	JFS_SBI(sb)->flag = ctx->flag;
-	JFS_SBI(sb)->uid = ctx->uid;
-	JFS_SBI(sb)->gid = ctx->gid;
-	JFS_SBI(sb)->umask = ctx->umask;
-	JFS_SBI(sb)->minblks_trim = ctx->minblks_trim;
+	/* Transfer results of parsing to the sbi. */
+	mutex_lock(&sbi->remount_mutex);
+	sbi->flag = ctx->flag;
+	sbi->uid = ctx->uid;
+	sbi->gid = ctx->gid;
+	sbi->umask = ctx->umask;
+	sbi->minblks_trim = ctx->minblks_trim;
+	mutex_unlock(&sbi->remount_mutex);
 	if (ctx->nls_map != (void *) -1) {
-		unload_nls(JFS_SBI(sb)->nls_tab);
-		JFS_SBI(sb)->nls_tab = ctx->nls_map;
+		unload_nls(sbi->nls_tab);
+		sbi->nls_tab = ctx->nls_map;
 	}
 	ctx->nls_map = NULL;
 
@@ -403,9 +413,9 @@ static int jfs_reconfigure(struct fs_context *fc)
 		 * Invalidate any previously read metadata.  fsck may have
 		 * changed the on-disk data since we mounted r/o
 		 */
-		truncate_inode_pages(JFS_SBI(sb)->direct_inode->i_mapping, 0);
+		truncate_inode_pages(sbi->direct_inode->i_mapping, 0);
 
-		JFS_SBI(sb)->flag = flag;
+		jfs_update_remount_flag(sbi, flag);
 		ret = jfs_mount_rw(sb, 1);
 
 		/* mark the fs r/w for quota activity */
@@ -419,21 +429,21 @@ static int jfs_reconfigure(struct fs_context *fc)
 		if (rc < 0)
 			return rc;
 		rc = jfs_umount_rw(sb);
-		JFS_SBI(sb)->flag = flag;
+		jfs_update_remount_flag(sbi, flag);
 		return rc;
 	}
-	if ((JFS_SBI(sb)->flag & JFS_NOINTEGRITY) != (flag & JFS_NOINTEGRITY)) {
+	if ((sbi->flag & JFS_NOINTEGRITY) != (flag & JFS_NOINTEGRITY)) {
 		if (!sb_rdonly(sb)) {
 			rc = jfs_umount_rw(sb);
 			if (rc)
 				return rc;
 
-			JFS_SBI(sb)->flag = flag;
+			jfs_update_remount_flag(sbi, flag);
 			ret = jfs_mount_rw(sb, 1);
 			return ret;
 		}
 	}
-	JFS_SBI(sb)->flag = flag;
+	jfs_update_remount_flag(sbi, flag);
 
 	return 0;
 }
@@ -453,6 +463,8 @@ static int jfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (!sbi)
 		return -ENOMEM;
 
+	mutex_init(&sbi->remount_mutex);
+
 	sb->s_fs_info = sbi;
 	sb->s_max_links = JFS_LINK_MAX;
 	sb->s_time_min = 0;
@@ -870,14 +882,17 @@ static void jfs_init_options(struct fs_context *fc, struct jfs_context *ctx)
 {
 	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
 		struct super_block *sb = fc->root->d_sb;
+		struct jfs_sb_info *sbi = JFS_SBI(sb);
 
 		/* Copy over current option values and mount flags */
-		ctx->uid = JFS_SBI(sb)->uid;
-		ctx->gid = JFS_SBI(sb)->gid;
-		ctx->umask = JFS_SBI(sb)->umask;
+		mutex_lock(&sbi->remount_mutex);
+		ctx->uid = sbi->uid;
+		ctx->gid = sbi->gid;
+		ctx->umask = sbi->umask;
+		ctx->minblks_trim = sbi->minblks_trim;
+		ctx->flag = sbi->flag;
+		mutex_unlock(&sbi->remount_mutex);
 		ctx->nls_map = (void *)-1;
-		ctx->minblks_trim = JFS_SBI(sb)->minblks_trim;
-		ctx->flag = JFS_SBI(sb)->flag;
 
 	} else {
 		/*