| Message ID | 20220517145039.3202184-11-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/15] btrfs: introduce a pure data checksum checking helper | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On 2022/5/17 22:50, Christoph Hellwig wrote: > This helpers works like submit_bio_wait, but goes through the btrfs bio > mapping using btrfs_map_bio. I hate the naming of btrfs_map_bio(), which should be btrfs_map_and_submit_bio(), but I also totally understand my poor naming scheme is even worse for most cases. Maybe we can add the "submit" part into the new function? > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/volumes.c | 21 +++++++++++++++++++++ > fs/btrfs/volumes.h | 2 ++ > 2 files changed, 23 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 0819db46dbc42..8925bc606db7e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6818,6 +6818,27 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, > return BLK_STS_OK; > } > > +static void btrfs_end_io_sync(struct bio *bio) > +{ > + complete(bio->bi_private); > +} > + > +blk_status_t btrfs_map_bio_wait(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror) > +{ > + DECLARE_COMPLETION_ONSTACK(done); > + blk_status_t ret; Is there any lockdep assert to make sure we're in wq context? Despite these nitpicks, it looks good to me. Thanks, Qu > + > + bio->bi_private = &done; > + bio->bi_end_io = btrfs_end_io_sync; > + ret = btrfs_map_bio(fs_info, bio, mirror); > + if (ret) > + return ret; > + > + wait_for_completion_io(&done); > + return bio->bi_status; > +} > + > static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args, > const struct btrfs_fs_devices *fs_devices) > { > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 6f784d4f54664..b346f6c401515 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -555,6 +555,8 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans, > 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); > +blk_status_t btrfs_map_bio_wait(struct btrfs_fs_info *fs_info, struct bio *bio, > + int mirror); > 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,
On Wed, May 18, 2022 at 06:26:08AM +0800, Qu Wenruo wrote: > > > On 2022/5/17 22:50, Christoph Hellwig wrote: >> This helpers works like submit_bio_wait, but goes through the btrfs bio >> mapping using btrfs_map_bio. > > I hate the naming of btrfs_map_bio(), which should be > btrfs_map_and_submit_bio(), but I also totally understand my poor naming > scheme is even worse for most cases. > > Maybe we can add the "submit" part into the new function? I was tempted to rename btrfs_map_bio to just btrfs_submit_bio a few times, but always had more important things to do first. But either way this helper should match the naming of the main async function. >> + DECLARE_COMPLETION_ONSTACK(done); >> + blk_status_t ret; > > Is there any lockdep assert to make sure we're in wq context? No. You can build some asserts manually, but wait_for_completion will already do that, so I'm not sure what the benefit would be.
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 0819db46dbc42..8925bc606db7e 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6818,6 +6818,27 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio, return BLK_STS_OK; } +static void btrfs_end_io_sync(struct bio *bio) +{ + complete(bio->bi_private); +} + +blk_status_t btrfs_map_bio_wait(struct btrfs_fs_info *fs_info, struct bio *bio, + int mirror) +{ + DECLARE_COMPLETION_ONSTACK(done); + blk_status_t ret; + + bio->bi_private = &done; + bio->bi_end_io = btrfs_end_io_sync; + ret = btrfs_map_bio(fs_info, bio, mirror); + if (ret) + return ret; + + wait_for_completion_io(&done); + return bio->bi_status; +} + static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args, const struct btrfs_fs_devices *fs_devices) { diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 6f784d4f54664..b346f6c401515 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -555,6 +555,8 @@ struct btrfs_block_group *btrfs_create_chunk(struct btrfs_trans_handle *trans, 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); +blk_status_t btrfs_map_bio_wait(struct btrfs_fs_info *fs_info, struct bio *bio, + int mirror); 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,
This helpers works like submit_bio_wait, but goes through the btrfs bio mapping using btrfs_map_bio. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/volumes.c | 21 +++++++++++++++++++++ fs/btrfs/volumes.h | 2 ++ 2 files changed, 23 insertions(+)