On 04/24/2013 04:48 PM, James Bottomley wrote:
> On Wed, 2013-04-24 at 13:32 +0200, Hannes Reinecke wrote:
>> scsi_send_eh_cmnd() is calling queuecommand() directly, so
>> it needs to check the return value here.
>> The only valid return codes for queuecommand() are 'busy'
>> states, so we need to wait for a bit to allow the LLDD
>> to recover.
>>
>> Based on an earlier patch from Wen Xiong.
> 
> This looks good, only one minor nitpick:
> 
>> Cc: Wen Xiong <wenxi...@linux.vnet.ibm.com>
>> Cc: Brian King <brk...@linux.vnet.ibm.com>
>> Signed-off-by: Hannes Reinecke <h...@suse.de>
>>
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index c1b05a8..1fc6da94 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -791,22 +791,32 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
>> unsigned char *cmnd,
>>      struct scsi_device *sdev = scmd->device;
>>      struct Scsi_Host *shost = sdev->host;
>>      DECLARE_COMPLETION_ONSTACK(done);
>> -    unsigned long timeleft;
>> +    unsigned long timeleft = timeout;
>>      struct scsi_eh_save ses;
>> +    const int stall_for = min(HZ/10, 1);
>>      int rtn;
>>  
>> +retry:
>>      scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
>>      shost->eh_action = &done;
>>  
>>      scsi_log_send(scmd);
>>      scmd->scsi_done = scsi_eh_done;
>> -    shost->hostt->queuecommand(shost, scmd);
>> -
>> -    timeleft = wait_for_completion_timeout(&done, timeout);
>> +    rtn = shost->hostt->queuecommand(shost, scmd);
>> +    if (rtn) {
>> +            if (timeleft) {
>> +                    scsi_eh_restore_cmnd(scmd, &ses);
>> +                    timeleft -= stall_for;
>> +                    msleep(stall_for);
> 
> Stall for is in HZ: need to convert to ms, so
> 
>                       msleep(stall_for * 1000/HZ);
> 
Right. Will be converting it.

> I also don't see a need to restore and reprep the command each time, but
> I don't see a problem with it either.
> 
This was mainly thought as a safeguard here; just in case someone
else might be fiddling with the command. But yeah, it's not required.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                   zSeries & Storage
h...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to