Re: Debugging scsi abort handling ?

[Hannes Reinecke <hare@suse.de>]

On 08/29/2014 12:14 PM, Finn Thain wrote:
>
> On Fri, 29 Aug 2014, Hannes Reinecke wrote:
>
>> On 08/29/2014 06:39 AM, Finn Thain wrote:
>>>
>>> On Thu, 28 Aug 2014, Hannes Reinecke wrote:
>>>
>>>> What might happen, though, that the command is already dead and gone
>>>> by the time you're calling ->scsi_done() (if you call it after
>>>> eh_abort). So there might not _be_ a command upon which you can call
>>>> ->scsi_done() to start with.
>>>>
>>>> Hence any LLDD need to clear up any internal references after a call
>>>> to eh_XXX to ensure it doesn't call ->scsi_done() an in invalid
>>>> command.
>>>>
>>>> So even if the LLDD returns 'FAILED' upon a call to eh_XXX it
>>>> _still_ needs to clear up the internal reference.
>>>
>>> This is a question that has been bothering me too. If the host's
>>> eh_abort_cmd() method returns FAILED, it seems the mid-layer is liable
>>> to re-issue the same command to the LLD (?)
>>>
>> No.
>> FAILED for any eh_abort_cmd() means that the TMF hasn't been sent.
>
> Makes sense, though it appears to contradict this advice about returning
> SUCCESS in some situations:
> http://marc.info/?l=linux-scsi&m=140923498632496&w=2
>
Well, if the LLDD detects an invalid command (ie if it cannot find 
any internal command matching the midlayer command) that's an 
automatic success, obviously.

So we should rephrase things to:

- The eh_XXX callback shall return 'SUCCESS' if the respective
   TMF (or equvalent) could be initiated or if the matching command
   reference has already been completed by the LLDD. Otherwise
   the eh_XXX callback shall return 'FAILED'.

>> The command will only ever be re-issued once EH completes.
>
> ...
>
>>
>> But indeed, 'FAILED' is not very meaningful here, leaving the midlayer
>> with no information about what happened to the command.
>>
>> Personally I would like to enforce this meaning on the eh_XXX callbacks:
>> - upon each eh_XXX callback the LLDD clears any internal references
>>    to the command / command scope (ie eh_abort_cmd clears the
>>    references to the command, eh_lun_reset clears all internal
>>    references to commands to this ITL nexus etc.)
>>    This happens irrespective of the return code.
>> - The eh_XXX callback shall return 'FAILED' if the respective
>>    TMF (or equivalent) could not be initiated.
>> - The eh_XXX callback shall return 'SUCCESS' if the respective
>>    TMF (or equvalent) could be initiated.
>> - After each eh_XXX callback control for this command / command
>>    scope is transferred back to the midlayer; the LLDD shall not
>>    assume the associated command structures to remain valid after
>>    that point.
>
> Perhaps that last constraint should be relaxed to "After the final EH
> callback (whether implemented or unimplemented by the host), command /
> command scope is transferred back to the midlayer..."
>
No, that's wrong.

By the time any eh_XXX callbacks are triggered control _is_ already 
back at the midlayer. IE the command timeout triggered and the block 
layer already set the REQ_ATOM_COMPLETED flag, short-circuiting any 
attempts to call ->scsi_done().
So with the callbacks the midlayer actually informs the LLDD about a 
certain fact; there is nothing the LLDD can do to change ownership 
at that point.

(Correction: During the call of any eh_XXX callbacks control _is_ 
back at the LLDD, otherwise the callbacks would be pointless. It's
just that the LLDD shouldn't assume the command is valid _after_
any of the eh_XXX callbacks has terminated.)

> A more severe TMF is probably mandatory (e.g. bus reset) but if the driver
> author later added a milder one (e.g. bus device reset), your rule would
> mean that the existing handler would then operate under new constraints,
> which might cause surprises.
>
Well, _if_ we were to adopt this rule we obviously have to audit
existing LLDDs if the rule is followed, and tweak them if not.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html