From 4237db03be4dae03eecd2955cfcbad0041bd9f0a Mon Sep 17 00:00:00 2001 From: Salvatore Bonaccorso Date: Sat, 8 Dec 2018 11:26:43 +0100 Subject: [PATCH] blk-mq: punt failed direct issue to dispatch list --- debian/changelog | 1 + ...failed-direct-issue-to-dispatch-list.patch | 124 ++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 126 insertions(+) create mode 100644 debian/patches/bugfix/all/blk-mq-punt-failed-direct-issue-to-dispatch-list.patch diff --git a/debian/changelog b/debian/changelog index 2c3039553..9355ecff3 100644 --- a/debian/changelog +++ b/debian/changelog @@ -36,6 +36,7 @@ linux (4.19.7-1~exp1) UNRELEASED; urgency=medium [ Salvatore Bonaccorso ] * blk-mq: fix corruption with direct issue (Closes: #915666) + * blk-mq: punt failed direct issue to dispatch list -- Uwe Kleine-König Wed, 28 Nov 2018 12:20:46 +0100 diff --git a/debian/patches/bugfix/all/blk-mq-punt-failed-direct-issue-to-dispatch-list.patch b/debian/patches/bugfix/all/blk-mq-punt-failed-direct-issue-to-dispatch-list.patch new file mode 100644 index 000000000..adf054eef --- /dev/null +++ b/debian/patches/bugfix/all/blk-mq-punt-failed-direct-issue-to-dispatch-list.patch @@ -0,0 +1,124 @@ +From c616cbee97aed4bc6178f148a7240206dcdb85a6 Mon Sep 17 00:00:00 2001 +From: Jens Axboe +Date: Thu, 6 Dec 2018 22:17:44 -0700 +Subject: blk-mq: punt failed direct issue to dispatch list + +From: Jens Axboe + +commit c616cbee97aed4bc6178f148a7240206dcdb85a6 upstream. + +After the direct dispatch corruption fix, we permanently disallow direct +dispatch of non read/write requests. This works fine off the normal IO +path, as they will be retried like any other failed direct dispatch +request. But for the blk_insert_cloned_request() that only DM uses to +bypass the bottom level scheduler, we always first attempt direct +dispatch. For some types of requests, that's now a permanent failure, +and no amount of retrying will make that succeed. This results in a +livelock. + +Instead of making special cases for what we can direct issue, and now +having to deal with DM solving the livelock while still retaining a BUSY +condition feedback loop, always just add a request that has been through +->queue_rq() to the hardware queue dispatch list. These are safe to use +as no merging can take place there. Additionally, if requests do have +prepped data from drivers, we aren't dependent on them not sharing space +in the request structure to safely add them to the IO scheduler lists. + +This basically reverts ffe81d45322c and is based on a patch from Ming, +but with the list insert case covered as well. + +Fixes: ffe81d45322c ("blk-mq: fix corruption with direct issue") +Cc: stable@vger.kernel.org +Suggested-by: Ming Lei +Reported-by: Bart Van Assche +Tested-by: Ming Lei +Acked-by: Mike Snitzer +Signed-off-by: Jens Axboe +Signed-off-by: Greg Kroah-Hartman + +--- + block/blk-mq.c | 33 +++++---------------------------- + 1 file changed, 5 insertions(+), 28 deletions(-) + +--- a/block/blk-mq.c ++++ b/block/blk-mq.c +@@ -1698,15 +1698,6 @@ static blk_status_t __blk_mq_issue_direc + break; + case BLK_STS_RESOURCE: + case BLK_STS_DEV_RESOURCE: +- /* +- * If direct dispatch fails, we cannot allow any merging on +- * this IO. Drivers (like SCSI) may have set up permanent state +- * for this request, like SG tables and mappings, and if we +- * merge to it later on then we'll still only do IO to the +- * original part. +- */ +- rq->cmd_flags |= REQ_NOMERGE; +- + blk_mq_update_dispatch_busy(hctx, true); + __blk_mq_requeue_request(rq); + break; +@@ -1719,18 +1710,6 @@ static blk_status_t __blk_mq_issue_direc + return ret; + } + +-/* +- * Don't allow direct dispatch of anything but regular reads/writes, +- * as some of the other commands can potentially share request space +- * with data we need for the IO scheduler. If we attempt a direct dispatch +- * on those and fail, we can't safely add it to the scheduler afterwards +- * without potentially overwriting data that the driver has already written. +- */ +-static bool blk_rq_can_direct_dispatch(struct request *rq) +-{ +- return req_op(rq) == REQ_OP_READ || req_op(rq) == REQ_OP_WRITE; +-} +- + static blk_status_t __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx, + struct request *rq, + blk_qc_t *cookie, +@@ -1752,7 +1731,7 @@ static blk_status_t __blk_mq_try_issue_d + goto insert; + } + +- if (!blk_rq_can_direct_dispatch(rq) || (q->elevator && !bypass_insert)) ++ if (q->elevator && !bypass_insert) + goto insert; + + if (!blk_mq_get_dispatch_budget(hctx)) +@@ -1768,7 +1747,7 @@ insert: + if (bypass_insert) + return BLK_STS_RESOURCE; + +- blk_mq_sched_insert_request(rq, false, run_queue, false); ++ blk_mq_request_bypass_insert(rq, run_queue); + return BLK_STS_OK; + } + +@@ -1784,7 +1763,7 @@ static void blk_mq_try_issue_directly(st + + ret = __blk_mq_try_issue_directly(hctx, rq, cookie, false); + if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE) +- blk_mq_sched_insert_request(rq, false, true, false); ++ blk_mq_request_bypass_insert(rq, true); + else if (ret != BLK_STS_OK) + blk_mq_end_request(rq, ret); + +@@ -1814,15 +1793,13 @@ void blk_mq_try_issue_list_directly(stru + struct request *rq = list_first_entry(list, struct request, + queuelist); + +- if (!blk_rq_can_direct_dispatch(rq)) +- break; +- + list_del_init(&rq->queuelist); + ret = blk_mq_request_issue_directly(rq); + if (ret != BLK_STS_OK) { + if (ret == BLK_STS_RESOURCE || + ret == BLK_STS_DEV_RESOURCE) { +- list_add(&rq->queuelist, list); ++ blk_mq_request_bypass_insert(rq, ++ list_empty(list)); + break; + } + blk_mq_end_request(rq, ret); diff --git a/debian/patches/series b/debian/patches/series index 8e0ab0b20..dc9998e5b 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -92,6 +92,7 @@ bugfix/all/partially-revert-usb-kconfig-using-select-for-usb_co.patch bugfix/all/kbuild-include-addtree-remove-quotes-before-matching-path.patch debian/revert-objtool-fix-config_stack_validation-y-warning.patch bugfix/all/blk-mq-fix-corruption-with-direct-issue.patch +bugfix/all/blk-mq-punt-failed-direct-issue-to-dispatch-list.patch # Miscellaneous features