nvme: make nvme error status codes converted to errno detailed

PATCH New Guan Junxiong 4 participants 7 messages Mar 29, 2017 – Mar 30, 2017 NVMe

lore

Share & tools

lore

Guan Junxiong

author

2017-03-29 10:08 UTC #0 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

From: Junxiong Guan <[email protected]>

For more detailed information about nvme error status when ending blk_mq request,some of nvme error status codes can be categorized into different errnos explicitly. For example, NVME_SC_ACCESS_DENIED and NVME_SC_CONNECT_CTRL_BUSY can be converted to EACCESS and EBUSY repec- tively.This patch makes conversion from those nvme error status to errno detailed.

---
 drivers/nvme/host/nvme.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

drivers/nvme/host/nvme.h +30 -0

index a3da1e90b99d..be699ca38cc8 100644
@@ -255,7 +255,37 @@ static inline int nvme_error_status(u16 status)
 	case NVME_SC_SUCCESS:
 		return 0;
 	case NVME_SC_CAP_EXCEEDED:
+	case NVME_SC_NS_INSUFFICENT_CAP:
 		return -ENOSPC;
+	case NVME_SC_READ_ONLY:
+	case NVME_SC_ACCESS_DENIED:
+		return -EACCES;
+	case NVME_SC_LBA_RANGE:
+		return -EFAULT;
+	case NVME_SC_CONNECT_CTRL_BUSY:
+		return -EBUSY;
+	case NVME_SC_INVALID_OPCODE:
+	case NVME_SC_INVALID_FIELD:
+	case NVME_SC_INVALID_NS:
+	case NVME_SC_SGL_INVALID_LAST:
+	case NVME_SC_SGL_INVALID_COUNT:
+	case NVME_SC_SGL_INVALID_DATA:
+	case NVME_SC_SGL_INVALID_METADATA:
+	case NVME_SC_SGL_INVALID_TYPE:
+	case NVME_SC_SGL_INVALID_OFFSET:
+	case NVME_SC_SGL_INVALID_SUBTYPE:
+	case NVME_SC_CQ_INVALID:
+	case NVME_SC_QID_INVALID:
+	case NVME_SC_QUEUE_SIZE:
+	case NVME_SC_FIRMWARE_SLOT:
+	case NVME_SC_FIRMWARE_IMAGE:
+	case NVME_SC_INVALID_VECTOR:
+	case NVME_SC_INVALID_LOG_PAGE:
+	case NVME_SC_INVALID_FORMAT:
+	case NVME_SC_CTRL_LIST_INVALID:
+		return -EINVAL;
+	case NVME_SC_CMD_SEQ_ERROR:
+		return -EPROTO;
 	default:
 		return -EIO;
 	}

Signature · 1 line

2.11.1

Sagi Grimberg

maintainer

2017-03-29 17:39 UTC #1 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

Quoted 8 lines · Guan Junxiong

From: Junxiong Guan <guanjunxiong at huawei.com>
For more detailed information about nvme error status when ending blk_mq request,some of nvme error status codes can be categorized into different errnos explicitly. For example, NVME_SC_ACCESS_DENIED and NVME_SC_CONNECT_CTRL_BUSY can be converted to EACCESS and EBUSY repec- tively.This patch makes conversion from those nvme error status to errno detailed.

This makes sense to me, see comments below.

Quoted 9 lines

---
 drivers/nvme/host/nvme.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

drivers/nvme/host/nvme.h +0 -0

index a3da1e90b99d..be699ca38cc8 100644

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 259

Quoted 5 lines

drivers/nvme/host/nvme.h +1 -0

@@ -255,7 +255,37 @@ static inline int nvme_error_status(u16 status)
 	case NVME_SC_SUCCESS:
 		return 0;
 	case NVME_SC_CAP_EXCEEDED:
+	case NVME_SC_NS_INSUFFICENT_CAP:
 		return -ENOSPC;

why is this ENOSPC?

Maybe we can add also some fabrics status codes?

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 265

Quoted 6 lines

drivers/nvme/host/nvme.h +6 -0

+	case NVME_SC_READ_ONLY:
+	case NVME_SC_ACCESS_DENIED:
+		return -EACCES;
+	case NVME_SC_LBA_RANGE:
+		return -EFAULT;
+	case NVME_SC_CONNECT_CTRL_BUSY:

case NVME_SC_CONNECT_CTRL_BUSY:

Quoted 13 lines

drivers/nvme/host/nvme.h +10 -0

+		return -EBUSY;
 
 
 
+	case NVME_SC_INVALID_OPCODE:
+	case NVME_SC_INVALID_FIELD:
+	case NVME_SC_INVALID_NS:
+	case NVME_SC_SGL_INVALID_LAST:
+	case NVME_SC_SGL_INVALID_COUNT:
+	case NVME_SC_SGL_INVALID_DATA:
+	case NVME_SC_SGL_INVALID_METADATA:
+	case NVME_SC_SGL_INVALID_TYPE:
+	case NVME_SC_SGL_INVALID_OFFSET:

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 285

Quoted 10 lines

drivers/nvme/host/nvme.h +10 -0

+	case NVME_SC_SGL_INVALID_SUBTYPE:
+	case NVME_SC_CQ_INVALID:
+	case NVME_SC_QID_INVALID:
+	case NVME_SC_QUEUE_SIZE:
+	case NVME_SC_FIRMWARE_SLOT:
+	case NVME_SC_FIRMWARE_IMAGE:
+	case NVME_SC_INVALID_VECTOR:
+	case NVME_SC_INVALID_LOG_PAGE:
+	case NVME_SC_INVALID_FORMAT:
+	case NVME_SC_CTRL_LIST_INVALID:

case NVME_SC_BAD_ATTRIBUTES:
case NVME_SC_INVALID_PI:
case NVME_SC_CONNECT_FORMAT:
case NVME_SC_CONNECT_INVALID_PARAM:
case NVME_SC_CONNECT_INVALID_HOST:

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 291

Quoted 7 lines

drivers/nvme/host/nvme.h +3 -0

+		return -EINVAL;
+	case NVME_SC_CMD_SEQ_ERROR:
+		return -EPROTO;

drivers/nvme/host/nvme.h +0 -0

 	default:
 		return -EIO;
 	}

It'll also be useful if we can log the specific error (as a human readable string).

guanjunxiong

author

2017-03-30 08:05 UTC #2 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

Quoted 10 lines

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 259
Quoted 8 lines
drivers/nvme/host/nvme.h +1 -0
@@ -255,7 +255,37 @@ static inline int nvme_error_status(u16 status)
     case NVME_SC_SUCCESS:
         return 0;
     case NVME_SC_CAP_EXCEEDED:
+    case NVME_SC_NS_INSUFFICENT_CAP:
         return -ENOSPC;
why is this ENOSPC?

In the NVMe Spec, Namespace Insufficient Capacity (NVME_SC_NS_INSUFFICENT_CAP) means that creating the namespace requires more free space than is currently available. However ENOSPC means no space is left on the device.
If there is a little free space on the device,NVME_SC_NS_INSUFFICENT_CAP will be returned. Your question is reasonble, so I will categorize NVME_SC_NS_INSUFFICENT_CAP into default EIO.

Quoted 25 lines

Quoted 9 lines

drivers/nvme/host/nvme.h +9 -0

+    case NVME_SC_INVALID_OPCODE:
+    case NVME_SC_INVALID_FIELD:
+    case NVME_SC_INVALID_NS:
+    case NVME_SC_SGL_INVALID_LAST:
+    case NVME_SC_SGL_INVALID_COUNT:
+    case NVME_SC_SGL_INVALID_DATA:
+    case NVME_SC_SGL_INVALID_METADATA:
+    case NVME_SC_SGL_INVALID_TYPE:
+    case NVME_SC_SGL_INVALID_OFFSET:

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 285

Quoted 10 lines

drivers/nvme/host/nvme.h +10 -0

+    case NVME_SC_SGL_INVALID_SUBTYPE:
+    case NVME_SC_CQ_INVALID:
+    case NVME_SC_QID_INVALID:
+    case NVME_SC_QUEUE_SIZE:
+    case NVME_SC_FIRMWARE_SLOT:
+    case NVME_SC_FIRMWARE_IMAGE:
+    case NVME_SC_INVALID_VECTOR:
+    case NVME_SC_INVALID_LOG_PAGE:
+    case NVME_SC_INVALID_FORMAT:
+    case NVME_SC_CTRL_LIST_INVALID:

case NVME_SC_BAD_ATTRIBUTES:
case NVME_SC_INVALID_PI:
case NVME_SC_CONNECT_FORMAT:
case NVME_SC_CONNECT_INVALID_PARAM:
case NVME_SC_CONNECT_INVALID_HOST:

Quoted 3 lines

drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 286
Quoted 1 line
drivers/nvme/host/nvme.h +1 -0
+        return -EINVAL;
Maybe we can add also some fabrics status codes?

Nice. In addition, because the value of some fabrics status codes are the same as the value of the PCIe status code, such as:

	/*
	 * I/O Command Set Specific - NVM commands:
	 */
	NVME_SC_BAD_ATTRIBUTES		= 0x180,
	NVME_SC_INVALID_PI		= 0x181,
	NVME_SC_READ_ONLY		= 0x182,

	/*
	 * I/O Command Set Specific - Fabrics commands:
	 */
	NVME_SC_CONNECT_FORMAT		= 0x180,
	NVME_SC_CONNECT_CTRL_BUSY	= 0x181,
	NVME_SC_CONNECT_INVALID_PARAM	= 0x182,

more consideration needs to be taken into the nvme_error_status(u16 status) function. Therefore , funct nvme_error_status(u16 status) need to accept one more parameter to distinguish the duplicated value of the status codes above. Here is the new prototype of nvme_error_status function:

	static inline int nvme_error_status(u16 status, bool is_fabrics_cmd);

If command in the request is fabrics command, then is_fabrics_cmd is set to true. Or else set to false

Quoted 3 lines

It'll also be useful if we can log the specific error (as a human readable string).

Nice advice. The nvme_status_to_string() function in the nvme-cli will be reused for this.

Thanks.

Quoted 83 lines · Sagi Grimberg

Quoted 8 lines · Guan Junxiong
From: Junxiong Guan <guanjunxiong at huawei.com>
For more detailed information about nvme error status when ending blk_mq request,some of nvme error status codes can be categorized into different errnos explicitly. For example, NVME_SC_ACCESS_DENIED and NVME_SC_CONNECT_CTRL_BUSY can be converted to EACCESS and EBUSY repec- tively.This patch makes conversion from those nvme error status to errno detailed.
This makes sense to me, see comments below.
Quoted 9 lines
---
 drivers/nvme/host/nvme.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
drivers/nvme/host/nvme.h +0 -0
index a3da1e90b99d..be699ca38cc8 100644
drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 259
Quoted 5 lines
drivers/nvme/host/nvme.h +1 -0
@@ -255,7 +255,37 @@ static inline int nvme_error_status(u16 status)
     case NVME_SC_SUCCESS:
         return 0;
     case NVME_SC_CAP_EXCEEDED:
+    case NVME_SC_NS_INSUFFICENT_CAP:
         return -ENOSPC;
why is this ENOSPC?
Maybe we can add also some fabrics status codes?
drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 265
Quoted 6 lines
drivers/nvme/host/nvme.h +6 -0
+    case NVME_SC_READ_ONLY:
+    case NVME_SC_ACCESS_DENIED:
+        return -EACCES;
+    case NVME_SC_LBA_RANGE:
+        return -EFAULT;
+    case NVME_SC_CONNECT_CTRL_BUSY:
case NVME_SC_CONNECT_CTRL_BUSY:
Quoted 13 lines
drivers/nvme/host/nvme.h +10 -0
+        return -EBUSY;
 
 
 
+    case NVME_SC_INVALID_OPCODE:
+    case NVME_SC_INVALID_FIELD:
+    case NVME_SC_INVALID_NS:
+    case NVME_SC_SGL_INVALID_LAST:
+    case NVME_SC_SGL_INVALID_COUNT:
+    case NVME_SC_SGL_INVALID_DATA:
+    case NVME_SC_SGL_INVALID_METADATA:
+    case NVME_SC_SGL_INVALID_TYPE:
+    case NVME_SC_SGL_INVALID_OFFSET:
drivers/nvme/host/nvme.h static inline int nvme_error_status(u16 status) · line 285
Quoted 10 lines
drivers/nvme/host/nvme.h +10 -0
+    case NVME_SC_SGL_INVALID_SUBTYPE:
+    case NVME_SC_CQ_INVALID:
+    case NVME_SC_QID_INVALID:
+    case NVME_SC_QUEUE_SIZE:
+    case NVME_SC_FIRMWARE_SLOT:
+    case NVME_SC_FIRMWARE_IMAGE:
+    case NVME_SC_INVALID_VECTOR:
+    case NVME_SC_INVALID_LOG_PAGE:
+    case NVME_SC_INVALID_FORMAT:
+    case NVME_SC_CTRL_LIST_INVALID:
case NVME_SC_BAD_ATTRIBUTES:
case NVME_SC_INVALID_PI:
case NVME_SC_CONNECT_FORMAT:
case NVME_SC_CONNECT_INVALID_PARAM:
case NVME_SC_CONNECT_INVALID_HOST:
Quoted 7 lines
drivers/nvme/host/nvme.h +3 -0
+        return -EINVAL;
+    case NVME_SC_CMD_SEQ_ERROR:
+        return -EPROTO;
     default:
         return -EIO;
     }
It'll also be useful if we can log the specific error (as a human readable string).
.

Christoph Hellwig

2017-03-30 08:11 UTC #3 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

In reply to Guan Junxiong

Quoted 8 lines · Guan Junxiong

From: Junxiong Guan <guanjunxiong at huawei.com>
For more detailed information about nvme error status when ending blk_mq request,some of nvme error status codes can be categorized into different errnos explicitly. For example, NVME_SC_ACCESS_DENIED and NVME_SC_CONNECT_CTRL_BUSY can be converted to EACCESS and EBUSY repec- tively.This patch makes conversion from those nvme error status to errno detailed.

NAK. Most of these codes arw not defined for the block layer and don't make any sense. Also I have a major rework in this area pending that would conflict with this patch.

guanjunxiong

author

2017-03-30 11:32 UTC #4 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

Quoted 10 lines · Christoph Hellwig

Quoted 8 lines · Guan Junxiong
From: Junxiong Guan <guanjunxiong at huawei.com>
For more detailed information about nvme error status when ending blk_mq request,some of nvme error status codes can be categorized into different errnos explicitly. For example, NVME_SC_ACCESS_DENIED and NVME_SC_CONNECT_CTRL_BUSY can be converted to EACCESS and EBUSY repec- tively.This patch makes conversion from those nvme error status to errno detailed.

Quoted 6 lines

NAK. Most of these codes arw not defined for the block layer and don't make any sense. Also I have a major rework in this area pending that would conflict with this patch.
.

Hi,Christoph
Current block layer ends the requests with errors and logs different errno in a
human readable string. So I think the detailed conversion above make sense.Also , as Sagi said, It'll be useful if we can log the specific error (as a human readable string).
Anyway, since this patch would conflict with your rework in this area,
I decide to suspend this patch.
BTW, what's the plan of your rework in this area?

Keith Busch

2017-03-30 17:22 UTC #5 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

Quoted 7 lines · guanjunxiong

Current block layer ends the requests with errors and logs different errno in a
human readable string. So I think the detailed conversion above make sense.Also , as Sagi said, It'll be useful if we can log the specific error (as a human readable string).
Anyway, since this patch would conflict with your rework in this area,
I decide to suspend this patch.
BTW, what's the plan of your rework in this area?

I don't see a problem having more appropriate errno's here. The block layer looks okay to accept any value, and the spec folks might be happy to see their thought out status codes don't all become EIO. :)

But just so we all know, the majority of those statuses are only possible to encounter with passthrough, and those are not translated to errno.

Christoph Hellwig

2017-03-30 17:20 UTC #6 lore raw

PATCH 1/1 nvme: make nvme error status codes converted to errno detailed

Quoted 3 lines · Keith Busch

I don't see a problem having more appropriate errno's here. The block layer looks okay to accept any value, and the spec folks might be happy to see their thought out status codes don't all become EIO. :)

The block layer accepts all and treats a few magically. That's why I want to get rid of errno values in the block layer entirely and replace the with a few status codes that make the special casing obvious.

Quoted 2 lines

But just so we all know, the majority of those statuses are only possible to encounter with passthrough, and those are not translated to errno.

Exactly.