Re: [PATCH 0/2] blk request timeout handler: mv scsi timer code to block layer

Mike Christie
2006-07-25T16:28:45Z

Jens Axboe wrote:
> On Tue, Jul 25 2006, Mike Christie wrote:
>> For the request based multipath I thought I would need to run some code
>> when a command times out. I did not want to duplicate the scsi code, so
>> I did the following patches which move the scsi timer code to the block
>> layer then convert scsi.
>>
>> I have tested the scsi_error.c and normal paths with iscsi. And, I have
>> tested the normal IO paths with libata. Since libata uses the strategy
>> handler it needs to be tested a lot more. Some of the drivers that were
>> touching the timeout_per_command field need to be compile tested still
>> too. I converted them, but I think some still need a "#include
>> blkdev.h".
>>
>> The patches only move the scsi timer code to the block layer and hook it
>> in so others can use it. I have not started on the abort, reset and
>> quiesce code since it is not really needed for multipath. I wanted to
>> see if the timer code move was ok on its own without the rest of the
>> scsi eh move because I do not want to manage the patches out of tree
>> with the other request multipath patches. I also wanted to check if the
>> scsi timer code was ok in general. Maybe scsi got it wrong and needed to
>> be rewritten :)
>
> Excellent, one item off my TODO list :-). I had pending code, but not
> completed yet.
>
> I had intended to make the timer addition/deletion implicit from the
> activate/deactive rq paths, both to have it happen automatically and
> from a cleanliness POV. That makes the timer only active when the
> request is in the driver, and should also make the deletion implicit for
> when the request gets requeued.
>

Ok I did that, almost. For the normal request_fn/dequeue, requeue, and
blk softiriq completion paths the block layer handles all the timer
addition, deletion and restarting. There is one nasty path in the scsi,
where we need to requeue the command only if the timer has not expired
and for that I cheated and allowed scsi to do the blk_delete_timer() so
it could check the return value. I will work on fixing that case for the
next resend of the patches.