Re: [PATCH 3/3] scsi: Return ENODATA on medium error

[Hannes Reinecke <hare@suse.de>]

On 06/06/2013 09:52 AM, Ren Mingxin wrote:
> Hi, Hannes:
> 
> On 06/05/2013 03:11 PM, Hannes Reinecke wrote:
>> When a medium error is detected the SCSI stack should return
>> ENODATA to the upper layers.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> ---
>>   drivers/scsi/scsi_error.c | 7 ++++++-
>>   drivers/scsi/scsi_lib.c   | 5 +++++
>>   include/scsi/scsi.h       | 2 ++
>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index bf5e61a..2ded10a 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -235,6 +235,7 @@ static inline void
>> scsi_eh_prt_fail_stats(struct Scsi_Host *shost,
>>    *    NEEDS_RETRY
>>    *    TARGET_ERROR
>>    *    ALLOC_ERROR
>> + *    MEDIA_FAILURE
>>    *
>>    * Notes:
>>    *    When a deferred error is detected the current command has
>> @@ -375,7 +376,7 @@ static int scsi_check_sense(struct scsi_cmnd
>> *scmd)
>>           if (sshdr.asc == 0x11 || /* UNRECOVERED READ ERR */
>>               sshdr.asc == 0x13 || /* AMNF DATA FIELD */
>>               sshdr.asc == 0x14) { /* RECORD NOT FOUND */
>> -            return TARGET_ERROR;
>> +            return MEDIA_FAILURE;
>>           }
>>           return NEEDS_RETRY;
>>
>> @@ -1598,6 +1599,10 @@ int scsi_decide_disposition(struct
>> scsi_cmnd *scmd)
>>               /* target hit out-of-space condition */
>>               set_host_byte(scmd, DID_ALLOC_FAILURE);
>>               rtn = SUCCESS;
>> +        } else if (rtn == MEDIA_FAILURE) {
>> +            /* medium error */
>> +            set_host_byte(scmd, DID_MEDIUM_ERROR);
>> +            rtn = SUCCESS;
>>           }
>>           /* if rtn == FAILED, we have no sense information;
>>            * returning FAILED will wake the error handler thread
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index 209a4d5..39d626e 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -711,6 +711,7 @@ EXPORT_SYMBOL(scsi_release_buffers);
>>    * -EREMOTEIO    permanent target failure, do not retry
>>    * -EBADE    permanent nexus failure, retry on other path
>>    * -ENOSPC    No write space available
>> + * -ENODATA    Medium error
>>    */
>>   static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd,
>> int result)
>>   {
>> @@ -732,6 +733,10 @@ static int __scsi_error_from_host_byte(struct
>> scsi_cmnd *cmd, int result)
>>           set_host_byte(cmd, DID_OK);
>>           error = -ENOSPC;
>>           break;
>> +    case DID_MEDIUM_ERROR:
>> +        set_host_byte(cmd, DID_OK);
>> +        error = -ENODATA;
>> +        break;
> 
> It seems that there is a debugging requirement to announce the
> meaning of these new added error codes in the function
> blk_update_request()like this:
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 33c33bc..a396eb6 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2315,6 +2315,12 @@ bool blk_update_request(struct request *req,
> int error, unsigned int nr_bytes)
>                 case -EBADE:
>                         error_type = "critical nexus";
>                         break;
> +               case -ENOSPC:
> +                       error_type = "critical space allocation";
> +                       break;
> +               case -ENODATA:
> +                       error_type = "critical medium";
> +                       break;
>                 case -EIO:
>                 default:
>                         error_type = "I/O";
> 
And indeed, that's true. I'll be updating the patches.

> # To tell the truth, I'm not understand why this patchset is needed
> # in practice for I've only just got limited info about LSF. I guess
> # this is one of the improvements for SCSI EH. Could you give an
> # example/condition the upper layers interest in?
> 
No, this is actually related, but independent of the SCSI EH rework.
Filesystem folks expressed their interest in these kind of errors.
'ENODATA' is especially of interest, as this mean you will _never_
get the data back. Any other type of error has at least a
theoretical chance to recover the data.

And ENOSPC needs to be differentiated, too, as this is some sort of
'temporary write error'. So when the fs is able to free-up some
space this error might actually be fixed, so it's worth inform the
filesystem about it.

Cheers,

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