Re: [PATCH 3/3] btrfs: pass the btrfs_bio_ctrl to submit_one_bio

[Nikolay Borisov <nborisov@suse.com>]

On 3.06.22 г. 10:11 ч., Christoph Hellwig wrote:
> submit_one_bio always works on the bio and compression flags from a
> btrfs_bio_ctrl structure.  Pass the explicitly and clean up the the
> calling conventions by handling a NULL bio in submit_one_bio, and
> using the btrfs_bio_ctrl to pass the mirror number as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/extent_io.c | 82 ++++++++++++++++++++------------------------
>   1 file changed, 37 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 72a258fa27947..facca7feb9a22 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -144,6 +144,7 @@ struct tree_entry {
>    */
>   struct btrfs_bio_ctrl {
>   	struct bio *bio;
> +	int mirror_num;
>   	enum btrfs_compression_type compress_type;
>   	u32 len_to_stripe_boundary;
>   	u32 len_to_oe_boundary;
> @@ -178,10 +179,11 @@ static int add_extent_changeset(struct extent_state *state, u32 bits,
>   	return ret;
>   }
>   
> -static void submit_one_bio(struct bio *bio, int mirror_num,
> -			   enum btrfs_compression_type compress_type)
> +static void __submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>   {
> +	struct bio *bio = bio_ctrl->bio;
>   	struct inode *inode = bio_first_page_all(bio)->mapping->host;
> +	int mirror_num = bio_ctrl->mirror_num;
>   
>   	/* Caller should ensure the bio has at least some range added */
>   	ASSERT(bio->bi_iter.bi_size);
> @@ -191,14 +193,17 @@ static void submit_one_bio(struct bio *bio, int mirror_num,
>   	else if (btrfs_op(bio) == BTRFS_MAP_WRITE)
>   		btrfs_submit_data_write_bio(inode, bio, mirror_num);
>   	else
> -		btrfs_submit_data_read_bio(inode, bio, mirror_num, compress_type);
> +		btrfs_submit_data_read_bio(inode, bio, mirror_num,
> +					   bio_ctrl->compress_type);
>   
> -	/*
> -	 * Above submission hooks will handle the error by ending the bio,
> -	 * which will do the cleanup properly.  So here we should not return
> -	 * any error, or the caller of submit_extent_page() will do cleanup
> -	 * again, causing problems.
> -	 */
> +	/* The bio is owned by the bi_end_io handler now */
> +	bio_ctrl->bio = NULL;
> +}
> +
> +static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
> +{
> +	if (bio_ctrl->bio)
> +		__submit_one_bio(bio_ctrl);
>   }

Why do you need a function just to put an if in it, just move this atop 
__submit_one_bio :

if (!bio_ctrl->bio)
     return

and rename it to submit_one_bio.

<snip>