[PATCH 0/2] nvme: add command retry driver control flag

[PATCH 0/2] nvme: add command retry driver control flag

[Minwoo Im]

Hello,

This series is to support command retry for passthru commands via ioctl.
This patch introduced `driver_ctrl` flag to the passthru ioctl command
structures by replacing the existing `rsvd` field with `driver_ctrl`
flag.

This flag can be used to give some control flag to the driver from
user-space.  The first flag introduced in this series is
`NVME_DRIVER_CTRL_RETRY` to make command retry-able in failure case
without DNR status.  (And of course, in case it's not a path-related
errors).

Please have a look.

Thanks,

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 1/2] nvme: retry command if driver control flag given for passthru

[Minwoo Im]

ioctl passthru commands are REQ_FAILFAST_DRIVER requests which are
non-retry requests if failed.  This patch introduced `driver_ctrl` flag
to the ioctl command structure: nvme_passthru_cmd, nvme_passthru_cmd64.
`driver_ctrl` field has not been appended, but replaced existing
reserved field.

Clear the REQ_FAILFASTER_DRIVER flag from the request if `driver_ctrl`
is given with NVME_DRIVER_CTRL_RETRY set to make it retry-able if failed
without DNR.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c        | 12 ++++++++----
 include/uapi/linux/nvme_ioctl.h | 15 +++++++++++----
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..20cd4a815944 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1143,7 +1143,7 @@ EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);
 static int nvme_submit_user_cmd(struct request_queue *q,
 		struct nvme_command *cmd, void __user *ubuffer,
 		unsigned bufflen, void __user *meta_buffer, unsigned meta_len,
-		u32 meta_seed, u64 *result, unsigned timeout)
+		u32 meta_seed, u64 *result, unsigned timeout, u16 driver_ctrl)
 {
 	bool write = nvme_is_write(cmd);
 	struct nvme_ns *ns = q->queuedata;
@@ -1157,6 +1157,9 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 
+	if (driver_ctrl & NVME_DRIVER_CTRL_RETRY)
+		req->cmd_flags &= ~REQ_FAILFAST_DRIVER;
+
 	if (timeout)
 		req->timeout = timeout;
 	nvme_req(req)->flags |= NVME_REQ_USERCMD;
@@ -1615,7 +1618,8 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 
 	return nvme_submit_user_cmd(ns->queue, &c,
 			nvme_to_user_ptr(io.addr), length,
-			metadata, meta_len, lower_32_bits(io.slba), NULL, 0);
+			metadata, meta_len, lower_32_bits(io.slba), NULL, 0,
+			io.driver_ctrl);
 }
 
 static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
@@ -1653,7 +1657,7 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
 			nvme_to_user_ptr(cmd.addr), cmd.data_len,
 			nvme_to_user_ptr(cmd.metadata), cmd.metadata_len,
-			0, &result, timeout);
+			0, &result, timeout, cmd.driver_ctrl);
 
 	if (status >= 0) {
 		if (put_user(result, &ucmd->result))
@@ -1697,7 +1701,7 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
 			nvme_to_user_ptr(cmd.addr), cmd.data_len,
 			nvme_to_user_ptr(cmd.metadata), cmd.metadata_len,
-			0, &cmd.result, timeout);
+			0, &cmd.result, timeout, cmd.driver_ctrl);
 
 	if (status >= 0) {
 		if (put_user(cmd.result, &ucmd->result))
diff --git a/include/uapi/linux/nvme_ioctl.h b/include/uapi/linux/nvme_ioctl.h
index d99b5a772698..b247b3eee2e3 100644
--- a/include/uapi/linux/nvme_ioctl.h
+++ b/include/uapi/linux/nvme_ioctl.h
@@ -14,7 +14,7 @@ struct nvme_user_io {
 	__u8	flags;
 	__u16	control;
 	__u16	nblocks;
-	__u16	rsvd;
+	__u16	driver_ctrl;
 	__u64	metadata;
 	__u64	addr;
 	__u64	slba;
@@ -27,7 +27,7 @@ struct nvme_user_io {
 struct nvme_passthru_cmd {
 	__u8	opcode;
 	__u8	flags;
-	__u16	rsvd1;
+	__u16	driver_ctrl;
 	__u32	nsid;
 	__u32	cdw2;
 	__u32	cdw3;
@@ -48,7 +48,7 @@ struct nvme_passthru_cmd {
 struct nvme_passthru_cmd64 {
 	__u8	opcode;
 	__u8	flags;
-	__u16	rsvd1;
+	__u16	driver_ctrl;
 	__u32	nsid;
 	__u32	cdw2;
 	__u32	cdw3;
@@ -63,7 +63,7 @@ struct nvme_passthru_cmd64 {
 	__u32	cdw14;
 	__u32	cdw15;
 	__u32	timeout_ms;
-	__u32   rsvd2;
+	__u32   rsvd;
 	__u64	result;
 };
 
@@ -79,4 +79,11 @@ struct nvme_passthru_cmd64 {
 #define NVME_IOCTL_ADMIN64_CMD	_IOWR('N', 0x47, struct nvme_passthru_cmd64)
 #define NVME_IOCTL_IO64_CMD	_IOWR('N', 0x48, struct nvme_passthru_cmd64)
 
+enum {
+	/* Retry command if failed with !DNR for <= nvme_max_retries */
+	__NVME_DRIVER_CTRL_RETRY,
+};
+
+#define NVME_DRIVER_CTRL_RETRY  (1 << __NVME_DRIVER_CTRL_RETRY)
+
 #endif /* _UAPI_LINUX_NVME_IOCTL_H */
-- 
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[nvme-cli PATCH 2/2] nvme-ioctl: add command retry driver control flag to passthru

[Minwoo Im]

Replace existing reserved field in passthru ioctl command structures.
This patch also removed the `--rsvd` option from the passthru command
because even if it's given, it's not going to be passthrough to the
kernel driver: meaningless input.

This patch introduced the first flag value for command retry in case of
!DNR failure case from controller.  If `--retry` option is given to
passthru command, it will be retried by the driver itself rather than
returning error to the user-space direcly.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 linux/nvme_ioctl.h | 12 +++++++++---
 nvme-ioctl.c       |  5 ++---
 nvme-ioctl.h       |  2 +-
 nvme.c             | 15 +++++++++------
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/linux/nvme_ioctl.h b/linux/nvme_ioctl.h
index d569414211d1..17dde016cb5c 100644
--- a/linux/nvme_ioctl.h
+++ b/linux/nvme_ioctl.h
@@ -23,7 +23,7 @@ struct nvme_user_io {
 	__u8	flags;
 	__u16	control;
 	__u16	nblocks;
-	__u16	rsvd;
+	__u16	driver_ctrl;
 	__u64	metadata;
 	__u64	addr;
 	__u64	slba;
@@ -36,7 +36,7 @@ struct nvme_user_io {
 struct nvme_passthru_cmd {
 	__u8	opcode;
 	__u8	flags;
-	__u16	rsvd1;
+	__u16	driver_ctrl;
 	__u32	nsid;
 	__u32	cdw2;
 	__u32	cdw3;
@@ -57,7 +57,7 @@ struct nvme_passthru_cmd {
 struct nvme_passthru_cmd64 {
     __u8    opcode;
     __u8    flags;
-    __u16   rsvd1;
+    __u16   driver_ctrl;
     __u32   nsid;
     __u32   cdw2;
     __u32   cdw3;
@@ -88,4 +88,10 @@ struct nvme_passthru_cmd64 {
 #define NVME_IOCTL_ADMIN64_CMD  _IOWR('N', 0x47, struct nvme_passthru_cmd64)
 #define NVME_IOCTL_IO64_CMD _IOWR('N', 0x48, struct nvme_passthru_cmd64)
 
+enum nvme_driver_ctrl_bits {
+	/* Retry command if failed with !DNR for <= nvme_max_retries */
+	__NVME_DRIVER_CTRL_RETRY,	};
+
+#define NVME_DRIVER_CTRL_RETRY	(1 << __NVME_DRIVER_CTRL_RETRY)
+
 #endif /* _UAPI_LINUX_NVME_IOCTL_H */
diff --git a/nvme-ioctl.c b/nvme-ioctl.c
index 3e19023be890..2176fb3747ec 100644
--- a/nvme-ioctl.c
+++ b/nvme-ioctl.c
@@ -90,7 +90,7 @@ int nvme_submit_io_passthru(int fd, struct nvme_passthru_cmd *cmd)
 }
 
 int nvme_passthru(int fd, unsigned long ioctl_cmd, __u8 opcode,
-		  __u8 flags, __u16 rsvd,
+		  __u8 flags, __u16 driver_ctrl,
 		  __u32 nsid, __u32 cdw2, __u32 cdw3, __u32 cdw10, __u32 cdw11,
 		  __u32 cdw12, __u32 cdw13, __u32 cdw14, __u32 cdw15,
 		  __u32 data_len, void *data, __u32 metadata_len,
@@ -99,7 +99,7 @@ int nvme_passthru(int fd, unsigned long ioctl_cmd, __u8 opcode,
 	struct nvme_passthru_cmd cmd = {
 		.opcode		= opcode,
 		.flags		= flags,
-		.rsvd1		= rsvd,
+		.driver_ctrl	= driver_ctrl,
 		.nsid		= nsid,
 		.cdw2		= cdw2,
 		.cdw3		= cdw3,
@@ -133,7 +133,6 @@ int nvme_io(int fd, __u8 opcode, __u64 slba, __u16 nblocks, __u16 control,
 		.flags		= 0,
 		.control	= control,
 		.nblocks	= nblocks,
-		.rsvd		= 0,
 		.metadata	= (__u64)(uintptr_t) metadata,
 		.addr		= (__u64)(uintptr_t) data,
 		.slba		= slba,
diff --git a/nvme-ioctl.h b/nvme-ioctl.h
index 2b7640734398..a442da0275cc 100644
--- a/nvme-ioctl.h
+++ b/nvme-ioctl.h
@@ -17,7 +17,7 @@ int nvme_submit_admin_passthru(int fd, struct nvme_passthru_cmd *cmd);
 int nvme_submit_io_passthru(int fd, struct nvme_passthru_cmd *cmd);
 
 int nvme_passthru(int fd, unsigned long ioctl_cmd, __u8 opcode, __u8 flags,
-		  __u16 rsvd, __u32 nsid, __u32 cdw2, __u32 cdw3,
+		  __u16 driver_ctrl, __u32 nsid, __u32 cdw2, __u32 cdw3,
 		  __u32 cdw10, __u32 cdw11, __u32 cdw12,
 		  __u32 cdw13, __u32 cdw14, __u32 cdw15,
 		  __u32 data_len, void *data, __u32 metadata_len,
diff --git a/nvme.c b/nvme.c
index 5324e8bbe79f..419b2ecbf30f 100644
--- a/nvme.c
+++ b/nvme.c
@@ -5060,11 +5060,11 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	int err = 0, wfd = STDIN_FILENO, fd;
 	__u32 result;
 	bool huge;
+	__u16 driver_ctrl = 0x0;
 
 	struct config {
 		__u8  opcode;
 		__u8  flags;
-		__u16 rsvd;
 		__u32 namespace_id;
 		__u32 data_len;
 		__u32 metadata_len;
@@ -5084,12 +5084,12 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		int   read;
 		int   write;
 		__u8  prefill;
+		bool  retry;
 	};
 
 	struct config cfg = {
 		.opcode       = 0,
 		.flags        = 0,
-		.rsvd         = 0,
 		.namespace_id = 0,
 		.data_len     = 0,
 		.metadata_len = 0,
@@ -5104,11 +5104,11 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		.cdw15        = 0,
 		.input_file   = "",
 		.prefill      = 0,
+		.retry        = false,
 	};
 
 	const char *opcode = "opcode (required)";
 	const char *flags = "command flags";
-	const char *rsvd = "value for reserved field";
 	const char *namespace_id = "desired namespace";
 	const char *data_len = "data I/O length (bytes)";
 	const char *metadata_len = "metadata seg. length (bytes)";
@@ -5128,12 +5128,12 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	const char *re = "set dataflow direction to receive";
 	const char *wr = "set dataflow direction to send";
 	const char *prefill = "prefill buffers with known byte-value, default 0";
+	const char *retry = "command retry if failed with !DNR";
 
 	OPT_ARGS(opts) = {
 		OPT_BYTE("opcode",       'o', &cfg.opcode,       opcode),
 		OPT_BYTE("flags",        'f', &cfg.flags,        flags),
 		OPT_BYTE("prefill",      'p', &cfg.prefill,      prefill),
-		OPT_SHRT("rsvd",         'R', &cfg.rsvd,         rsvd),
 		OPT_UINT("namespace-id", 'n', &cfg.namespace_id, namespace_id),
 		OPT_UINT("data-len",     'l', &cfg.data_len,     data_len),
 		OPT_UINT("metadata-len", 'm', &cfg.metadata_len, metadata_len),
@@ -5152,6 +5152,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		OPT_FLAG("dry-run",      'd', &cfg.dry_run,      dry),
 		OPT_FLAG("read",         'r', &cfg.read,         re),
 		OPT_FLAG("write",        'w', &cfg.write,        wr),
+		OPT_FLAG("retry",        'R', &cfg.retry,        retry),
 		OPT_END()
 	};
 
@@ -5213,7 +5214,6 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	if (cfg.show_command) {
 		printf("opcode       : %02x\n", cfg.opcode);
 		printf("flags        : %02x\n", cfg.flags);
-		printf("rsvd1        : %04x\n", cfg.rsvd);
 		printf("nsid         : %08x\n", cfg.namespace_id);
 		printf("cdw2         : %08x\n", cfg.cdw2);
 		printf("cdw3         : %08x\n", cfg.cdw3);
@@ -5232,7 +5232,10 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	if (cfg.dry_run)
 		goto free_data;
 
-	err = nvme_passthru(fd, ioctl_cmd, cfg.opcode, cfg.flags, cfg.rsvd,
+	if (cfg.retry)
+		driver_ctrl |= NVME_DRIVER_CTRL_RETRY;
+
+	err = nvme_passthru(fd, ioctl_cmd, cfg.opcode, cfg.flags, driver_ctrl,
 				cfg.namespace_id, cfg.cdw2, cfg.cdw3, cfg.cdw10,
 				cfg.cdw11, cfg.cdw12, cfg.cdw13, cfg.cdw14, cfg.cdw15,
 				cfg.data_len, data, cfg.metadata_len, metadata,
-- 
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme