Re: [PATCH RESEND 3/5] block: Add a new helper to attempt to merge a bio
From: Christoph Hellwig <hch@lst.de>
Date: 2020-08-17 06:26:41
Also in:
lkml
On Mon, Aug 17, 2020 at 12:09:17PM +0800, Baolin Wang wrote:
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_mq_bio_list_merge() and blk_attempt_plug_merge().
Looks sensible, but two comments:
+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;Can't we also switch the bio_attempt_*merge helpers to return enum bio_merge_status to simplify this a bit? Also I think these helpers can be marked static now, although I didn't actually apply your series, so I might have missed something.
quoted hunk ↗ jump to hunk
+++ b/block/blk-mq-sched.c@@ -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; + merge = blk_attempt_bio_merge(q, rq, bio, nr_segs); + if (merge == BIO_MERGE_NONE) continue; + return merge == BIO_MERGE_OK ? true: false;
Maybe write this a little more explicit:
switch (blk_attempt_bio_merge(q, rq, bio, nr_segs)) {
case BIO_MERGE_NONE:
continue:
case BIO_MERGE_OK:
return true;
case BIO_MERGE_FAILED:
return false;
}
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);
/*
--
1.8.3.1---end quoted text---