Hi, Ming! 在 2022/07/26 12:21, Ming Lei 写道:
On Tue, Jul 26, 2022 at 11:35:19AM +0800, Yu Kuai wrote:From: Yu Kuai <yukuai3@xxxxxxxxxx> Currently, in virtio_scsi, if 'bd->last' is not set to true while dispatching request, such io will stay in driver's queue, and driver will wait for block layer to dispatch more rqs. However, if block layer failed to dispatch more rq, it should trigger commit_rqs to inform driver. There is a problem in blk_mq_try_issue_list_directly() that commit_rqs won't be called: // assume that queue_depth is set to 1, list contains two rq blk_mq_try_issue_list_directly blk_mq_request_issue_directly // dispatch first rq // last is false __blk_mq_try_issue_directly blk_mq_get_dispatch_budget // succeed to get first budget __blk_mq_issue_directly scsi_queue_rq cmd->flags |= SCMD_LAST virtscsi_queuecommand kick = (sc->flags & SCMD_LAST) != 0 // kick is false, first rq won't issue to disk queued++ blk_mq_request_issue_directly // dispatch second rq __blk_mq_try_issue_directly blk_mq_get_dispatch_budget // failed to get second budget ret == BLK_STS_RESOURCE blk_mq_request_bypass_insert // errors is still 0 if (!list_empty(list) || errors && ...) // won't pass, commit_rqs won't be called In this situation, first rq relied on second rq to dispatch, while second rq relied on first rq to complete, thus they will both hung. And same problem exists in blk_mq_dispatch_rq_list() Fix the problem by also treat 'BLK_STS_*RESOURCE' as 'errors' since it means that request is not queued successfully. Fixes: d666ba98f849 ("blk-mq: add mq_ops->commit_rqs()") Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx> --- Changes in v2: - suggested by Ming, handle blk_mq_dispatch_rq_list() as well. - change title and modify commit message. block/blk-mq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 70177ee74295..ee1e065fe63f 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1909,6 +1909,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list, fallthrough; case BLK_STS_DEV_RESOURCE: blk_mq_handle_dev_resource(rq, list); + errors++; goto out; case BLK_STS_ZONE_RESOURCE: /* @@ -1918,6 +1919,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list, */ blk_mq_handle_zone_resource(rq, &zone_list); needs_resource = true; + errors++;But accounting error here may break return value of blk_mq_dispatch_rq_list(), see: /* Returns true if we did some work AND can potentially do more. */
Yes, you're right. I'll try to fix that in v3. Thanks, Kuai
