Re: [PATCH RFC] Remove the cancel_delayed_work() call from scsi_put_command()

[Bart Van Assche <bvanassche@acm.org>]

On 05/22/14 18:22, Paolo Bonzini wrote:
> Il 21/05/2014 15:30, Bart Van Assche ha scritto:
>> +static bool scmd_being_handled_in_other_context(struct scsi_cmnd *scmd)
>> +{
>> +    struct Scsi_Host *shost = scmd->device->host;
>> +    struct scsi_cmnd *c;
>> +    unsigned long flags;
>> +    bool ret = false;
>> +
>> +    if (!blk_rq_completed(scmd->request))
>> +        return true;
>> +
>> +    spin_lock_irqsave(shost->host_lock, flags);
>> +    list_for_each_entry(c, &shost->eh_cmd_q, eh_entry) {
>> +        if (c == scmd) {
>> +            ret = true;
>> +            break;
>> +        }
>> +    }
>> +    spin_unlock_irqrestore(shost->host_lock, flags);
>> +
>> +    return ret;
>> +}
>> +
>>  /**
>>   * scmd_eh_abort_handler - Handle command aborts
>>   * @work:    command to be aborted.
>> @@ -120,6 +142,8 @@ scmd_eh_abort_handler(struct work_struct *work)
>>      struct scsi_device *sdev = scmd->device;
>>      int rtn;
>>
>> +    WARN_ON_ONCE(scmd_being_handled_in_other_context(scmd));
> 
> What about a simpler, though less accuracte
> 
>     WARN_ON(!blk_rq_completed(scmd->request));
> 
> that doesn't need the host_lock?

One reason why I posted this patch as an RFC was to invite feedback. I'm
fine with leaving out the loop over the eh_cmd_q list although I do not
expect that will make a significant performance difference. None of the
functions in which a check was added are in the hot path.

Bart.