Re: [PATCH] ext4: propagate errors from fast commit range replay

[Jan Kara]

On Wed 08-07-26 08:12:04, Guanghui Yang wrote:
> ext4_fc_replay() stops replaying fast commit tags only when a tag
> handler returns a negative error. However, ext4_fc_replay_add_range()
> and ext4_fc_replay_del_range() currently return 0 from their common
> exit paths even after internal failures.
> 
> This hides errors from ext4_fc_record_modified_inode(),
> ext4_map_blocks(), ext4_find_extent(), ext4_ext_insert_extent(),
> ext4_ext_replay_update_ex(), and ext4_ext_remove_space(). As a result,
> a failed ADD_RANGE or DEL_RANGE replay can be treated as successful and
> the replay code may continue with subsequent fast commit tags.
> 
> This is particularly problematic for DEL_RANGE because it may already
> have marked blocks as free before ext4_ext_remove_space() fails. If the
> error is swallowed, replay may continue from a partially applied range
> operation.
> 
> Return the saved error from the common exit paths and make the
> ERR_PTR() cases in ADD_RANGE store PTR_ERR() before jumping to out.
> 
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guanghui Yang <3497809730@xxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ext4/fast_commit.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 8e2259799614..fbb486d917b0 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2196,8 +2196,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>  		if (ret == 0) {
>  			/* Range is not mapped */
>  			path = ext4_find_extent(inode, cur, path, 0);
> -			if (IS_ERR(path))
> +			if (IS_ERR(path)) {
> +				ret = PTR_ERR(path);
> +				path = NULL;
>  				goto out;
> +			}
>  			memset(&newex, 0, sizeof(newex));
>  			newex.ee_block = cpu_to_le32(cur);
>  			ext4_ext_store_pblock(
> @@ -2209,8 +2212,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>  			path = ext4_ext_insert_extent(NULL, inode,
>  						      path, &newex, 0);
>  			up_write((&EXT4_I(inode)->i_data_sem));
> -			if (IS_ERR(path))
> +			if (IS_ERR(path)) {
> +				ret = PTR_ERR(path);
> +				path = NULL;
>  				goto out;
> +			}
>  			goto next;
>  		}
>  
> @@ -2257,10 +2263,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>  	}
>  	ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >>
>  					sb->s_blocksize_bits);
> +	ret = 0;
>  out:
>  	ext4_free_ext_path(path);
>  	iput(inode);
> -	return 0;
> +	return ret;
>  }
>  
>  /* Replay DEL_RANGE tag */
> @@ -2320,9 +2327,10 @@ ext4_fc_replay_del_range(struct super_block *sb, u8 *val)
>  	ext4_ext_replay_shrink_inode(inode,
>  		i_size_read(inode) >> sb->s_blocksize_bits);
>  	ext4_mark_inode_dirty(NULL, inode);
> +	ret = 0;
>  out:
>  	iput(inode);
> -	return 0;
> +	return ret;
>  }
>  
>  static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)
> 
> base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR