| Message ID | 20220615151515.888424-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] btrfs: remove a bunch of pointles stripe_len arguments | expand |
On Wed, Jun 15, 2022 at 05:15:13PM +0200, Christoph Hellwig wrote: > Always consume the bio and call the end_io handler on error instead of > returning an error and letting the caller handle it. This matches > what the block layer submission does and avoids any confusion on who > needs to handle errors. > > As this requires touching all the callers, rename the function to > btrfs_submit_bio, which describes the functionality much better. One high level question: should we make btrfs_wq_submit_bio follow the same API of not having the caller call bio_endio? The names are similar enough that having them behave the same seems useful. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/compression.c | 8 ++------ > fs/btrfs/disk-io.c | 21 ++++++++++----------- > fs/btrfs/inode.c | 25 ++++++++++--------------- > fs/btrfs/volumes.c | 13 ++++++++----- > fs/btrfs/volumes.h | 4 ++-- > 5 files changed, 32 insertions(+), 39 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 63d542961b78a..907fc8a4c092c 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -593,9 +593,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, > } > > ASSERT(bio->bi_iter.bi_size); > - ret = btrfs_map_bio(fs_info, bio, 0); > - if (ret) > - goto finish_cb; > + btrfs_submit_bio(fs_info, bio, 0); > bio = NULL; > } > cond_resched(); > @@ -931,9 +929,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > sums += fs_info->csum_size * nr_sectors; > > ASSERT(comp_bio->bi_iter.bi_size); > - ret = btrfs_map_bio(fs_info, comp_bio, mirror_num); > - if (ret) > - goto finish_cb; > + btrfs_submit_bio(fs_info, comp_bio, mirror_num); > comp_bio = NULL; > } > } > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 800ad3a9c68ed..5df6865428a5c 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -728,7 +728,6 @@ static void run_one_async_done(struct btrfs_work *work) > { > struct async_submit_bio *async; > struct inode *inode; > - blk_status_t ret; > > async = container_of(work, struct async_submit_bio, work); > inode = async->inode; > @@ -746,11 +745,7 @@ static void run_one_async_done(struct btrfs_work *work) > * This changes nothing when cgroups aren't in use. > */ > async->bio->bi_opf |= REQ_CGROUP_PUNT; > - ret = btrfs_map_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); > - if (ret) { > - async->bio->bi_status = ret; > - bio_endio(async->bio); > - } > + btrfs_submit_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); > } > > static void run_one_async_free(struct btrfs_work *work) > @@ -814,7 +809,7 @@ static blk_status_t btree_submit_bio_start(struct inode *inode, struct bio *bio, > { > /* > * when we're called for a write, we're already in the async > - * submission context. Just jump into btrfs_map_bio > + * submission context. Just jump into btrfs_submit_bio. > */ > return btree_csum_one_bio(bio); > } > @@ -839,11 +834,15 @@ void btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio, int mirror_ > bio->bi_opf |= REQ_META; > > if (btrfs_op(bio) != BTRFS_MAP_WRITE) { > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > - } else if (!should_async_write(fs_info, BTRFS_I(inode))) { > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > + } > + if (!should_async_write(fs_info, BTRFS_I(inode))) { > ret = btree_csum_one_bio(bio); > - if (!ret) > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > + if (!ret) { > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > + } > } else { > /* > * kthread helpers are used to submit writes so that > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 7a54f964ff378..6f665bf59f620 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -2617,7 +2617,8 @@ void btrfs_submit_data_write_bio(struct inode *inode, struct bio *bio, int mirro > goto out; > } > } > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > out: > if (ret) { > bio->bi_status = ret; > @@ -2645,14 +2646,13 @@ void btrfs_submit_data_read_bio(struct inode *inode, struct bio *bio, > * not, which is why we ignore skip_sum here. > */ > ret = btrfs_lookup_bio_sums(inode, bio, NULL); > - if (ret) > - goto out; > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > -out: > if (ret) { > bio->bi_status = ret; > bio_endio(bio); > + return; > } > + > + btrfs_submit_bio(fs_info, bio, mirror_num); > } > > /* > @@ -7863,8 +7863,7 @@ static void submit_dio_repair_bio(struct inode *inode, struct bio *bio, > BUG_ON(bio_op(bio) == REQ_OP_WRITE); > > refcount_inc(&dip->refs); > - if (btrfs_map_bio(fs_info, bio, mirror_num)) With this change, won't we no longer do this refcount_dec on error? Does bio_endio do it or something? On the other hand, I feel like we would not have called bio_endio in this path before and now we do. More generally, were you able to exercise the error paths in this code? > - refcount_dec(&dip->refs); > + btrfs_submit_bio(fs_info, bio, mirror_num); > } > > static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip, > @@ -7972,7 +7971,8 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio, > file_offset - dip->file_offset); > } > map: > - return btrfs_map_bio(fs_info, bio, 0); > + btrfs_submit_bio(fs_info, bio, 0); > + return BLK_STS_OK; > } > > static void btrfs_submit_direct(const struct iomap_iter *iter, > @@ -10277,7 +10277,6 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, > struct bio *bio, int mirror_num) > { > struct btrfs_encoded_read_private *priv = bio->bi_private; > - struct btrfs_bio *bbio = btrfs_bio(bio); > struct btrfs_fs_info *fs_info = inode->root->fs_info; > blk_status_t ret; > > @@ -10288,12 +10287,8 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, > } > > atomic_inc(&priv->pending); > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > - if (ret) { > - atomic_dec(&priv->pending); Same question as above > - btrfs_bio_free_csum(bbio); > - } > - return ret; > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return BLK_STS_OK; > } > > static blk_status_t btrfs_encoded_read_verify_csum(struct btrfs_bio *bbio) > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index ff777e39d5f4a..739676944e94d 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6724,8 +6724,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, > } > } > btrfs_debug_in_rcu(fs_info, > - "btrfs_map_bio: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", > - bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, > + "%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", > + __func__, bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, > (unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name), > dev->devid, bio->bi_iter.bi_size); > > @@ -6735,8 +6735,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, > submit_bio(bio); > } > > -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > - int mirror_num) > +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror_num) > { > u64 logical = bio->bi_iter.bi_sector << 9; > u64 length = bio->bi_iter.bi_size; > @@ -6781,7 +6781,10 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > } > out_dec: > btrfs_bio_counter_dec(fs_info); > - return errno_to_blk_status(ret); > + if (ret) { > + bio->bi_status = errno_to_blk_status(ret); > + bio_endio(bio); > + } > } > > static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args, > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 588367c76c463..c0f5bbba9c6ac 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -573,8 +573,8 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info); > struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans, > u64 type); > void btrfs_mapping_tree_free(struct extent_map_tree *tree); > -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > - int mirror_num); > +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror_num); > int btrfs_open_devices(struct btrfs_fs_devices *fs_devices, > fmode_t flags, void *holder); > struct btrfs_device *btrfs_scan_one_device(const char *path, > -- > 2.30.2 >
On Wed, Jun 15, 2022 at 09:59:38AM -0700, Boris Burkov wrote: > One high level question: should we make btrfs_wq_submit_bio follow the > same API of not having the caller call bio_endio? The names are similar > enough that having them behave the same seems useful. If we don't end up killing it in the current form it should eventually get the same treatment, yes.
On 2022/6/15 23:15, Christoph Hellwig wrote: > Always consume the bio and call the end_io handler on error instead of > returning an error and letting the caller handle it. This matches > what the block layer submission does and avoids any confusion on who > needs to handle errors. > > As this requires touching all the callers, rename the function to > btrfs_submit_bio, which describes the functionality much better. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/compression.c | 8 ++------ > fs/btrfs/disk-io.c | 21 ++++++++++----------- > fs/btrfs/inode.c | 25 ++++++++++--------------- > fs/btrfs/volumes.c | 13 ++++++++----- > fs/btrfs/volumes.h | 4 ++-- > 5 files changed, 32 insertions(+), 39 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 63d542961b78a..907fc8a4c092c 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -593,9 +593,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, > } > > ASSERT(bio->bi_iter.bi_size); > - ret = btrfs_map_bio(fs_info, bio, 0); > - if (ret) > - goto finish_cb; > + btrfs_submit_bio(fs_info, bio, 0); > bio = NULL; > } > cond_resched(); > @@ -931,9 +929,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > sums += fs_info->csum_size * nr_sectors; > > ASSERT(comp_bio->bi_iter.bi_size); > - ret = btrfs_map_bio(fs_info, comp_bio, mirror_num); > - if (ret) > - goto finish_cb; > + btrfs_submit_bio(fs_info, comp_bio, mirror_num); > comp_bio = NULL; > } > } > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 800ad3a9c68ed..5df6865428a5c 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -728,7 +728,6 @@ static void run_one_async_done(struct btrfs_work *work) > { > struct async_submit_bio *async; > struct inode *inode; > - blk_status_t ret; > > async = container_of(work, struct async_submit_bio, work); > inode = async->inode; > @@ -746,11 +745,7 @@ static void run_one_async_done(struct btrfs_work *work) > * This changes nothing when cgroups aren't in use. > */ > async->bio->bi_opf |= REQ_CGROUP_PUNT; > - ret = btrfs_map_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); > - if (ret) { > - async->bio->bi_status = ret; > - bio_endio(async->bio); > - } > + btrfs_submit_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); > } > > static void run_one_async_free(struct btrfs_work *work) > @@ -814,7 +809,7 @@ static blk_status_t btree_submit_bio_start(struct inode *inode, struct bio *bio, > { > /* > * when we're called for a write, we're already in the async > - * submission context. Just jump into btrfs_map_bio > + * submission context. Just jump into btrfs_submit_bio. > */ > return btree_csum_one_bio(bio); > } > @@ -839,11 +834,15 @@ void btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio, int mirror_ > bio->bi_opf |= REQ_META; > > if (btrfs_op(bio) != BTRFS_MAP_WRITE) { > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > - } else if (!should_async_write(fs_info, BTRFS_I(inode))) { > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > + } > + if (!should_async_write(fs_info, BTRFS_I(inode))) { > ret = btree_csum_one_bio(bio); > - if (!ret) > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > + if (!ret) { > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > + } > } else { > /* > * kthread helpers are used to submit writes so that > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 7a54f964ff378..6f665bf59f620 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -2617,7 +2617,8 @@ void btrfs_submit_data_write_bio(struct inode *inode, struct bio *bio, int mirro > goto out; > } > } > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return; > out: > if (ret) { > bio->bi_status = ret; > @@ -2645,14 +2646,13 @@ void btrfs_submit_data_read_bio(struct inode *inode, struct bio *bio, > * not, which is why we ignore skip_sum here. > */ > ret = btrfs_lookup_bio_sums(inode, bio, NULL); > - if (ret) > - goto out; > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > -out: > if (ret) { > bio->bi_status = ret; > bio_endio(bio); > + return; > } > + > + btrfs_submit_bio(fs_info, bio, mirror_num); > } > > /* > @@ -7863,8 +7863,7 @@ static void submit_dio_repair_bio(struct inode *inode, struct bio *bio, > BUG_ON(bio_op(bio) == REQ_OP_WRITE); > > refcount_inc(&dip->refs); > - if (btrfs_map_bio(fs_info, bio, mirror_num)) > - refcount_dec(&dip->refs); > + btrfs_submit_bio(fs_info, bio, mirror_num); > } > > static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip, > @@ -7972,7 +7971,8 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio, > file_offset - dip->file_offset); > } > map: > - return btrfs_map_bio(fs_info, bio, 0); > + btrfs_submit_bio(fs_info, bio, 0); > + return BLK_STS_OK; > } > > static void btrfs_submit_direct(const struct iomap_iter *iter, > @@ -10277,7 +10277,6 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, > struct bio *bio, int mirror_num) > { > struct btrfs_encoded_read_private *priv = bio->bi_private; > - struct btrfs_bio *bbio = btrfs_bio(bio); > struct btrfs_fs_info *fs_info = inode->root->fs_info; > blk_status_t ret; > > @@ -10288,12 +10287,8 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, > } > > atomic_inc(&priv->pending); > - ret = btrfs_map_bio(fs_info, bio, mirror_num); > - if (ret) { > - atomic_dec(&priv->pending); > - btrfs_bio_free_csum(bbio); > - } > - return ret; > + btrfs_submit_bio(fs_info, bio, mirror_num); > + return BLK_STS_OK; > } > > static blk_status_t btrfs_encoded_read_verify_csum(struct btrfs_bio *bbio) > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index ff777e39d5f4a..739676944e94d 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6724,8 +6724,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, > } > } > btrfs_debug_in_rcu(fs_info, > - "btrfs_map_bio: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", > - bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, > + "%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", > + __func__, bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, > (unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name), > dev->devid, bio->bi_iter.bi_size); > > @@ -6735,8 +6735,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, > submit_bio(bio); > } > > -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > - int mirror_num) > +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror_num) > { > u64 logical = bio->bi_iter.bi_sector << 9; > u64 length = bio->bi_iter.bi_size; > @@ -6781,7 +6781,10 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > } > out_dec: > btrfs_bio_counter_dec(fs_info); > - return errno_to_blk_status(ret); > + if (ret) { > + bio->bi_status = errno_to_blk_status(ret); > + bio_endio(bio); > + } > } > > static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args, > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 588367c76c463..c0f5bbba9c6ac 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -573,8 +573,8 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info); > struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans, > u64 type); > void btrfs_mapping_tree_free(struct extent_map_tree *tree); > -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > - int mirror_num); > +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror_num); > int btrfs_open_devices(struct btrfs_fs_devices *fs_devices, > fmode_t flags, void *holder); > struct btrfs_device *btrfs_scan_one_device(const char *path,
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 63d542961b78a..907fc8a4c092c 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -593,9 +593,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, } ASSERT(bio->bi_iter.bi_size); - ret = btrfs_map_bio(fs_info, bio, 0); - if (ret) - goto finish_cb; + btrfs_submit_bio(fs_info, bio, 0); bio = NULL; } cond_resched(); @@ -931,9 +929,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, sums += fs_info->csum_size * nr_sectors; ASSERT(comp_bio->bi_iter.bi_size); - ret = btrfs_map_bio(fs_info, comp_bio, mirror_num); - if (ret) - goto finish_cb; + btrfs_submit_bio(fs_info, comp_bio, mirror_num); comp_bio = NULL; } } diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 800ad3a9c68ed..5df6865428a5c 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -728,7 +728,6 @@ static void run_one_async_done(struct btrfs_work *work) { struct async_submit_bio *async; struct inode *inode; - blk_status_t ret; async = container_of(work, struct async_submit_bio, work); inode = async->inode; @@ -746,11 +745,7 @@ static void run_one_async_done(struct btrfs_work *work) * This changes nothing when cgroups aren't in use. */ async->bio->bi_opf |= REQ_CGROUP_PUNT; - ret = btrfs_map_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); - if (ret) { - async->bio->bi_status = ret; - bio_endio(async->bio); - } + btrfs_submit_bio(btrfs_sb(inode->i_sb), async->bio, async->mirror_num); } static void run_one_async_free(struct btrfs_work *work) @@ -814,7 +809,7 @@ static blk_status_t btree_submit_bio_start(struct inode *inode, struct bio *bio, { /* * when we're called for a write, we're already in the async - * submission context. Just jump into btrfs_map_bio + * submission context. Just jump into btrfs_submit_bio. */ return btree_csum_one_bio(bio); } @@ -839,11 +834,15 @@ void btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio, int mirror_ bio->bi_opf |= REQ_META; if (btrfs_op(bio) != BTRFS_MAP_WRITE) { - ret = btrfs_map_bio(fs_info, bio, mirror_num); - } else if (!should_async_write(fs_info, BTRFS_I(inode))) { + btrfs_submit_bio(fs_info, bio, mirror_num); + return; + } + if (!should_async_write(fs_info, BTRFS_I(inode))) { ret = btree_csum_one_bio(bio); - if (!ret) - ret = btrfs_map_bio(fs_info, bio, mirror_num); + if (!ret) { + btrfs_submit_bio(fs_info, bio, mirror_num); + return; + } } else { /* * kthread helpers are used to submit writes so that diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 7a54f964ff378..6f665bf59f620 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2617,7 +2617,8 @@ void btrfs_submit_data_write_bio(struct inode *inode, struct bio *bio, int mirro goto out; } } - ret = btrfs_map_bio(fs_info, bio, mirror_num); + btrfs_submit_bio(fs_info, bio, mirror_num); + return; out: if (ret) { bio->bi_status = ret; @@ -2645,14 +2646,13 @@ void btrfs_submit_data_read_bio(struct inode *inode, struct bio *bio, * not, which is why we ignore skip_sum here. */ ret = btrfs_lookup_bio_sums(inode, bio, NULL); - if (ret) - goto out; - ret = btrfs_map_bio(fs_info, bio, mirror_num); -out: if (ret) { bio->bi_status = ret; bio_endio(bio); + return; } + + btrfs_submit_bio(fs_info, bio, mirror_num); } /* @@ -7863,8 +7863,7 @@ static void submit_dio_repair_bio(struct inode *inode, struct bio *bio, BUG_ON(bio_op(bio) == REQ_OP_WRITE); refcount_inc(&dip->refs); - if (btrfs_map_bio(fs_info, bio, mirror_num)) - refcount_dec(&dip->refs); + btrfs_submit_bio(fs_info, bio, mirror_num); } static blk_status_t btrfs_check_read_dio_bio(struct btrfs_dio_private *dip, @@ -7972,7 +7971,8 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio, file_offset - dip->file_offset); } map: - return btrfs_map_bio(fs_info, bio, 0); + btrfs_submit_bio(fs_info, bio, 0); + return BLK_STS_OK; } static void btrfs_submit_direct(const struct iomap_iter *iter, @@ -10277,7 +10277,6 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, struct bio *bio, int mirror_num) { struct btrfs_encoded_read_private *priv = bio->bi_private; - struct btrfs_bio *bbio = btrfs_bio(bio); struct btrfs_fs_info *fs_info = inode->root->fs_info; blk_status_t ret; @@ -10288,12 +10287,8 @@ static blk_status_t submit_encoded_read_bio(struct btrfs_inode *inode, } atomic_inc(&priv->pending); - ret = btrfs_map_bio(fs_info, bio, mirror_num); - if (ret) { - atomic_dec(&priv->pending); - btrfs_bio_free_csum(bbio); - } - return ret; + btrfs_submit_bio(fs_info, bio, mirror_num); + return BLK_STS_OK; } static blk_status_t btrfs_encoded_read_verify_csum(struct btrfs_bio *bbio) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index ff777e39d5f4a..739676944e94d 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6724,8 +6724,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, } } btrfs_debug_in_rcu(fs_info, - "btrfs_map_bio: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", - bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, + "%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u", + __func__, bio_op(bio), bio->bi_opf, bio->bi_iter.bi_sector, (unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name), dev->devid, bio->bi_iter.bi_size); @@ -6735,8 +6735,8 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, submit_bio(bio); } -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, - int mirror_num) +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, + int mirror_num) { u64 logical = bio->bi_iter.bi_sector << 9; u64 length = bio->bi_iter.bi_size; @@ -6781,7 +6781,10 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, } out_dec: btrfs_bio_counter_dec(fs_info); - return errno_to_blk_status(ret); + if (ret) { + bio->bi_status = errno_to_blk_status(ret); + bio_endio(bio); + } } static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args, diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 588367c76c463..c0f5bbba9c6ac 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -573,8 +573,8 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info); struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans, u64 type); void btrfs_mapping_tree_free(struct extent_map_tree *tree); -blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, - int mirror_num); +void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio, + int mirror_num); int btrfs_open_devices(struct btrfs_fs_devices *fs_devices, fmode_t flags, void *holder); struct btrfs_device *btrfs_scan_one_device(const char *path,
Always consume the bio and call the end_io handler on error instead of returning an error and letting the caller handle it. This matches what the block layer submission does and avoids any confusion on who needs to handle errors. As this requires touching all the callers, rename the function to btrfs_submit_bio, which describes the functionality much better. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/compression.c | 8 ++------ fs/btrfs/disk-io.c | 21 ++++++++++----------- fs/btrfs/inode.c | 25 ++++++++++--------------- fs/btrfs/volumes.c | 13 ++++++++----- fs/btrfs/volumes.h | 4 ++-- 5 files changed, 32 insertions(+), 39 deletions(-)