* [PATCH] nvme: retry commands if DNR bit is not set
@ 2022-11-02 7:36 Hannes Reinecke
2022-11-02 8:00 ` Chao Leng
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2022-11-02 7:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke
Add a 'retries' argument to __nvme_submit_sync_cmd() to instruct
the function to retry the command if the DNR bit is not set in
the command result, and modify the authentication code to allow
for retries.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/nvme/host/auth.c | 2 +-
drivers/nvme/host/core.c | 29 ++++++++++++++++++++++++-----
drivers/nvme/host/fabrics.c | 10 +++++-----
drivers/nvme/host/nvme.h | 6 +++++-
4 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index c8a6db7c4498..3b63aa155beb 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -69,7 +69,7 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
qid == 0 ? NVME_QID_ANY : qid,
- 0, flags);
+ 0, flags, nvme_max_retries);
if (ret > 0)
dev_warn(ctrl->device,
"qid %d auth_send failed with status %d\n", qid, ret);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dc4220600585..d0de1a85596a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -54,7 +54,7 @@ static unsigned char shutdown_timeout = 5;
module_param(shutdown_timeout, byte, 0644);
MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
-static u8 nvme_max_retries = 5;
+u8 nvme_max_retries = 5;
module_param_named(max_retries, nvme_max_retries, byte, 0644);
MODULE_PARM_DESC(max_retries, "max number of retries a command may have");
@@ -1016,7 +1016,7 @@ static int nvme_execute_rq(struct request *rq, bool at_head)
*/
int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
union nvme_result *result, void *buffer, unsigned bufflen,
- int qid, int at_head, blk_mq_req_flags_t flags)
+ int qid, int at_head, blk_mq_req_flags_t flags, int retries)
{
struct request *req;
int ret;
@@ -1030,6 +1030,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
if (IS_ERR(req))
return PTR_ERR(req);
nvme_init_request(req, cmd);
+ nvme_req(req)->retries = retries;
if (buffer && bufflen) {
ret = blk_rq_map_kern(q, req, buffer, bufflen, GFP_KERNEL);
@@ -1037,8 +1038,18 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
goto out;
}
+retry:
req->rq_flags |= RQF_QUIET;
ret = nvme_execute_rq(req, at_head);
+ if (ret > 0) {
+ struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+
+ if (ctrl->kas)
+ ctrl->comp_seen = true;
+
+ if (retries-- && nvme_decide_disposition(req) != COMPLETE)
+ goto retry;
+ }
if (result && ret >= 0)
*result = nvme_req(req)->result;
out:
@@ -1051,10 +1062,18 @@ 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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, 0);
}
EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
+int nvme_submit_sync_cmd_retry(struct request_queue *q,
+ struct nvme_command *cmd, void *buffer, unsigned bufflen)
+{
+ return __nvme_submit_sync_cmd(q, cmd, NULL, buffer, bufflen,
+ NVME_QID_ANY, 0, 0, nvme_max_retries);
+}
+EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd_retry);
+
static u32 nvme_known_admin_effects(u8 opcode)
{
switch (opcode) {
@@ -1500,7 +1519,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, NVME_QID_ANY, 0, 0);
+ buffer, buflen, NVME_QID_ANY, 0, 0, 0);
if (ret >= 0 && result)
*result = le32_to_cpu(res.u32);
return ret;
@@ -2195,7 +2214,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,
- NVME_QID_ANY, 1, 0);
+ NVME_QID_ANY, 1, 0, 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 ce27276f552d..f4bfbaf733e9 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -153,7 +153,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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, 0);
if (ret >= 0)
*val = le64_to_cpu(res.u64);
@@ -199,7 +199,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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, 0);
if (unlikely(ret))
dev_err(ctrl->device,
"Property Set error: %d, offset %#x\n",
@@ -401,7 +401,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res,
data, sizeof(*data), NVME_QID_ANY, 1,
- BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+ BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, 0);
if (ret) {
nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
&cmd, data);
@@ -480,7 +480,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
ret = __nvme_submit_sync_cmd(ctrl->connect_q, &cmd, &res,
data, sizeof(*data), qid, 1,
- BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+ BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, 0);
if (ret) {
nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
&cmd, data);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a29877217ee6..32d9dc2d957e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -47,6 +47,8 @@ extern struct workqueue_struct *nvme_wq;
extern struct workqueue_struct *nvme_reset_wq;
extern struct workqueue_struct *nvme_delete_wq;
+extern u8 nvme_max_retries;
+
/*
* List of workarounds for devices that required behavior not specified in
* the standard.
@@ -811,10 +813,12 @@ static inline bool nvme_is_unique_nsid(struct nvme_ctrl *ctrl,
int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
void *buf, unsigned bufflen);
+int nvme_submit_sync_cmd_retry(struct request_queue *q,
+ struct nvme_command *cmd, void *buf, unsigned bufflen);
int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
union nvme_result *result, void *buffer, unsigned bufflen,
int qid, int at_head,
- blk_mq_req_flags_t flags);
+ blk_mq_req_flags_t flags, int retries);
int nvme_set_features(struct nvme_ctrl *dev, unsigned int fid,
unsigned int dword11, void *buffer, size_t buflen,
u32 *result);
--
2.35.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] nvme: retry commands if DNR bit is not set
2022-11-02 7:36 [PATCH] nvme: retry commands if DNR bit is not set Hannes Reinecke
@ 2022-11-02 8:00 ` Chao Leng
2022-11-02 14:19 ` Hannes Reinecke
0 siblings, 1 reply; 7+ messages in thread
From: Chao Leng @ 2022-11-02 8:00 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme
On 2022/11/2 15:36, Hannes Reinecke wrote:
> Add a 'retries' argument to __nvme_submit_sync_cmd() to instruct
> the function to retry the command if the DNR bit is not set in
> the command result, and modify the authentication code to allow
> for retries.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/nvme/host/auth.c | 2 +-
> drivers/nvme/host/core.c | 29 ++++++++++++++++++++++++-----
> drivers/nvme/host/fabrics.c | 10 +++++-----
> drivers/nvme/host/nvme.h | 6 +++++-
> 4 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index c8a6db7c4498..3b63aa155beb 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -69,7 +69,7 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
>
> ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
> qid == 0 ? NVME_QID_ANY : qid,
> - 0, flags);
> + 0, flags, nvme_max_retries);
> if (ret > 0)
> dev_warn(ctrl->device,
> "qid %d auth_send failed with status %d\n", qid, ret);
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index dc4220600585..d0de1a85596a 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -54,7 +54,7 @@ static unsigned char shutdown_timeout = 5;
> module_param(shutdown_timeout, byte, 0644);
> MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
>
> -static u8 nvme_max_retries = 5;
> +u8 nvme_max_retries = 5;
> module_param_named(max_retries, nvme_max_retries, byte, 0644);
> MODULE_PARM_DESC(max_retries, "max number of retries a command may have");
>
> @@ -1016,7 +1016,7 @@ static int nvme_execute_rq(struct request *rq, bool at_head)
> */
> int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> union nvme_result *result, void *buffer, unsigned bufflen,
> - int qid, int at_head, blk_mq_req_flags_t flags)
> + int qid, int at_head, blk_mq_req_flags_t flags, int retries)
> {
> struct request *req;
> int ret;
> @@ -1030,6 +1030,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> if (IS_ERR(req))
> return PTR_ERR(req);
> nvme_init_request(req, cmd);
> + nvme_req(req)->retries = retries;
>
> if (buffer && bufflen) {
> ret = blk_rq_map_kern(q, req, buffer, bufflen, GFP_KERNEL);
> @@ -1037,8 +1038,18 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> goto out;
> }
>
> +retry:
> req->rq_flags |= RQF_QUIET;
> ret = nvme_execute_rq(req, at_head);
> + if (ret > 0) {
> + struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
> +
> + if (ctrl->kas)
> + ctrl->comp_seen = true;
> +
> + if (retries-- && nvme_decide_disposition(req) != COMPLETE)
> + goto retry;
> + }
> if (result && ret >= 0)
> *result = nvme_req(req)->result;
Reusing the existing retry mechanism may be better.
We can do like this:
Do not set REQ_FAILFAST_DRIVER in nvme_init_request If need retry.
if (noretry)
req->cmd_flags |= REQ_FAILFAST_DRIVER;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] nvme: retry commands if DNR bit is not set
2022-11-02 8:00 ` Chao Leng
@ 2022-11-02 14:19 ` Hannes Reinecke
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2022-11-02 14:19 UTC (permalink / raw)
To: Chao Leng, Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme
On 11/2/22 09:00, Chao Leng wrote:
>
>
> On 2022/11/2 15:36, Hannes Reinecke wrote:
>> Add a 'retries' argument to __nvme_submit_sync_cmd() to instruct
>> the function to retry the command if the DNR bit is not set in
>> the command result, and modify the authentication code to allow
>> for retries.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/nvme/host/auth.c | 2 +-
>> drivers/nvme/host/core.c | 29 ++++++++++++++++++++++++-----
>> drivers/nvme/host/fabrics.c | 10 +++++-----
>> drivers/nvme/host/nvme.h | 6 +++++-
>> 4 files changed, 35 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
>> index c8a6db7c4498..3b63aa155beb 100644
>> --- a/drivers/nvme/host/auth.c
>> +++ b/drivers/nvme/host/auth.c
>> @@ -69,7 +69,7 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl,
>> int qid,
>> ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
>> qid == 0 ? NVME_QID_ANY : qid,
>> - 0, flags);
>> + 0, flags, nvme_max_retries);
>> if (ret > 0)
>> dev_warn(ctrl->device,
>> "qid %d auth_send failed with status %d\n", qid, ret);
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index dc4220600585..d0de1a85596a 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -54,7 +54,7 @@ static unsigned char shutdown_timeout = 5;
>> module_param(shutdown_timeout, byte, 0644);
>> MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for
>> controller shutdown");
>> -static u8 nvme_max_retries = 5;
>> +u8 nvme_max_retries = 5;
>> module_param_named(max_retries, nvme_max_retries, byte, 0644);
>> MODULE_PARM_DESC(max_retries, "max number of retries a command may
>> have");
>> @@ -1016,7 +1016,7 @@ static int nvme_execute_rq(struct request *rq,
>> bool at_head)
>> */
>> int __nvme_submit_sync_cmd(struct request_queue *q, struct
>> nvme_command *cmd,
>> union nvme_result *result, void *buffer, unsigned bufflen,
>> - int qid, int at_head, blk_mq_req_flags_t flags)
>> + int qid, int at_head, blk_mq_req_flags_t flags, int retries)
>> {
>> struct request *req;
>> int ret;
>> @@ -1030,6 +1030,7 @@ int __nvme_submit_sync_cmd(struct request_queue
>> *q, struct nvme_command *cmd,
>> if (IS_ERR(req))
>> return PTR_ERR(req);
>> nvme_init_request(req, cmd);
>> + nvme_req(req)->retries = retries;
>> if (buffer && bufflen) {
>> ret = blk_rq_map_kern(q, req, buffer, bufflen, GFP_KERNEL);
>> @@ -1037,8 +1038,18 @@ int __nvme_submit_sync_cmd(struct request_queue
>> *q, struct nvme_command *cmd,
>> goto out;
>> }
>> +retry:
>> req->rq_flags |= RQF_QUIET;
>> ret = nvme_execute_rq(req, at_head);
>> + if (ret > 0) {
>> + struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
>> +
>> + if (ctrl->kas)
>> + ctrl->comp_seen = true;
>> +
>> + if (retries-- && nvme_decide_disposition(req) != COMPLETE)
>> + goto retry;
>> + }
>> if (result && ret >= 0)
>> *result = nvme_req(req)->result;
> Reusing the existing retry mechanism may be better.
> We can do like this:
> Do not set REQ_FAILFAST_DRIVER in nvme_init_request If need retry.
> if (noretry)
> req->cmd_flags |= REQ_FAILFAST_DRIVER;
>
But then we still would need to set the retries; however, it's a good
suggestion. Will be reworking the patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] nvme: retry commands if DNR bit is not set
@ 2023-02-01 11:50 Hannes Reinecke
2023-02-01 13:09 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2023-02-01 11:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke
Add a 'retry' argument to __nvme_submit_sync_cmd() to instruct
the function to not set the FAILFAST_DRIVER bit for the command,
causing it to be retried in nvme_decide_disposition() if the DNR
bit is not set in the command result.
And modify the authentication code to allow for retries.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/nvme/host/auth.c | 2 +-
drivers/nvme/host/core.c | 18 ++++++++++--------
drivers/nvme/host/fabrics.c | 10 +++++-----
drivers/nvme/host/ioctl.c | 2 +-
drivers/nvme/host/nvme.h | 5 +++--
drivers/nvme/host/pci.c | 4 ++--
drivers/nvme/target/passthru.c | 2 +-
7 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 787537454f7f..cf4b12436a43 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -78,7 +78,7 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
qid == 0 ? NVME_QID_ANY : qid,
- 0, flags);
+ 0, flags, true);
if (ret > 0)
dev_warn(ctrl->device,
"qid %d auth_send failed with status %d\n", qid, ret);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f9af062f82f4..e7390e55ee14 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -663,7 +663,8 @@ static inline void nvme_clear_nvme_request(struct request *req)
}
/* initialize a passthrough request */
-void nvme_init_request(struct request *req, struct nvme_command *cmd)
+void nvme_init_request(struct request *req, struct nvme_command *cmd,
+ bool retry)
{
if (req->q->queuedata)
req->timeout = NVME_IO_TIMEOUT;
@@ -673,7 +674,8 @@ void nvme_init_request(struct request *req, struct nvme_command *cmd)
/* passthru commands should let the driver set the SGL flags */
cmd->common.flags &= ~NVME_CMD_SGL_ALL;
- req->cmd_flags |= REQ_FAILFAST_DRIVER;
+ if (!retry)
+ req->cmd_flags |= REQ_FAILFAST_DRIVER;
if (req->mq_hctx->type == HCTX_TYPE_POLL)
req->cmd_flags |= REQ_POLLED;
nvme_clear_nvme_request(req);
@@ -1023,7 +1025,7 @@ EXPORT_SYMBOL_NS_GPL(nvme_execute_rq, NVME_TARGET_PASSTHRU);
*/
int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
union nvme_result *result, void *buffer, unsigned bufflen,
- int qid, int at_head, blk_mq_req_flags_t flags)
+ int qid, int at_head, blk_mq_req_flags_t flags, bool retry)
{
struct request *req;
int ret;
@@ -1036,7 +1038,7 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
if (IS_ERR(req))
return PTR_ERR(req);
- nvme_init_request(req, cmd);
+ nvme_init_request(req, cmd, retry);
if (buffer && bufflen) {
ret = blk_rq_map_kern(q, req, buffer, bufflen, GFP_KERNEL);
@@ -1057,7 +1059,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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, false);
}
EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
@@ -1238,7 +1240,7 @@ static void nvme_keep_alive_work(struct work_struct *work)
nvme_reset_ctrl(ctrl);
return;
}
- nvme_init_request(rq, &ctrl->ka_cmd);
+ nvme_init_request(rq, &ctrl->ka_cmd, false);
rq->timeout = ctrl->kato * HZ;
rq->end_io = nvme_keep_alive_end_io;
@@ -1515,7 +1517,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, NVME_QID_ANY, 0, 0);
+ buffer, buflen, NVME_QID_ANY, 0, 0, false);
if (ret >= 0 && result)
*result = le32_to_cpu(res.u32);
return ret;
@@ -2247,7 +2249,7 @@ static int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t l
cmd.common.cdw11 = cpu_to_le32(len);
return __nvme_submit_sync_cmd(ctrl->admin_q, &cmd, NULL, buffer, len,
- NVME_QID_ANY, 1, 0);
+ NVME_QID_ANY, 1, 0, false);
}
static void nvme_configure_opal(struct nvme_ctrl *ctrl, bool was_suspended)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index bbaa04a0c502..c6d7c89939e0 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -153,7 +153,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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, false);
if (ret >= 0)
*val = le64_to_cpu(res.u64);
@@ -199,7 +199,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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, false);
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,
- NVME_QID_ANY, 0, 0);
+ NVME_QID_ANY, 0, 0, false);
if (unlikely(ret))
dev_err(ctrl->device,
"Property Set error: %d, offset %#x\n",
@@ -401,7 +401,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res,
data, sizeof(*data), NVME_QID_ANY, 1,
- BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+ BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, false);
if (ret) {
nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
&cmd, data);
@@ -487,7 +487,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
ret = __nvme_submit_sync_cmd(ctrl->connect_q, &cmd, &res,
data, sizeof(*data), qid, 1,
- BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+ BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, false);
if (ret) {
nvmf_log_connect_error(ctrl, ret, le32_to_cpu(res.u32),
&cmd, data);
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 723e7d5b778f..11f03a0696c2 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -154,7 +154,7 @@ static struct request *nvme_alloc_user_request(struct request_queue *q,
req = blk_mq_alloc_request(q, nvme_req_op(cmd) | rq_flags, blk_flags);
if (IS_ERR(req))
return req;
- nvme_init_request(req, cmd);
+ nvme_init_request(req, cmd, false);
nvme_req(req)->flags |= NVME_REQ_USERCMD;
return req;
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bf46f122e9e1..efe043ed492a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -774,7 +774,8 @@ static inline enum req_op nvme_req_op(struct nvme_command *cmd)
}
#define NVME_QID_ANY -1
-void nvme_init_request(struct request *req, struct nvme_command *cmd);
+void nvme_init_request(struct request *req, struct nvme_command *cmd,
+ bool retry);
void nvme_cleanup_cmd(struct request *req);
blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req);
blk_status_t nvme_fail_nonready_command(struct nvme_ctrl *ctrl,
@@ -816,7 +817,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,
int qid, int at_head,
- blk_mq_req_flags_t flags);
+ blk_mq_req_flags_t flags, bool retry);
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/pci.c b/drivers/nvme/host/pci.c
index 6d0217c82f4a..9fbd3c1124e1 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1382,7 +1382,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req)
atomic_inc(&dev->ctrl.abort_limit);
return BLK_EH_RESET_TIMER;
}
- nvme_init_request(abort_req, &cmd);
+ nvme_init_request(abort_req, &cmd, false);
abort_req->end_io = abort_endio;
abort_req->end_io_data = NULL;
@@ -2394,7 +2394,7 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode)
req = blk_mq_alloc_request(q, nvme_req_op(&cmd), BLK_MQ_REQ_NOWAIT);
if (IS_ERR(req))
return PTR_ERR(req);
- nvme_init_request(req, &cmd);
+ nvme_init_request(req, &cmd, false);
if (opcode == nvme_admin_delete_cq)
req->end_io = nvme_del_cq_end;
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 511c980d538d..2d51480ce02d 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -321,7 +321,7 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
status = NVME_SC_INTERNAL;
goto out_put_ns;
}
- nvme_init_request(rq, req->cmd);
+ nvme_init_request(rq, req->cmd, false);
if (timeout)
rq->timeout = timeout;
--
2.35.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] nvme: retry commands if DNR bit is not set
2023-02-01 11:50 Hannes Reinecke
@ 2023-02-01 13:09 ` Christoph Hellwig
2023-02-01 13:55 ` Hannes Reinecke
2023-02-01 14:45 ` Jens Axboe
0 siblings, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2023-02-01 13:09 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme
On Wed, Feb 01, 2023 at 12:50:01PM +0100, Hannes Reinecke wrote:
> Add a 'retry' argument to __nvme_submit_sync_cmd() to instruct
> the function to not set the FAILFAST_DRIVER bit for the command,
> causing it to be retried in nvme_decide_disposition() if the DNR
> bit is not set in the command result.
> And modify the authentication code to allow for retries.
This is missing the why. And maybe we can find a better interace than
a magic bool flag.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: retry commands if DNR bit is not set
2023-02-01 13:09 ` Christoph Hellwig
@ 2023-02-01 13:55 ` Hannes Reinecke
2023-02-01 14:45 ` Jens Axboe
1 sibling, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2023-02-01 13:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme
On 2/1/23 14:09, Christoph Hellwig wrote:
> On Wed, Feb 01, 2023 at 12:50:01PM +0100, Hannes Reinecke wrote:
>> Add a 'retry' argument to __nvme_submit_sync_cmd() to instruct
>> the function to not set the FAILFAST_DRIVER bit for the command,
>> causing it to be retried in nvme_decide_disposition() if the DNR
>> bit is not set in the command result.
>> And modify the authentication code to allow for retries.
>
> This is missing the why. And maybe we can find a better interace than
> a magic bool flag.
The 'why' is that commands submitted internally (via
nvme_submit_sync_cmd() and friends are typically normal NVMe commands,
and as such should be subjected to the normal 'DNR' evaluation rules.
This has been an issue with the authentication protocol commands in
particular, but I've got reports that the 'connect' command itself might
also be affected.
Of course we can discuss the interface; personally I found the flag not
a bad idea...
Cheers,
Hannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: retry commands if DNR bit is not set
2023-02-01 13:09 ` Christoph Hellwig
2023-02-01 13:55 ` Hannes Reinecke
@ 2023-02-01 14:45 ` Jens Axboe
1 sibling, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2023-02-01 14:45 UTC (permalink / raw)
To: Christoph Hellwig, Hannes Reinecke; +Cc: Sagi Grimberg, Keith Busch, linux-nvme
On 2/1/23 6:09 AM, Christoph Hellwig wrote:
> On Wed, Feb 01, 2023 at 12:50:01PM +0100, Hannes Reinecke wrote:
>> Add a 'retry' argument to __nvme_submit_sync_cmd() to instruct
>> the function to not set the FAILFAST_DRIVER bit for the command,
>> causing it to be retried in nvme_decide_disposition() if the DNR
>> bit is not set in the command result.
>> And modify the authentication code to allow for retries.
>
> This is missing the why. And maybe we can find a better interace than
> a magic bool flag.
The bool addition is awful, I think it's about high time to refactor
that part of the interface rather than piling more junk on top. It's
at 8 arguments even without this change...
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-01 14:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-02 7:36 [PATCH] nvme: retry commands if DNR bit is not set Hannes Reinecke
2022-11-02 8:00 ` Chao Leng
2022-11-02 14:19 ` Hannes Reinecke
-- strict thread matches above, loose matches on Subject: below --
2023-02-01 11:50 Hannes Reinecke
2023-02-01 13:09 ` Christoph Hellwig
2023-02-01 13:55 ` Hannes Reinecke
2023-02-01 14:45 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox