btrfs bio usage cleanups for multi-page bvec enabledment
Subsystems (2)
Hi btrfs maintainers,
can you review and ACK these two btrfs patches that clean up some interactions with the bio code? I'd like to get these in as the base of Mings multi-page bio_vec work, which currently contain some not quite as nice workaround in this area.
PATCH 1/2 btrfs: remove various bio_offset arguments
The btrfs write path passes a bio_offset argument through some deep callchains including async offloading. In the end this is easily calculatable using page_offset plus the bvec offset for the first page in the bio, and only actually used by by a single function. Just move the calculation of the offset there.
--- fs/btrfs/disk-io.c | 21 +++++---------------- fs/btrfs/disk-io.h | 2 +- fs/btrfs/extent_io.c | 9 ++------- fs/btrfs/extent_io.h | 5 ++--- fs/btrfs/inode.c | 17 ++++++++--------- 5 files changed, 18 insertions(+), 36 deletions(-)
fs/btrfs/disk-io.c +5 -16
@@ -108,11 +108,6 @@ struct async_submit_bio {struct bio *bio;extent_submit_bio_start_t *submit_bio_start;int mirror_num; -/* -* bio_offset is optional, can be used if the pages in the bio -* can't tell us where in the file the bio should go -*/ -u64 bio_offset;struct btrfs_work work;blk_status_t status; };
@@ -754,8 +749,7 @@ static void run_one_async_start(struct btrfs_work *work)blk_status_t ret; async = container_of(work, struct async_submit_bio, work); - ret = async->submit_bio_start (async->private_data, async->bio, - async->bio_offset); + ret = async->submit_bio_start (async->private_data, async->bio);if (ret) async->status = ret; }
@@ -786,7 +780,7 @@ static void run_one_async_free(struct btrfs_work *work)blk_status_t btrfs_wq_submit_bio (struct btrfs_fs_info *fs_info,struct bio *bio,int mirror_num,unsigned long bio_flags, -u64 bio_offset,void *private_data, +void *private_data,extent_submit_bio_start_t *submit_bio_start) {struct async_submit_bio *async;
@@ -803,8 +797,6 @@ blk_status_t btrfs_wq_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,btrfs_init_work (&async->work, btrfs_worker_helper, run_one_async_start, run_one_async_done, run_one_async_free); - async->bio_offset = bio_offset; - async->status =0 ;if (op_is_sync (bio->bi_opf))
@@ -831,8 +823,7 @@ static blk_status_t btree_csum_one_bio(struct bio *bio)return errno_to_blk_status (ret); } -static blk_status_t btree_submit_bio_start (void *private_data,struct bio *bio, -u64 bio_offset) +static blk_status_t btree_submit_bio_start (void *private_data,struct bio *bio) {/* * when we're called for a write, we're already in the async
@@ -853,8 +844,7 @@ static int check_async_write(struct btrfs_inode *bi) }static blk_status_t btree_submit_bio_hook (void *private_data,struct bio *bio, -int mirror_num,unsigned long bio_flags, -u64 bio_offset) +int mirror_num,unsigned long bio_flags) {struct inode *inode = private_data;struct btrfs_fs_info *fs_info =btrfs_sb (inode->i_sb);
@@ -882,8 +872,7 @@ static blk_status_t btree_submit_bio_hook(void *private_data, struct bio *bio,* checksumming can happen in parallel across all CPUs */ ret =btrfs_wq_submit_bio (fs_info, bio, mirror_num,0 , - bio_offset, private_data, - btree_submit_bio_start); + private_data, btree_submit_bio_start); }if (ret)
fs/btrfs/disk-io.h +1 -1
@@ -119,7 +119,7 @@ blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio, enum btrfs_wq_endio_type metadata);blk_status_t btrfs_wq_submit_bio (struct btrfs_fs_info *fs_info,struct bio *bio,int mirror_num,unsigned long bio_flags, -u64 bio_offset,void *private_data, +void *private_data,extent_submit_bio_start_t *submit_bio_start);blk_status_t btrfs_submit_bio_done (void *private_data,struct bio *bio,int mirror_num);
fs/btrfs/extent_io.c +2 -7
@@ -2397,7 +2397,7 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset, read_mode, failrec->this_mirror, failrec->in_validation); status = tree->ops->submit_bio_hook (tree->private_data, bio, failrec->this_mirror, - failrec->bio_flags,0 ); + failrec->bio_flags);if (status) {free_io_failure (failure_tree, tree, failrec);bio_put (bio);
@@ -2719,18 +2719,13 @@ static int __must_check submit_one_bio(struct bio *bio, int mirror_num, unsigned long bio_flags) {blk_status_t ret =0 ; -struct bio_vec *bvec =bio_last_bvec_all (bio); -struct page *page = bvec->bv_page;struct extent_io_tree *tree = bio->bi_private; -u64 start; - - start =page_offset (page) + bvec->bv_offset; bio->bi_private =NULL ;if (tree->ops) ret = tree->ops->submit_bio_hook (tree->private_data, bio, - mirror_num, bio_flags, start); + mirror_num, bio_flags);else btrfsic_submit_bio (bio);
fs/btrfs/extent_io.h +2 -3
@@ -86,11 +86,10 @@ struct btrfs_io_bio;struct io_failure_record ;typedef blk_status_t (extent_submit_bio_hook_t )(void *private_data,struct bio *bio, -int mirror_num,unsigned long bio_flags, -u64 bio_offset); +int mirror_num,unsigned long bio_flags);typedef blk_status_t (extent_submit_bio_start_t )(void *private_data, -struct bio *bio,u64 bio_offset); +struct bio *bio);struct extent_io_ops {/*
fs/btrfs/inode.c +8 -9
@@ -1921,8 +1921,7 @@ int btrfs_merge_bio_hook(struct page *page, unsigned long offset,* At IO completion time the cums attached on the ordered extent record * are inserted into the btree */ -static blk_status_t btrfs_submit_bio_start (void *private_data,struct bio *bio, -u64 bio_offset) +static blk_status_t btrfs_submit_bio_start (void *private_data,struct bio *bio) {struct inode *inode = private_data;blk_status_t ret =0 ;
@@ -1974,8 +1973,7 @@ blk_status_t btrfs_submit_bio_done(void *private_data, struct bio *bio,* c-3) otherwise: async submit */ static blk_status_t btrfs_submit_bio_hook (void *private_data,struct bio *bio, -int mirror_num,unsigned long bio_flags, -u64 bio_offset) +int mirror_num,unsigned long bio_flags) {struct inode *inode = private_data;struct btrfs_fs_info *fs_info =btrfs_sb (inode->i_sb);
@@ -2012,8 +2010,7 @@ static blk_status_t btrfs_submit_bio_hook(void *private_data, struct bio *bio, goto mapit;/* we're doing a write, do the async checksumming */ ret =btrfs_wq_submit_bio (fs_info, bio, mirror_num, bio_flags, - bio_offset, inode, - btrfs_submit_bio_start); + inode, btrfs_submit_bio_start); goto out; }else if (!skip_sum) { ret =btrfs_csum_one_bio (inode, bio,0 ,0 );
@@ -8112,10 +8109,13 @@ static void btrfs_endio_direct_write(struct bio *bio) }static blk_status_t btrfs_submit_bio_start_direct_io (void *private_data, -struct bio *bio,u64 offset) +struct bio *bio) {struct inode *inode = private_data; +struct bio_vec *bvec =bio_first_bvec_all (bio); +u64 offset =page_offset (bvec->bv_page) + bvec->bv_offset;blk_status_t ret; + ret =btrfs_csum_one_bio (inode, bio, offset,1 );BUG_ON (ret);/* -ENOMEM */ return 0 ;
@@ -8214,8 +8214,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio, goto map;if (write && async_submit) { - ret =btrfs_wq_submit_bio (fs_info, bio,0 ,0 , - file_offset, inode, + ret =btrfs_wq_submit_bio (fs_info, bio,0 ,0 , inode, btrfs_submit_bio_start_direct_io); goto err; }else if (write) {
Signature · 1 line
2.19.1
Re: [PATCH 1/2] btrfs: remove various bio_offset arguments
Quoted 7 lines · Christoph Hellwig
The btrfs write path passes a
bio_offsetargument through some deep callchains including async offloading. In the end this is easily calculatable usingpage_offsetplus the bvec offset for the first page in the bio, and only actually used by by a single function. Just move the calculation of the offset there.Signed-off-by Christoph Hellwig <[email protected]>
Ack for taking it with the bio updates.
PATCH 2/2 btrfs: look at bi_size for repair decisions
In reply to Christoph Hellwigbio_readpage_error currently uses bi_vcnt to decide if it is worth retrying an I/O. But the vector count is mostly an implementation artifact - it really should figure out if there is more than a single sector worth retrying. Use bi_size for that and shift by PAGE_SHIFT. This really should be blocks/sectors, but given that btrfs doesn't support a sector size different from the PAGE_SIZE using the page size keeps the changes to a minimum.
--- fs/btrfs/extent_io.c | 2 +- include/linux/bio.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-)
fs/btrfs/extent_io.c +1 -1
@@ -2368,7 +2368,7 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset,int read_mode =0 ;blk_status_t status;int ret; -unsigned failed_bio_pages =bio_pages_all (failed_bio); +unsigned failed_bio_pages = failed_bio->bi_iter.bi_size >>PAGE_SHIFT ;BUG_ON (bio_op (failed_bio) ==REQ_OP_WRITE );
include/linux/bio.h +0 -6
@@ -263,12 +263,6 @@ static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv) bv->bv_len = iter.bi_bvec_done; } -static inline unsigned bio_pages_all (struct bio *bio) -{ -WARN_ON_ONCE (bio_flagged (bio,BIO_CLONED )); -return bio->bi_vcnt; -} -static inline struct bio_vec *bio_first_bvec_all (struct bio *bio) {WARN_ON_ONCE (bio_flagged (bio,BIO_CLONED ));
Signature · 1 line
2.19.1
Re: [PATCH 2/2] btrfs: look at bi_size for repair decisions
Quoted 9 lines · Christoph Hellwig
bio_readpage_errorcurrently usesbi_vcntto decide if it is worth retrying an I/O. But the vector count is mostly an implementation artifact - it really should figure out if there is more than a single sector worth retrying. Usebi_sizefor that and shift byPAGE_SHIFT. This really should be blocks/sectors, but given that btrfs doesn't support a sector size different from thePAGE_SIZEusing the page size keeps the changes to a minimum.Signed-off-by Christoph Hellwig <[email protected]>