Re: [PATCH 5/5] btrfs: do not return errors from submit_bio_hook_t instances

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/4/15 22:33, Christoph Hellwig wrote:
> Both btrfs_repair_one_sector and submit_bio_one as the direct caller of
> one of the instances ignore errors as they expect the methods themselves
> to call ->bi_end_io on error.  Remove the unused and dangerous return
> value.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/ctree.h     |  4 ++--
>   fs/btrfs/extent_io.h |  2 +-
>   fs/btrfs/inode.c     | 23 ++++++++---------------
>   3 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 0fd3a21cd5a89..67e169ba55e87 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3249,8 +3249,8 @@ void btrfs_inode_safe_disk_i_size_write(struct btrfs_inode *inode, u64 new_i_siz
>   u64 btrfs_file_extent_end(const struct btrfs_path *path);
>
>   /* inode.c */
> -blk_status_t btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
> -				   int mirror_num, unsigned long bio_flags);
> +void btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
> +			   int mirror_num, unsigned long bio_flags);
>   unsigned int btrfs_verify_data_csum(struct btrfs_bio *bbio,
>   				    u32 bio_offset, struct page *page,
>   				    u64 start, u64 end);
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index c94df8e2ab9d6..b390ec79f9a86 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -71,7 +71,7 @@ struct btrfs_fs_info;
>   struct io_failure_record;
>   struct extent_io_tree;
>
> -typedef blk_status_t (submit_bio_hook_t)(struct inode *inode, struct bio *bio,
> +typedef void (submit_bio_hook_t)(struct inode *inode, struct bio *bio,
>   					 int mirror_num,
>   					 unsigned long bio_flags);
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 414156c0ac38a..a37da2decf958 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2581,9 +2581,8 @@ static blk_status_t extract_ordered_extent(struct btrfs_inode *inode,
>    *
>    *    c-3) otherwise:			async submit
>    */
> -blk_status_t btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
> -				   int mirror_num, unsigned long bio_flags)
> -
> +void btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
> +			   int mirror_num, unsigned long bio_flags)
>   {
>   	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>   	struct btrfs_root *root = BTRFS_I(inode)->root;
> @@ -2620,7 +2619,7 @@ blk_status_t btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
>   			 */
>   			btrfs_submit_compressed_read(inode, bio, mirror_num,
>   						     bio_flags);
> -			return BLK_STS_OK;
> +			return;
>   		} else {
>   			/*
>   			 * Lookup bio sums does extra checks around whether we
> @@ -2654,7 +2653,6 @@ blk_status_t btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
>   		bio->bi_status = ret;
>   		bio_endio(bio);
>   	}
> -	return ret;
>   }
>
>   /*
> @@ -7798,25 +7796,20 @@ static void btrfs_dio_private_put(struct btrfs_dio_private *dip)
>   	kfree(dip);
>   }
>
> -static blk_status_t submit_dio_repair_bio(struct inode *inode, struct bio *bio,
> -					  int mirror_num,
> -					  unsigned long bio_flags)
> +static void submit_dio_repair_bio(struct inode *inode, struct bio *bio,
> +				  int mirror_num, unsigned long bio_flags)
>   {
>   	struct btrfs_dio_private *dip = bio->bi_private;
>   	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> -	blk_status_t ret;
>
>   	BUG_ON(bio_op(bio) == REQ_OP_WRITE);
>
> -	ret = btrfs_bio_wq_end_io(fs_info, bio, BTRFS_WQ_ENDIO_DATA);
> -	if (ret)
> -		return ret;
> +	if (btrfs_bio_wq_end_io(fs_info, bio, BTRFS_WQ_ENDIO_DATA))
> +		return;
>
>   	refcount_inc(&dip->refs);
> -	ret = btrfs_map_bio(fs_info, bio, mirror_num);
> -	if (ret)
> +	if (btrfs_map_bio(fs_info, bio, mirror_num))
>   		refcount_dec(&dip->refs);
> -	return ret;
>   }
>
>   static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip,