| Message ID | 78e6abb1da3bb93961254526172dc70138f8f39b.1560880630.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Minor cleanups | expand |
On 18.06.19 г. 21:00 ч., David Sterba wrote: > The block device is passed around for the only purpose to set it in new > bios. Move the assignment one level up. This is a preparatory patch for > further bdev cleanups. > > Signed-off-by: David Sterba <dsterba@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> Albeit I'd go as far as suggesting to make btrfs_bio_alloc a void function similar to the generic bio alloc functions. It should be up to the callers of bio alloc functions to initialize the bio in one place. Even after your patch initialization is split across btrfs_bio_alloc and its caller which seems a bit idiosyncratic. > --- > fs/btrfs/compression.c | 12 ++++++++---- > fs/btrfs/extent_io.c | 6 +++--- > fs/btrfs/extent_io.h | 2 +- > 3 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index db41315f11eb..60c47b417a4b 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -340,7 +340,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, > > bdev = fs_info->fs_devices->latest_bdev; > > - bio = btrfs_bio_alloc(bdev, first_byte); > + bio = btrfs_bio_alloc(first_byte); > + bio_set_dev(bio, bdev); > bio->bi_opf = REQ_OP_WRITE | write_flags; > bio->bi_private = cb; > bio->bi_end_io = end_compressed_bio_write; > @@ -382,7 +383,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, > bio_endio(bio); > } > > - bio = btrfs_bio_alloc(bdev, first_byte); > + bio = btrfs_bio_alloc(first_byte); > + bio_set_dev(bio, bdev); > bio->bi_opf = REQ_OP_WRITE | write_flags; > bio->bi_private = cb; > bio->bi_end_io = end_compressed_bio_write; > @@ -620,7 +622,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > /* include any pages we added in add_ra-bio_pages */ > cb->len = bio->bi_iter.bi_size; > > - comp_bio = btrfs_bio_alloc(bdev, cur_disk_byte); > + comp_bio = btrfs_bio_alloc(cur_disk_byte); > + bio_set_dev(comp_bio, bdev); > comp_bio->bi_opf = REQ_OP_READ; > comp_bio->bi_private = cb; > comp_bio->bi_end_io = end_compressed_bio_read; > @@ -670,7 +673,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > bio_endio(comp_bio); > } > > - comp_bio = btrfs_bio_alloc(bdev, cur_disk_byte); > + comp_bio = btrfs_bio_alloc(cur_disk_byte); > + bio_set_dev(comp_bio, bdev); > comp_bio->bi_opf = REQ_OP_READ; > comp_bio->bi_private = cb; > comp_bio->bi_end_io = end_compressed_bio_read; > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index a6ad2f6f2bf7..f21be5d3724f 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2863,12 +2863,11 @@ static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio) > * never fail. We're returning a bio right now but you can call btrfs_io_bio > * for the appropriate container_of magic > */ > -struct bio *btrfs_bio_alloc(struct block_device *bdev, u64 first_byte) > +struct bio *btrfs_bio_alloc(u64 first_byte) > { > struct bio *bio; > > bio = bio_alloc_bioset(GFP_NOFS, BIO_MAX_PAGES, &btrfs_bioset); > - bio_set_dev(bio, bdev); > bio->bi_iter.bi_sector = first_byte >> 9; > btrfs_io_bio_init(btrfs_io_bio(bio)); > return bio; > @@ -2979,7 +2978,8 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree, > } > } > > - bio = btrfs_bio_alloc(bdev, offset); > + bio = btrfs_bio_alloc(offset); > + bio_set_dev(bio, bdev); > bio_add_page(bio, page, page_size, pg_offset); > bio->bi_end_io = end_io_func; > bio->bi_private = tree; > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h > index 844e595cde5b..6e13a62a2974 100644 > --- a/fs/btrfs/extent_io.h > +++ b/fs/btrfs/extent_io.h > @@ -497,7 +497,7 @@ void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end, > u64 delalloc_end, struct page *locked_page, > unsigned bits_to_clear, > unsigned long page_ops); > -struct bio *btrfs_bio_alloc(struct block_device *bdev, u64 first_byte); > +struct bio *btrfs_bio_alloc(u64 first_byte); > struct bio *btrfs_io_bio_alloc(unsigned int nr_iovecs); > struct bio *btrfs_bio_clone(struct bio *bio); > struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size); >
On Wed, Jun 19, 2019 at 10:02:53AM +0300, Nikolay Borisov wrote: > > > On 18.06.19 г. 21:00 ч., David Sterba wrote: > > The block device is passed around for the only purpose to set it in new > > bios. Move the assignment one level up. This is a preparatory patch for > > further bdev cleanups. > > > > Signed-off-by: David Sterba <dsterba@suse.com> > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > > Albeit I'd go as far as suggesting to make btrfs_bio_alloc a void > function similar to the generic bio alloc functions. It should be up to > the callers of bio alloc functions to initialize the bio in one place. > Even after your patch initialization is split across btrfs_bio_alloc and > its caller which seems a bit idiosyncratic. The bio_set_dev will go away (device for a bio is set right before it's submitted), so there will be only the allocation and the code as before. Some kind of split is probably inevitable, btrfs_bio_alloc needs the bioset that's private to extent_io.c and the callers know what to set to bi_opf etc. Maybe, as I'm looking at it now, btrfs_bio_alloc can take the value for opf, private and end_io as argument. All the callsites fill these three. That would make it consistent.
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index db41315f11eb..60c47b417a4b 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -340,7 +340,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, bdev = fs_info->fs_devices->latest_bdev; - bio = btrfs_bio_alloc(bdev, first_byte); + bio = btrfs_bio_alloc(first_byte); + bio_set_dev(bio, bdev); bio->bi_opf = REQ_OP_WRITE | write_flags; bio->bi_private = cb; bio->bi_end_io = end_compressed_bio_write; @@ -382,7 +383,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, bio_endio(bio); } - bio = btrfs_bio_alloc(bdev, first_byte); + bio = btrfs_bio_alloc(first_byte); + bio_set_dev(bio, bdev); bio->bi_opf = REQ_OP_WRITE | write_flags; bio->bi_private = cb; bio->bi_end_io = end_compressed_bio_write; @@ -620,7 +622,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, /* include any pages we added in add_ra-bio_pages */ cb->len = bio->bi_iter.bi_size; - comp_bio = btrfs_bio_alloc(bdev, cur_disk_byte); + comp_bio = btrfs_bio_alloc(cur_disk_byte); + bio_set_dev(comp_bio, bdev); comp_bio->bi_opf = REQ_OP_READ; comp_bio->bi_private = cb; comp_bio->bi_end_io = end_compressed_bio_read; @@ -670,7 +673,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, bio_endio(comp_bio); } - comp_bio = btrfs_bio_alloc(bdev, cur_disk_byte); + comp_bio = btrfs_bio_alloc(cur_disk_byte); + bio_set_dev(comp_bio, bdev); comp_bio->bi_opf = REQ_OP_READ; comp_bio->bi_private = cb; comp_bio->bi_end_io = end_compressed_bio_read; diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index a6ad2f6f2bf7..f21be5d3724f 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2863,12 +2863,11 @@ static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio) * never fail. We're returning a bio right now but you can call btrfs_io_bio * for the appropriate container_of magic */ -struct bio *btrfs_bio_alloc(struct block_device *bdev, u64 first_byte) +struct bio *btrfs_bio_alloc(u64 first_byte) { struct bio *bio; bio = bio_alloc_bioset(GFP_NOFS, BIO_MAX_PAGES, &btrfs_bioset); - bio_set_dev(bio, bdev); bio->bi_iter.bi_sector = first_byte >> 9; btrfs_io_bio_init(btrfs_io_bio(bio)); return bio; @@ -2979,7 +2978,8 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree, } } - bio = btrfs_bio_alloc(bdev, offset); + bio = btrfs_bio_alloc(offset); + bio_set_dev(bio, bdev); bio_add_page(bio, page, page_size, pg_offset); bio->bi_end_io = end_io_func; bio->bi_private = tree; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index 844e595cde5b..6e13a62a2974 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -497,7 +497,7 @@ void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end, u64 delalloc_end, struct page *locked_page, unsigned bits_to_clear, unsigned long page_ops); -struct bio *btrfs_bio_alloc(struct block_device *bdev, u64 first_byte); +struct bio *btrfs_bio_alloc(u64 first_byte); struct bio *btrfs_io_bio_alloc(unsigned int nr_iovecs); struct bio *btrfs_bio_clone(struct bio *bio); struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size);
The block device is passed around for the only purpose to set it in new bios. Move the assignment one level up. This is a preparatory patch for further bdev cleanups. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/compression.c | 12 ++++++++---- fs/btrfs/extent_io.c | 6 +++--- fs/btrfs/extent_io.h | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-)