Re: [PATCH v13] blk-mq: Rework blk-mq timeout handling again

[Jens Axboe <axboe@kernel.dk>]

On 5/22/18 2:44 PM, Bart Van Assche wrote:
> On Tue, 2018-05-22 at 14:38 -0600, Jens Axboe wrote:
>> On 5/22/18 2:33 PM, Bart Van Assche wrote:
>>> Thanks for having reported this. How about using the following change to suppress
>>> that warning:
>>>
>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>> index bb99c03e7a34..84e55ea55baf 100644
>>> --- a/block/blk-mq.c
>>> +++ b/block/blk-mq.c
>>> @@ -844,6 +844,7 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved)
>>>  
>>>  	switch (ret) {
>>>  	case BLK_EH_HANDLED:
>>> +		blk_mq_change_rq_state(req, MQ_RQ_TIMED_OUT, MQ_RQ_COMPLETE);
>>>  		__blk_mq_complete_request(req);
>>>  		break;
>>>  	case BLK_EH_RESET_TIMER:
>>>
>>> I think this will work better than what was proposed in your last e-mail. I'm afraid
>>> that with that change that a completion that occurs while the timeout handler is
>>> running can be ignored.
>>
>> What if that races with eg requeue? We get the completion from IRQ, decide we
>> need to requeue/restart the request rather than complete it.
> 
> Shouldn't block drivers that requeue a request from inside their timeout handler
> return BLK_EH_NOT_HANDLED instead of BLK_EH_HANDLED?

Yeah, I guess they should. See reply to Christoph as well, making this
simpler would be a great idea. Could be done as a prep patch (series)
to this one.

-- 
Jens Axboe