Re: [PATCH 3/5] btrfs: do not return errors from btrfs_submit_metadata_bio

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

On 2022/4/15 22:33, Christoph Hellwig wrote:
> btrfs_submit_metadata_bio already calls ->bi_end_io on error and the
> caller must ignore the return value, so remove it.

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

I think your cleanup is doing the first step towards a proper defined
behavior on how we should handle bios: just fire and forget.

Thanks,
Qu
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/disk-io.c | 26 ++++++++++----------------
>   fs/btrfs/disk-io.h |  4 ++--
>   2 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 28b9cf020b8df..9c488224edc73 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -920,8 +920,8 @@ static bool should_async_write(struct btrfs_fs_info *fs_info,
>   	return true;
>   }
>
> -blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
> -				       int mirror_num)
> +void btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
> +			       int mirror_num)
>   {
>   	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>   	blk_status_t ret;
> @@ -933,14 +933,12 @@ blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
>   		 */
>   		ret = btrfs_bio_wq_end_io(fs_info, bio,
>   					  BTRFS_WQ_ENDIO_METADATA);
> -		if (ret)
> -			goto out_w_error;
> -		ret = btrfs_map_bio(fs_info, bio, mirror_num);
> +		if (!ret)
> +			ret = btrfs_map_bio(fs_info, bio, mirror_num);
>   	} else if (!should_async_write(fs_info, BTRFS_I(inode))) {
>   		ret = btree_csum_one_bio(bio);
> -		if (ret)
> -			goto out_w_error;
> -		ret = btrfs_map_bio(fs_info, bio, mirror_num);
> +		if (!ret)
> +			ret = btrfs_map_bio(fs_info, bio, mirror_num);
>   	} else {
>   		/*
>   		 * kthread helpers are used to submit writes so that
> @@ -950,14 +948,10 @@ blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
>   					  0, btree_submit_bio_start);
>   	}
>
> -	if (ret)
> -		goto out_w_error;
> -	return 0;
> -
> -out_w_error:
> -	bio->bi_status = ret;
> -	bio_endio(bio);
> -	return ret;
> +	if (ret) {
> +		bio->bi_status = ret;
> +		bio_endio(bio);
> +	}
>   }
>
>   #ifdef CONFIG_MIGRATION
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 56607abe75aa4..1312d93c02edb 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -87,8 +87,8 @@ void btrfs_drop_and_free_fs_root(struct btrfs_fs_info *fs_info,
>   int btrfs_validate_metadata_buffer(struct btrfs_bio *bbio,
>   				   struct page *page, u64 start, u64 end,
>   				   int mirror);
> -blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
> -				       int mirror_num);
> +void btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
> +			       int mirror_num);
>   #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>   struct btrfs_root *btrfs_alloc_dummy_root(struct btrfs_fs_info *fs_info);
>   #endif