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 <lczerner@xxxxxxxxxx>
>
> Reported-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> 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
>
