[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ac242680-ff9b-da3f-056c-da2d14947959@huaweicloud.com>
Date: Tue, 26 Jul 2022 10:55:53 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Ming Lei <ming.lei@...hat.com>, Yu Kuai <yukuai1@...weicloud.com>
Cc: axboe@...nel.dk, osandov@...com, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org, yi.zhang@...wei.com
Subject: Re: [PATCH] blk-mq: fix io hung due to missing commit_rqs while
scheduler is none
Hi, Ming
在 2022/07/26 10:48, Ming Lei 写道:
> On Tue, Jul 26, 2022 at 10:38:52AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@...wei.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.
>>
>> Fix the problem by also treat 'BLK_STS_RESOURCE' and
>> 'BLK_STS_DEV_RESOURCE' as 'errors' in blk_mq_try_issue_list_directly(),
>> so that 'commit_rqs' will be called when dispatch of last rq failed.
>>
>> Fixes: d666ba98f849 ("blk-mq: add mq_ops->commit_rqs()")
>> Signed-off-by: Yu Kuai <yukuai3@...wei.com>
>> ---
>> block/blk-mq.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 70177ee74295..752b0fe4c128 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -2680,6 +2680,7 @@ void blk_mq_try_issue_list_directly(struct blk_mq_hw_ctx *hctx,
>> list_del_init(&rq->queuelist);
>> ret = blk_mq_request_issue_directly(rq, list_empty(list));
>> if (ret != BLK_STS_OK) {
>> + errors++;
>
> OK, it is because that list becomes empty, but .queue_rq(last_rq_in_list)
> returns BLK_STS_*RESOURCE, but scsi can't call ->queuecommand() for this
> real last rq. Then blk_mq_try_issue_list_directly() doesn't call
> ->commit_rqs() too.
>
> Here errors means that request not queued successfully, so this patch
> looks fine.
>
> Also I think blk_mq_dispatch_rq_list() needs similar handling too,
> right?
Yes, you're right, I'll handle blk_mq_dispatch_rq_list() in v2, and also
change the title.
Thanks,
Kuai
Powered by blists - more mailing lists