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

Qu Wenruo
2022-05-22T12:22:05Z

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;
> }
>