Re: [PATCH 09/40] btrfs: simplify scrub_recheck_block

From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Christoph Hellwig <hch@lst.de>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>, Qu Wenruo <wqu@suse.com>
Cc: Naohiro Aota <naohiro.aota@wdc.com>,
	linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 09/40] btrfs: simplify scrub_recheck_block
Date: Wed, 23 Mar 2022 08:10:51 +0800	[thread overview]
Message-ID: <2e4640cc-4b21-bbcb-9ba3-23267efb582a@gmx.com> (raw)
In-Reply-To: <20220322155606.1267165-10-hch@lst.de>

On 2022/3/22 23:55, Christoph Hellwig wrote:
> The I/O in repair_io_failue is synchronous and doesn't need a btrfs_bio,
> so just use an on-stack bio.

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

Just an unrelated question below.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/scrub.c | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 605ecc675ba7c..508c91e26b6e9 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1462,8 +1462,9 @@ static void scrub_recheck_block(struct btrfs_fs_info *fs_info,
>   		return scrub_recheck_block_on_raid56(fs_info, sblock);
>
>   	for (page_num = 0; page_num < sblock->page_count; page_num++) {
> -		struct bio *bio;
>   		struct scrub_page *spage = sblock->pagev[page_num];
> +		struct bio bio;
> +		struct bio_vec bvec;
>
>   		if (spage->dev->bdev == NULL) {
>   			spage->io_error = 1;
> @@ -1472,20 +1473,17 @@ static void scrub_recheck_block(struct btrfs_fs_info *fs_info,
>   		}
>
>   		WARN_ON(!spage->page);
> -		bio = btrfs_bio_alloc(1);
> -		bio_set_dev(bio, spage->dev->bdev);
> -
> -		bio_add_page(bio, spage->page, fs_info->sectorsize, 0);
> -		bio->bi_iter.bi_sector = spage->physical >> 9;
> -		bio->bi_opf = REQ_OP_READ;
> +		bio_init(&bio, spage->dev->bdev, &bvec, 1, REQ_OP_READ);
> +		__bio_add_page(&bio, spage->page, fs_info->sectorsize, 0);

Can we make the naming for __bio_add_page() better?

With more on-stack bio usage, such __bio_add_page() is really a little
embarrassing.

Thanks,
Qu

> +		bio.bi_iter.bi_sector = spage->physical >> 9;
>
> -		btrfsic_check_bio(bio);
> -		if (submit_bio_wait(bio)) {
> +		btrfsic_check_bio(&bio);
> +		if (submit_bio_wait(&bio)) {
>   			spage->io_error = 1;
>   			sblock->no_io_error_seen = 0;
>   		}
>
> -		bio_put(bio);
> +		bio_uninit(&bio);
>   	}
>
>   	if (sblock->no_io_error_seen)