Re: [PATCH 8/8] btrfs: use btrfs_bio_for_each_sector in btrfs_check_read_dio_bio

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

On 2022/5/22 19:47, Christoph Hellwig wrote:
> Use the new btrfs_bio_for_each_sector iterator to simplify
> btrfs_check_read_dio_bio.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

In fact, in my version I also want to convert the buffered read endio to
use the helper, and get rid of the error bitmap thing.

But you're so fast sending out the patchset...

Thanks,
Qu
> ---
>   fs/btrfs/inode.c | 56 +++++++++++++++++++-----------------------------
>   1 file changed, 22 insertions(+), 34 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 095632977a798..34466b543ed97 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7886,47 +7886,35 @@ static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip,
>   {
>   	struct inode *inode = dip->inode;
>   	struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
> -	const u32 sectorsize = fs_info->sectorsize;
>   	struct extent_io_tree *failure_tree = &BTRFS_I(inode)->io_failure_tree;
>   	struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
>   	const bool csum = !(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM);
> -	struct bio_vec bvec;
> -	struct bvec_iter iter;
> -	u32 bio_offset = 0;
>   	blk_status_t err = BLK_STS_OK;
> +	struct bvec_iter iter;
> +	struct bio_vec bv;
> +	u32 offset;
> +
> +	btrfs_bio_for_each_sector(fs_info, bv, bbio, iter, offset) {
> +		u64 start = bbio->file_offset + offset;
> +
> +		if (uptodate &&
> +		    (!csum || !check_data_csum(inode, bbio, offset, bv.bv_page,
> +				bv.bv_offset, start))) {
> +			clean_io_failure(fs_info, failure_tree, io_tree, start,
> +					 bv.bv_page, btrfs_ino(BTRFS_I(inode)),
> +					 bv.bv_offset);
> +		} else {
> +			int ret;
>
> -	__bio_for_each_segment(bvec, &bbio->bio, iter, bbio->iter) {
> -		unsigned int i, nr_sectors, pgoff;
> -
> -		nr_sectors = BTRFS_BYTES_TO_BLKS(fs_info, bvec.bv_len);
> -		pgoff = bvec.bv_offset;
> -		for (i = 0; i < nr_sectors; i++) {
> -			u64 start = bbio->file_offset + bio_offset;
> -
> -			ASSERT(pgoff < PAGE_SIZE);
> -			if (uptodate &&
> -			    (!csum || !check_data_csum(inode, bbio,
> -						       bio_offset, bvec.bv_page,
> -						       pgoff, start))) {
> -				clean_io_failure(fs_info, failure_tree, io_tree,
> -						 start, bvec.bv_page,
> -						 btrfs_ino(BTRFS_I(inode)),
> -						 pgoff);
> -			} else {
> -				int ret;
> -
> -				ret = btrfs_repair_one_sector(inode, &bbio->bio,
> -						bio_offset, bvec.bv_page, pgoff,
> -						start, bbio->mirror_num,
> -						submit_dio_repair_bio);
> -				if (ret)
> -					err = errno_to_blk_status(ret);
> -			}
> -			ASSERT(bio_offset + sectorsize > bio_offset);
> -			bio_offset += sectorsize;
> -			pgoff += sectorsize;
> +			ret = btrfs_repair_one_sector(inode, &bbio->bio, offset,
> +					bv.bv_page, bv.bv_offset, start,
> +					bbio->mirror_num,
> +					submit_dio_repair_bio);
> +			if (ret)
> +				err = errno_to_blk_status(ret);
>   		}
>   	}
> +
>   	return err;
>   }
>