2020-07-27 11:46:10

by Jan Kara

[permalink] [raw]
Subject: [PATCH 6/6] ext4: Correctly restore system zone info when remount fails

When remounting filesystem fails late during remount handling and
block_validity mount option is also changed during the remount, we fail
to restore system zone information to a state matching the mount option.
This is mostly harmless, just the block validity checking will not match
the situation described by the mount option. Make sure these two are always
consistent.

Reported-by: Lukas Czerner <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/block_validity.c | 8 --------
fs/ext4/super.c | 29 +++++++++++++++++++++--------
2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
index 2d008c1b58f2..c54ba52f2dd4 100644
--- a/fs/ext4/block_validity.c
+++ b/fs/ext4/block_validity.c
@@ -220,14 +220,6 @@ int ext4_setup_system_zone(struct super_block *sb)
int flex_size = ext4_flex_bg_size(sbi);
int ret;

- if (!test_opt(sb, BLOCK_VALIDITY)) {
- if (sbi->system_blks)
- ext4_release_system_zone(sb);
- return 0;
- }
- if (sbi->system_blks)
- return 0;
-
system_blks = kzalloc(sizeof(*system_blks), GFP_KERNEL);
if (!system_blks)
return -ENOMEM;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8e055ec57a2c..37f09ecca0df 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4698,11 +4698,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)

ext4_set_resv_clusters(sb);

- err = ext4_setup_system_zone(sb);
- if (err) {
- ext4_msg(sb, KERN_ERR, "failed to initialize system "
- "zone (%d)", err);
- goto failed_mount4a;
+ if (test_opt(sb, BLOCK_VALIDITY)) {
+ err = ext4_setup_system_zone(sb);
+ if (err) {
+ ext4_msg(sb, KERN_ERR, "failed to initialize system "
+ "zone (%d)", err);
+ goto failed_mount4a;
+ }
}

ext4_ext_init(sb);
@@ -5653,9 +5655,16 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
ext4_register_li_request(sb, first_not_zeroed);
}

- err = ext4_setup_system_zone(sb);
- if (err)
- goto restore_opts;
+ /*
+ * Handle creation of system zone data early because it can fail.
+ * Releasing of existing data is done when we are sure remount will
+ * succeed.
+ */
+ if (test_opt(sb, BLOCK_VALIDITY) && !sbi->system_blks) {
+ err = ext4_setup_system_zone(sb);
+ if (err)
+ goto restore_opts;
+ }

if (sbi->s_journal == NULL && !(old_sb_flags & SB_RDONLY)) {
err = ext4_commit_super(sb, 1);
@@ -5677,6 +5686,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
}
}
#endif
+ if (!test_opt(sb, BLOCK_VALIDITY) && sbi->system_blks)
+ ext4_release_system_zone(sb);

*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data);
@@ -5692,6 +5703,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
sbi->s_commit_interval = old_opts.s_commit_interval;
sbi->s_min_batch_time = old_opts.s_min_batch_time;
sbi->s_max_batch_time = old_opts.s_max_batch_time;
+ if (!test_opt(sb, BLOCK_VALIDITY) && sbi->system_blks)
+ ext4_release_system_zone(sb);
#ifdef CONFIG_QUOTA
sbi->s_jquota_fmt = old_opts.s_jquota_fmt;
for (i = 0; i < EXT4_MAXQUOTAS; i++) {
--
2.16.4


2020-07-27 12:45:57

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH 6/6] ext4: Correctly restore system zone info when remount fails

On Mon, Jul 27, 2020 at 01:44:29PM +0200, Jan Kara wrote:
> When remounting filesystem fails late during remount handling and
> block_validity mount option is also changed during the remount, we fail
> to restore system zone information to a state matching the mount option.
> This is mostly harmless, just the block validity checking will not match
> the situation described by the mount option. Make sure these two are always
> consistent.

Looks good, thanks!

Reviewed-by: Lukas Czerner <[email protected]>

>
> Reported-by: Lukas Czerner <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> fs/ext4/block_validity.c | 8 --------
> fs/ext4/super.c | 29 +++++++++++++++++++++--------
> 2 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
> index 2d008c1b58f2..c54ba52f2dd4 100644
> --- a/fs/ext4/block_validity.c
> +++ b/fs/ext4/block_validity.c
> @@ -220,14 +220,6 @@ int ext4_setup_system_zone(struct super_block *sb)
> int flex_size = ext4_flex_bg_size(sbi);
> int ret;
>
> - if (!test_opt(sb, BLOCK_VALIDITY)) {
> - if (sbi->system_blks)
> - ext4_release_system_zone(sb);
> - return 0;
> - }
> - if (sbi->system_blks)
> - return 0;
> -
> system_blks = kzalloc(sizeof(*system_blks), GFP_KERNEL);
> if (!system_blks)
> return -ENOMEM;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 8e055ec57a2c..37f09ecca0df 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4698,11 +4698,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>
> ext4_set_resv_clusters(sb);
>
> - err = ext4_setup_system_zone(sb);
> - if (err) {
> - ext4_msg(sb, KERN_ERR, "failed to initialize system "
> - "zone (%d)", err);
> - goto failed_mount4a;
> + if (test_opt(sb, BLOCK_VALIDITY)) {
> + err = ext4_setup_system_zone(sb);
> + if (err) {
> + ext4_msg(sb, KERN_ERR, "failed to initialize system "
> + "zone (%d)", err);
> + goto failed_mount4a;
> + }
> }
>
> ext4_ext_init(sb);
> @@ -5653,9 +5655,16 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> ext4_register_li_request(sb, first_not_zeroed);
> }
>
> - err = ext4_setup_system_zone(sb);
> - if (err)
> - goto restore_opts;
> + /*
> + * Handle creation of system zone data early because it can fail.
> + * Releasing of existing data is done when we are sure remount will
> + * succeed.
> + */
> + if (test_opt(sb, BLOCK_VALIDITY) && !sbi->system_blks) {
> + err = ext4_setup_system_zone(sb);
> + if (err)
> + goto restore_opts;
> + }
>
> if (sbi->s_journal == NULL && !(old_sb_flags & SB_RDONLY)) {
> err = ext4_commit_super(sb, 1);
> @@ -5677,6 +5686,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> }
> }
> #endif
> + if (!test_opt(sb, BLOCK_VALIDITY) && sbi->system_blks)
> + ext4_release_system_zone(sb);
>
> *flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
> ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data);
> @@ -5692,6 +5703,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> sbi->s_commit_interval = old_opts.s_commit_interval;
> sbi->s_min_batch_time = old_opts.s_min_batch_time;
> sbi->s_max_batch_time = old_opts.s_max_batch_time;
> + if (!test_opt(sb, BLOCK_VALIDITY) && sbi->system_blks)
> + ext4_release_system_zone(sb);
> #ifdef CONFIG_QUOTA
> sbi->s_jquota_fmt = old_opts.s_jquota_fmt;
> for (i = 0; i < EXT4_MAXQUOTAS; i++) {
> --
> 2.16.4
>