On 01/21/2011 09:49 PM, Mike Christie wrote:
> On 01/21/2011 02:07 AM, Hannes Reinecke wrote:
>> Consider a scenario where a SCSI EH command like TUR fails with a NOT
>> READY
>> status - MANUAL INTERVENTION REQUIRED (02/04/03). As evident in the
>> ASC/ASCQ
>> description, manual intervention is required in this case i.e. the
>> target wants
>> TUR to fail here so that the host administrator can take corrective
>> action.
>>
>> But this particular ASC/ASCQ is not handled in the scsi_check_sense,
>> which
>> ultimately causes scsi_eh_tur to erroneously report a SUCCESS (device
>> ready)
>> instead of the actual FAILURE state (device not ready).
>>
>> This patch converts scsi_check_sense() to return SOFT_ERROR in
>> cases where an error has been signalled via the sense code, but
>> we should not wake the error handler. This error code will be
>> reverted back to SUCCESS for normal command handling, but
>> scsi_eh_completed_normally() will convert it to FAILED.
>>
>> Reported-by: Martin George<marting@xxxxxxxxxx>
>> Signed-off-by: Hannes Reinecke<hare@xxxxxxx>
>>
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index 45c7564..e86e62e 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -223,7 +223,7 @@ static inline void scsi_eh_prt_fail_stats(struct
>> Scsi_Host *shost,
>> * @scmd: Cmd to have sense checked.
>> *
>> * Return value:
>> - * SUCCESS or FAILED or NEEDS_RETRY
>> + * SUCCESS or FAILED or NEEDS_RETRY or SOFT_ERROR
>> *
>> * Notes:
>> * When a deferred error is detected the current command has
>> @@ -278,7 +278,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
>>
>> case ABORTED_COMMAND:
>> if (sshdr.asc == 0x10) /* DIF */
>> - return SUCCESS;
>> + return SOFT_ERROR;
>>
>> return NEEDS_RETRY;
>> case NOT_READY:
>> @@ -324,19 +324,19 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
>> * Pass the UA upwards for a determination in the completion
>> * functions.
>> */
>> - return SUCCESS;
>> + return SOFT_ERROR;
>>
>
>
> For normal IO execution the UA, not ready, and illegal request handling
> in scsi_io_completion would determine that some of these are retryable
> conditions, but with the patch in the scsi_error.cs path we will return
> soft error and fail. Also a lot of these that were retryable were
> returned as success in the past, but with the patch are being failed.
>
No. scsi_check_sense() is not called from scsi_io_completion; that
just calls scsi_normalize_sense().
scsi_check_sense() is only called from
- scsi_eh_completed_normally()
- scsi_eh_stu()
- scsi_decide_disposition()
All of which have been checked against this patch.
scsi_io_completion() has it's own set of checks for sense codes
which I don't modify here.
> Martin made me the same bugzilla :) I was thinking we needed to make
> check_sense smarter or move some of the scsi_io_completion code to some
> lib helpers so scsi_error.c could call them.
>
> And does scsi_eh_stu need to be covnerted to check for soft error and
> failed.
>
No. scsi_eh_stu() needs to check if a START STOP UNIT command needs
to be send. The trigger for this is that scsi_check_sense() returns
FAILED:
list_for_each_entry(scmd, work_q, eh_entry)
if (scmd->device == sdev && SCSI_SENSE_VALID(scmd) &&
scsi_check_sense(scmd) == FAILED ) {
stu_scmd = scmd;
break;
}
if (!stu_scmd)
continue;
so this logic hasn't changed.
> Also if we did this patch, then I think we also have to convert the
> device handlers.
Not necessarily. This patch just allows check_sense() to return
an additional error code; the device handlers are not forced to
use them. If they don't things will work like before.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
