On 10/28/2014 08:04 PM, Elliott, Robert (Server Storage) wrote:
> 
> 
>> -----Original Message-----
>> From: Hannes Reinecke [mailto:[email protected]]
>> Sent: Friday, 24 October, 2014 7:27 AM
>> To: James Bottomley
>> Cc: Christoph Hellwig; Elliott, Robert (Server Storage); linux-
>> [email protected]; Hannes Reinecke
>> Subject: [PATCH 25/27] scsi: check for correct return code in
>> scsi_eh_abort_cmds()
>>
>> scsi_try_to_abort_cmd() might return SUCCESS, FAILED, or
>> FAST_IO_FAIL. So just checking for FAILED will treat
>> FAST_IO_FAIL as SUCCESS, which is wrong.
>>
>> Cc: Robert Elliott <[email protected]>
>> Signed-off-by: Hannes Reinecke <[email protected]>
>> ---
>>  drivers/scsi/scsi_error.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index fa7b5ec..e94baf1 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -1345,7 +1345,7 @@ static int scsi_eh_abort_cmds(struct list_head
>> *work_q,
>>                      scmd_printk(KERN_INFO, scmd,
>>                                   "%s: aborting cmd\n", current->comm));
>>              rtn = scsi_try_to_abort_cmd(shost->hostt, scmd);
>> -            if (rtn == FAILED) {
>> +            if (rtn != SUCCESS) {
>>                      SCSI_LOG_ERROR_RECOVERY(3,
>>                              scmd_printk(KERN_INFO, scmd,
>>                                          "%s: aborting cmd failed\n",
> 
> The rest of the code in that function is:
>                                               current->comm));
>                       list_splice_init(&check_list, work_q);
>                        return list_empty(work_q);
>               }       [closing the rtn != SUCCESS block]
>               scmd->eh_eflags &= ~SCSI_EH_CANCEL_CMD;
>               if (rtn == FAST_IO_FAIL)
>                       scsi_eh_finish_cmd(scmd, done_q);
>               else
>                       list_move_tail(&scmd->eh_entry, &check_list);
>       }
> 
>       return scsi_eh_test_devices(&check_list, work_q, done_q, 0);
> }
> 
> With this change, if rtn is FAST_IO_FAIL, the 
>       return list_empty(work_q);
> will be taken and it'll never get to
>       if (rtn == FAST_IO_FAIL)
> 
Gnaa. You are correct.

Christoph, please drop this patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                   zSeries & Storage
[email protected]                          +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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to