[PATCH] ext4: handle option set by mount flags correctly

[PATCH] ext4: handle option set by mount flags correctly

[Lukas Czerner]

Currently there is a problem with mount options that can be both set by
vfs using mount flags or by a string parsing in ext4.

i_version/iversion options gets lost after remount, for example

$ mount -o i_version /dev/pmem0 /mnt
$ grep pmem0 /proc/self/mountinfo | grep i_version
310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,seclabel,i_version
$ mount -o remount,ro /mnt
$ grep pmem0 /proc/self/mountinfo | grep i_version

nolazytime gets ignored by ext4 on remount, for example

$ mount -o lazytime /dev/pmem0 /mnt
$ grep pmem0 /proc/self/mountinfo | grep lazytime
310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel
$ mount -o remount,nolazytime /mnt
$ grep pmem0 /proc/self/mountinfo | grep lazytime
310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel

Fix it by applying the SB_LAZYTIME and SB_I_VERSION flags from *flags to
s_flags before we parse the option and use the resulting state of the
same flags in *flags at the end of successful remount.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/super.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 330957ed1f05..caab4c57f909 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5445,7 +5445,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct ext4_super_block *es;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	unsigned long old_sb_flags;
+	unsigned long old_sb_flags, vfs_flags;
 	struct ext4_mount_options old_opts;
 	int enable_quota = 0;
 	ext4_group_t g;
@@ -5488,6 +5488,14 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	if (sbi->s_journal && sbi->s_journal->j_task->io_context)
 		journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
 
+	/*
+	 * Some options can be enabled by ext4 and/or by VFS mount flag
+	 * either way we need to make sure it matches in both *flags and
+	 * s_flags. Copy those selected flags from *flags to s_flags
+	 */
+	vfs_flags = SB_LAZYTIME | SB_I_VERSION;
+	sb->s_flags = (sb->s_flags & ~vfs_flags) | (*flags & vfs_flags);
+
 	if (!parse_options(data, sb, NULL, &journal_ioprio, 1)) {
 		err = -EINVAL;
 		goto restore_opts;
@@ -5541,9 +5549,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		set_task_ioprio(sbi->s_journal->j_task, journal_ioprio);
 	}
 
-	if (*flags & SB_LAZYTIME)
-		sb->s_flags |= SB_LAZYTIME;
-
 	if ((bool)(*flags & SB_RDONLY) != sb_rdonly(sb)) {
 		if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED) {
 			err = -EROFS;
@@ -5675,7 +5680,13 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	}
 #endif
 
-	*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
+	/*
+	 * Some options can be enabled by ext4 and/or by VFS mount flag
+	 * either way we need to make sure it matches in both *flags and
+	 * s_flags. Copy those selected flags from s_flags to *flags
+	 */
+	*flags = (*flags & ~vfs_flags) | (sb->s_flags & vfs_flags);
+
 	ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data);
 	kfree(orig_data);
 	return 0;
-- 
2.21.3

Re: [PATCH] ext4: handle option set by mount flags correctly

[Ritesh Harjani]

On 7/23/20 8:35 PM, Lukas Czerner wrote:
> Currently there is a problem with mount options that can be both set by
> vfs using mount flags or by a string parsing in ext4.
> 
> i_version/iversion options gets lost after remount, for example
> 
> $ mount -o i_version /dev/pmem0 /mnt
> $ grep pmem0 /proc/self/mountinfo | grep i_version
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,seclabel,i_version
> $ mount -o remount,ro /mnt
> $ grep pmem0 /proc/self/mountinfo | grep i_version
> 
> nolazytime gets ignored by ext4 on remount, for example
> 
> $ mount -o lazytime /dev/pmem0 /mnt
> $ grep pmem0 /proc/self/mountinfo | grep lazytime
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel
> $ mount -o remount,nolazytime /mnt
> $ grep pmem0 /proc/self/mountinfo | grep lazytime
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel
> 
> Fix it by applying the SB_LAZYTIME and SB_I_VERSION flags from *flags to
> s_flags before we parse the option and use the resulting state of the
> same flags in *flags at the end of successful remount.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Looks good to me. I did some small unit testing to observe the
behavior. Below logs are *with your patch applied* for nolazytime e.g.
which you referred above. But without your patch, as you mentioned
nolazytime was not taking into effect.

           mount  8635 [007]  5477.595642:      probe:ext4_remount: 
(ffffffff813bd270) s_flags=0x72810000 flags=0x0 data_string="i_version"
            mount  8635 [007]  5477.595672:  probe:ext4_remount_L55: 
(ffffffff813bd3d1) s_flags=0x70010000 flags=0x0 data_string="i_version"
            mount  8635 [007]  5477.595758: probe:ext4_remount_L217: 
(ffffffff813bd787) s_flags=0x70810000 flags=0x0
            mount  8635 [007]  5477.595772: probe:ext4_remount_L246: 
(ffffffff813bd820) s_flags=0x70810000 flags=0x800000



Please feel free to add.
Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>


> ---
>   fs/ext4/super.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 330957ed1f05..caab4c57f909 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5445,7 +5445,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>   {
>   	struct ext4_super_block *es;
>   	struct ext4_sb_info *sbi = EXT4_SB(sb);
> -	unsigned long old_sb_flags;
> +	unsigned long old_sb_flags, vfs_flags;
>   	struct ext4_mount_options old_opts;
>   	int enable_quota = 0;
>   	ext4_group_t g;
> @@ -5488,6 +5488,14 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>   	if (sbi->s_journal && sbi->s_journal->j_task->io_context)
>   		journal_ioprio = sbi->s_journal->j_task->io_context->ioprio;
> 
> +	/*
> +	 * Some options can be enabled by ext4 and/or by VFS mount flag
> +	 * either way we need to make sure it matches in both *flags and
> +	 * s_flags. Copy those selected flags from *flags to s_flags
> +	 */
> +	vfs_flags = SB_LAZYTIME | SB_I_VERSION;
> +	sb->s_flags = (sb->s_flags & ~vfs_flags) | (*flags & vfs_flags);
> +
>   	if (!parse_options(data, sb, NULL, &journal_ioprio, 1)) {
>   		err = -EINVAL;
>   		goto restore_opts;
> @@ -5541,9 +5549,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>   		set_task_ioprio(sbi->s_journal->j_task, journal_ioprio);
>   	}
> 
> -	if (*flags & SB_LAZYTIME)
> -		sb->s_flags |= SB_LAZYTIME;
> -
>   	if ((bool)(*flags & SB_RDONLY) != sb_rdonly(sb)) {
>   		if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED) {
>   			err = -EROFS;
> @@ -5675,7 +5680,13 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>   	}
>   #endif
> 
> -	*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
> +	/*
> +	 * Some options can be enabled by ext4 and/or by VFS mount flag
> +	 * either way we need to make sure it matches in both *flags and
> +	 * s_flags. Copy those selected flags from s_flags to *flags
> +	 */
> +	*flags = (*flags & ~vfs_flags) | (sb->s_flags & vfs_flags);
> +
>   	ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data);
>   	kfree(orig_data);
>   	return 0;
> 

Re: [PATCH] ext4: handle option set by mount flags correctly

[tytso]

On Thu, Jul 23, 2020 at 05:05:26PM +0200, Lukas Czerner wrote:
> Currently there is a problem with mount options that can be both set by
> vfs using mount flags or by a string parsing in ext4.
> 
> i_version/iversion options gets lost after remount, for example
> 
> $ mount -o i_version /dev/pmem0 /mnt
> $ grep pmem0 /proc/self/mountinfo | grep i_version
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,seclabel,i_version
> $ mount -o remount,ro /mnt
> $ grep pmem0 /proc/self/mountinfo | grep i_version
> 
> nolazytime gets ignored by ext4 on remount, for example
> 
> $ mount -o lazytime /dev/pmem0 /mnt
> $ grep pmem0 /proc/self/mountinfo | grep lazytime
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel
> $ mount -o remount,nolazytime /mnt
> $ grep pmem0 /proc/self/mountinfo | grep lazytime
> 310 95 259:0 / /mnt rw,relatime shared:163 - ext4 /dev/pmem0 rw,lazytime,seclabel
> 
> Fix it by applying the SB_LAZYTIME and SB_I_VERSION flags from *flags to
> s_flags before we parse the option and use the resulting state of the
> same flags in *flags at the end of successful remount.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, applied.

						- Ted