Some clean-ups for bio merge
cover Some clean-ups for bio merge
Hi,
There are some duplicated code when trying to merge bio from pluged list and software queue, thus this patch set did some clean-ups when merging a bio. Any comments are welcome. Thanks.
Baolin Wang (5):
block: Move bio merge related functions into blk-merge.c
block: Remove redundant blk_mq_sched_allow_merge() validation
block: Add a new helper to attempt to merge a bio
block: Remove blk_mq_attempt_merge() function
block: Remove __blk_mq_sched_bio_merge() helper
block/blk-core.c | 156 ----------------------------------------------- block/blk-merge.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++ block/blk-mq-sched.c | 64 ++++++-------------- block/blk-mq-sched.h | 13 +--- block/blk.h | 9 +++ 5 files changed, 197 insertions(+), 213 deletions(-)
Signature · 1 line
1.8.3.1
PATCH 1/5 block: Move bio merge related functions into blk-merge.c
It's better to move bio merge related functions into blk-merge.c, which contains all merge related functions.
--- block/blk-core.c | 156 ----------------------------------------------------- block/blk-merge.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 156 deletions(-)
block/blk-core.c +0 -156
@@ -642,162 +642,6 @@ void blk_put_request(struct request *req) }EXPORT_SYMBOL (blk_put_request); -static void blk_account_io_merge_bio (struct request *req) -{ -if (!blk_do_io_stat (req)) -return ; - -part_stat_lock (); -part_stat_inc (req->part, merges[op_stat_group (req_op (req))]); -part_stat_unlock (); -} - -bool bio_attempt_back_merge (struct request *req,struct bio *bio, -unsigned int nr_segs) -{ -const int ff = bio->bi_opf &REQ_FAILFAST_MASK ; - -if (!ll_back_merge_fn (req, bio, nr_segs)) -return false; - -trace_block_bio_backmerge (req->q, req, bio); -rq_qos_merge (req->q, req, bio); - -if ((req->cmd_flags &REQ_FAILFAST_MASK ) != ff) -blk_rq_set_mixed_merge (req); - - req->biotail->bi_next = bio; - req->biotail = bio; - req->__data_len += bio->bi_iter.bi_size; - -bio_crypt_free_ctx (bio); - -blk_account_io_merge_bio (req); -return true; -} - -bool bio_attempt_front_merge (struct request *req,struct bio *bio, -unsigned int nr_segs) -{ -const int ff = bio->bi_opf &REQ_FAILFAST_MASK ; - -if (!ll_front_merge_fn (req, bio, nr_segs)) -return false; - -trace_block_bio_frontmerge (req->q, req, bio); -rq_qos_merge (req->q, req, bio); - -if ((req->cmd_flags &REQ_FAILFAST_MASK ) != ff) -blk_rq_set_mixed_merge (req); - - bio->bi_next = req->bio; - req->bio = bio; - - req->__sector = bio->bi_iter.bi_sector; - req->__data_len += bio->bi_iter.bi_size; - -bio_crypt_do_front_merge (req, bio); - -blk_account_io_merge_bio (req); -return true; -} - -bool bio_attempt_discard_merge (struct request_queue *q,struct request *req, -struct bio *bio) -{ -unsigned short segments =blk_rq_nr_discard_segments (req); - -if (segments >=queue_max_discard_segments (q)) - goto no_merge; -if (blk_rq_sectors (req) +bio_sectors (bio) > -blk_rq_get_max_sectors (req,blk_rq_pos (req))) - goto no_merge; - -rq_qos_merge (q, req, bio); - - req->biotail->bi_next = bio; - req->biotail = bio; - req->__data_len += bio->bi_iter.bi_size; - req->nr_phys_segments = segments +1 ; - -blk_account_io_merge_bio (req); -return true; -no_merge: -req_set_nomerge (q, req); -return false; -} - -/** -* blk_attempt_plug_merge - try to merge with %current's plugged list -* @q: request_queue new bio is being queued at -* @bio: new bio being queued -* @nr_segs: number of segments in @bio -* @same_queue_rq: pointer to &struct request that gets filled in when -* another request associated with @q is found on the plug list -* (optional, may be %NULL) -* -* Determine whether @bio being queued on @q can be merged with a request -* on %current's plugged list. Returns %true if merge was successful, -* otherwise %false. -* -* Plugging coalesces IOs from the same issuer for the same purpose without -* going through @q->queue_lock. As such it's more of an issuing mechanism -* than scheduling, and the request, while may have elvpriv data, is not -* added on the elevator at this point. In addition, we don't have -* reliable access to the elevator outside queue lock. Only check basic -* merging parameters without querying the elevator. -* -* Caller must ensure !blk_queue_nomerges(q) beforehand. -*/ -bool blk_attempt_plug_merge (struct request_queue *q,struct bio *bio, -unsigned int nr_segs,struct request **same_queue_rq) -{ -struct blk_plug *plug; -struct request *rq; -struct list_head *plug_list; - - plug =blk_mq_plug (q, bio); -if (!plug) -return false; - - plug_list = &plug->mq_list; - -list_for_each_entry_reverse (rq, plug_list, queuelist) { -bool merged = false; - -if (rq->q == q && same_queue_rq) { -/* -* Only blk-mq multiple hardware queues case checks the -* rq in the same queue, there should be only one such -* rq in a queue -**/ - *same_queue_rq = rq; - } - -if (rq->q != q || !blk_rq_merge_ok (rq, bio)) -continue ; - -switch (blk_try_merge (rq, bio)) { -case ELEVATOR_BACK_MERGE : - merged =bio_attempt_back_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_FRONT_MERGE : - merged =bio_attempt_front_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_DISCARD_MERGE : - merged =bio_attempt_discard_merge (q, rq, bio); -break ; -default : -break ; - } - -if (merged) -return true; - } - -return false; -} -static void handle_bad_sector (struct bio *bio,sector_t maxsector) {char b[BDEVNAME_SIZE ];
block/blk-merge.c +157 -0
166 lines not on this page · raw message
Signature · 1 line
1.8.3.1
PATCH 2/5 block: Remove redundant blk_mq_sched_allow_merge() validation
In reply to Baolin WangOnly software queue and kyber IO scheduler will call blk_mq_bio_list_merge() to merge a bio, and kyber IO scheduler did not implement the ops->allow_merge(). Thus we can remove the redundant blk_mq_sched_allow_merge() in blk_mq_bio_list_merge() function.
--- block/blk-mq-sched.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
block/blk-mq-sched.c +2 -6
@@ -403,14 +403,10 @@ bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list,switch (blk_try_merge (rq, bio)) {case ELEVATOR_BACK_MERGE : -if (blk_mq_sched_allow_merge (q, rq, bio)) - merged =bio_attempt_back_merge (rq, bio, - nr_segs); + merged =bio_attempt_back_merge (rq, bio, nr_segs);break ;case ELEVATOR_FRONT_MERGE : -if (blk_mq_sched_allow_merge (q, rq, bio)) - merged =bio_attempt_front_merge (rq, bio, - nr_segs); + merged =bio_attempt_front_merge (rq, bio, nr_segs);break ;case ELEVATOR_DISCARD_MERGE : merged =bio_attempt_discard_merge (q, rq, bio);
Signature · 1 line
1.8.3.1
PATCH 3/5 block: Add a new helper to attempt to merge a bio
In reply to Baolin WangThere are lots of duplicated code when trying to merge a bio from plug list and sw queue, we can introduce a new helper to attempt to merge a bio, which can simplify the blk_mq_bio_list_merge() and blk_attempt_plug_merge().
--- block/blk-merge.c | 47 +++++++++++++++++++++++++++++------------------ block/blk-mq-sched.c | 22 ++++------------------ block/blk.h | 9 +++++++++ 3 files changed, 42 insertions(+), 36 deletions(-)
block/blk-merge.c +29 -18
@@ -975,6 +975,33 @@ bool bio_attempt_discard_merge(struct request_queue *q, struct request *req,return false; } +enum bio_merge_status blk_attempt_bio_merge (struct request_queue *q, +struct request *rq, +struct bio *bio, +unsigned int nr_segs) +{ +bool merged = false; + +if (!blk_rq_merge_ok (rq, bio)) +return BIO_MERGE_NONE ; + +switch (blk_try_merge (rq, bio)) { +case ELEVATOR_BACK_MERGE : + merged =bio_attempt_back_merge (rq, bio, nr_segs); +break ; +case ELEVATOR_FRONT_MERGE : + merged =bio_attempt_front_merge (rq, bio, nr_segs); +break ; +case ELEVATOR_DISCARD_MERGE : + merged =bio_attempt_discard_merge (q, rq, bio); +break ; +default : +return BIO_MERGE_NONE ; + } + +return merged ?BIO_MERGE_OK :BIO_MERGE_FAILED ; +} +/** * blk_attempt_plug_merge - try to merge with %current's plugged list * @q: request_queue new bio is being queued at
@@ -1011,8 +1038,6 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio, plug_list = &plug->mq_list;list_for_each_entry_reverse (rq, plug_list, queuelist) { -bool merged = false; -if (rq->q == q && same_queue_rq) {/* * Only blk-mq multiple hardware queues case checks the
@@ -1022,24 +1047,10 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio, *same_queue_rq = rq; } -if (rq->q != q || !blk_rq_merge_ok (rq, bio)) +if (rq->q != q)continue ; -switch (blk_try_merge (rq, bio)) { -case ELEVATOR_BACK_MERGE : - merged =bio_attempt_back_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_FRONT_MERGE : - merged =bio_attempt_front_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_DISCARD_MERGE : - merged =bio_attempt_discard_merge (q, rq, bio); -break ; -default : -break ; - } - -if (merged) +if (blk_attempt_bio_merge (q, rq, bio, nr_segs) ==BIO_MERGE_OK )return true; }
block/blk-mq-sched.c +4 -18
@@ -391,31 +391,17 @@ bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list, {struct request *rq;int checked =8 ; +enum bio_merge_status merge;list_for_each_entry_reverse (rq, list, queuelist) { -bool merged = false; -if (!checked--)break ; -if (!blk_rq_merge_ok (rq, bio)) + merge =blk_attempt_bio_merge (q, rq, bio, nr_segs); +if (merge ==BIO_MERGE_NONE )continue ; -switch (blk_try_merge (rq, bio)) { -case ELEVATOR_BACK_MERGE : - merged =bio_attempt_back_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_FRONT_MERGE : - merged =bio_attempt_front_merge (rq, bio, nr_segs); -break ; -case ELEVATOR_DISCARD_MERGE : - merged =bio_attempt_discard_merge (q, rq, bio); -break ; -default : -continue ; - } - -return merged; +return merge ==BIO_MERGE_OK ? true: false; }return false;
block/blk.h +9 -0
@@ -234,6 +234,15 @@ int blk_attempt_req_merge(struct request_queue *q, struct request *rq,bool blk_rq_merge_ok (struct request *rq,struct bio *bio);enum elv_merge blk_try_merge (struct request *rq,struct bio *bio); +enum bio_merge_status { +BIO_MERGE_OK , +BIO_MERGE_NONE , +BIO_MERGE_FAILED , +}; + +enum bio_merge_status blk_attempt_bio_merge (struct request_queue *q, +struct request *rq,struct bio *bio,unsigned int nr_segs); +int blk_dev_init (void );/*
Signature · 1 line
1.8.3.1
PATCH 4/5 block: Remove blk_mq_attempt_merge() function
In reply to Baolin WangThe small blk_mq_attempt_merge() function is only called by __blk_mq_sched_bio_merge(), just open code it.
--- block/blk-mq-sched.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-)
block/blk-mq-sched.c +10 -23
@@ -408,28 +408,6 @@ bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list, }EXPORT_SYMBOL_GPL (blk_mq_bio_list_merge); -/* -* Reverse check our software queue for entries that we could potentially -* merge with. Currently includes a hand-wavy stop count of 8, to not spend -* too much time checking for merges. -*/ -static bool blk_mq_attempt_merge (struct request_queue *q, -struct blk_mq_hw_ctx *hctx, -struct blk_mq_ctx *ctx,struct bio *bio, -unsigned int nr_segs) -{ -enum hctx_type type = hctx->type; - -lockdep_assert_held (&ctx->lock); - -if (blk_mq_bio_list_merge (q, &ctx->rq_lists[type], bio, nr_segs)) { - ctx->rq_merged++; -return true; - } - -return false; -} -bool __blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio,unsigned int nr_segs) {
@@ -447,7 +425,16 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, !list_empty_careful(&ctx->rq_lists[type])) {/* default per sw-queue merge */ spin_lock (&ctx->lock); - ret =blk_mq_attempt_merge (q, hctx, ctx, bio, nr_segs); +/* +* Reverse check our software queue for entries that we could +* potentially merge with. Currently includes a hand-wavy stop +* count of 8, to not spend too much time checking for merges. +*/ +if (blk_mq_bio_list_merge (q, &ctx->rq_lists[type], bio, nr_segs)) { + ctx->rq_merged++; + ret = true; + } +spin_unlock (&ctx->lock); }
Signature · 1 line
1.8.3.1
PATCH 5/5 block: Remove __blk_mq_sched_bio_merge() helper
In reply to Baolin WangThe blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and no other places will use __blk_mq_sched_bio_merge(). Thus we can combine these 2 similar functions into one function.
--- block/blk-mq-sched.c | 5 ++++- block/blk-mq-sched.h | 13 ++----------- 2 files changed, 6 insertions(+), 12 deletions(-)
block/blk-mq-sched.c +4 -1
@@ -408,7 +408,7 @@ bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list, }EXPORT_SYMBOL_GPL (blk_mq_bio_list_merge); -bool __blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio, +bool blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio,unsigned int nr_segs) {struct elevator_queue *e = q->elevator;
@@ -417,6 +417,9 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,bool ret = false;enum hctx_type type; +if (blk_queue_nomerges (q) || !bio_mergeable (bio)) +return false; +if (e && e->type->ops.bio_merge)return e->type->ops.bio_merge (hctx, bio, nr_segs);
block/blk-mq-sched.h +2 -11
@@ -13,8 +13,6 @@ void blk_mq_sched_free_hctx_data(struct request_queue *q,void blk_mq_sched_request_inserted (struct request *rq);bool blk_mq_sched_try_merge (struct request_queue *q,struct bio *bio,unsigned int nr_segs,struct request **merged_request); -bool __blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio, -unsigned int nr_segs);bool blk_mq_sched_try_insert_merge (struct request_queue *q,struct request *rq);void blk_mq_sched_mark_restart_hctx (struct blk_mq_hw_ctx *hctx);void blk_mq_sched_restart (struct blk_mq_hw_ctx *hctx);
@@ -31,15 +29,8 @@ void blk_mq_sched_insert_requests(struct blk_mq_hw_ctx *hctx,void blk_mq_exit_sched (struct request_queue *q,struct elevator_queue *e);void blk_mq_sched_free_requests (struct request_queue *q); -static inline bool -blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio, -unsigned int nr_segs) -{ -if (blk_queue_nomerges (q) || !bio_mergeable (bio)) -return false; - -return __blk_mq_sched_bio_merge (q, bio, nr_segs); -} +bool blk_mq_sched_bio_merge (struct request_queue *q,struct bio *bio, +unsigned int nr_segs); static inline bool blk_mq_sched_allow_merge(struct request_queue *q, struct request *rq,
Signature · 1 line
1.8.3.1
Hi,
Quoted 5 lines · Baolin Wang
Hi,
There are some duplicated code when trying to merge bio from pluged list and software queue, thus this patch set did some clean-ups when merging a bio. Any comments are welcome. Thanks.
Any comments for this patch set? Thanks.
Quoted 17 lines
Baolin Wang (5):
block: Move bio merge related functions intoblk-merge.c
block: Remove redundantblk_mq_sched_allow_merge()validation
block: Add a new helper to attempt to merge a bio
block: Removeblk_mq_attempt_merge()function
block: Remove__blk_mq_sched_bio_merge()helperblock/blk-core.c | 156 ----------------------------------------------- block/blk-merge.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++ block/blk-mq-sched.c | 64 ++++++-------------- block/blk-mq-sched.h | 13 +--- block/blk.h | 9 +++ 5 files changed, 197 insertions(+), 213 deletions(-)Signature · 1 line
1.8.3.1