[6/6] btrfs: lift bio_set_dev from bio allocation helpers
diff mbox series

Message ID 78e6abb1da3bb93961254526172dc70138f8f39b.1560880630.git.dsterba@suse.com
State New
Headers show
Series
  • Minor cleanups
Related show

Commit Message

David Sterba June 18, 2019, 6 p.m. UTC
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(-)

Comments

Nikolay Borisov June 19, 2019, 7:02 a.m. UTC | #1
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);
>
David Sterba June 19, 2019, 12:03 p.m. UTC | #2
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.

Patch
diff mbox series

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