Skip to contentSkip to conversation

blk-mq: include errors in did_work calculation

PATCH In review Jens Axboe 4 participants 4 messages Mar 24, 2017 Block layer

lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

PATCH 1/1 blk-mq: include errors in did_work calculation

Currently we return true in blk_mq_dispatch_rq_list() if we queued IO successfully, but we really want to return whether or not the we made progress. Progress includes if we got an error return. If we don't, this can lead to a hang in blk_mq_sched_dispatch_requests() when a driver is draining IO by returning BLK_MQ_QUEUE_ERROR instead of manually ending the IO in error and return BLK_MQ_QUEUE_OK.

Signed-off-by Jens Axboe <[email protected]>
block/blk-mq.c +4 -3
index a4546f060e80..e3b09abf9d5b 100644
@@ -978,7 +978,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)
 	struct request *rq;
 	LIST_HEAD(driver_list);
 	struct list_head *dptr;
-	int queued, ret = BLK_MQ_RQ_QUEUE_OK;
+	int errors, queued, ret = BLK_MQ_RQ_QUEUE_OK;
 
 	/*
 	 * Start off with dptr being NULL, so we start the first request

@@ -989,7 +989,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)
 	/*
 	 * Now process all the entries, sending them to the driver.
 	 */
-	queued = 0;
+	errors = queued = 0;
 	while (!list_empty(list)) {
 		struct blk_mq_queue_data bd;
 

@@ -1046,6 +1046,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)
 		default:
 			pr_err("blk-mq: bad return on queue: %d\n", ret);
 		case BLK_MQ_RQ_QUEUE_ERROR:
+			errors++;
 			rq->errors = -EIO;
 			blk_mq_end_request(rq, rq->errors);
 			break;

@@ -1097,7 +1098,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)
 			blk_mq_run_hw_queue(hctx, true);
 	}
 
-	return queued != 0;
+	return (queued + errors) != 0;
 }
 
 static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
Tested-by on v1 1/1
#1 lore raw

block/blk-mq.c bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx =

Quoted 17 lines · Jens Axboe

=20
Currently we return true in blk_mq_dispatch_rq_list() if we queued IO successfully, but we really want to return whether or not the we made progress. Progress includes if we got an error return. If we don't, this can lead to a hang in blk_mq_sched_dispatch_requests() when a driver is draining IO by returning BLK_MQ_QUEUE_ERROR instead of manually ending the IO in error and return BLK_MQ_QUEUE_OK. =20

Signed-off-by Jens Axboe <[email protected]>

=20

block/blk-mq.c +0 -0
index a4546f060e80..e3b09abf9d5b 100644
@@ -978,7 +978,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx =

*hctx, struct list_head *list)

block/blk-mq.c bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx =

Quoted 9 lines
block/blk-mq.c +1 -1
 	struct request *rq;
 	LIST_HEAD(driver_list);
 	struct list_head *dptr;
-	int queued, ret =3D BLK_MQ_RQ_QUEUE_OK;
+	int errors, queued, ret =3D BLK_MQ_RQ_QUEUE_OK;

=20

	/*
	 * Start off with dptr being NULL, so we start the first request
block/blk-mq.c +0 -0
@@ -989,7 +989,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx =

*hctx, struct list_head *list)

block/blk-mq.c bool blk_mq_dispatch_rq_list(struct =

Quoted 9 lines
block/blk-mq.c +1 -1
 	/*
 	 * Now process all the entries, sending them to the driver.
 	 */
-	queued =3D 0;
+	errors =3D queued =3D 0;
 	while (!list_empty(list)) {
 		struct blk_mq_queue_data bd;

=20

block/blk-mq.c +0 -0
@@ -1046,6 +1046,7 @@ bool blk_mq_dispatch_rq_list(struct =

blk_mq_hw_ctx *hctx, struct list_head *list)

block/blk-mq.c bool blk_mq_dispatch_rq_list(struct = · line 1047

Quoted 2 lines
block/blk-mq.c +0 -0
 		default:
 			pr_err("blk-mq: bad return on queue: %d\n", =

ret);

block/blk-mq.c bool blk_mq_dispatch_rq_list(struct =

Quoted 6 lines
block/blk-mq.c +1 -0
 		case BLK_MQ_RQ_QUEUE_ERROR:
+			errors++;
 			rq->errors =3D -EIO;
 			blk_mq_end_request(rq, rq->errors);
 			break;

@@ -1097,7 +1098,7 @@ bool blk_mq_dispatch_rq_list(struct =

blk_mq_hw_ctx *hctx, struct list_head *list)

Quoted 9 lines
block/blk-mq.c +0 -0
 			blk_mq_run_hw_queue(hctx, true);
 	}

=20

block/blk-mq.c +1 -1
-	return queued !=3D 0;
+	return (queued + errors) !=3D 0;

}
=20
static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx) =20

Thanks this fixed it, you can add

Tested-by Josef Bacik <[email protected]>

Thanks,

Josef=

Reviewed-by on v1 1/1
#2 lore raw
In reply to Jens Axboe
Quoted 6 lines · Jens Axboe

Currently we return true in blk_mq_dispatch_rq_list() if we queued IO successfully, but we really want to return whether or not the we made progress. Progress includes if we got an error return. If we don't, this can lead to a hang in blk_mq_sched_dispatch_requests() when a driver is draining IO by returning BLK_MQ_QUEUE_ERROR instead of manually ending the IO in error and return BLK_MQ_QUEUE_OK.

Reviewed-by Bart Van Assche <[email protected]>=
Reviewed-by on v1 1/1
#3 lore raw
In reply to Jens Axboe
Quoted 8 lines · Jens Axboe

Currently we return true in blk_mq_dispatch_rq_list() if we queued IO successfully, but we really want to return whether or not the we made progress. Progress includes if we got an error return. If we don't, this can lead to a hang in blk_mq_sched_dispatch_requests() when a driver is draining IO by returning BLK_MQ_QUEUE_ERROR instead of manually ending the IO in error and return BLK_MQ_QUEUE_OK.

Signed-off-by Jens Axboe <[email protected]>
Reviewed-by Omar Sandoval <[email protected]>