Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 27 Jul 2020 14:43:40 +0200
From:   Lukas Czerner <lczerner@redhat.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 6/6] ext4: Correctly restore system zone info when
 remount fails
Message-ID: <20200727124340.doyenujsriu7gtw3@work>
References: <20200727114429.1478-1-jack@suse.cz>
 <20200727114429.1478-7-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200727114429.1478-7-jack@suse.cz>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

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@redhat.com>

> 
> Reported-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  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
>