lkml.org 
[lkml]   [2022]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] blk-mq: fix io hung due to missing commit_rqs
From
Date
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@huawei.com>
>>
>> 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@huawei.com>
>> ---
>> 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
>

\
 
 \ /
  Last update: 2022-07-26 08:31    [W:0.047 / U:0.876 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site