Some clean-ups for bio merge
v3 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.
Changes from v2:
- Split blk_mq_bio_list_merge() moving into a separate patch.
- Add reviewed-by tag from Christoph.
- Coding style improvement.
Changes from v1:
- Drop patch 2 and patch 5 in v1 patch set.
- Add reviewed-by tag from Christoph.
- Move blk_mq_bio_list_merge() into blk-merge.c and rename it.
- Some coding style improvements.
Baolin Wang (4):
block: Move bio merge related functions into blk-merge.c
block: Move blk_mq_bio_list_merge() into blk-merge.c
block: Add a new helper to attempt to merge a bio
block: Remove blk_mq_attempt_merge() function
block/blk-core.c | 156 ------------------------------------- block/blk-merge.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++ block/blk-mq-sched.c | 94 +++++------------------ block/blk.h | 23 ++++-- block/kyber-iosched.c | 2 +- include/linux/blk-mq.h | 2 - 6 files changed, 240 insertions(+), 240 deletions(-)
Signature · 1 line
1.8.3.1
PATCH v3 1/4 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 v3 2/4 block: Move blk_mq_bio_list_merge() into blk-merge.c
In reply to Baolin WangMove the blk_mq_bio_list_merge() into blk-merge.c and rename it as a generic name.
--- block/blk-merge.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ block/blk-mq-sched.c | 46 +--------------------------------------------- block/blk.h | 2 ++ block/kyber-iosched.c | 2 +- include/linux/blk-mq.h | 2 -- 5 files changed, 48 insertions(+), 48 deletions(-)
block/blk-merge.c +44 -0
@@ -1052,3 +1052,47 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,return false; } + +/* +* Iterate list of requests and see if we can merge this bio with any +* of them. +*/ +bool blk_bio_list_merge (struct request_queue *q,struct list_head *list, +struct bio *bio,unsigned int nr_segs) +{ +struct request *rq; +int checked =8 ; + +list_for_each_entry_reverse (rq, list, queuelist) { +bool merged = false; + +if (!checked--) +break ; + +if (!blk_rq_merge_ok (rq, bio)) +continue ; + +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); +break ; +case ELEVATOR_FRONT_MERGE : +if (blk_mq_sched_allow_merge (q, rq, bio)) + 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 false; +} +EXPORT_SYMBOL_GPL (blk_bio_list_merge);
block/blk-mq-sched.c +1 -45
58 lines not on this page · raw message
block/blk.h +2 -0
@@ -177,6 +177,8 @@ bool bio_attempt_discard_merge(struct request_queue *q, struct request *req, struct bio *bio);bool blk_attempt_plug_merge (struct request_queue *q,struct bio *bio,unsigned int nr_segs,struct request **same_queue_rq); +bool blk_bio_list_merge (struct request_queue *q,struct list_head *list, +struct bio *bio,unsigned int nr_segs);void blk_account_io_start (struct request *req);void blk_account_io_done (struct request *req,u64 now);
block/kyber-iosched.c +1 -1
@@ -573,7 +573,7 @@ static bool kyber_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio,bool merged;spin_lock (&kcq->lock); - merged =blk_mq_bio_list_merge (hctx->queue, rq_list, bio, nr_segs); + merged =blk_bio_list_merge (hctx->queue, rq_list, bio, nr_segs);spin_unlock (&kcq->lock);return merged;
include/linux/blk-mq.h +0 -2
@@ -489,8 +489,6 @@ static inline int blk_mq_request_completed(struct request *rq)void blk_mq_delay_kick_requeue_list (struct request_queue *q,unsigned long msecs);void blk_mq_complete_request (struct request *rq);bool blk_mq_complete_request_remote (struct request *rq); -bool blk_mq_bio_list_merge (struct request_queue *q,struct list_head *list, -struct bio *bio,unsigned int nr_segs);bool blk_mq_queue_stopped (struct request_queue *q);void blk_mq_stop_hw_queue (struct blk_mq_hw_ctx *hctx);void blk_mq_start_hw_queue (struct blk_mq_hw_ctx *hctx);
Signature · 1 line
1.8.3.1
PATCH v3 3/4 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_bio_list_merge() and blk_attempt_plug_merge().
--- block/blk-merge.c | 104 ++++++++++++++++++++++++++------------------------- block/blk-mq-sched.c | 6 +-- block/blk.h | 21 ++++++++--- 3 files changed, 71 insertions(+), 60 deletions(-)
block/blk-merge.c +53 -51
@@ -907,13 +907,14 @@ static void blk_account_io_merge_bio(struct request *req)part_stat_unlock (); } -bool bio_attempt_back_merge (struct request *req,struct bio *bio, -unsigned int nr_segs) +enum bio_merge_status 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; +return BIO_MERGE_FAILED ;trace_block_bio_backmerge (req->q, req, bio);rq_qos_merge (req->q, req, bio);
@@ -928,16 +929,17 @@ bool bio_attempt_back_merge(struct request *req, struct bio *bio,bio_crypt_free_ctx (bio);blk_account_io_merge_bio (req); -return true; +return BIO_MERGE_OK ; } -bool bio_attempt_front_merge (struct request *req,struct bio *bio, -unsigned int nr_segs) +enum bio_merge_status 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; +return BIO_MERGE_FAILED ;trace_block_bio_frontmerge (req->q, req, bio);rq_qos_merge (req->q, req, bio);
@@ -954,11 +956,12 @@ bool bio_attempt_front_merge(struct request *req, struct bio *bio,bio_crypt_do_front_merge (req, bio);blk_account_io_merge_bio (req); -return true; +return BIO_MERGE_OK ; } -bool bio_attempt_discard_merge (struct request_queue *q,struct request *req, -struct bio *bio) +enum bio_merge_status bio_attempt_discard_merge (struct request_queue *q, +struct request *req, +struct bio *bio) {unsigned short segments =blk_rq_nr_discard_segments (req);
@@ -976,10 +979,39 @@ bool bio_attempt_discard_merge(struct request_queue *q, struct request *req, req->nr_phys_segments = segments +1 ;blk_account_io_merge_bio (req); -return true; +return BIO_MERGE_OK ; no_merge:req_set_nomerge (q, req); -return false; +return BIO_MERGE_FAILED ; +} + +static enum bio_merge_status blk_attempt_bio_merge (struct request_queue *q, +struct request *rq, +struct bio *bio, +unsigned int nr_segs, +bool sched_allow_merge) +{ +if (!blk_rq_merge_ok (rq, bio)) +return BIO_MERGE_NONE ; + +switch (blk_try_merge (rq, bio)) { +case ELEVATOR_BACK_MERGE : +if (!sched_allow_merge || + (sched_allow_merge &&blk_mq_sched_allow_merge (q, rq, bio))) +return bio_attempt_back_merge (rq, bio, nr_segs); +break ; +case ELEVATOR_FRONT_MERGE : +if (!sched_allow_merge || + (sched_allow_merge &&blk_mq_sched_allow_merge (q, rq, bio))) +return bio_attempt_front_merge (rq, bio, nr_segs); +break ; +case ELEVATOR_DISCARD_MERGE : +return bio_attempt_discard_merge (q, rq, bio); +default : +return BIO_MERGE_NONE ; + } + +return BIO_MERGE_FAILED ; }/**
@@ -1018,8 +1050,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
@@ -1029,24 +1059,11 @@ 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, false) == +BIO_MERGE_OK )return true; }
@@ -1064,33 +1081,18 @@ bool blk_bio_list_merge(struct request_queue *q, struct list_head *list,int checked =8 ;list_for_each_entry_reverse (rq, list, queuelist) { -bool merged = false; -if (!checked--)break ; -if (!blk_rq_merge_ok (rq, bio)) -continue ; - -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); -break ; -case ELEVATOR_FRONT_MERGE : -if (blk_mq_sched_allow_merge (q, rq, bio)) - merged =bio_attempt_front_merge (rq, bio, - nr_segs); -break ; -case ELEVATOR_DISCARD_MERGE : - merged =bio_attempt_discard_merge (q, rq, bio); -break ; -default : +switch (blk_attempt_bio_merge (q, rq, bio, nr_segs, true)) { +case BIO_MERGE_NONE :continue ; +case BIO_MERGE_OK : +return true; +case BIO_MERGE_FAILED : +return false; } -return merged; }return false;
block/blk-mq-sched.c +3 -3
@@ -368,7 +368,7 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,case ELEVATOR_BACK_MERGE :if (!blk_mq_sched_allow_merge (q, rq, bio))return false; -if (!bio_attempt_back_merge (rq, bio, nr_segs)) +if (bio_attempt_back_merge (rq, bio, nr_segs) !=BIO_MERGE_OK )return false; *merged_request =attempt_back_merge (q, rq);if (!*merged_request)
@@ -377,14 +377,14 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,case ELEVATOR_FRONT_MERGE :if (!blk_mq_sched_allow_merge (q, rq, bio))return false; -if (!bio_attempt_front_merge (rq, bio, nr_segs)) +if (bio_attempt_front_merge (rq, bio, nr_segs) !=BIO_MERGE_OK )return false; *merged_request =attempt_front_merge (q, rq);if (!*merged_request)elv_merged_request (q, rq,ELEVATOR_FRONT_MERGE );return true;case ELEVATOR_DISCARD_MERGE : -return bio_attempt_discard_merge (q, rq, bio); +return bio_attempt_discard_merge (q, rq, bio) ==BIO_MERGE_OK ;default :return false; }
block/blk.h +15 -6
@@ -29,6 +29,12 @@ struct blk_flush_queue {spinlock_t mq_flush_lock; }; +enum bio_merge_status { +BIO_MERGE_OK , +BIO_MERGE_NONE , +BIO_MERGE_FAILED , +}; +extern struct kmem_cache *blk_requestq_cachep;extern struct kobj_type blk_queue_ktype;extern struct ida blk_queue_ida;
@@ -169,12 +175,15 @@ static inline void blk_integrity_del(struct gendisk *disk)unsigned long blk_rq_timeout (unsigned long timeout);void blk_add_timer (struct request *req); -bool bio_attempt_front_merge (struct request *req,struct bio *bio, -unsigned int nr_segs); -bool bio_attempt_back_merge (struct request *req,struct bio *bio, -unsigned int nr_segs); -bool bio_attempt_discard_merge (struct request_queue *q,struct request *req, -struct bio *bio); +enum bio_merge_status bio_attempt_front_merge (struct request *req, +struct bio *bio, +unsigned int nr_segs); +enum bio_merge_status bio_attempt_back_merge (struct request *req, +struct bio *bio, +unsigned int nr_segs); +enum bio_merge_status bio_attempt_discard_merge (struct request_queue *q, +struct request *req, +struct bio *bio);bool blk_attempt_plug_merge (struct request_queue *q,struct bio *bio,unsigned int nr_segs,struct request **same_queue_rq); bool blk_bio_list_merge(struct request_queue *q, struct list_head *list,
Signature · 1 line
1.8.3.1
PATCH v3 4/4 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 | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-)
block/blk-mq-sched.c +16 -28
@@ -391,28 +391,6 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio, }EXPORT_SYMBOL_GPL (blk_mq_sched_try_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_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) {
@@ -426,14 +404,24 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,return e->type->ops.bio_merge (hctx, bio, nr_segs); type = hctx->type; -if ((hctx->flags &BLK_MQ_F_SHOULD_MERGE ) && - !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); -spin_unlock (&ctx->lock); +if (!(hctx->flags &BLK_MQ_F_SHOULD_MERGE ) || +list_empty_careful (&ctx->rq_lists[type])) +return false; + +/* default per sw-queue merge */ +spin_lock (&ctx->lock); +/* +* 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_bio_list_merge (q, &ctx->rq_lists[type], bio, nr_segs)) { + ctx->rq_merged++; + ret = true; } +spin_unlock (&ctx->lock); +return ret; }
Signature · 1 line
1.8.3.1
Looks good,
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.
Applied, thanks.
Signature · 1 line
Jens Axboe