Skip to contentSkip to conversation

blk request timeout handler: mv scsi timer code to block layer

PATCH In review Mike Christie 2 participants 4 messages Jul 25, 2006 SCSI

lore
Share & tools
lore
#0 lore raw

cover blk request timeout handler: mv scsi timer code to block layer

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 :)

Both patches were made over 2.6.18-rc2.

#1 lore raw
Quoted 12 lines · Mike Christie

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".

Quoted 8 lines

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.

Signature · 1 line
Jens Axboe
#2 lore raw
Quoted 22 lines · Jens Axboe
Quoted 12 lines · Mike Christie

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".

Quoted 8 lines

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 :)

Quoted 9 lines

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.

#3 lore raw
Quoted 32 lines · Mike Christie
Quoted 21 lines · Jens Axboe
Quoted 12 lines · Mike Christie

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".

Quoted 8 lines

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 :)

Quoted 9 lines

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.

Quoted 3 lines

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,

Oops, during testing and my own code review I noticed there was also some places in the hotplug removal error paths that I used the wrong block layer function and did not delete the timer. I will fix those up as well.