Re: [PATCH 2/3] block: Introduce blk_rq_completed()

[Paolo Bonzini <pbonzini@redhat.com>]

Il 27/05/2014 13:26, James Bottomley ha scritto:
>> You could use a different mechanism than a softirq to tell the abort
>> were successful, for example by overriding scsi_done.  But with respect
>> to the block layer, the mechanics of avoiding the race and double-free
>> would probably be the same.
>
> I think there's some confusion about what the race and double free is:
> It only occurs with timeouts.  In a timeout situation, the host had
> decided it's not waiting any longer for the target to respond and
> proceeds to error recovery.  At any time between the host making this
> decision up to the point it kicks the target hard enough to clear all
> in-flight commands, the target may return the command.  If we didn't
> have some ignore function on command completions while we're handling
> errors, this would lead to double completion.
>
> If we decided to allow arbitrary aborts of running commands, we would
> send a TMF in during the normal (i.e. un timed out) command period.
> Because there's no timeout involved, there's no double free problem.
> The race in this case is whether the abort catches the command or not
> and to mediate that race we need the normal status return.

I'm not sure why "no timeout" implies "no double free".  There would 
still be a race between the interrupt handler and softirq on one side, 
and the abort handler on the other.  The interrupt handler's call to 
cmd->scsi_done ends up triggering the softirq and thus freeing the 
command with scsi_put_command.  The abort handler, as you mentioned, 
wants the status return so it needs the interrupt handler to run---but 
not the softirq.

A simple way to avoid this could be to skip the softirq processing, by 
marking the request block-layer-complete, and do everything in the abort 
handler.  The interrupt handler would still run and fill in the status.

Paolo