Re: [PATCH 5/5] btrfs: remove bioc->stripes_pending

[Boris Burkov <boris@bur.io>]

On Wed, Jun 15, 2022 at 05:15:15PM +0200, Christoph Hellwig wrote:
> Replace the the stripes_pending field with the pending counter in the
> bio.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/btrfs/volumes.c | 100 ++++++++++++++++++++++-----------------------
>  fs/btrfs/volumes.h |   1 -
>  2 files changed, 48 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 3a8c437bdd65b..bf80a850347cd 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5889,7 +5889,6 @@ static struct btrfs_io_context *alloc_btrfs_io_context(struct btrfs_fs_info *fs_
>  		sizeof(u64) * (total_stripes),
>  		GFP_NOFS|__GFP_NOFAIL);
>  
> -	atomic_set(&bioc->error, 0);
>  	refcount_set(&bioc->refs, 1);
>  
>  	bioc->fs_info = fs_info;
> @@ -6619,46 +6618,21 @@ static void btrfs_end_bio_work(struct work_struct *work)
>  	struct btrfs_bio *bbio =
>  		container_of(work, struct btrfs_bio, end_io_work);
>  
> -	bio_endio(&bbio->bio);
> -}
> -
> -static void btrfs_end_bioc(struct btrfs_io_context *bioc, bool async)
> -{
> -	struct bio *orig_bio = bioc->orig_bio;
> -	struct btrfs_bio *bbio = btrfs_bio(orig_bio);
> -
> -	bbio->mirror_num = bioc->mirror_num;
> -	orig_bio->bi_private = bioc->private;
> -	orig_bio->bi_end_io = bioc->end_io;
> -
> -	/*
> -	 * Only send an error to the higher layers if it is beyond the tolerance
> -	 * threshold.
> -	 */
> -	if (atomic_read(&bioc->error) > bioc->max_errors)
> -		orig_bio->bi_status = BLK_STS_IOERR;
> -	else
> -		orig_bio->bi_status = BLK_STS_OK;
> -
> -	if (btrfs_op(orig_bio) == BTRFS_MAP_READ && async) {
> -		INIT_WORK(&bbio->end_io_work, btrfs_end_bio_work);
> -		queue_work(btrfs_end_io_wq(bioc), &bbio->end_io_work);
> -	} else {
> -		bio_endio(orig_bio);
> -	}
> -
> -	btrfs_put_bioc(bioc);
> +	bbio->bio.bi_end_io(&bbio->bio);
>  }
>  
>  static void btrfs_end_bio(struct bio *bio)
>  {
>  	struct btrfs_io_stripe *stripe = bio->bi_private;
>  	struct btrfs_io_context *bioc = stripe->bioc;
> +	struct bio *orig_bio = bioc->orig_bio;
> +	struct btrfs_bio *bbio = btrfs_bio(orig_bio);
>  
>  	if (bio->bi_status) {
>  		atomic_inc(&bioc->error);
> -		if (bio->bi_status == BLK_STS_IOERR ||
> -		    bio->bi_status == BLK_STS_TARGET) {
> +		if (stripe->dev && stripe->dev->bdev &&
> +		    (bio->bi_status == BLK_STS_IOERR ||
> +		     bio->bi_status == BLK_STS_TARGET)) {
>  			if (btrfs_op(bio) == BTRFS_MAP_WRITE)
>  				btrfs_dev_stat_inc_and_print(stripe->dev,
>  						BTRFS_DEV_STAT_WRITE_ERRS);
> @@ -6671,12 +6645,35 @@ static void btrfs_end_bio(struct bio *bio)
>  		}
>  	}
>  
> -	if (bio != bioc->orig_bio)
> +	btrfs_bio_counter_dec(bioc->fs_info);
> +
> +	if (bio != orig_bio) {
> +		bio_endio(orig_bio);
>  		bio_put(bio);
> +		return;
> +	}
>  
> -	btrfs_bio_counter_dec(bioc->fs_info);
> -	if (atomic_dec_and_test(&bioc->stripes_pending))
> -		btrfs_end_bioc(bioc, true);
> +	/*
> +	 * Only send an error to the higher layers if it is beyond the tolerance
> +	 * threshold.
> +	 */
> +	if (atomic_read(&bioc->error) > bioc->max_errors)
> +		orig_bio->bi_status = BLK_STS_IOERR;
> +	else
> +		orig_bio->bi_status = BLK_STS_OK;
> +
> +	bbio->mirror_num = bioc->mirror_num;
> +	orig_bio->bi_end_io = bioc->end_io;
> +	orig_bio->bi_private = bioc->private;
> +	if (btrfs_op(orig_bio) == BTRFS_MAP_READ) {
> +		bbio->device = stripe->dev;
> +		INIT_WORK(&bbio->end_io_work, btrfs_end_bio_work);
> +		queue_work(btrfs_end_io_wq(bioc), &bbio->end_io_work);
> +	} else {
> +		orig_bio->bi_end_io(orig_bio);
> +	}
> +
> +	btrfs_put_bioc(bioc);
>  }
>  
>  static void submit_stripe_bio(struct btrfs_io_context *bioc,
> @@ -6687,28 +6684,30 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc,
>  	u64 physical = bioc->stripes[dev_nr].physical;
>  	struct bio *bio;
>  
> -	if (!dev || !dev->bdev ||
> -	    test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) ||
> -	    (btrfs_op(orig_bio) == BTRFS_MAP_WRITE &&
> -	     !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))) {
> -		atomic_inc(&bioc->error);
> -		if (atomic_dec_and_test(&bioc->stripes_pending))
> -			btrfs_end_bioc(bioc, false);
> -		return;
> -	}
> -
>  	if (clone) {
> -		bio = bio_alloc_clone(dev->bdev, orig_bio, GFP_NOFS, &fs_bio_set);
> +		bio = bio_alloc_clone(NULL, orig_bio, GFP_NOFS, &fs_bio_set);
> +		bio_inc_remaining(orig_bio);
>  	} else {
>  		bio = orig_bio;
> -		bio_set_dev(bio, dev->bdev);
> -		btrfs_bio(bio)->device = dev;
>  	}
>  
>  	bioc->stripes[dev_nr].bioc = bioc;
>  	bio->bi_private = &bioc->stripes[dev_nr];
>  	bio->bi_end_io = btrfs_end_bio;
>  	bio->bi_iter.bi_sector = physical >> 9;
> +
> +	btrfs_bio_counter_inc_noblocked(fs_info);
> +
> +	if (!dev || !dev->bdev ||
> +	    test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) ||
> +	    (btrfs_op(bio) == BTRFS_MAP_WRITE &&
> +	     !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))) {
> +		bio_io_error(bio);
> +		return;
> +	}
> +
> +	bio_set_dev(bio, dev->bdev);
> +	

Extra tabbed newline. There's at least one more checkpatch failure like
this one in the series.

>  	/*
>  	 * For zone append writing, bi_sector must point the beginning of the
>  	 * zone
> @@ -6729,8 +6728,6 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc,
>  		(unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name),
>  		dev->devid, bio->bi_iter.bi_size);
>  
> -	btrfs_bio_counter_inc_noblocked(fs_info);
> -
>  	btrfsic_check_bio(bio);
>  	submit_bio(bio);
>  }
> @@ -6760,7 +6757,6 @@ void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
>  	bioc->orig_bio = bio;
>  	bioc->private = bio->bi_private;
>  	bioc->end_io = bio->bi_end_io;
> -	atomic_set(&bioc->stripes_pending, total_devs);
>  
>  	if ((bioc->map_type & BTRFS_BLOCK_GROUP_RAID56_MASK) &&
>  	    ((btrfs_op(bio) == BTRFS_MAP_WRITE) || (mirror_num > 1))) {
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index c0f5bbba9c6ac..ecbaf92323030 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -444,7 +444,6 @@ struct btrfs_discard_stripe {
>   */
>  struct btrfs_io_context {
>  	refcount_t refs;
> -	atomic_t stripes_pending;
>  	struct btrfs_fs_info *fs_info;
>  	u64 map_type; /* get from map_lookup->type */
>  	bio_end_io_t *end_io;
> -- 
> 2.30.2
>