On Thu, 2019-04-04 at 15:08 +0800, Ming Lei wrote:
On Wed, Apr 03, 2019 at 01:11:26PM -0700, Bart Van Assche wrote:
If blk_mq_try_issue_directly() returns BLK_STS*_RESOURCE that means that the request has not been queued and that the caller should retry to submit the request. Both blk_mq_request_bypass_insert() and blk_mq_sched_insert_request() guarantee that a request will be processed. Hence return BLK_STS_OK if one of these functions is called. This patch avoids that blk_mq_dispatch_rq_list() crashes when using dm-mpath.
Cc: Christoph Hellwig hch@infradead.org Cc: Hannes Reinecke hare@suse.com Cc: James Smart james.smart@broadcom.com Cc: Ming Lei ming.lei@redhat.com Cc: Jianchao Wang jianchao.w.wang@oracle.com Cc: Keith Busch keith.busch@intel.com Cc: Dongli Zhang dongli.zhang@oracle.com Cc: Laurence Oberman loberman@redhat.com Tested-by: Laurence Oberman loberman@redhat.com Reviewed-by: Laurence Oberman loberman@redhat.com Reported-by: Laurence Oberman loberman@redhat.com Fixes: 7f556a44e61d ("blk-mq: refactor the code of issue request directly") # v5.0. Cc: stable@vger.kernel.org Signed-off-by: Bart Van Assche bvanassche@acm.org
block/blk-mq.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c index 652d0c6d5945..b2c20dce8a30 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1859,16 +1859,11 @@ blk_status_t blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx, case BLK_STS_RESOURCE: if (force) { blk_mq_request_bypass_insert(rq, run_queue);
/** We have to return BLK_STS_OK for the DM* to avoid livelock. Otherwise, we return* the real result to indicate whether the* request is direct-issued successfully.*/ret = bypass ? BLK_STS_OK : ret;
ret = BLK_STS_OK;ret = BLK_STS_OK;This change itself is correct.
However, there is other issue introduced by 7f556a44e61d.
We need blk_insert_cloned_request() to pass back BLK_STS_RESOURCE/BLK_STS_RESOURCE to caller, so that dm-rq driver may see the underlying queue is busy, then tell blk-mq to deal with the busy condition from dm-rq queue, so that IO merge can get improved.
That is exactly what 396eaf21ee17c476e8f6 ("blk-mq: improve DM's blk-mq IO merging via blk_insert_cloned_request feedback") did.
There must be performance regression with 7f556a44e61d by cut the feedback.
So could you fix them all in one patch?
Since commit 7f556a44e61d introduced multiple problems and since fixing these is nontrivial, how about reverting that commit?
Thanks,
Bart.