Skip to contentSkip to conversation

nvme: make use of nvme error status codes in block layer

PATCH Superseded Guan Junxiong 3 participants 5 messages Apr 17, 2017 – Apr 18, 2017 NVMe

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 nvme: make use of nvme error status codes in block layer

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.This patch makes conversion from those nvme error status to errno detail so that block layer can make use of the detailed error information.

---
 drivers/nvme/host/core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/nvme/host/core.c +29 -0
index 9bf758e3c911..95180ac0c710 100644
@@ -73,6 +73,35 @@ static int nvme_error_status(struct request *req)
 		return 0;
 	case NVME_SC_CAP_EXCEEDED:
 		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 · 5 lines
2.11.1


change from V1:
  rebase on the request-errors 
#1 lore raw

PATCH v2 1/1 nvme: make use of nvme error status codes in block layer

Quoted 7 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.This patch makes conversion from those nvme error status to errno detail so that block layer can make use of the detailed error information.

This is a good list but it's not the complete list. It also seems to be somewhat arbitrary list of chosen NVMe error codes to convert, with respect to include/linux/nvme.h, which is a little tedious to track.

How about fold the conversion based on how the NVMe errors are grouped in nvme.h? So there would be one patch for:

	/*
	 * Generic Command Status:
	 */

one patch for:

	/*
	 * Command Specific Status:
	 */

etc...

This would make it easier to track and maintain what NVMe errors have been accounted for the errno conversion between submitted patches and what is implemented in the code.

Thanks,
Jay

Quoted 44 lines

---
?drivers/nvme/host/core.c | 29 +++++++++++++++++++++++++++++ ?1 file changed, 29 insertions(+)

drivers/nvme/host/core.c +0 -0
index 9bf758e3c911..95180ac0c710 100644
@@ -73,6 +73,35 @@ static int nvme_error_status(struct request *req)
 ?		return 0;

? case NVME_SC_CAP_EXCEEDED:
? return -ENOSPC;

drivers/nvme/host/core.c +29 -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:
+		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;
? }

#2 lore raw

PATCH v2 1/1 nvme: make use of nvme error status codes in block layer

Hi,Jay:

Quoted 10 lines · J Freyensee
Quoted 7 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.This patch makes conversion from those nvme error status to errno detail so that block layer can make use of the detailed error information.

Quoted 3 lines

This is a good list but it's not the complete list. It also seems to be somewhat arbitrary list of chosen NVMe error codes to convert, with respect to include/linux/nvme.h, which is a little tedious to track.

You are right it's not the complete list. Actually, some of the error codes such as NVME_SC_FIRMWARE_SLOT can not be converted to errno properly, so it is converted to EIO by default.

Quoted 13 lines

How about fold the conversion based on how the NVMe errors are grouped in nvme.h? So there would be one patch for:

	/*
	 * Generic Command Status:
	 */

one patch for:

	/*
	 * Command Specific Status:
	 */
Quoted 6 lines

etc...

This would make it easier to track and maintain what NVMe errors have been accounted for the errno conversion between submitted patches and what is implemented in the code.

That's a good suggestion I will alter this patch folding the conversion based on how the NVMe errors and resubmit.

Quoted 47 lines
Quoted 34 lines
---
 drivers/nvme/host/core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/nvme/host/core.c +22 -0
index 9bf758e3c911..95180ac0c710 100644
@@ -73,6 +73,35 @@ static int nvme_error_status(struct request *req)
 		return 0;
 	case NVME_SC_CAP_EXCEEDED:
 		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:

drivers/nvme/host/core.c static int nvme_error_status(struct request *req) · line 107

Quoted 10 lines
drivers/nvme/host/core.c +7 -0
+	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;
 	}

.

#3 lore raw

PATCH v2 1/1 nvme: make use of nvme error status codes in block layer

In reply to Guan Junxiong

NAK. Please don't start exposing random error codes to the block layer. There are a few (very few) with special meaning in the block layer, mostly just for multipathing. If you have an actual, demonstrated need for those we can add it. Otherwise you're just creating noise.

#4 lore raw

PATCH v2 1/1 nvme: make use of nvme error status codes in block layer

Quoted 8 lines · Christoph Hellwig

NAK. Please don't start exposing random error codes to the block layer. There are a few (very few) with special meaning in the block layer, mostly just for multipathing. If you have an actual, demonstrated need for those we can add it. Otherwise you're just creating noise.

.

Thanks for your reply which makes sense. We really need to let dm-multipath make use of the nvme error codes. Current all error codes are converted to EIO except NVME_SC_CAP_EXCEEDED, which means almost IO are retried on other path.

If some nvme error codes such as NVME_SC_WRITE_FAULT,NVME_SC_READ_ERROR into -ENODATA so that it will not retry to save bandwith. Does it make sense?