[PATCH 0/5] blk-mq updates

[PATCH 0/5] blk-mq updates

[Christoph Hellwig]

Patch 1 makes sure bios are completed more carefully, fixing the regression
with the earlier patch from Mike as well as a few issues found during code
inspection.  Patches 2 and 3 are a split up and better documented version
of "blk-mq: blk-mq should free bios in pass through case", and the last two
are minor cleanups.

[PATCH 1/5] blk-mq: more careful bio completion

[Christoph Hellwig]

[-- Attachment #1: 0001-blk-mq-more-careful-bio-completion.patch --]
[-- Type: text/plain, Size: 1253 bytes --]

Make sure we set bio errors correctly and don't complete bios midway
through a flush sequence.  Largely copied from the old I/O path.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index dece2e2..d2e568e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -269,6 +269,21 @@ void blk_mq_free_request(struct request *rq)
 	__blk_mq_free_request(hctx, ctx, rq);
 }
 
+static void blk_mq_bio_endio(struct request *rq, struct bio *bio, int error)
+{
+	if (error)
+		clear_bit(BIO_UPTODATE, &bio->bi_flags);
+	else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
+		error = -EIO;
+
+	if (unlikely(rq->cmd_flags & REQ_QUIET))
+		set_bit(BIO_QUIET, &bio->bi_flags);
+
+	/* don't actually finish bio if it's part of flush sequence */
+	if (!(rq->cmd_flags & REQ_FLUSH_SEQ))
+		bio_endio(bio, error);
+}
+
 void blk_mq_finish_request(struct request *rq, int error)
 {
 	struct bio *bio = rq->bio;
@@ -279,7 +294,7 @@ void blk_mq_finish_request(struct request *rq, int error)
 
 		bio->bi_next = NULL;
 		bytes += bio->bi_size;
-		bio_endio(bio, error);
+		blk_mq_bio_endio(rq, bio, error);
 		bio = next;
 	}
 
-- 
1.7.10.4

[PATCH 2/5] blk-mq: dont call blk_mq_free_request from blk_mq_finish_request

[Christoph Hellwig]

[-- Attachment #1: 0002-blk-mq-don-t-call-blk_mq_free_request-from-blk_mq_fi.patch --]
[-- Type: text/plain, Size: 1378 bytes --]

From: Mike Christie <michaelc@cs.wisc.edu>

These are fundamentally different operations, so keep them separate.

[hch: split up and reworded the changelog]

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-flush.c |    1 +
 block/blk-mq.c    |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index c56c37d..d835243 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -232,6 +232,7 @@ static void flush_end_io(struct request *flush_rq, int error)
 
 	if (q->mq_ops) {
 		blk_mq_finish_request(flush_rq, error);
+		blk_mq_free_request(flush_rq);
 		spin_lock_irqsave(&q->mq_flush_lock, flags);
 	}
 	running = &q->flush_queue[q->flush_running_idx];
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d2e568e..9c32719 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -300,7 +300,6 @@ void blk_mq_finish_request(struct request *rq, int error)
 
 	blk_account_io_completion(rq, bytes);
 	blk_account_io_done(rq);
-	blk_mq_free_request(rq);
 }
 
 void blk_mq_complete_request(struct request *rq, int error)
@@ -313,8 +312,10 @@ void blk_mq_complete_request(struct request *rq, int error)
 	 */
 	if (rq->end_io)
 		rq->end_io(rq, error);
-	else
+	else {
 		blk_mq_finish_request(rq, error);
+		blk_mq_free_request(rq);
+	}
 
 }
 
-- 
1.7.10.4

[PATCH 3/5] blk-mq: always complete bios in blk_mq_complete_request

[Christoph Hellwig]

[-- Attachment #1: 0003-blk-mq-always-complete-bios-in-blk_mq_complete_reque.patch --]
[-- Type: text/plain, Size: 2256 bytes --]

From: Mike Christie <michaelc@cs.wisc.edu>

Complete bios for all requests going through blk_mq_complete_request.

This mirrors the old I/O path and avoids blk-mq special casing in every
->end_io handler.

[hch: split up and reworded the changelog]

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-flush.c |    1 -
 block/blk-mq.c    |   13 ++++---------
 block/blk-mq.h    |    1 -
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index d835243..3e4cc9c 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -231,7 +231,6 @@ static void flush_end_io(struct request *flush_rq, int error)
 	unsigned long flags = 0;
 
 	if (q->mq_ops) {
-		blk_mq_finish_request(flush_rq, error);
 		blk_mq_free_request(flush_rq);
 		spin_lock_irqsave(&q->mq_flush_lock, flags);
 	}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9c32719..c137541 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -284,7 +284,7 @@ static void blk_mq_bio_endio(struct request *rq, struct bio *bio, int error)
 		bio_endio(bio, error);
 }
 
-void blk_mq_finish_request(struct request *rq, int error)
+static void blk_mq_finish_request(struct request *rq, int error)
 {
 	struct bio *bio = rq->bio;
 	unsigned int bytes = 0;
@@ -306,17 +306,12 @@ void blk_mq_complete_request(struct request *rq, int error)
 {
 	trace_block_rq_complete(rq->q, rq);
 
-	/*
-	 * If ->end_io is set, it's responsible for doing the rest of the
-	 * completion.
-	 */
+	blk_mq_finish_request(rq, error);
+
 	if (rq->end_io)
 		rq->end_io(rq, error);
-	else {
-		blk_mq_finish_request(rq, error);
+	else
 		blk_mq_free_request(rq);
-	}
-
 }
 
 void __blk_mq_end_io(struct request *rq, int error)
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 42d0110..52bf1f9 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -27,7 +27,6 @@ void blk_mq_complete_request(struct request *rq, int error);
 void blk_mq_run_request(struct request *rq, bool run_queue, bool async);
 void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
 void blk_mq_init_flush(struct request_queue *q);
-void blk_mq_finish_request(struct request *rq, int error);
 
 /*
  * CPU hotplug helpers
-- 
1.7.10.4

[PATCH 4/5] blk-mq: kill blk_mq_finish_request

[Christoph Hellwig]

[-- Attachment #1: 0004-blk-mq-kill-blk_mq_finish_request.patch --]
[-- Type: text/plain, Size: 1274 bytes --]

It can be merged into the only caller, and doing so allows accounting for
I/O completion in the correct place as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c137541..99a600e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -284,11 +284,13 @@ static void blk_mq_bio_endio(struct request *rq, struct bio *bio, int error)
 		bio_endio(bio, error);
 }
 
-static void blk_mq_finish_request(struct request *rq, int error)
+void blk_mq_complete_request(struct request *rq, int error)
 {
 	struct bio *bio = rq->bio;
 	unsigned int bytes = 0;
 
+	trace_block_rq_complete(rq->q, rq);
+
 	while (bio) {
 		struct bio *next = bio->bi_next;
 
@@ -299,19 +301,13 @@ static void blk_mq_finish_request(struct request *rq, int error)
 	}
 
 	blk_account_io_completion(rq, bytes);
-	blk_account_io_done(rq);
-}
-
-void blk_mq_complete_request(struct request *rq, int error)
-{
-	trace_block_rq_complete(rq->q, rq);
-
-	blk_mq_finish_request(rq, error);
 
 	if (rq->end_io)
 		rq->end_io(rq, error);
 	else
 		blk_mq_free_request(rq);
+
+	blk_account_io_done(rq);
 }
 
 void __blk_mq_end_io(struct request *rq, int error)
-- 
1.7.10.4

[PATCH 5/5] blk-mq: cleanup blk_mq_bio_to_request

[Christoph Hellwig]

[-- Attachment #1: 0005-blk-mq-cleanup-blk_mq_bio_to_request.patch --]
[-- Type: text/plain, Size: 1731 bytes --]

Remove dead code and a dead argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |   15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 99a600e..2b85029 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -857,15 +857,8 @@ void blk_mq_insert_requests(struct request_queue *q, struct list_head *list,
 						from_schedule);
 }
 
-static void blk_mq_bio_to_request(struct request_queue *q,
-				  struct request *rq, struct bio *bio)
+static void blk_mq_bio_to_request(struct request *rq, struct bio *bio)
 {
-	unsigned int rw_flags;
-
-	rw_flags = bio_data_dir(bio);
-	if (rw_is_sync(bio->bi_rw))
-		rw_flags |= REQ_SYNC;
-
 	init_request_from_bio(rq, bio);
 	blk_account_io_start(rq, 1);
 }
@@ -915,7 +908,7 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
 	hctx->queued++;
 
 	if (unlikely(is_flush_fua)) {
-		blk_mq_bio_to_request(q, rq, bio);
+		blk_mq_bio_to_request(rq, bio);
 		blk_mq_put_ctx(ctx);
 		blk_insert_flush(rq);
 		goto run_queue;
@@ -930,7 +923,7 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		struct blk_plug *plug = current->plug;
 
 		if (plug) {
-			blk_mq_bio_to_request(q, rq, bio);
+			blk_mq_bio_to_request(rq, bio);
 			if (list_empty(&plug->list))
 				trace_block_plug(q);
 			else if (request_count >= BLK_MAX_REQUEST_COUNT) {
@@ -949,7 +942,7 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
 	    blk_mq_attempt_merge(q, ctx, bio))
 		__blk_mq_free_request(hctx, ctx, rq);
 	else {
-		blk_mq_bio_to_request(q, rq, bio);
+		blk_mq_bio_to_request(rq, bio);
 		__blk_mq_insert_request(hctx, rq);
 	}
 
-- 
1.7.10.4

Re: [PATCH 0/5] blk-mq updates

[Jens Axboe]

On Sun, Oct 06 2013, Christoph Hellwig wrote:
> Patch 1 makes sure bios are completed more carefully, fixing the regression
> with the earlier patch from Mike as well as a few issues found during code
> inspection.  Patches 2 and 3 are a split up and better documented version
> of "blk-mq: blk-mq should free bios in pass through case", and the last two
> are minor cleanups.

Looks good, applied! Thanks Christoph.

-- 
Jens Axboe