Re: SCSI EH tidbit

[Tejun Heo <htejun@gmail.com>]

  Hello, Jeff.

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>>
>>  Hi, Jeff.
>>
>> Jeff Garzik wrote:
>>
>>>
>>> I'm thinking that __scsi_done() might be more appropriate than 
>>> scsi_finish_command(), for use in completing commands in the EH path. 
>>> There may need to be some timer-related mods somewhere to make that 
>>> work, though.
>>
>>
>>
>>  Can you elaborate on why you think that would be a good idea?  I see 
>> cons but not many pros.  My cons are...
>>
>> * SCSI EH uses scsi_finish_command()
>>
>> * iodone/err counters will be incremented twice
>>
>> * has possibility of looping
> 
> 
> 
> scsi_finish_command unconditionally completes the command, rather than 
> running it through scsi_decide_disposition() decision tree again.  That 
> prevents us from using the standard command-retry path, for PCI/ATA bus 
> errors where we want to resubmit the command.

  That can be done by finishing with scsi_queue_insert as done by the 
current SCSI EH implementation.  And, yes, that also can be done by 
generating appropriate sense data and run it through __scsi_done again.

  The thing is that I don't really see much difference between finishing 
with scsi_queue_insert/scsi_queue_insert and using __scsi_done.  I'm 
opting for not using __scsi_done mainly because that's how the current 
SCSI EH implementation is implemented and I don't really think it would 
be a good idea to do things differently from SCSI EH without clear reason.

  Also, please note that for some errors, there might not be clearly 
matching sense code.  We can read_scsi_decide_dispostion() and find out 
sense code which does what we want and then emulate that value, but I 
don't know.  It just doesn't seem like a very good idea to me.

> There would be no loop, as __scsi_done() simply raises a softirq.  There 
> are just a few details to consider in case you're in the error handler.

  I don't know whether you already have decided ATAPI sense data should 
be collected outside EH as you haven't replied to my message yet, but 
let's say we perform request sensing in EH.  The sequence would be...

1. ATAPI error occurs
2. somehow __scsi_done is called, which raises SOFTIRQ
3. scsi_softirq kicks in and sees that there's no sense data
4. SCSI EH invoked, libata EH invoked
5. EH requests SENSE

  Let's assume that request sense returns NO SENSE here.

6. EH invokes __scsi_done
7. softirq raised
8. scsi_softirq runs cmd through scsi_decide_disposition
9. disposition is FAILED, invoke scsi_eh_scmd_add
10. the scmd enters EH again

  Note that any sense data which returns FAILED from 
scsi_decide_disposition will trigger the same sequence.  We can of 
course change sense data to something for which 
scsi_decide_disposition() does not return FAILED, but I think that would 
be a bit fragile.

  The situation is similar when generating sense data.  We can always 
generate sense data for which scsi_decide_disposition() would never 
return FAILED.  But IMHO it doesn't look like a very good idea.

  Thanks.

-- 
tejun