Re: [PATCH] btrfs: fix dev-replace after the scrub rework

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

On 2023/6/1 17:37, Christoph Hellwig wrote:
> This fixes the tests case for me.  I'd much prefer to not
> duplicate all this logic though, so something like this should
> be folded in:

Yes, the cleanup is also in my mind, but currently I prefer to do the
fix first in-place, then you can do the tide up after the more critical fix.

Thanks,
Qu
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 4e74105bc97191..6fbf30bea27ab0 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1102,6 +1102,33 @@ static void scrub_write_endio(struct btrfs_bio *bbio)
>   		wake_up(&stripe->io_wait);
>   }
>
> +static void btrfs_submit_scrub_bio(struct scrub_ctx *sctx,
> +				   struct scrub_stripe *stripe,
> +				   struct btrfs_bio *bbio,
> +				   bool dev_replace)
> +{
> +	struct btrfs_fs_info *fs_info = stripe->bg->fs_info;
> +	u32 bio_len = bbio->bio.bi_iter.bi_size;
> +	u32 bio_off = (bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT) -
> +		stripe->logical;
> +
> +	fill_writer_pointer_gap(sctx, stripe->physical + bio_off);
> +	atomic_inc(&stripe->pending_io);
> +	btrfs_submit_repair_write(bbio, stripe->mirror_num, dev_replace);
> +
> +	/* For zoned writeback, queue depth must be 1. */
> +	if (!btrfs_is_zoned(fs_info))
> +		return;
> +
> +	/*
> +	 * If the write finished without error, forward the write pointer.
> +	 */
> +	wait_scrub_stripe_io(stripe);
> +	if (!test_bit(bio_off >> fs_info->sectorsize_bits,
> +		      &stripe->write_error_bitmap))
> +		sctx->write_pointer += bio_len;
> +}
> +
>   /*
>    * Submit the write bio(s) for the sectors specified by @write_bitmap.
>    *
> @@ -1120,7 +1147,6 @@ static void scrub_write_sectors(struct scrub_ctx *sctx, struct scrub_stripe *str
>   {
>   	struct btrfs_fs_info *fs_info = stripe->bg->fs_info;
>   	struct btrfs_bio *bbio = NULL;
> -	const bool zoned = btrfs_is_zoned(fs_info);
>   	int sector_nr;
>
>   	for_each_set_bit(sector_nr, &write_bitmap, stripe->nr_sectors) {
> @@ -1133,25 +1159,7 @@ static void scrub_write_sectors(struct scrub_ctx *sctx, struct scrub_stripe *str
>
>   		/* Cannot merge with previous sector, submit the current one. */
>   		if (bbio && sector_nr && !test_bit(sector_nr - 1, &write_bitmap)) {
> -			u32 bio_len = bbio->bio.bi_iter.bi_size;
> -			u32 bio_off = (bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT) -
> -				      stripe->logical;
> -
> -			fill_writer_pointer_gap(sctx, stripe->physical + bio_off);
> -			atomic_inc(&stripe->pending_io);
> -			btrfs_submit_repair_write(bbio, stripe->mirror_num, dev_replace);
> -			/* For zoned writeback, queue depth must be 1. */
> -			if (zoned) {
> -				wait_scrub_stripe_io(stripe);
> -
> -				/*
> -				 * Write finished without error, forward the
> -				 * write pointer.
> -				 */
> -				if (!test_bit(bio_off >> fs_info->sectorsize_bits,
> -					     &stripe->write_error_bitmap))
> -					sctx->write_pointer += bio_len;
> -			}
> +			btrfs_submit_scrub_bio(sctx, stripe, bbio, dev_replace);
>   			bbio = NULL;
>   		}
>   		if (!bbio) {
> @@ -1164,26 +1172,9 @@ static void scrub_write_sectors(struct scrub_ctx *sctx, struct scrub_stripe *str
>   		ret = bio_add_page(&bbio->bio, page, fs_info->sectorsize, pgoff);
>   		ASSERT(ret == fs_info->sectorsize);
>   	}
> -	if (bbio) {
> -		u32 bio_len = bbio->bio.bi_iter.bi_size;
> -		u32 bio_off = (bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT) -
> -			      stripe->logical;
>
> -		fill_writer_pointer_gap(sctx, stripe->physical + bio_off);
> -		atomic_inc(&stripe->pending_io);
> -		btrfs_submit_repair_write(bbio, stripe->mirror_num, dev_replace);
> -		if (zoned) {
> -			wait_scrub_stripe_io(stripe);
> -
> -			/*
> -			 * Write finished without error, forward the
> -			 * write pointer.
> -			 */
> -			if (!test_bit(bio_off >> fs_info->sectorsize_bits,
> -				     &stripe->write_error_bitmap))
> -				sctx->write_pointer += bio_len;
> -		}
> -	}
> +	if (bbio)
> +		btrfs_submit_scrub_bio(sctx, stripe, bbio, dev_replace);
>   }
>
>   /*