mbox series

[V5,0/9] Enable ext4 support for per-file/directory DAX operations

Message ID 20200528150003.828793-1-ira.weiny@intel.com
Headers show
Series Enable ext4 support for per-file/directory DAX operations | expand

Message

Ira Weiny May 28, 2020, 2:59 p.m. UTC
From: Ira Weiny <ira.weiny@intel.com>

Changes from V4:
	Fix up DAX mutual exclusion with other flags.
	Add clean up patch (remove jflags)

Changes from V3:
	Change EXT4_DAX_FL to bit24
	Cache device DAX support in the super block and use that is
		ext4_should_use_dax()

Changes from V2:
	Rework DAX exclusivity with verity and encryption based on feedback
	from Eric

Enable the same per file DAX support in ext4 as was done for xfs.  This series
builds and depends on the V11 series for xfs.[1]

This passes the same xfstests test as XFS.

The only issue is that this modifies the old mount option parsing code rather
than waiting for the new parsing code to be finalized.

This series starts with 3 fixes which include making Verity and Encrypt truly
mutually exclusive from DAX.  I think these first 3 patches should be picked up
for 5.8 regardless of what is decided regarding the mount parsing.

[1] https://lore.kernel.org/lkml/20200428002142.404144-1-ira.weiny@intel.com/

To: linux-ext4@vger.kernel.org
To: Andreas Dilger <adilger.kernel@dilger.ca>
To: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
To: Eric Biggers <ebiggers@kernel.org>

Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Ira Weiny (9):
  fs/ext4: Narrow scope of DAX check in setflags
  fs/ext4: Disallow verity if inode is DAX
  fs/ext4: Change EXT4_MOUNT_DAX to EXT4_MOUNT_DAX_ALWAYS
  fs/ext4: Update ext4_should_use_dax()
  fs/ext4: Only change S_DAX on inode load
  fs/ext4: Make DAX mount option a tri-state
  fs/ext4: Remove jflag variable
  fs/ext4: Introduce DAX inode flag
  Documentation/dax: Update DAX enablement for ext4

 Documentation/filesystems/dax.txt         |  6 +-
 Documentation/filesystems/ext4/verity.rst |  3 +
 fs/ext4/ext4.h                            | 27 +++++--
 fs/ext4/ialloc.c                          |  2 +-
 fs/ext4/inode.c                           | 26 +++++--
 fs/ext4/ioctl.c                           | 65 ++++++++++++++---
 fs/ext4/super.c                           | 85 ++++++++++++++++++-----
 fs/ext4/verity.c                          |  5 +-
 include/uapi/linux/fs.h                   |  1 +
 9 files changed, 175 insertions(+), 45 deletions(-)

Comments

Theodore Ts'o May 29, 2020, 2:54 a.m. UTC | #1
On Thu, May 28, 2020 at 07:59:54AM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Changes from V4:
> 	Fix up DAX mutual exclusion with other flags.
> 	Add clean up patch (remove jflags)
> 
> Changes from V3:
> 	Change EXT4_DAX_FL to bit24
> 	Cache device DAX support in the super block and use that is
> 		ext4_should_use_dax()
> 
> Changes from V2:
> 	Rework DAX exclusivity with verity and encryption based on feedback
> 	from Eric
> 
> Enable the same per file DAX support in ext4 as was done for xfs.  This series
> builds and depends on the V11 series for xfs.[1]
> 
> This passes the same xfstests test as XFS.
> 
> The only issue is that this modifies the old mount option parsing code rather
> than waiting for the new parsing code to be finalized.
> 
> This series starts with 3 fixes which include making Verity and Encrypt truly
> mutually exclusive from DAX.  I think these first 3 patches should be picked up
> for 5.8 regardless of what is decided regarding the mount parsing.
> 
> [1] https://lore.kernel.org/lkml/20200428002142.404144-1-ira.weiny@intel.com/
> 
> To: linux-ext4@vger.kernel.org
> To: Andreas Dilger <adilger.kernel@dilger.ca>
> To: "Theodore Y. Ts'o" <tytso@mit.edu>
> To: Jan Kara <jack@suse.cz>
> To: Eric Biggers <ebiggers@kernel.org>

Thanks, applied to the ext4-dax branch.

						- Ted
Theodore Ts'o May 29, 2020, 4:17 a.m. UTC | #2
On Thu, May 28, 2020 at 10:54:41PM -0400, Theodore Y. Ts'o wrote:
> 
> Thanks, applied to the ext4-dax branch.
> 

I spoke too soon.  While I tried merging with the ext4.git dev branch,
a merge conflict made me look closer and I realize I needed to make
the following changes (see diff between your patch set and what is
currently in ext4-dax).

Essentially, I needed to rework the branch to take into account commit
e0198aff3ae3 ("ext4: reject mount options not supported when
remounting in handle_mount_opt()").

The problem is that if you allow handle_mount_opt() to apply the
changes to the dax settings, and then later on, ext4_remount() realize
that we're remounting, and we need to reject the change, there's a
race if we restore the mount options to the original configuration.
Specifically, as Syzkaller pointed out, between when we change the dax
settings and then reset them, it's possible for some file to be opened
with "wrong" dax setting, and then when they are reset, *boom*.

The correct way to deal with this is to reject the mount option change
much earlier, in handle_mount_opt(), *before* we mess with the dax
settings.

Please take a look at the ext4-dax for the actual changes which I
made.

Cheers,

					- Ted


diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3658e3016999..9a37d70394b2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1733,7 +1733,7 @@ static int clear_qf_name(struct super_block *sb, int qtype)
 #define MOPT_NO_EXT3	0x0200
 #define MOPT_EXT4_ONLY	(MOPT_NO_EXT2 | MOPT_NO_EXT3)
 #define MOPT_STRING	0x0400
-#define MOPT_SKIP	0x0800
+#define MOPT_NO_REMOUNT	0x0800
 
 static const struct mount_opts {
 	int	token;
@@ -1783,18 +1783,15 @@ static const struct mount_opts {
 	{Opt_min_batch_time, 0, MOPT_GTE0},
 	{Opt_inode_readahead_blks, 0, MOPT_GTE0},
 	{Opt_init_itable, 0, MOPT_GTE0},
-	{Opt_dax, EXT4_MOUNT_DAX_ALWAYS, MOPT_SET | MOPT_SKIP},
-	{Opt_dax_always, EXT4_MOUNT_DAX_ALWAYS,
-		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
-	{Opt_dax_inode, EXT4_MOUNT2_DAX_INODE,
-		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
-	{Opt_dax_never, EXT4_MOUNT2_DAX_NEVER,
-		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
+	{Opt_dax, 0, MOPT_NO_REMOUNT},
+	{Opt_dax_always, 0, MOPT_NO_REMOUNT},
+	{Opt_dax_inode, 0, MOPT_NO_REMOUNT},
+	{Opt_dax_never, 0, MOPT_NO_REMOUNT},
 	{Opt_stripe, 0, MOPT_GTE0},
 	{Opt_resuid, 0, MOPT_GTE0},
 	{Opt_resgid, 0, MOPT_GTE0},
-	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0},
-	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING},
+	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0 | MOPT_NO_REMOUNT},
+	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING | MOPT_NO_REMOUNT},
 	{Opt_journal_ioprio, 0, MOPT_NO_EXT2 | MOPT_GTE0},
 	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
 	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
@@ -1831,7 +1828,7 @@ static const struct mount_opts {
 	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
 	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
 	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
-	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
+	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET | MOPT_NO_REMOUNT},
 	{Opt_err, 0, 0}
 };
 
@@ -1929,6 +1926,12 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 			 "Mount option \"%s\" incompatible with ext3", opt);
 		return -1;
 	}
+	if ((m->flags & MOPT_NO_REMOUNT) && is_remount) {
+		ext4_msg(sb, KERN_ERR,
+			 "Mount option \"%s\" not supported when remounting",
+			 opt);
+		return -1;
+	}
 
 	if (args->from && !(m->flags & MOPT_STRING) && match_int(args, &arg))
 		return -1;
@@ -2008,11 +2011,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		}
 		sbi->s_resgid = gid;
 	} else if (token == Opt_journal_dev) {
-		if (is_remount) {
-			ext4_msg(sb, KERN_ERR,
-				 "Cannot specify journal on remount");
-			return -1;
-		}
 		*journal_devnum = arg;
 	} else if (token == Opt_journal_path) {
 		char *journal_path;
@@ -2020,11 +2018,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		struct path path;
 		int error;
 
-		if (is_remount) {
-			ext4_msg(sb, KERN_ERR,
-				 "Cannot specify journal on remount");
-			return -1;
-		}
 		journal_path = match_strdup(&args[0]);
 		if (!journal_path) {
 			ext4_msg(sb, KERN_ERR, "error: could not dup "
@@ -2287,7 +2280,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
 	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
 		int want_set = m->flags & MOPT_SET;
 		if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
-		    (m->flags & MOPT_CLEAR_ERR) || m->flags & MOPT_SKIP)
+		    (m->flags & MOPT_CLEAR_ERR))
 			continue;
 		if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
 			continue; /* skip if same as the default */
@@ -5474,24 +5467,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		}
 	}
 
-	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_NO_MBCACHE) {
-		ext4_msg(sb, KERN_ERR, "can't enable nombcache during remount");
-		err = -EINVAL;
-		goto restore_opts;
-	}
-
-	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX_ALWAYS ||
-	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_NEVER ||
-	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_INODE) {
-		ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
-			"dax mount option with busy inodes while remounting");
-		sbi->s_mount_opt &= ~EXT4_MOUNT_DAX_ALWAYS;
-		sbi->s_mount_opt |= old_opts.s_mount_opt & EXT4_MOUNT_DAX_ALWAYS;
-		sbi->s_mount_opt2 &= ~(EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
-		sbi->s_mount_opt2 |= old_opts.s_mount_opt2 &
-				     (EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
-	}
-
 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
 		ext4_abort(sb, EXT4_ERR_ESHUTDOWN, "Abort forced by user");
Ira Weiny May 29, 2020, 6:12 p.m. UTC | #3
On Fri, May 29, 2020 at 12:17:17AM -0400, Theodore Y. Ts'o wrote:
> On Thu, May 28, 2020 at 10:54:41PM -0400, Theodore Y. Ts'o wrote:
> > 
> > Thanks, applied to the ext4-dax branch.
> > 
> 
> I spoke too soon.  While I tried merging with the ext4.git dev branch,
> a merge conflict made me look closer and I realize I needed to make
> the following changes (see diff between your patch set and what is
> currently in ext4-dax).
> 
> Essentially, I needed to rework the branch to take into account commit
> e0198aff3ae3 ("ext4: reject mount options not supported when
> remounting in handle_mount_opt()").
> 
> The problem is that if you allow handle_mount_opt() to apply the
> changes to the dax settings, and then later on, ext4_remount() realize
> that we're remounting, and we need to reject the change, there's a
> race if we restore the mount options to the original configuration.
> Specifically, as Syzkaller pointed out, between when we change the dax
> settings and then reset them, it's possible for some file to be opened
> with "wrong" dax setting, and then when they are reset, *boom*.
> 
> The correct way to deal with this is to reject the mount option change
> much earlier, in handle_mount_opt(), *before* we mess with the dax
> settings.
> 
> Please take a look at the ext4-dax for the actual changes which I
> made.
> 
> Cheers,
> 
> 					- Ted
> 
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 3658e3016999..9a37d70394b2 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1733,7 +1733,7 @@ static int clear_qf_name(struct super_block *sb, int qtype)
>  #define MOPT_NO_EXT3	0x0200
>  #define MOPT_EXT4_ONLY	(MOPT_NO_EXT2 | MOPT_NO_EXT3)
>  #define MOPT_STRING	0x0400
> -#define MOPT_SKIP	0x0800

I think we still need MOPT_SKIP...

This was put in to skip these options when printing to deal with printing only
dax=inode when it was specified by the user.

Ah but I see now.  By taking MOPT_SET away you have created the same behavior?

This is  orthogonal to the remount issue right?

> +#define MOPT_NO_REMOUNT	0x0800
>  
>  static const struct mount_opts {
>  	int	token;
> @@ -1783,18 +1783,15 @@ static const struct mount_opts {
>  	{Opt_min_batch_time, 0, MOPT_GTE0},
>  	{Opt_inode_readahead_blks, 0, MOPT_GTE0},
>  	{Opt_init_itable, 0, MOPT_GTE0},
> -	{Opt_dax, EXT4_MOUNT_DAX_ALWAYS, MOPT_SET | MOPT_SKIP},
> -	{Opt_dax_always, EXT4_MOUNT_DAX_ALWAYS,
> -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> -	{Opt_dax_inode, EXT4_MOUNT2_DAX_INODE,
> -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> -	{Opt_dax_never, EXT4_MOUNT2_DAX_NEVER,
> -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> +	{Opt_dax, 0, MOPT_NO_REMOUNT},
> +	{Opt_dax_always, 0, MOPT_NO_REMOUNT},
> +	{Opt_dax_inode, 0, MOPT_NO_REMOUNT},
> +	{Opt_dax_never, 0, MOPT_NO_REMOUNT},

Even if MOPT_SET is redundant.  Why don't we need still need MOPT_EXT4_ONLY?

And why don't we need to associate the defines; EXT4_MOUNT_DAX_ALWAYS etc?

>  	{Opt_stripe, 0, MOPT_GTE0},
>  	{Opt_resuid, 0, MOPT_GTE0},
>  	{Opt_resgid, 0, MOPT_GTE0},
> -	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0},
> -	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING},
> +	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0 | MOPT_NO_REMOUNT},
> +	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING | MOPT_NO_REMOUNT},
>  	{Opt_journal_ioprio, 0, MOPT_NO_EXT2 | MOPT_GTE0},
>  	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
>  	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
> @@ -1831,7 +1828,7 @@ static const struct mount_opts {
>  	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
>  	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
>  	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
> -	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
> +	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET | MOPT_NO_REMOUNT},
>  	{Opt_err, 0, 0}
>  };
>  
> @@ -1929,6 +1926,12 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
>  			 "Mount option \"%s\" incompatible with ext3", opt);
>  		return -1;
>  	}
> +	if ((m->flags & MOPT_NO_REMOUNT) && is_remount) {
> +		ext4_msg(sb, KERN_ERR,
> +			 "Mount option \"%s\" not supported when remounting",
> +			 opt);
> +		return -1;
> +	}

I think this is cleaner!

Thanks, I did test this but not while trying to manipulate files as the same time
as a remount.  So a race would not have been caught.

Thanks!
Ira

>  
>  	if (args->from && !(m->flags & MOPT_STRING) && match_int(args, &arg))
>  		return -1;
> @@ -2008,11 +2011,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
>  		}
>  		sbi->s_resgid = gid;
>  	} else if (token == Opt_journal_dev) {
> -		if (is_remount) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "Cannot specify journal on remount");
> -			return -1;
> -		}
>  		*journal_devnum = arg;
>  	} else if (token == Opt_journal_path) {
>  		char *journal_path;
> @@ -2020,11 +2018,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
>  		struct path path;
>  		int error;
>  
> -		if (is_remount) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "Cannot specify journal on remount");
> -			return -1;
> -		}
>  		journal_path = match_strdup(&args[0]);
>  		if (!journal_path) {
>  			ext4_msg(sb, KERN_ERR, "error: could not dup "
> @@ -2287,7 +2280,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>  	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
>  		int want_set = m->flags & MOPT_SET;
>  		if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
> -		    (m->flags & MOPT_CLEAR_ERR) || m->flags & MOPT_SKIP)
> +		    (m->flags & MOPT_CLEAR_ERR))
>  			continue;
>  		if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
>  			continue; /* skip if same as the default */
> @@ -5474,24 +5467,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>  		}
>  	}
>  
> -	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_NO_MBCACHE) {
> -		ext4_msg(sb, KERN_ERR, "can't enable nombcache during remount");
> -		err = -EINVAL;
> -		goto restore_opts;
> -	}
> -
> -	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX_ALWAYS ||
> -	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_NEVER ||
> -	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_INODE) {
> -		ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
> -			"dax mount option with busy inodes while remounting");
> -		sbi->s_mount_opt &= ~EXT4_MOUNT_DAX_ALWAYS;
> -		sbi->s_mount_opt |= old_opts.s_mount_opt & EXT4_MOUNT_DAX_ALWAYS;
> -		sbi->s_mount_opt2 &= ~(EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
> -		sbi->s_mount_opt2 |= old_opts.s_mount_opt2 &
> -				     (EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
> -	}
> -
>  	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
>  		ext4_abort(sb, EXT4_ERR_ESHUTDOWN, "Abort forced by user");
>
Ira Weiny June 2, 2020, 3:58 p.m. UTC | #4
Ted,

Sorry for the top post but did you catch this reply?  Generally the patch looks
good but I had a couple of questions because I don't fully grok the mount code
especially with regard to EXT2 support.

If you already saw it sorry for bothering you I just know that our email
servers sometimes 'file' things for me and I miss them...  ;-)

Thanks,
Ira

On Fri, May 29, 2020 at 11:12:38AM -0700, 'Ira Weiny' wrote:
> On Fri, May 29, 2020 at 12:17:17AM -0400, Theodore Y. Ts'o wrote:
> > On Thu, May 28, 2020 at 10:54:41PM -0400, Theodore Y. Ts'o wrote:
> > > 
> > > Thanks, applied to the ext4-dax branch.
> > > 
> > 
> > I spoke too soon.  While I tried merging with the ext4.git dev branch,
> > a merge conflict made me look closer and I realize I needed to make
> > the following changes (see diff between your patch set and what is
> > currently in ext4-dax).
> > 
> > Essentially, I needed to rework the branch to take into account commit
> > e0198aff3ae3 ("ext4: reject mount options not supported when
> > remounting in handle_mount_opt()").
> > 
> > The problem is that if you allow handle_mount_opt() to apply the
> > changes to the dax settings, and then later on, ext4_remount() realize
> > that we're remounting, and we need to reject the change, there's a
> > race if we restore the mount options to the original configuration.
> > Specifically, as Syzkaller pointed out, between when we change the dax
> > settings and then reset them, it's possible for some file to be opened
> > with "wrong" dax setting, and then when they are reset, *boom*.
> > 
> > The correct way to deal with this is to reject the mount option change
> > much earlier, in handle_mount_opt(), *before* we mess with the dax
> > settings.
> > 
> > Please take a look at the ext4-dax for the actual changes which I
> > made.
> > 
> > Cheers,
> > 
> > 					- Ted
> > 
> > 
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 3658e3016999..9a37d70394b2 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -1733,7 +1733,7 @@ static int clear_qf_name(struct super_block *sb, int qtype)
> >  #define MOPT_NO_EXT3	0x0200
> >  #define MOPT_EXT4_ONLY	(MOPT_NO_EXT2 | MOPT_NO_EXT3)
> >  #define MOPT_STRING	0x0400
> > -#define MOPT_SKIP	0x0800
> 
> I think we still need MOPT_SKIP...
> 
> This was put in to skip these options when printing to deal with printing only
> dax=inode when it was specified by the user.
> 
> Ah but I see now.  By taking MOPT_SET away you have created the same behavior?
> 
> This is  orthogonal to the remount issue right?
> 
> > +#define MOPT_NO_REMOUNT	0x0800
> >  
> >  static const struct mount_opts {
> >  	int	token;
> > @@ -1783,18 +1783,15 @@ static const struct mount_opts {
> >  	{Opt_min_batch_time, 0, MOPT_GTE0},
> >  	{Opt_inode_readahead_blks, 0, MOPT_GTE0},
> >  	{Opt_init_itable, 0, MOPT_GTE0},
> > -	{Opt_dax, EXT4_MOUNT_DAX_ALWAYS, MOPT_SET | MOPT_SKIP},
> > -	{Opt_dax_always, EXT4_MOUNT_DAX_ALWAYS,
> > -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> > -	{Opt_dax_inode, EXT4_MOUNT2_DAX_INODE,
> > -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> > -	{Opt_dax_never, EXT4_MOUNT2_DAX_NEVER,
> > -		MOPT_EXT4_ONLY | MOPT_SET | MOPT_SKIP},
> > +	{Opt_dax, 0, MOPT_NO_REMOUNT},
> > +	{Opt_dax_always, 0, MOPT_NO_REMOUNT},
> > +	{Opt_dax_inode, 0, MOPT_NO_REMOUNT},
> > +	{Opt_dax_never, 0, MOPT_NO_REMOUNT},
> 
> Even if MOPT_SET is redundant.  Why don't we need still need MOPT_EXT4_ONLY?
> 
> And why don't we need to associate the defines; EXT4_MOUNT_DAX_ALWAYS etc?
> 
> >  	{Opt_stripe, 0, MOPT_GTE0},
> >  	{Opt_resuid, 0, MOPT_GTE0},
> >  	{Opt_resgid, 0, MOPT_GTE0},
> > -	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0},
> > -	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING},
> > +	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0 | MOPT_NO_REMOUNT},
> > +	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING | MOPT_NO_REMOUNT},
> >  	{Opt_journal_ioprio, 0, MOPT_NO_EXT2 | MOPT_GTE0},
> >  	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
> >  	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
> > @@ -1831,7 +1828,7 @@ static const struct mount_opts {
> >  	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
> >  	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
> >  	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
> > -	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
> > +	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET | MOPT_NO_REMOUNT},
> >  	{Opt_err, 0, 0}
> >  };
> >  
> > @@ -1929,6 +1926,12 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> >  			 "Mount option \"%s\" incompatible with ext3", opt);
> >  		return -1;
> >  	}
> > +	if ((m->flags & MOPT_NO_REMOUNT) && is_remount) {
> > +		ext4_msg(sb, KERN_ERR,
> > +			 "Mount option \"%s\" not supported when remounting",
> > +			 opt);
> > +		return -1;
> > +	}
> 
> I think this is cleaner!
> 
> Thanks, I did test this but not while trying to manipulate files as the same time
> as a remount.  So a race would not have been caught.
> 
> Thanks!
> Ira
> 
> >  
> >  	if (args->from && !(m->flags & MOPT_STRING) && match_int(args, &arg))
> >  		return -1;
> > @@ -2008,11 +2011,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> >  		}
> >  		sbi->s_resgid = gid;
> >  	} else if (token == Opt_journal_dev) {
> > -		if (is_remount) {
> > -			ext4_msg(sb, KERN_ERR,
> > -				 "Cannot specify journal on remount");
> > -			return -1;
> > -		}
> >  		*journal_devnum = arg;
> >  	} else if (token == Opt_journal_path) {
> >  		char *journal_path;
> > @@ -2020,11 +2018,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> >  		struct path path;
> >  		int error;
> >  
> > -		if (is_remount) {
> > -			ext4_msg(sb, KERN_ERR,
> > -				 "Cannot specify journal on remount");
> > -			return -1;
> > -		}
> >  		journal_path = match_strdup(&args[0]);
> >  		if (!journal_path) {
> >  			ext4_msg(sb, KERN_ERR, "error: could not dup "
> > @@ -2287,7 +2280,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
> >  	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
> >  		int want_set = m->flags & MOPT_SET;
> >  		if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
> > -		    (m->flags & MOPT_CLEAR_ERR) || m->flags & MOPT_SKIP)
> > +		    (m->flags & MOPT_CLEAR_ERR))
> >  			continue;
> >  		if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
> >  			continue; /* skip if same as the default */
> > @@ -5474,24 +5467,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> >  		}
> >  	}
> >  
> > -	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_NO_MBCACHE) {
> > -		ext4_msg(sb, KERN_ERR, "can't enable nombcache during remount");
> > -		err = -EINVAL;
> > -		goto restore_opts;
> > -	}
> > -
> > -	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX_ALWAYS ||
> > -	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_NEVER ||
> > -	    (sbi->s_mount_opt2 ^ old_opts.s_mount_opt2) & EXT4_MOUNT2_DAX_INODE) {
> > -		ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
> > -			"dax mount option with busy inodes while remounting");
> > -		sbi->s_mount_opt &= ~EXT4_MOUNT_DAX_ALWAYS;
> > -		sbi->s_mount_opt |= old_opts.s_mount_opt & EXT4_MOUNT_DAX_ALWAYS;
> > -		sbi->s_mount_opt2 &= ~(EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
> > -		sbi->s_mount_opt2 |= old_opts.s_mount_opt2 &
> > -				     (EXT4_MOUNT2_DAX_NEVER | EXT4_MOUNT2_DAX_INODE);
> > -	}
> > -
> >  	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
> >  		ext4_abort(sb, EXT4_ERR_ESHUTDOWN, "Abort forced by user");
> >