[PATCHv3 0/4] scsi: More detailed I/O errors

[PATCHv3 0/4] scsi: More detailed I/O errors

[Hannes Reinecke]

As discussed at LSF the SCSI stack already knows about several
error conditions, it's just not well documented.
Also there are some more conditions upper layers might be
interested in.

This patchset improves the documentation of the existing
I/O error codes and adds two more error codes, ENOSPC
for thin provisioning failure and ENODATA for medium error.

Changes since the initial version:
- Correctly document all return codes in
  __scsi_error_from_host_byte()
- Set hostbyte status in scsi_check_sense()

Changes since v2:
- Modify dm-mpath to handle ENOSPC

Hannes Reinecke (4):
  scsi: Document enhanced error codes
  scsi: Set hostbyte status in scsi_check_sense()
  scsi: return ENOSPC on thin provisioning failure
  scsi: Return ENODATA on medium error

 block/blk-core.c          |  6 ++++++
 drivers/md/dm-mpath.c     | 16 +++++++++++++++-
 drivers/scsi/scsi_error.c | 28 +++++++++++++---------------
 drivers/scsi/scsi_lib.c   | 22 ++++++++++++++++++++++
 include/scsi/scsi.h       |  3 ++-
 5 files changed, 58 insertions(+), 17 deletions(-)

-- 
1.7.12.4

[PATCH 1/4] scsi: Document enhanced error codes

[Hannes Reinecke]

Document the various error codes returned on I/O failure.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 86d5220..d3fc50f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -700,6 +700,18 @@ void scsi_release_buffers(struct scsi_cmnd *cmd)
 }
 EXPORT_SYMBOL(scsi_release_buffers);
 
+/**
+ * __scsi_error_from_host_byte - translate SCSI error code into errno
+ * @cmd:	SCSI command (unused)
+ * @result:	scsi error code
+ *
+ * Translate SCSI error code into standard UNIX errno.
+ * Return values:
+ * -ENOLINK	temporary transport failure
+ * -EREMOTEIO	permanent target failure, do not retry
+ * -EBADE	permanent nexus failure, retry on other path
+ * -EIO		unspecified I/O error
+ */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 {
 	int error = 0;
-- 
1.7.12.4

[PATCH 2/4] scsi: Set hostbyte status in scsi_check_sense()

[Hannes Reinecke]

We should be modifying the host_byte status in scsi_check_sense()
directly; this saves us to introduce a special return code for
each and every condition.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 21 +++++++--------------
 include/scsi/scsi.h       |  1 -
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 84369f2..e5cb162 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -248,7 +248,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 or TARGET_ERROR
+ *	SUCCESS or FAILED or NEEDS_RETRY or ADD_TO_MLQUEUE
  *
  * Notes:
  *	When a deferred error is detected the current command has
@@ -379,13 +379,15 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 	case MISCOMPARE:
 	case BLANK_CHECK:
 	case DATA_PROTECT:
-		return TARGET_ERROR;
+		set_host_byte(scmd, DID_TARGET_FAILURE);
+		return SUCCESS;
 
 	case MEDIUM_ERROR:
 		if (sshdr.asc == 0x11 || /* UNRECOVERED READ ERR */
 		    sshdr.asc == 0x13 || /* AMNF DATA FIELD */
 		    sshdr.asc == 0x14) { /* RECORD NOT FOUND */
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
+			return SUCCESS;
 		}
 		return NEEDS_RETRY;
 
@@ -393,14 +395,14 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		if (scmd->device->retry_hwerror)
 			return ADD_TO_MLQUEUE;
 		else
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
 
 	case ILLEGAL_REQUEST:
 		if (sshdr.asc == 0x20 || /* Invalid command operation code */
 		    sshdr.asc == 0x21 || /* Logical block address out of range */
 		    sshdr.asc == 0x24 || /* Invalid field in cdb */
 		    sshdr.asc == 0x26) { /* Parameter value invalid */
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
 		}
 		return SUCCESS;
 
@@ -863,7 +865,6 @@ retry:
 		case SUCCESS:
 		case NEEDS_RETRY:
 		case FAILED:
-		case TARGET_ERROR:
 			break;
 		case ADD_TO_MLQUEUE:
 			rtn = NEEDS_RETRY;
@@ -1690,14 +1691,6 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
 		rtn = scsi_check_sense(scmd);
 		if (rtn == NEEDS_RETRY)
 			goto maybe_retry;
-		else if (rtn == TARGET_ERROR) {
-			/*
-			 * Need to modify host byte to signal a
-			 * permanent target failure
-			 */
-			set_host_byte(scmd, DID_TARGET_FAILURE);
-			rtn = SUCCESS;
-		}
 		/* if rtn == FAILED, we have no sense information;
 		 * returning FAILED will wake the error handler thread
 		 * to collect the sense and redo the decide
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 66216c1..07d4bd916 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -481,7 +481,6 @@ static inline int scsi_is_wlun(unsigned int lun)
 #define TIMEOUT_ERROR   0x2007
 #define SCSI_RETURN_NOT_HANDLED   0x2008
 #define FAST_IO_FAIL	0x2009
-#define TARGET_ERROR    0x200A
 
 /*
  * Midlevel queue return values.
-- 
1.7.12.4

[PATCH 3/4] scsi: return ENOSPC on thin provisioning failure

[Hannes Reinecke]

When the thin provisioning hard threshold is reached we
should return ENOSPC to inform upper layers about this fact.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          | 3 +++
 drivers/scsi/scsi_error.c | 7 ++++++-
 drivers/scsi/scsi_lib.c   | 5 +++++
 include/scsi/scsi.h       | 1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index d5745b5..170607c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2315,6 +2315,9 @@ 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 -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index e5cb162..aeec0e7 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -374,11 +374,16 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		return SUCCESS;
 
 		/* these are not supported */
+	case DATA_PROTECT:
+		if (sshdr.asc == 0x27 && sshdr.ascq == 0x07) {
+			/* Thin provisioning hard threshold reached */
+			set_host_byte(scmd, DID_ALLOC_FAILURE);
+			return SUCCESS;
+		}
 	case COPY_ABORTED:
 	case VOLUME_OVERFLOW:
 	case MISCOMPARE:
 	case BLANK_CHECK:
-	case DATA_PROTECT:
 		set_host_byte(scmd, DID_TARGET_FAILURE);
 		return SUCCESS;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d3fc50f..8a2f92d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -710,6 +710,7 @@ EXPORT_SYMBOL(scsi_release_buffers);
  * -ENOLINK	temporary transport failure
  * -EREMOTEIO	permanent target failure, do not retry
  * -EBADE	permanent nexus failure, retry on other path
+ * -ENOSPC	No write space available
  * -EIO		unspecified I/O error
  */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
@@ -728,6 +729,10 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 		set_host_byte(cmd, DID_OK);
 		error = -EBADE;
 		break;
+	case DID_ALLOC_FAILURE:
+		set_host_byte(cmd, DID_OK);
+		error = -ENOSPC;
+		break;
 	default:
 		error = -EIO;
 		break;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 07d4bd916..7d5b8e2 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -452,6 +452,7 @@ static inline int scsi_is_wlun(unsigned int lun)
 				 * other paths */
 #define DID_NEXUS_FAILURE 0x11  /* Permanent nexus failure, retry on other
 				 * paths might yield different results */
+#define DID_ALLOC_FAILURE 0x12  /* Space allocation on the device failed */
 #define DRIVER_OK       0x00	/* Driver status                           */
 
 /*
-- 
1.7.12.4

[PATCH 4/4] scsi: Return ENODATA on medium error

[Hannes Reinecke]

When a medium error is detected the SCSI stack should return
ENODATA to the upper layers.

Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          |  3 +++
 drivers/md/dm-mpath.c     | 16 +++++++++++++++-
 drivers/scsi/scsi_error.c |  2 +-
 drivers/scsi/scsi_lib.c   |  5 +++++
 include/scsi/scsi.h       |  1 +
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 170607c..1093ca1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2318,6 +2318,9 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 		case -ENOSPC:
 			error_type = "critical space allocation";
 			break;
+		case -ENODATA:
+			error_type = "critical medium";
+			break;
 		case -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index bdf26f5..57896cea 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1261,6 +1261,20 @@ static void activate_path(struct work_struct *work)
 				pg_init_done, pgpath);
 }
 
+static int noretry_error(int error)
+{
+	switch(error) {
+	case -EOPNOTSUPP:
+	case -EREMOTEIO:
+	case -EILSEQ:
+	case -ENODATA:
+		return 1;
+	}
+
+	/* Anything else could be a path failure, so should be retried */
+	return 0;
+}
+
 /*
  * end_io handling
  */
@@ -1284,7 +1298,7 @@ static int do_end_io(struct multipath *m, struct request *clone,
 	if (!error && !clone->errors)
 		return 0;	/* I/O complete */
 
-	if (error == -EOPNOTSUPP || error == -EREMOTEIO || error == -EILSEQ)
+	if (noretry_error(error))
 		return error;
 
 	if (mpio->pgpath)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index aeec0e7..7f794d4 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -391,7 +391,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 */
-			set_host_byte(scmd, DID_TARGET_FAILURE);
+			set_host_byte(scmd, DID_MEDIUM_ERROR);
 			return SUCCESS;
 		}
 		return NEEDS_RETRY;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8a2f92d..70ca1f6 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
  * -EIO		unspecified I/O error
  */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
@@ -733,6 +734,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;
 	default:
 		error = -EIO;
 		break;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 7d5b8e2..95ce204 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -453,6 +453,7 @@ static inline int scsi_is_wlun(unsigned int lun)
 #define DID_NEXUS_FAILURE 0x11  /* Permanent nexus failure, retry on other
 				 * paths might yield different results */
 #define DID_ALLOC_FAILURE 0x12  /* Space allocation on the device failed */
+#define DID_MEDIUM_ERROR  0x13  /* Medium error */
 #define DRIVER_OK       0x00	/* Driver status                           */
 
 /*
-- 
1.7.12.4

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

[Jun'ichi Nomura]

Hi Hannes,

On 07/01/13 22:16, Hannes Reinecke wrote:
> When a medium error is detected the SCSI stack should return
> ENODATA to the upper layers.
> 
> Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/blk-core.c          |  3 +++
>  drivers/md/dm-mpath.c     | 16 +++++++++++++++-
>  drivers/scsi/scsi_error.c |  2 +-
>  drivers/scsi/scsi_lib.c   |  5 +++++
>  include/scsi/scsi.h       |  1 +
>  5 files changed, 25 insertions(+), 2 deletions(-)
..
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index bdf26f5..57896cea 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c

Thank you for including the change.
But I think this change to dm should be a separate patch in this series
(prerequisite to the SCSI error code change) and Cc to dm-devel.

> @@ -1261,6 +1261,20 @@ static void activate_path(struct work_struct *work)
>  				pg_init_done, pgpath);
>  }
>  
> +static int noretry_error(int error)
> +{
> +	switch(error) {
> +	case -EOPNOTSUPP:
> +	case -EREMOTEIO:
> +	case -EILSEQ:
> +	case -ENODATA:

   	case -ENOSPC:

We don't want to fail the path for -ENOSPC, do we?

> +		return 1;
> +	}
> +
> +	/* Anything else could be a path failure, so should be retried */
> +	return 0;
> +}
> +
>  /*
>   * end_io handling
>   */
> @@ -1284,7 +1298,7 @@ static int do_end_io(struct multipath *m, struct request *clone,
>  	if (!error && !clone->errors)
>  		return 0;	/* I/O complete */
>  
> -	if (error == -EOPNOTSUPP || error == -EREMOTEIO || error == -EILSEQ)
> +	if (noretry_error(error))
>  		return error;
>  
>  	if (mpio->pgpath)

-- 
Jun'ichi Nomura, NEC Corporation