[6/7] block: prevent merging of requests with different priorities

Commit Message

Damien Le Moal Nov. 19, 2018, 3:51 a.m. UTC

Growing in size a high priority request by merging it with a lower
priority BIO or request will increase the request execution time. This
is the opposite result of the desired effect of high I/O priorities,
namely getting low I/O latencies. Prevent merging of requests and BIOs
that have different I/O priorities to fix this.

Signed-off-by: Damien Le Moal <[e-mail]>
---
 block/blk-core.c  |  3 ---
 block/blk-merge.c | 14 ++++++++++++--
 2 files changed, 12 insertions(+), 5 deletions(-)

Comments

Christoph Hellwig Nov. 19, 2018, 8:19 a.m. UTC | #1

> +	/*
> +	 * Don't allow merge of different I/O priorities.
> +	 */
> +	if (req->ioprio != next->ioprio)
> +		return NULL;

> +	/*
> +	 * Don't allow merge of different I/O priorities.
> +	 */
> +	if (rq->ioprio != bio_prio(bio))
> +		return false;
> +

These comments just restate what is obvious in the code.  I would
recommend to drop them.

Johannes Thumshirn Nov. 19, 2018, 8:31 a.m. UTC | #2

Looks good,
Reviewed-by: Johannes Thumshirn <[e-mail]>

Patch

diff --git a/block/blk-core.c b/block/blk-core.c
index 4450d3c08f25..dde30b08aa14 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -662,7 +662,6 @@  bool bio_attempt_back_merge(struct request_queue *q, struct request *req,
 	req->biotail->bi_next = bio;
 	req->biotail = bio;
 	req->__data_len += bio->bi_iter.bi_size;
-	req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
 
 	blk_account_io_start(req, false);
 	return true;
@@ -686,7 +685,6 @@  bool bio_attempt_front_merge(struct request_queue *q, struct request *req,
 
 	req->__sector = bio->bi_iter.bi_sector;
 	req->__data_len += bio->bi_iter.bi_size;
-	req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
 
 	blk_account_io_start(req, false);
 	return true;
@@ -706,7 +704,6 @@  bool bio_attempt_discard_merge(struct request_queue *q, struct request *req,
 	req->biotail->bi_next = bio;
 	req->biotail = bio;
 	req->__data_len += bio->bi_iter.bi_size;
-	req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
 	req->nr_phys_segments = segments + 1;
 
 	blk_account_io_start(req, false);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index b1df622cbd85..129f554d250d 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -752,6 +752,12 @@  static struct request *attempt_merge(struct request_queue *q,
 	if (req->write_hint != next->write_hint)
 		return NULL;
 
+	/*
+	 * Don't allow merge of different I/O priorities.
+	 */
+	if (req->ioprio != next->ioprio)
+		return NULL;
+
 	/*
 	 * If we are allowed to merge, then append bio list
 	 * from next to rq and release next. merge_requests_fn
@@ -807,8 +813,6 @@  static struct request *attempt_merge(struct request_queue *q,
 	 */
 	blk_account_io_merge(next);
 
-	req->ioprio = ioprio_best(req->ioprio, next->ioprio);
-
 	/*
 	 * ownership of bio passed from next to req, return 'next' for
 	 * the caller to free
@@ -883,6 +887,12 @@  bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
 	if (rq->write_hint != bio->bi_write_hint)
 		return false;
 
+	/*
+	 * Don't allow merge of different I/O priorities.
+	 */
+	if (rq->ioprio != bio_prio(bio))
+		return false;
+
 	return true;
 }