* [PATCHv2 1/5] block: support polling through blk_execute_rq
2021-04-23 22:05 [PATCHv2 0/5] block and nvme passthrough error handling Keith Busch
@ 2021-04-23 22:05 ` Keith Busch
2021-04-26 6:34 ` Ming Lei
` (2 more replies)
2021-04-23 22:05 ` [PATCHv2 2/5] nvme: use blk_execute_rq() for passthrough commands Keith Busch
` (3 subsequent siblings)
4 siblings, 3 replies; 20+ messages in thread
From: Keith Busch @ 2021-04-23 22:05 UTC (permalink / raw)
To: linux-nvme, sagi, hch, axboe, linux-block
Cc: Yuanyuan Zhong, Casey Chen, Keith Busch
Poll for completions if the request's hctx is a polling type.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-exec.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/block/blk-exec.c b/block/blk-exec.c
index beae70a0e5e5..b960ad187ba5 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -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);
--
2.25.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCHv2 1/5] block: support polling through blk_execute_rq
2021-04-23 22:05 ` [PATCHv2 1/5] block: support polling through blk_execute_rq Keith Busch
@ 2021-04-26 6:34 ` Ming Lei
2021-04-26 14:35 ` Christoph Hellwig
2021-05-17 16:43 ` Kanchan Joshi
2 siblings, 0 replies; 20+ messages in thread
From: Ming Lei @ 2021-04-26 6:34 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:54PM -0700, Keith Busch wrote:
> Poll for completions if the request's hctx is a polling type.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> block/blk-exec.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index beae70a0e5e5..b960ad187ba5 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -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);
> --
> 2.25.4
>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
--
Ming
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCHv2 1/5] block: support polling through blk_execute_rq
2021-04-23 22:05 ` [PATCHv2 1/5] block: support polling through blk_execute_rq Keith Busch
2021-04-26 6:34 ` Ming Lei
@ 2021-04-26 14:35 ` Christoph Hellwig
2021-05-17 16:43 ` Kanchan Joshi
2 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2021-04-26 14:35 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:54PM -0700, Keith Busch wrote:
> Poll for completions if the request's hctx is a polling type.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> block/blk-exec.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index beae70a0e5e5..b960ad187ba5 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -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));
I think it would be nice to split this into a little helper.
> + } 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..
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCHv2 1/5] block: support polling through blk_execute_rq
2021-04-23 22:05 ` [PATCHv2 1/5] block: support polling through blk_execute_rq Keith Busch
2021-04-26 6:34 ` Ming Lei
2021-04-26 14:35 ` Christoph Hellwig
@ 2021-05-17 16:43 ` Kanchan Joshi
2 siblings, 0 replies; 20+ messages in thread
From: Kanchan Joshi @ 2021-05-17 16:43 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, Sagi Grimberg, Christoph Hellwig, Jens Axboe,
linux-block, Yuanyuan Zhong, Casey Chen
On Sat, Apr 24, 2021 at 3:37 AM Keith Busch <kbusch@kernel.org> wrote:
>
> Poll for completions if the request's hctx is a polling type.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> block/blk-exec.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index beae70a0e5e5..b960ad187ba5 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -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);
> --
Looks good.
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
--
Kanchan
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCHv2 2/5] nvme: use blk_execute_rq() for passthrough commands
2021-04-23 22:05 [PATCHv2 0/5] block and nvme passthrough error handling Keith Busch
2021-04-23 22:05 ` [PATCHv2 1/5] block: support polling through blk_execute_rq Keith Busch
@ 2021-04-23 22:05 ` Keith Busch
2021-04-26 14:35 ` Christoph Hellwig
2021-04-23 22:05 ` [PATCHv2 3/5] block: return errors from blk_execute_rq() Keith Busch
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-04-23 22:05 UTC (permalink / raw)
To: linux-nvme, sagi, hch, axboe, linux-block
Cc: Yuanyuan Zhong, Casey Chen, 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 <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 85538c38aae9..10bb8406e067 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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 */
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 604ab0e5a2ad..e468f66878d7 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -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);
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index 888b108d87a4..2e296f951679 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -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);
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 9b9b7be0f412..aafa38882d71 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -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;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index c6102ce83bb4..c8f6ec5b8d2b 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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);
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 660c774fa9e1..f02212040573 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -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);
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 75435cdb156c..dc523818b822 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -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);
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 6665da3b634f..bf83b4d1cef7 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -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);
--
2.25.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCHv2 3/5] block: return errors from blk_execute_rq()
2021-04-23 22:05 [PATCHv2 0/5] block and nvme passthrough error handling Keith Busch
2021-04-23 22:05 ` [PATCHv2 1/5] block: support polling through blk_execute_rq Keith Busch
2021-04-23 22:05 ` [PATCHv2 2/5] nvme: use blk_execute_rq() for passthrough commands Keith Busch
@ 2021-04-23 22:05 ` Keith Busch
2021-04-26 6:42 ` Ming Lei
2021-04-26 14:36 ` Christoph Hellwig
2021-04-23 22:05 ` [PATCHv2 4/5] nvme: use return value " Keith Busch
2021-04-23 22:05 ` [PATCHv2 5/5] nvme: allow user passthrough commands to poll Keith Busch
4 siblings, 2 replies; 20+ messages in thread
From: Keith Busch @ 2021-04-23 22:05 UTC (permalink / raw)
To: linux-nvme, sagi, hch, axboe, linux-block
Cc: Yuanyuan Zhong, Casey Chen, 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 <kbusch@kernel.org>
---
block/blk-exec.c | 6 ++++--
include/linux/blkdev.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/block/blk-exec.c b/block/blk-exec.c
index b960ad187ba5..4e8e6fe20956 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -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);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index b91ba6207365..15e4ffac33af 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -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 *);
--
2.25.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCHv2 3/5] block: return errors from blk_execute_rq()
2021-04-23 22:05 ` [PATCHv2 3/5] block: return errors from blk_execute_rq() Keith Busch
@ 2021-04-26 6:42 ` Ming Lei
2021-04-26 14:36 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Ming Lei @ 2021-04-26 6:42 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:56PM -0700, Keith Busch wrote:
> 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 <kbusch@kernel.org>
> ---
> block/blk-exec.c | 6 ++++--
> include/linux/blkdev.h | 2 +-
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index b960ad187ba5..4e8e6fe20956 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -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);
Reviewed-by: Ming Lei <ming.lei@redhat.com>
--
Ming
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCHv2 3/5] block: return errors from blk_execute_rq()
2021-04-23 22:05 ` [PATCHv2 3/5] block: return errors from blk_execute_rq() Keith Busch
2021-04-26 6:42 ` Ming Lei
@ 2021-04-26 14:36 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2021-04-26 14:36 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:56PM -0700, Keith Busch wrote:
> 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCHv2 4/5] nvme: use return value from blk_execute_rq()
2021-04-23 22:05 [PATCHv2 0/5] block and nvme passthrough error handling Keith Busch
` (2 preceding siblings ...)
2021-04-23 22:05 ` [PATCHv2 3/5] block: return errors from blk_execute_rq() Keith Busch
@ 2021-04-23 22:05 ` Keith Busch
2021-04-26 14:42 ` Christoph Hellwig
2021-04-26 17:10 ` Yuanyuan Zhong
2021-04-23 22:05 ` [PATCHv2 5/5] nvme: allow user passthrough commands to poll Keith Busch
4 siblings, 2 replies; 20+ messages in thread
From: Keith Busch @ 2021-04-23 22:05 UTC (permalink / raw)
To: linux-nvme, sagi, hch, axboe, linux-block
Cc: Yuanyuan Zhong, Casey Chen, 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 <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 10bb8406e067..62af5fe7a0ce 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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);
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 8e05d65c9e93..9cdd8bfebb80 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -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) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index c8f6ec5b8d2b..76a7ed0728b9 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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);
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 2798944899b7..d9a649d9903b 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -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);
--
2.25.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()
2021-04-23 22:05 ` [PATCHv2 4/5] nvme: use return value " Keith Busch
@ 2021-04-26 14:42 ` Christoph Hellwig
2021-04-26 17:10 ` Yuanyuan Zhong
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2021-04-26 14:42 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:57PM -0700, Keith Busch wrote:
> 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 <kbusch@kernel.org>
> ---
> 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(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 10bb8406e067..62af5fe7a0ce 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -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;
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;
> + 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()
2021-04-23 22:05 ` [PATCHv2 4/5] nvme: use return value " Keith Busch
2021-04-26 14:42 ` Christoph Hellwig
@ 2021-04-26 17:10 ` Yuanyuan Zhong
2021-04-26 17:15 ` Keith Busch
1 sibling, 1 reply; 20+ messages in thread
From: Yuanyuan Zhong @ 2021-04-26 17:10 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, Christoph Hellwig, axboe, linux-block, Casey Chen
On Fri, Apr 23, 2021 at 3:06 PM Keith Busch <kbusch@kernel.org> wrote:
>
> 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 <kbusch@kernel.org>
> ---
> 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(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 10bb8406e067..62af5fe7a0ce 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -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)
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()
2021-04-26 17:10 ` Yuanyuan Zhong
@ 2021-04-26 17:15 ` Keith Busch
2021-04-26 17:39 ` Yuanyuan Zhong
0 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-04-26 17:15 UTC (permalink / raw)
To: Yuanyuan Zhong
Cc: linux-nvme, sagi, Christoph Hellwig, axboe, linux-block, Casey Chen
On Mon, Apr 26, 2021 at 10:10:09AM -0700, Yuanyuan Zhong wrote:
> On Fri, Apr 23, 2021 at 3:06 PM Keith Busch <kbusch@kernel.org> wrote:
> >
> > 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 <kbusch@kernel.org>
> > ---
> > 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(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 10bb8406e067..62af5fe7a0ce 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -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)
>
> 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 4/5] nvme: use return value from blk_execute_rq()
2021-04-26 17:15 ` Keith Busch
@ 2021-04-26 17:39 ` Yuanyuan Zhong
0 siblings, 0 replies; 20+ messages in thread
From: Yuanyuan Zhong @ 2021-04-26 17:39 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, Christoph Hellwig, axboe, linux-block, Casey Chen
On Mon, Apr 26, 2021 at 10:15 AM Keith Busch <kbusch@kernel.org> wrote:
>
> On Mon, Apr 26, 2021 at 10:10:09AM -0700, Yuanyuan Zhong wrote:
> > On Fri, Apr 23, 2021 at 3:06 PM Keith Busch <kbusch@kernel.org> wrote:
> > >
> > > 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 <kbusch@kernel.org>
> > > ---
> > > 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(-)
> > >
> > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > > index 10bb8406e067..62af5fe7a0ce 100644
> > > --- a/drivers/nvme/host/core.c
> > > +++ b/drivers/nvme/host/core.c
> > > @@ -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)
> >
> > 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.
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?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCHv2 5/5] nvme: allow user passthrough commands to poll
2021-04-23 22:05 [PATCHv2 0/5] block and nvme passthrough error handling Keith Busch
` (3 preceding siblings ...)
2021-04-23 22:05 ` [PATCHv2 4/5] nvme: use return value " Keith Busch
@ 2021-04-23 22:05 ` Keith Busch
2021-04-26 14:43 ` Christoph Hellwig
4 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-04-23 22:05 UTC (permalink / raw)
To: linux-nvme, sagi, hch, axboe, linux-block
Cc: Yuanyuan Zhong, Casey Chen, 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 <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 62af5fe7a0ce..3af4d955405b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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));
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9cdd8bfebb80..79fc90f010b0 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -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))
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index e9d9ad47f70f..a7fff633cdee 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -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;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 76a7ed0728b9..6005a61c7f4c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 09d4c5f99fc3..0400ac53964f 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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);
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index d9a649d9903b..be7c6d50d23a 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -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;
diff --git a/include/uapi/linux/nvme_ioctl.h b/include/uapi/linux/nvme_ioctl.h
index d99b5a772698..683d33c37a96 100644
--- a/include/uapi/linux/nvme_ioctl.h
+++ b/include/uapi/linux/nvme_ioctl.h
@@ -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;
--
2.25.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll
2021-04-23 22:05 ` [PATCHv2 5/5] nvme: allow user passthrough commands to poll Keith Busch
@ 2021-04-26 14:43 ` Christoph Hellwig
2021-04-26 15:15 ` Keith Busch
0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2021-04-26 14:43 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Fri, Apr 23, 2021 at 03:05:58PM -0700, Keith Busch wrote:
> 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll
2021-04-26 14:43 ` Christoph Hellwig
@ 2021-04-26 15:15 ` Keith Busch
2021-05-17 16:55 ` Kanchan Joshi
0 siblings, 1 reply; 20+ messages in thread
From: Keith Busch @ 2021-04-26 15:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-nvme, sagi, axboe, linux-block, Yuanyuan Zhong, Casey Chen
On Mon, Apr 26, 2021 at 04:43:16PM +0200, Christoph Hellwig wrote:
> On Fri, Apr 23, 2021 at 03:05:58PM -0700, Keith Busch wrote:
> > 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll
2021-04-26 15:15 ` Keith Busch
@ 2021-05-17 16:55 ` Kanchan Joshi
2021-05-17 17:14 ` Keith Busch
0 siblings, 1 reply; 20+ messages in thread
From: Kanchan Joshi @ 2021-05-17 16:55 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, linux-nvme, Sagi Grimberg, Jens Axboe,
linux-block, Yuanyuan Zhong, Casey Chen
On Mon, Apr 26, 2021 at 8:46 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Mon, Apr 26, 2021 at 04:43:16PM +0200, Christoph Hellwig wrote:
> > On Fri, Apr 23, 2021 at 03:05:58PM -0700, Keith Busch wrote:
> > > 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.
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.
--
Kanchan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCHv2 5/5] nvme: allow user passthrough commands to poll
2021-05-17 16:55 ` Kanchan Joshi
@ 2021-05-17 17:14 ` Keith Busch
0 siblings, 0 replies; 20+ messages in thread
From: Keith Busch @ 2021-05-17 17:14 UTC (permalink / raw)
To: Kanchan Joshi
Cc: Christoph Hellwig, linux-nvme, Sagi Grimberg, Jens Axboe,
linux-block, Yuanyuan Zhong, Casey Chen
On Mon, May 17, 2021 at 10:25:21PM +0530, Kanchan Joshi wrote:
> On Mon, Apr 26, 2021 at 8:46 PM Keith Busch <kbusch@kernel.org> wrote:
> >
> > On Mon, Apr 26, 2021 at 04:43:16PM +0200, Christoph Hellwig wrote:
> > > On Fri, Apr 23, 2021 at 03:05:58PM -0700, Keith Busch wrote:
> > > > 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.
>
> 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.
^ permalink raw reply [flat|nested] 20+ messages in thread