nvme: make use of nvme error status codes in block layer
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
@@ -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
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_mqrequest,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@@ -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;
? }
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_mqrequest,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@@ -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 ; }.
PATCH v2 1/1 nvme: make use of nvme error status codes in block layer
In reply to Guan JunxiongNAK. 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.
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?