Skip to contentSkip to conversation

block and nvme passthrough error handling

PATCH Superseded Keith Busch 5 participants 20 messages Apr 23, 2021 – May 17, 2021

Subsystems (4)
Block layer NVMe NVMe target NVMe FC transport

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Keith Busch

author maintainer
#0 lore raw

v2 cover block and nvme passthrough error handling

v1 -> v2:

Unify nvme's polled and non-polled requests by pushing that logic to the generic block layer.

Include the ioctl and passthrough users for getting the error status from the passthrough execution.

Added nvme patch allowing a user to specify a polled ioctl request.

Keith Busch (5):
block: support polling through blk_execute_rq
nvme: use blk_execute_rq() for passthrough commands
block: return errors from blk_execute_rq()
nvme: use return value from blk_execute_rq()
nvme: allow user passthrough commands to poll

 block/blk-exec.c                | 18 ++++++++--
 drivers/nvme/host/core.c        | 62 ++++++++++++---------------------
 drivers/nvme/host/fabrics.c     | 13 ++++---
 drivers/nvme/host/fabrics.h     |  2 +-
 drivers/nvme/host/fc.c          |  2 +-
 drivers/nvme/host/ioctl.c       | 38 ++++++++++----------
 drivers/nvme/host/lightnvm.c    |  4 +--
 drivers/nvme/host/nvme.h        |  7 ++--
 drivers/nvme/host/pci.c         |  4 +--
 drivers/nvme/host/rdma.c        |  3 +-
 drivers/nvme/host/tcp.c         |  2 +-
 drivers/nvme/target/loop.c      |  2 +-
 drivers/nvme/target/passthru.c  | 10 +++---
 include/linux/blkdev.h          |  2 +-
 include/uapi/linux/nvme_ioctl.h |  4 +++
 15 files changed, 85 insertions(+), 88 deletions(-)
Signature · 1 line
2.25.4
author maintainer
#1 lore raw

PATCH v2 1/5 block: support polling through blk_execute_rq

Poll for completions if the request's hctx is a polling type.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
block/blk-exec.c +11 -1
index beae70a0e5e5..b960ad187ba5 100644
@@ -63,6 +63,11 @@ void blk_execute_rq_nowait(struct gendisk *bd_disk, struct request *rq,
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
 
+static bool blk_rq_is_poll(struct request *rq)
+{
+	return rq->mq_hctx && rq->mq_hctx->type == HCTX_TYPE_POLL;
+}
+
 /**
  * blk_execute_rq - insert a request into queue for execution
  * @bd_disk:	matching gendisk

@@ -83,7 +88,12 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 
 	/* Prevent hang_check timer from firing at us during very long I/O */
 	hang_check = sysctl_hung_task_timeout_secs;
-	if (hang_check)
+	if (blk_rq_is_poll(rq)) {
+		do {
+			blk_poll(rq->q, request_to_qc_t(rq->mq_hctx, rq), true);
+			cond_resched();
+		} while (!completion_done(&wait));
+	} else if (hang_check)
 		while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
 	else
 		wait_for_completion_io(&wait);
Signature · 1 line
2.25.4
Reviewed-by on v2 1/5
#2 lore raw

Re: [PATCHv2 1/5] block: support polling through blk_execute_rq

Quoted 40 lines · Keith Busch

Poll for completions if the request's hctx is a polling type.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
block/blk-exec.c +11 -1
index beae70a0e5e5..b960ad187ba5 100644
@@ -63,6 +63,11 @@ void blk_execute_rq_nowait(struct gendisk *bd_disk, struct request *rq,
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
 
+static bool blk_rq_is_poll(struct request *rq)
+{
+	return rq->mq_hctx && rq->mq_hctx->type == HCTX_TYPE_POLL;
+}
+
 /**
  * blk_execute_rq - insert a request into queue for execution
  * @bd_disk:	matching gendisk

@@ -83,7 +88,12 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 
 	/* Prevent hang_check timer from firing at us during very long I/O */
 	hang_check = sysctl_hung_task_timeout_secs;
-	if (hang_check)
+	if (blk_rq_is_poll(rq)) {
+		do {
+			blk_poll(rq->q, request_to_qc_t(rq->mq_hctx, rq), true);
+			cond_resched();
+		} while (!completion_done(&wait));
+	} else if (hang_check)
 		while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
 	else
 		wait_for_completion_io(&wait);
Signature · 1 line
2.25.4
Reviewed-by Ming Lei <[email protected]>
Signature · 1 line
Ming
maintainer
#3 lore raw

Re: [PATCHv2 1/5] block: support polling through blk_execute_rq

In reply to Keith Busch
Quoted 24 lines · Keith Busch

Poll for completions if the request's hctx is a polling type.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
block/blk-exec.c +5 -0
index beae70a0e5e5..b960ad187ba5 100644
@@ -63,6 +63,11 @@ void blk_execute_rq_nowait(struct gendisk *bd_disk, struct request *rq,
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
 
+static bool blk_rq_is_poll(struct request *rq)
+{
+	return rq->mq_hctx && rq->mq_hctx->type == HCTX_TYPE_POLL;
+}
+
 /**
  * blk_execute_rq - insert a request into queue for execution
  * @bd_disk:	matching gendisk

block/blk-exec.c void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head) · line 95

Quoted 9 lines
block/blk-exec.c +5 -1
@@ -83,7 +88,12 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 
 	/* Prevent hang_check timer from firing at us during very long I/O */
 	hang_check = sysctl_hung_task_timeout_secs;
-	if (hang_check)
+	if (blk_rq_is_poll(rq)) {
+		do {
+			blk_poll(rq->q, request_to_qc_t(rq->mq_hctx, rq), true);
+			cond_resched();
+		} while (!completion_done(&wait));

I think it would be nice to split this into a little helper.

block/blk-exec.c void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head) · line 97

Quoted 2 lines
block/blk-exec.c +1 -0
+	} else if (hang_check)
 		while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));

And while we're at it, it would be nice to split this out as well and document this mess..

Reviewed-by on v2 1/5
#4 lore raw

Re: [PATCHv2 1/5] block: support polling through blk_execute_rq

In reply to Keith Busch
Quoted 29 lines · Keith Busch

Poll for completions if the request's hctx is a polling type.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
block/blk-exec.c +5 -1
index beae70a0e5e5..b960ad187ba5 100644
@@ -63,6 +63,11 @@ void blk_execute_rq_nowait(struct gendisk *bd_disk, struct request *rq,
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
 
+static bool blk_rq_is_poll(struct request *rq)
+{
+       return rq->mq_hctx && rq->mq_hctx->type == HCTX_TYPE_POLL;
+}
+
 /**
  * blk_execute_rq - insert a request into queue for execution
  * @bd_disk:   matching gendisk

@@ -83,7 +88,12 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 
        /* Prevent hang_check timer from firing at us during very long I/O */
        hang_check = sysctl_hung_task_timeout_secs;
-       if (hang_check)

block/blk-exec.c void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)

Quoted 10 lines
block/blk-exec.c +6 -1
+       if (blk_rq_is_poll(rq)) {
+               do {
+                       blk_poll(rq->q, request_to_qc_t(rq->mq_hctx, rq), true);
+                       cond_resched();
+               } while (!completion_done(&wait));
+       } else if (hang_check)
                while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
        else
                wait_for_completion_io(&wait);
--

Looks good.

Reviewed-by Kanchan Joshi <[email protected]>

--
Kanchan

_______________________________________________
Linux-nvme mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-nvme

author maintainer
#5 lore raw

PATCH v2 2/5 nvme: use blk_execute_rq() for passthrough commands

In reply to Keith Busch

The generic blk_execute_rq() knows how to handle polled completions. Use that instead of implementing an nvme specific handler.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c    | 38 +++++--------------------------------
 drivers/nvme/host/fabrics.c | 13 ++++++-------
 drivers/nvme/host/fabrics.h |  2 +-
 drivers/nvme/host/fc.c      |  2 +-
 drivers/nvme/host/nvme.h    |  2 +-
 drivers/nvme/host/rdma.c    |  3 +--
 drivers/nvme/host/tcp.c     |  2 +-
 drivers/nvme/target/loop.c  |  2 +-
 8 files changed, 17 insertions(+), 47 deletions(-)
drivers/nvme/host/core.c +5 -33
index 85538c38aae9..10bb8406e067 100644
@@ -944,31 +944,6 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 }
 EXPORT_SYMBOL_GPL(nvme_setup_cmd);
 
-static void nvme_end_sync_rq(struct request *rq, blk_status_t error)
-{
-	struct completion *waiting = rq->end_io_data;
-
-	rq->end_io_data = NULL;
-	complete(waiting);
-}
-
-static void nvme_execute_rq_polled(struct request_queue *q,
-		struct gendisk *bd_disk, struct request *rq, int at_head)
-{
-	DECLARE_COMPLETION_ONSTACK(wait);
-
-	WARN_ON_ONCE(!test_bit(QUEUE_FLAG_POLL, &q->queue_flags));
-
-	rq->cmd_flags |= REQ_HIPRI;
-	rq->end_io_data = &wait;
-	blk_execute_rq_nowait(bd_disk, rq, at_head, nvme_end_sync_rq);
-
-	while (!completion_done(&wait)) {
-		blk_poll(q, request_to_qc_t(rq->mq_hctx, rq), true);
-		cond_resched();
-	}
-}
-
 /*
  * Returns 0 on success.  If the result is negative, it's a Linux error code;
  * if the result is positive, it's an NVM Express status code

@@ -976,7 +951,7 @@ static void nvme_execute_rq_polled(struct request_queue *q,
 int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 		union nvme_result *result, void *buffer, unsigned bufflen,
 		unsigned timeout, int qid, int at_head,
-		blk_mq_req_flags_t flags, bool poll)
+		blk_mq_req_flags_t flags)
 {
 	struct request *req;
 	int ret;

@@ -997,10 +972,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 			goto out;
 	}
 
-	if (poll)
-		nvme_execute_rq_polled(req->q, NULL, req, at_head);
-	else
-		blk_execute_rq(NULL, req, at_head);
+	blk_execute_rq(NULL, req, at_head);
 	if (result)
 		*result = nvme_req(req)->result;
 	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)

@@ -1017,7 +989,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 		void *buffer, unsigned bufflen)
 {
 	return __nvme_submit_sync_cmd(q, cmd, NULL, buffer, bufflen, 0,
-			NVME_QID_ANY, 0, 0, false);
+			NVME_QID_ANY, 0, 0);
 }
 EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
 

@@ -1370,7 +1342,7 @@ static int nvme_features(struct nvme_ctrl *dev, u8 op, unsigned int fid,
 	c.features.dword11 = cpu_to_le32(dword11);
 
 	ret = __nvme_submit_sync_cmd(dev->admin_q, &c, &res,
-			buffer, buflen, 0, NVME_QID_ANY, 0, 0, false);
+			buffer, buflen, 0, NVME_QID_ANY, 0, 0);
 	if (ret >= 0 && result)
 		*result = le32_to_cpu(res.u32);
 	return ret;

@@ -1969,7 +1941,7 @@ int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t len,
 	cmd.common.cdw11 = cpu_to_le32(len);
 
 	return __nvme_submit_sync_cmd(ctrl->admin_q, &cmd, NULL, buffer, len, 0,
-			NVME_QID_ANY, 1, 0, false);
+			NVME_QID_ANY, 1, 0);
 }
 EXPORT_SYMBOL_GPL(nvme_sec_submit);
 #endif /* CONFIG_BLK_SED_OPAL */
drivers/nvme/host/fabrics.c +6 -7
index 604ab0e5a2ad..e468f66878d7 100644
@@ -151,7 +151,7 @@ int nvmf_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val)
 	cmd.prop_get.offset = cpu_to_le32(off);
 
 	ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res, NULL, 0, 0,
-			NVME_QID_ANY, 0, 0, false);
+			NVME_QID_ANY, 0, 0);
 
 	if (ret >= 0)
 		*val = le64_to_cpu(res.u64);

@@ -198,7 +198,7 @@ int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val)
 	cmd.prop_get.offset = cpu_to_le32(off);
 
 	ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res, NULL, 0, 0,
-			NVME_QID_ANY, 0, 0, false);
+			NVME_QID_ANY, 0, 0);
 
 	if (ret >= 0)
 		*val = le64_to_cpu(res.u64);

@@ -244,7 +244,7 @@ int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val)
 	cmd.prop_set.value = cpu_to_le64(val);
 
 	ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, NULL, NULL, 0, 0,
-			NVME_QID_ANY, 0, 0, false);
+			NVME_QID_ANY, 0, 0);
 	if (unlikely(ret))
 		dev_err(ctrl->device,
 			"Property Set error: %d, offset %#x\n",

@@ -398,7 +398,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 
 	ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res,
 			data, sizeof(*data), 0, NVME_QID_ANY, 1,
-			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, false);
+			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
 	if (ret) {
 		nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
 				       &cmd, data);

@@ -422,7 +422,6 @@ EXPORT_SYMBOL_GPL(nvmf_connect_admin_queue);
  * @qid:	NVMe I/O queue number for the new I/O connection between
  *		host and target (note qid == 0 is illegal as this is
  *		the Admin queue, per NVMe standard).
- * @poll:	Whether or not to poll for the completion of the connect cmd.
  *
  * This function issues a fabrics-protocol connection
  * of a NVMe I/O queue (via NVMe Fabrics "Connect" command)

@@ -434,7 +433,7 @@ EXPORT_SYMBOL_GPL(nvmf_connect_admin_queue);
  *	> 0: NVMe error status code
  *	< 0: Linux errno error code
  */
-int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid, bool poll)
+int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
 {
 	struct nvme_command cmd;
 	struct nvmf_connect_data *data;

@@ -461,7 +460,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid, bool poll)
 
 	ret = __nvme_submit_sync_cmd(ctrl->connect_q, &cmd, &res,
 			data, sizeof(*data), 0, qid, 1,
-			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, poll);
+			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
 	if (ret) {
 		nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
 				       &cmd, data);
drivers/nvme/host/fabrics.h +1 -1
index 888b108d87a4..2e296f951679 100644
@@ -178,7 +178,7 @@ int nvmf_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val);
 int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val);
 int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val);
 int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl);
-int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid, bool poll);
+int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid);
 int nvmf_register_transport(struct nvmf_transport_ops *ops);
 void nvmf_unregister_transport(struct nvmf_transport_ops *ops);
 void nvmf_free_options(struct nvmf_ctrl_options *opts);
drivers/nvme/host/fc.c +1 -1
index 9b9b7be0f412..aafa38882d71 100644
@@ -2346,7 +2346,7 @@ nvme_fc_connect_io_queues(struct nvme_fc_ctrl *ctrl, u16 qsize)
 					(qsize / 5));
 		if (ret)
 			break;
-		ret = nvmf_connect_io_queue(&ctrl->ctrl, i, false);
+		ret = nvmf_connect_io_queue(&ctrl->ctrl, i);
 		if (ret)
 			break;
drivers/nvme/host/nvme.h +1 -1
index c6102ce83bb4..c8f6ec5b8d2b 100644
@@ -637,7 +637,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 		union nvme_result *result, void *buffer, unsigned bufflen,
 		unsigned timeout, int qid, int at_head,
-		blk_mq_req_flags_t flags, bool poll);
+		blk_mq_req_flags_t flags);
 int nvme_set_features(struct nvme_ctrl *dev, unsigned int fid,
 		      unsigned int dword11, void *buffer, size_t buflen,
 		      u32 *result);
drivers/nvme/host/rdma.c +1 -2
index 660c774fa9e1..f02212040573 100644
@@ -680,11 +680,10 @@ static void nvme_rdma_stop_io_queues(struct nvme_rdma_ctrl *ctrl)
 static int nvme_rdma_start_queue(struct nvme_rdma_ctrl *ctrl, int idx)
 {
 	struct nvme_rdma_queue *queue = &ctrl->queues[idx];
-	bool poll = nvme_rdma_poll_queue(queue);
 	int ret;
 
 	if (idx)
-		ret = nvmf_connect_io_queue(&ctrl->ctrl, idx, poll);
+		ret = nvmf_connect_io_queue(&ctrl->ctrl, idx);
 	else
 		ret = nvmf_connect_admin_queue(&ctrl->ctrl);
drivers/nvme/host/tcp.c +1 -1
index 75435cdb156c..dc523818b822 100644
@@ -1558,7 +1558,7 @@ static int nvme_tcp_start_queue(struct nvme_ctrl *nctrl, int idx)
 	int ret;
 
 	if (idx)
-		ret = nvmf_connect_io_queue(nctrl, idx, false);
+		ret = nvmf_connect_io_queue(nctrl, idx);
 	else
 		ret = nvmf_connect_admin_queue(nctrl);
drivers/nvme/target/loop.c +1 -1
index 6665da3b634f..bf83b4d1cef7 100644
@@ -335,7 +335,7 @@ static int nvme_loop_connect_io_queues(struct nvme_loop_ctrl *ctrl)
 	int i, ret;
 
 	for (i = 1; i < ctrl->ctrl.queue_count; i++) {
-		ret = nvmf_connect_io_queue(&ctrl->ctrl, i, false);
+		ret = nvmf_connect_io_queue(&ctrl->ctrl, i);
 		if (ret)
 			return ret;
 		set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[i].flags);
Signature · 1 line
2.25.4
author maintainer
#7 lore raw

PATCH v2 3/5 block: return errors from blk_execute_rq()

In reply to Keith Busch

The synchronous blk_execute_rq() had not provided a way for its callers to know if its request was successful or not. Return the errno from the dispatch status.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c       | 6 ++++--
 include/linux/blkdev.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)
block/blk-exec.c +4 -2
index b960ad187ba5..4e8e6fe20956 100644
@@ -21,7 +21,7 @@ static void blk_end_sync_rq(struct request *rq, blk_status_t error)
 {
 	struct completion *waiting = rq->end_io_data;
 
-	rq->end_io_data = NULL;
+	rq->end_io_data = ERR_PTR(blk_status_to_errno(error));
 
 	/*
 	 * complete last, if this is a stack request the process (and thus

@@ -77,8 +77,9 @@ static bool blk_rq_is_poll(struct request *rq)
  * Description:
  *    Insert a fully prepared request at the back of the I/O scheduler queue
  *    for execution and wait for completion.
+ * Return: The errno value of the blk_status_t provided to blk_mq_end_request().
  */
-void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
+int blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
 	unsigned long hang_check;

@@ -97,5 +98,6 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 		while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
 	else
 		wait_for_completion_io(&wait);
+	return PTR_ERR_OR_ZERO(rq->end_io_data);
 }
 EXPORT_SYMBOL(blk_execute_rq);
include/linux/blkdev.h +1 -1
index b91ba6207365..15e4ffac33af 100644
@@ -938,7 +938,7 @@ extern int blk_rq_map_kern(struct request_queue *, struct request *, void *, uns
 extern int blk_rq_map_user_iov(struct request_queue *, struct request *,
 			       struct rq_map_data *, const struct iov_iter *,
 			       gfp_t);
-extern void blk_execute_rq(struct gendisk *, struct request *, int);
+extern int blk_execute_rq(struct gendisk *, struct request *, int);
 extern void blk_execute_rq_nowait(struct gendisk *,
 				  struct request *, int, rq_end_io_fn *);
Signature · 1 line
2.25.4
Reviewed-by on v2 3/5
#8 lore raw

Re: [PATCHv2 3/5] block: return errors from blk_execute_rq()

Quoted 35 lines · Keith Busch

The synchronous blk_execute_rq() had not provided a way for its callers to know if its request was successful or not. Return the errno from the dispatch status.

Signed-off-by Keith Busch <[email protected]>
---
 block/blk-exec.c       | 6 ++++--
 include/linux/blkdev.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)
block/blk-exec.c +3 -2
index b960ad187ba5..4e8e6fe20956 100644
@@ -21,7 +21,7 @@ static void blk_end_sync_rq(struct request *rq, blk_status_t error)
 {
 	struct completion *waiting = rq->end_io_data;
 
-	rq->end_io_data = NULL;
+	rq->end_io_data = ERR_PTR(blk_status_to_errno(error));
 
 	/*
 	 * complete last, if this is a stack request the process (and thus

@@ -77,8 +77,9 @@ static bool blk_rq_is_poll(struct request *rq)
  * Description:
  *    Insert a fully prepared request at the back of the I/O scheduler queue
  *    for execution and wait for completion.
+ * Return: The errno value of the blk_status_t provided to blk_mq_end_request().
  */
-void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
+int blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
 	unsigned long hang_check;

block/blk-exec.c void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head) · line 101

Quoted 4 lines
block/blk-exec.c +1 -0
@@ -97,5 +98,6 @@ void blk_execute_rq(struct gendisk *bd_disk, struct request *rq, int at_head)
 		while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
 	else
 		wait_for_completion_io(&wait);
+	return PTR_ERR_OR_ZERO(rq->end_io_data);
Reviewed-by Ming Lei <[email protected]>
Signature · 7 lines
Ming


_______________________________________________
Linux-nvme mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-nvme
maintainer
#9 lore raw

Re: [PATCHv2 3/5] block: return errors from blk_execute_rq()

In reply to Keith Busch
Quoted 3 lines · Keith Busch

The synchronous blk_execute_rq() had not provided a way for its callers to know if its request was successful or not. Return the errno from the dispatch status.

Same comments on the accidentally sent other version of the patch.

author maintainer
#10 lore raw

PATCH v2 4/5 nvme: use return value from blk_execute_rq()

In reply to Keith Busch

We don't have an nvme status to report if the driver's .queue_rq() returns an error without dispatching the requested nvme command. Use the return value from blk_execute_rq() for all passthrough commands so the caller may know their command was not successful.

If the command is from the target passthrough interface and fails to dispatch, synthesize the response back to the host as a internal target error.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c       | 16 ++++++++++++----
 drivers/nvme/host/ioctl.c      |  6 +-----
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/target/passthru.c |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)
drivers/nvme/host/core.c +12 -4
index 10bb8406e067..62af5fe7a0ce 100644
@@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 			goto out;
 	}
 
-	blk_execute_rq(NULL, req, at_head);
+	ret = blk_execute_rq(NULL, req, at_head);
 	if (result)
 		*result = nvme_req(req)->result;
 	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
 		ret = -EINTR;
-	else
+	else if (nvme_req(req)->status)
 		ret = nvme_req(req)->status;
  out:
 	blk_mq_free_request(req);

@@ -1066,18 +1066,26 @@ static void nvme_passthru_end(struct nvme_ctrl *ctrl, u32 effects)
 	}
 }
 
-void nvme_execute_passthru_rq(struct request *rq)
+int nvme_execute_passthru_rq(struct request *rq)
 {
 	struct nvme_command *cmd = nvme_req(rq)->cmd;
 	struct nvme_ctrl *ctrl = nvme_req(rq)->ctrl;
 	struct nvme_ns *ns = rq->q->queuedata;
 	struct gendisk *disk = ns ? ns->disk : NULL;
 	u32 effects;
+	int ret;
 
 	effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
-	blk_execute_rq(disk, rq, 0);
+	ret = blk_execute_rq(disk, rq, 0);
 	if (effects) /* nothing to be done for zero cmd effects */
 		nvme_passthru_end(ctrl, effects);
+
+	if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
+		ret = -EINTR;
+	else if (nvme_req(rq)->status)
+		ret = nvme_req(rq)->status;
+
+	return ret;
 }
 EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);
drivers/nvme/host/ioctl.c +1 -5
index 8e05d65c9e93..9cdd8bfebb80 100644
@@ -93,11 +93,7 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 		}
 	}
 
-	nvme_execute_passthru_rq(req);
-	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
-		ret = -EINTR;
-	else
-		ret = nvme_req(req)->status;
+	ret = nvme_execute_passthru_rq(req);
 	if (result)
 		*result = le64_to_cpu(nvme_req(req)->result.u64);
 	if (meta && !ret && !write) {
drivers/nvme/host/nvme.h +1 -1
index c8f6ec5b8d2b..76a7ed0728b9 100644
@@ -847,7 +847,7 @@ static inline void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
 
 u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 			 u8 opcode);
-void nvme_execute_passthru_rq(struct request *rq);
+int nvme_execute_passthru_rq(struct request *rq);
 struct nvme_ctrl *nvme_ctrl_from_file(struct file *file);
 struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid);
 void nvme_put_ns(struct nvme_ns *ns);
drivers/nvme/target/passthru.c +4 -4
index 2798944899b7..d9a649d9903b 100644
@@ -153,11 +153,10 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
 {
 	struct nvmet_req *req = container_of(w, struct nvmet_req, p.work);
 	struct request *rq = req->p.rq;
-	u16 status;
+	int status;
 
-	nvme_execute_passthru_rq(rq);
+	status = nvme_execute_passthru_rq(rq);
 
-	status = nvme_req(rq)->status;
 	if (status == NVME_SC_SUCCESS &&
 	    req->cmd->common.opcode == nvme_admin_identify) {
 		switch (req->cmd->identify.cns) {

@@ -168,7 +167,8 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
 			nvmet_passthru_override_id_ns(req);
 			break;
 		}
-	}
+	} else if (status < 0)
+		status = NVME_SC_INTERNAL;
 
 	req->cqe->result = nvme_req(rq)->result;
 	nvmet_req_complete(req, status);
Signature · 1 line
2.25.4
maintainer
#11 lore raw

Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()

Quoted 24 lines · Keith Busch

We don't have an nvme status to report if the driver's .queue_rq() returns an error without dispatching the requested nvme command. Use the return value from blk_execute_rq() for all passthrough commands so the caller may know their command was not successful.

If the command is from the target passthrough interface and fails to dispatch, synthesize the response back to the host as a internal target error.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c       | 16 ++++++++++++----
 drivers/nvme/host/ioctl.c      |  6 +-----
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/target/passthru.c |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)
drivers/nvme/host/core.c +0 -0
index 10bb8406e067..62af5fe7a0ce 100644
@@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 			goto out;
 	}

drivers/nvme/host/core.c int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, · line 981

Quoted 10 lines
drivers/nvme/host/core.c +2 -2
 
-	blk_execute_rq(NULL, req, at_head);
+	ret = blk_execute_rq(NULL, req, at_head);
 	if (result)
 		*result = nvme_req(req)->result;
 	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
 		ret = -EINTR;
-	else
+	else if (nvme_req(req)->status)
 		ret = nvme_req(req)->status;

Just cosmetic, and already in the existing code, but I'd prefer if we could keep the ret assignments together, something like:

	status = blk_execute_rq(NULL, req, at_head);
  	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
 		ret = -EINTR;
	else if (nvme_req(req)->status)
		ret = nvme_req(req)->status;
	else
		ret = blk_status_to_errno(status);

if (result)
*result = nvme_req(req)->result;

drivers/nvme/host/core.c static void nvme_passthru_end(struct nvme_ctrl *ctrl, u32 effects) · line 1087

Quoted 9 lines
drivers/nvme/host/core.c +7 -0
+	ret = blk_execute_rq(disk, rq, 0);
 	if (effects) /* nothing to be done for zero cmd effects */
 		nvme_passthru_end(ctrl, effects);
+
+	if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
+		ret = -EINTR;
+	else if (nvme_req(rq)->status)
+		ret = nvme_req(rq)->status;
+

I think we want a helper for all this return value magic instead of duplicating it.

#12 lore raw

Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()

In reply to Keith Busch
Quoted 24 lines · Keith Busch

We don't have an nvme status to report if the driver's .queue_rq() returns an error without dispatching the requested nvme command. Use the return value from blk_execute_rq() for all passthrough commands so the caller may know their command was not successful.

If the command is from the target passthrough interface and fails to dispatch, synthesize the response back to the host as a internal target error.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c       | 16 ++++++++++++----
 drivers/nvme/host/ioctl.c      |  6 +-----
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/target/passthru.c |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)
drivers/nvme/host/core.c +0 -0
index 10bb8406e067..62af5fe7a0ce 100644
@@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
                        goto out;

drivers/nvme/host/core.c int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, · line 980

Quoted 10 lines
drivers/nvme/host/core.c +2 -2
        }
 
-       blk_execute_rq(NULL, req, at_head);
+       ret = blk_execute_rq(NULL, req, at_head);
        if (result)
                *result = nvme_req(req)->result;
        if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
                ret = -EINTR;
-       else
+       else if (nvme_req(req)->status)

Since nvme_req(req)->status is uninitialized for a command failed to dispatch, it's valid only if ret from blk_execute_rq() is 0.

author maintainer
#13 lore raw

Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()

Quoted 27 lines · Yuanyuan Zhong
Quoted 26 lines · Keith Busch

We don't have an nvme status to report if the driver's .queue_rq() returns an error without dispatching the requested nvme command. Use the return value from blk_execute_rq() for all passthrough commands so the caller may know their command was not successful.

If the command is from the target passthrough interface and fails to dispatch, synthesize the response back to the host as a internal target error.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c       | 16 ++++++++++++----
 drivers/nvme/host/ioctl.c      |  6 +-----
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/target/passthru.c |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)
drivers/nvme/host/core.c +0 -0
index 10bb8406e067..62af5fe7a0ce 100644
@@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
                        goto out;
        }
Quoted 11 lines

drivers/nvme/host/core.c int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, · line 980

Quoted 8 lines
drivers/nvme/host/core.c +2 -2
-       blk_execute_rq(NULL, req, at_head);
+       ret = blk_execute_rq(NULL, req, at_head);
        if (result)
                *result = nvme_req(req)->result;
        if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
                ret = -EINTR;
-       else
+       else if (nvme_req(req)->status)

Since nvme_req(req)->status is uninitialized for a command failed to dispatch, it's valid only if ret from blk_execute_rq() is 0.

That's not quite right. If queue_rq() succeeds, but the SSD returns an error, blk_execute_rq() returns a non-zero value with a valid nvme_req status.

#14 lore raw

Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()

Quoted 41 lines · Keith Busch
Quoted 27 lines · Yuanyuan Zhong
Quoted 26 lines · Keith Busch

We don't have an nvme status to report if the driver's .queue_rq() returns an error without dispatching the requested nvme command. Use the return value from blk_execute_rq() for all passthrough commands so the caller may know their command was not successful.

If the command is from the target passthrough interface and fails to dispatch, synthesize the response back to the host as a internal target error.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c       | 16 ++++++++++++----
 drivers/nvme/host/ioctl.c      |  6 +-----
 drivers/nvme/host/nvme.h       |  2 +-
 drivers/nvme/target/passthru.c |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)
drivers/nvme/host/core.c +0 -0
index 10bb8406e067..62af5fe7a0ce 100644
@@ -972,12 +972,12 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
                        goto out;
        }
Quoted 11 lines

drivers/nvme/host/core.c int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd, · line 980

Quoted 8 lines
drivers/nvme/host/core.c +2 -2
-       blk_execute_rq(NULL, req, at_head);
+       ret = blk_execute_rq(NULL, req, at_head);
        if (result)
                *result = nvme_req(req)->result;
        if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
                ret = -EINTR;
-       else
+       else if (nvme_req(req)->status)

Since nvme_req(req)->status is uninitialized for a command failed to dispatch, it's valid only if ret from blk_execute_rq() is 0.

Quoted 3 lines

That's not quite right. If queue_rq() succeeds, but the SSD returns an error, blk_execute_rq() returns a non-zero value with a valid nvme_req status.

Agreed. But after that, freeing the req let nvme_req(req)->status from SSD stay. If the same req get re-allocated and get dispatching failure, shouldn't the dispatching
error take precedence over the stale nvme_req(req)->status?

author maintainer
#15 lore raw

PATCH v2 5/5 nvme: allow user passthrough commands to poll

In reply to Keith Busch

The block layer knows how to deal with polled requests. Let the NVMe driver use the previously reserved user "flags" fields to define an option to allocate the request from the polled hardware contexts. If polling is not enabled, then the block layer will automatically fallback to a non-polled request.

Signed-off-by Keith Busch <[email protected]>
---
 drivers/nvme/host/core.c        | 10 ++++++----
 drivers/nvme/host/ioctl.c       | 32 ++++++++++++++++++--------------
 drivers/nvme/host/lightnvm.c    |  4 ++--
 drivers/nvme/host/nvme.h        |  3 ++-
 drivers/nvme/host/pci.c         |  4 ++--
 drivers/nvme/target/passthru.c  |  2 +-
 include/uapi/linux/nvme_ioctl.h |  4 ++++
 7 files changed, 35 insertions(+), 24 deletions(-)
drivers/nvme/host/core.c +6 -4
index 62af5fe7a0ce..3af4d955405b 100644
@@ -610,11 +610,13 @@ static inline void nvme_init_request(struct request *req,
 }
 
 struct request *nvme_alloc_request(struct request_queue *q,
-		struct nvme_command *cmd, blk_mq_req_flags_t flags)
+		struct nvme_command *cmd, blk_mq_req_flags_t flags,
+		unsigned rq_flags)
 {
+	unsigned cmd_flags = nvme_req_op(cmd) | rq_flags;
 	struct request *req;
 
-	req = blk_mq_alloc_request(q, nvme_req_op(cmd), flags);
+	req = blk_mq_alloc_request(q, cmd_flags, flags);
 	if (!IS_ERR(req))
 		nvme_init_request(req, cmd);
 	return req;

@@ -957,7 +959,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 	int ret;
 
 	if (qid == NVME_QID_ANY)
-		req = nvme_alloc_request(q, cmd, flags);
+		req = nvme_alloc_request(q, cmd, flags, 0);
 	else
 		req = nvme_alloc_request_qid(q, cmd, flags, qid);
 	if (IS_ERR(req))

@@ -1130,7 +1132,7 @@ static void nvme_keep_alive_work(struct work_struct *work)
 	}
 
 	rq = nvme_alloc_request(ctrl->admin_q, &ctrl->ka_cmd,
-				BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+				BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, 0);
 	if (IS_ERR(rq)) {
 		/* allocation failure, reset the controller */
 		dev_err(ctrl->device, "keep-alive failed: %ld\n", PTR_ERR(rq));
drivers/nvme/host/ioctl.c +18 -14
index 9cdd8bfebb80..79fc90f010b0 100644
@@ -56,7 +56,7 @@ static void *nvme_add_user_metadata(struct bio *bio, void __user *ubuf,
 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, unsigned rq_flags)
 {
 	bool write = nvme_is_write(cmd);
 	struct nvme_ns *ns = q->queuedata;

@@ -66,7 +66,7 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	void *meta = NULL;
 	int ret;
 
-	req = nvme_alloc_request(q, cmd, 0);
+	req = nvme_alloc_request(q, cmd, 0, rq_flags);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 

@@ -116,11 +116,12 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 	struct nvme_command c;
 	unsigned length, meta_len;
 	void __user *metadata;
+	unsigned rq_flags = 0;
 
 	if (copy_from_user(&io, uio, sizeof(io)))
 		return -EFAULT;
-	if (io.flags)
-		return -EINVAL;
+	if (io.flags & NVME_HIPRI)
+		rq_flags |= REQ_HIPRI;
 
 	switch (io.opcode) {
 	case nvme_cmd_write:

@@ -158,7 +159,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 
 	memset(&c, 0, sizeof(c));
 	c.rw.opcode = io.opcode;
-	c.rw.flags = io.flags;
+	c.rw.flags = 0;
 	c.rw.nsid = cpu_to_le32(ns->head->ns_id);
 	c.rw.slba = cpu_to_le64(io.slba);
 	c.rw.length = cpu_to_le16(io.nblocks);

@@ -170,7 +171,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,
+			rq_flags);
 }
 
 static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

@@ -178,6 +180,7 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 {
 	struct nvme_passthru_cmd cmd;
 	struct nvme_command c;
+	unsigned rq_flags = 0;
 	unsigned timeout = 0;
 	u64 result;
 	int status;

@@ -186,8 +189,8 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 		return -EACCES;
 	if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
 		return -EFAULT;
-	if (cmd.flags)
-		return -EINVAL;
+	if (cmd.flags & NVME_HIPRI)
+		rq_flags |= REQ_HIPRI;
 	if (ns && cmd.nsid != ns->head->ns_id) {
 		dev_err(ctrl->device,
 			"%s: nsid (%u) in cmd does not match nsid (%u) of namespace\n",

@@ -197,7 +200,7 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 
 	memset(&c, 0, sizeof(c));
 	c.common.opcode = cmd.opcode;
-	c.common.flags = cmd.flags;
+	c.common.flags = 0;
 	c.common.nsid = cpu_to_le32(cmd.nsid);
 	c.common.cdw2[0] = cpu_to_le32(cmd.cdw2);
 	c.common.cdw2[1] = cpu_to_le32(cmd.cdw3);

@@ -214,7 +217,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, rq_flags);
 
 	if (status >= 0) {
 		if (put_user(result, &ucmd->result))

@@ -229,6 +232,7 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 {
 	struct nvme_passthru_cmd64 cmd;
 	struct nvme_command c;
+	unsigned rq_flags = 0;
 	unsigned timeout = 0;
 	int status;
 

@@ -236,8 +240,8 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 		return -EACCES;
 	if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
 		return -EFAULT;
-	if (cmd.flags)
-		return -EINVAL;
+	if (cmd.flags & NVME_HIPRI)
+		rq_flags |= REQ_HIPRI;
 	if (ns && cmd.nsid != ns->head->ns_id) {
 		dev_err(ctrl->device,
 			"%s: nsid (%u) in cmd does not match nsid (%u) of namespace\n",

@@ -247,7 +251,7 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 
 	memset(&c, 0, sizeof(c));
 	c.common.opcode = cmd.opcode;
-	c.common.flags = cmd.flags;
+	c.common.flags = 0;
 	c.common.nsid = cpu_to_le32(cmd.nsid);
 	c.common.cdw2[0] = cpu_to_le32(cmd.cdw2);
 	c.common.cdw2[1] = cpu_to_le32(cmd.cdw3);

@@ -264,7 +268,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, rq_flags);
 
 	if (status >= 0) {
 		if (put_user(cmd.result, &ucmd->result))
drivers/nvme/host/lightnvm.c +2 -2
index e9d9ad47f70f..a7fff633cdee 100644
@@ -653,7 +653,7 @@ static struct request *nvme_nvm_alloc_request(struct request_queue *q,
 
 	nvme_nvm_rqtocmd(rqd, ns, cmd);
 
-	rq = nvme_alloc_request(q, (struct nvme_command *)cmd, 0);
+	rq = nvme_alloc_request(q, (struct nvme_command *)cmd, 0, 0);
 	if (IS_ERR(rq))
 		return rq;
 

@@ -766,7 +766,7 @@ static int nvme_nvm_submit_user_cmd(struct request_queue *q,
 	DECLARE_COMPLETION_ONSTACK(wait);
 	int ret = 0;
 
-	rq = nvme_alloc_request(q, (struct nvme_command *)vcmd, 0);
+	rq = nvme_alloc_request(q, (struct nvme_command *)vcmd, 0, 0);
 	if (IS_ERR(rq)) {
 		ret = -ENOMEM;
 		goto err_cmd;
drivers/nvme/host/nvme.h +2 -1
index 76a7ed0728b9..6005a61c7f4c 100644
@@ -629,7 +629,8 @@ void nvme_start_freeze(struct nvme_ctrl *ctrl);
 
 #define NVME_QID_ANY -1
 struct request *nvme_alloc_request(struct request_queue *q,
-		struct nvme_command *cmd, blk_mq_req_flags_t flags);
+		struct nvme_command *cmd, blk_mq_req_flags_t flags,
+		unsigned rq_flags);
 void nvme_cleanup_cmd(struct request *req);
 blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req);
 int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
drivers/nvme/host/pci.c +2 -2
index 09d4c5f99fc3..0400ac53964f 100644
@@ -1344,7 +1344,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 		 req->tag, nvmeq->qid);
 
 	abort_req = nvme_alloc_request(dev->ctrl.admin_q, &cmd,
-			BLK_MQ_REQ_NOWAIT);
+			BLK_MQ_REQ_NOWAIT, 0);
 	if (IS_ERR(abort_req)) {
 		atomic_inc(&dev->ctrl.abort_limit);
 		return BLK_EH_RESET_TIMER;

@@ -2268,7 +2268,7 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
 	cmd.delete_queue.opcode = opcode;
 	cmd.delete_queue.qid = cpu_to_le16(nvmeq->qid);
 
-	req = nvme_alloc_request(q, &cmd, BLK_MQ_REQ_NOWAIT);
+	req = nvme_alloc_request(q, &cmd, BLK_MQ_REQ_NOWAIT, 0);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
drivers/nvme/target/passthru.c +1 -1
index d9a649d9903b..be7c6d50d23a 100644
@@ -244,7 +244,7 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
 		timeout = nvmet_req_subsys(req)->admin_timeout;
 	}
 
-	rq = nvme_alloc_request(q, req->cmd, 0);
+	rq = nvme_alloc_request(q, req->cmd, 0, 0);
 	if (IS_ERR(rq)) {
 		status = NVME_SC_INTERNAL;
 		goto out_put_ns;
include/uapi/linux/nvme_ioctl.h +4 -0
index d99b5a772698..683d33c37a96 100644
@@ -9,6 +9,10 @@
 
 #include <linux/types.h>
 
+enum nvme_io_flags {
+	NVME_HIPRI	= 1 << 0, /* use polling queue if available */
+};
+
 struct nvme_user_io {
 	__u8	opcode;
 	__u8	flags;
Signature · 1 line
2.25.4
maintainer
#16 lore raw

Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll

Quoted 5 lines · Keith Busch

The block layer knows how to deal with polled requests. Let the NVMe driver use the previously reserved user "flags" fields to define an option to allocate the request from the polled hardware contexts. If polling is not enabled, then the block layer will automatically fallback to a non-polled request.

So this only support synchronous polling for a single command. What use case do we have for that? I think io_uring based polling would be much more useful once we support NVMe passthrough through that.

author maintainer
#17 lore raw

Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll

Quoted 10 lines · Christoph Hellwig
Quoted 5 lines · Keith Busch

The block layer knows how to deal with polled requests. Let the NVMe driver use the previously reserved user "flags" fields to define an option to allocate the request from the polled hardware contexts. If polling is not enabled, then the block layer will automatically fallback to a non-polled request.

So this only support synchronous polling for a single command. What use case do we have for that? I think io_uring based polling would be much more useful once we support NVMe passthrough through that.

There is no significant use case here. I just needed a simple way to test the polled exec from earlier in the series. It was simple enough so I included the patch here, but it's really not important compared to the preceeding patches.

#18 lore raw

Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll

Quoted 9 lines · Keith Busch
Quoted 7 lines · Christoph Hellwig
Quoted 5 lines · Keith Busch

The block layer knows how to deal with polled requests. Let the NVMe driver use the previously reserved user "flags" fields to define an option to allocate the request from the polled hardware contexts. If polling is not enabled, then the block layer will automatically fallback to a non-polled request.

Quoted 8 lines
Quoted 3 lines

So this only support synchronous polling for a single command. What use case do we have for that? I think io_uring based polling would be much more useful once we support NVMe passthrough through that.

There is no significant use case here. I just needed a simple way to test the polled exec from earlier in the series. It was simple enough so I included the patch here, but it's really not important compared to the preceeding patches.

It would be great to see this in at some point; helps in making passthrough more useful.
I'll look into integrating this with async-passthrough.

Signature · 1 line
Kanchan
author maintainer
#19 lore raw

Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll

Quoted 14 lines · Kanchan Joshi
Quoted 13 lines · Keith Busch
Quoted 10 lines · Christoph Hellwig
Quoted 5 lines · Keith Busch

The block layer knows how to deal with polled requests. Let the NVMe driver use the previously reserved user "flags" fields to define an option to allocate the request from the polled hardware contexts. If polling is not enabled, then the block layer will automatically fallback to a non-polled request.

So this only support synchronous polling for a single command. What use case do we have for that? I think io_uring based polling would be much more useful once we support NVMe passthrough through that.

Quoted 8 lines
Quoted 4 lines

There is no significant use case here. I just needed a simple way to test the polled exec from earlier in the series. It was simple enough so I included the patch here, but it's really not important compared to the preceeding patches.

It would be great to see this in at some point; helps in making passthrough more useful.
I'll look into integrating this with async-passthrough.

Right, async ioctl would really provide better justification for passthrough polling. I'll post a new version of this series this week to address the previously submitted feedback, but without this patch for now.