2020-08-28 02:54:20

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v3 0/4] 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(-)

--
1.8.3.1


2020-08-28 02:54:25

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v3 2/4] block: Move blk_mq_bio_list_merge() into blk-merge.c

Move the blk_mq_bio_list_merge() into blk-merge.c and
rename it as a generic name.

Signed-off-by: Baolin Wang <[email protected]>
---
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(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 3aa2de5..b09e9fc 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -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);
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index d2790e5..82acff9 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -392,50 +392,6 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
EXPORT_SYMBOL_GPL(blk_mq_sched_try_merge);

/*
- * Iterate list of requests and see if we can merge this bio with any
- * of them.
- */
-bool blk_mq_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_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.
@@ -449,7 +405,7 @@ static bool blk_mq_attempt_merge(struct request_queue *q,

lockdep_assert_held(&ctx->lock);

- if (blk_mq_bio_list_merge(q, &ctx->rq_lists[type], bio, nr_segs)) {
+ if (blk_bio_list_merge(q, &ctx->rq_lists[type], bio, nr_segs)) {
ctx->rq_merged++;
return true;
}
diff --git a/block/blk.h b/block/blk.h
index 49e2928..d6152d2 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -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);
diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c
index a38c5ab..6d4ba0e 100644
--- a/block/kyber-iosched.c
+++ b/block/kyber-iosched.c
@@ -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;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 9d2d5ad..21a02e0 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -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);
--
1.8.3.1

2020-08-28 02:54:49

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v3 3/4] block: Add a new helper to attempt to merge a bio

There 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().

Signed-off-by: Baolin Wang <[email protected]>
---
block/blk-merge.c | 104 ++++++++++++++++++++++++++-------------------------
block/blk-mq-sched.c | 6 +--
block/blk.h | 21 ++++++++---
3 files changed, 71 insertions(+), 60 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index b09e9fc..80c9744 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -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;
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 82acff9..94db0c9 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -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;
}
diff --git a/block/blk.h b/block/blk.h
index d6152d2..a180443 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -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,
--
1.8.3.1

2020-08-28 02:56:53

by Baolin Wang

[permalink] [raw]
Subject: [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.

Signed-off-by: Baolin Wang <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
---
block/blk-core.c | 156 -----------------------------------------------------
block/blk-merge.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 157 insertions(+), 156 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index d9d6326..ed79109 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -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];
diff --git a/block/blk-merge.c b/block/blk-merge.c
index f685d63..3aa2de5 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -11,6 +11,7 @@
#include <trace/events/block.h>

#include "blk.h"
+#include "blk-rq-qos.h"

static inline bool bio_will_gap(struct request_queue *q,
struct request *prev_rq, struct bio *prev, struct bio *next)
@@ -895,3 +896,159 @@ enum elv_merge blk_try_merge(struct request *rq, struct bio *bio)
return ELEVATOR_FRONT_MERGE;
return ELEVATOR_NO_MERGE;
}
+
+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;
+}
--
1.8.3.1

2020-08-28 02:58:22

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v3 4/4] block: Remove blk_mq_attempt_merge() function

The small blk_mq_attempt_merge() function is only called by
__blk_mq_sched_bio_merge(), just open code it.

Signed-off-by: Baolin Wang <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
---
block/blk-mq-sched.c | 44 ++++++++++++++++----------------------------
1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 94db0c9..205d971 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -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;
}

--
1.8.3.1

2020-09-01 07:45:25

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Some clean-ups for bio merge

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2020-09-01 14:44:48

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Some clean-ups for bio merge

On 8/27/20 8:52 PM, Baolin Wang wrote:
> 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.

--
Jens Axboe