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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> ---
> 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;
>
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 <[email protected]>
Thanks, applied.
- Ted