[PATCH] nvme-tcp: handle failing req immediately for REQ_FAILFAST_DRIVER
Daniel Wagner
dwagner at suse.de
Thu Apr 7 08:01:55 PDT 2022
On Thu, Apr 07, 2022 at 03:53:54PM +0200, Daniel Wagner wrote:
> On Thu, Apr 07, 2022 at 04:42:55PM +0300, Sagi Grimberg wrote:
> > > Understood. Would something like this work
> > >
> > > struct request *rq = blk_mq_rq_from_pdu(queue->request);
> > > bool queue_ready = test_bit(NVME_TCP_Q_LIVE, &queue->flags);
> > >
> > > dev_err(queue->ctrl->ctrl.device,
> > > "failed to send request %d\n", ret);
> > > if ((ret != -EPIPE && ret != -ECONNRESET) ||
> > > (!queue_ready && rq->cmd_flags & REQ_FAILFAST_DRIVER))
> > > nvme_tcp_fail_request(queue->request);
> > >
> > > ?
> >
> > But in your case the queue is live...
>
> Could we not just test for NVME_CTRL_ADMIN_Q_STOPPED instead as it is
> set in nvme_stop_admin_queue:
>
> nvme_tcp_teardown_ctrl
> nvme_stop_admin_queue
> nvme_shutdown_ctrl
>
> instead?
>
> > I'm thinking that we maybe need a register access timeout value for
> > fabrics...
>
> I see there is a shutdown_timeout which is 5 seconds on default. Seems
> not to trigger though.
Obviously, the shutdown_timeout is not connected to the register
writes. So we would need something like:
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f204c6f78b5b..a146ea25f386 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2133,7 +2133,8 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
u32 csts, bit = enabled ? NVME_CSTS_RDY : 0;
int ret;
- while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
+ while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS,
+ &csts, timeout)) == 0) {
if (csts == ~0)
return -ENODEV;
if ((csts & NVME_CSTS_RDY) == bit)
@@ -2166,7 +2167,8 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
ctrl->ctrl_config &= ~NVME_CC_ENABLE;
- ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+ ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config,
+ NVME_REG_ACCESS_TIMEOUT);
if (ret)
return ret;
@@ -2182,7 +2184,8 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
unsigned dev_page_min;
int ret;
- ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
+ ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap,
+ NVME_REG_ACCESS_TIMEOUT);
if (ret) {
dev_err(ctrl->device, "Reading CAP failed (%d)\n", ret);
return ret;
@@ -2205,7 +2208,8 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
ctrl->ctrl_config |= NVME_CC_ENABLE;
- ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+ ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config,
+ NVME_REG_ACCESS_TIMEOUT);
if (ret)
return ret;
return nvme_wait_ready(ctrl, ctrl->cap, true);
@@ -2221,11 +2225,13 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
ctrl->ctrl_config |= NVME_CC_SHN_NORMAL;
- ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+ ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config,
+ NVME_REG_ACCESS_TIMEOUT);
if (ret)
return ret;
- while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
+ while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts,
+ NVME_REG_ACCESS_TIMEOUT)) == 0) {
if ((csts & NVME_CSTS_SHST_MASK) == NVME_CSTS_SHST_CMPLT)
break;
@@ -3084,7 +3090,8 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
{
int ret;
- ret = ctrl->ops->reg_read32(ctrl, NVME_REG_VS, &ctrl->vs);
+ ret = ctrl->ops->reg_read32(ctrl, NVME_REG_VS, &ctrl->vs,
+ NVME_REG_ACCESS_TIMEOUT);
if (ret) {
dev_err(ctrl->device, "Reading VS failed (%d)\n", ret);
return ret;
@@ -4386,7 +4393,8 @@ static bool nvme_ctrl_pp_status(struct nvme_ctrl *ctrl)
u32 csts;
- if (ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts))
+ if (ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts,
+ NVME_REG_ACCESS_TIMEOUT))
return false;
if (csts == ~0)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index ee79a6d639b4..7882559e89bc 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -142,7 +142,7 @@ EXPORT_SYMBOL_GPL(nvmf_get_address);
* > 0: NVMe error status code
* < 0: Linux errno error code
*/
-int nvmf_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val)
+int nvmf_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val, int timeout)
{
struct nvme_command cmd = { };
union nvme_result res;
@@ -152,8 +152,8 @@ int nvmf_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val)
cmd.prop_get.fctype = nvme_fabrics_type_property_get;
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);
+ ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res, NULL, 0,
+ timeout, NVME_QID_ANY, 0, 0);
if (ret >= 0)
*val = le64_to_cpu(res.u64);
@@ -187,7 +187,7 @@ EXPORT_SYMBOL_GPL(nvmf_reg_read32);
* > 0: NVMe error status code
* < 0: Linux errno error code
*/
-int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val)
+int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val, int timeout)
{
struct nvme_command cmd = { };
union nvme_result res;
@@ -198,8 +198,8 @@ int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val)
cmd.prop_get.attrib = 1;
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);
+ ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, &res, NULL, 0,
+ timeout, NVME_QID_ANY, 0, 0);
if (ret >= 0)
*val = le64_to_cpu(res.u64);
@@ -232,7 +232,7 @@ EXPORT_SYMBOL_GPL(nvmf_reg_read64);
* > 0: NVMe error status code
* < 0: Linux errno error code
*/
-int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val)
+int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val, int timeout)
{
struct nvme_command cmd = { };
int ret;
@@ -243,8 +243,8 @@ int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val)
cmd.prop_set.offset = cpu_to_le32(off);
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);
+ ret = __nvme_submit_sync_cmd(ctrl->fabrics_q, &cmd, NULL, NULL, 0,
+ timeout, NVME_QID_ANY, 0, 0);
if (unlikely(ret))
dev_err(ctrl->device,
"Property Set error: %d, offset %#x\n",
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index c3203ff1c654..c2ae0c04cd4a 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -186,9 +186,9 @@ static inline char *nvmf_ctrl_subsysnqn(struct nvme_ctrl *ctrl)
return ctrl->subsys->subnqn;
}
-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_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val, int timeout);
+int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val, int timeout);
+int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val, int timeout);
int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl);
int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid);
int nvmf_register_transport(struct nvmf_transport_ops *ops);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a1f8403ffd78..f17a4df17e11 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -27,6 +27,8 @@ extern unsigned int admin_timeout;
#define NVME_DEFAULT_KATO 5
+#define NVME_REG_ACCESS_TIMEOUT NVME_DEFAULT_KATO
+
#ifdef CONFIG_ARCH_NO_SG_CHAIN
#define NVME_INLINE_SG_CNT 0
#define NVME_INLINE_METADATA_SG_CNT 0
@@ -489,9 +491,12 @@ struct nvme_ctrl_ops {
#define NVME_F_FABRICS (1 << 0)
#define NVME_F_METADATA_SUPPORTED (1 << 1)
#define NVME_F_PCI_P2PDMA (1 << 2)
- int (*reg_read32)(struct nvme_ctrl *ctrl, u32 off, u32 *val);
- int (*reg_write32)(struct nvme_ctrl *ctrl, u32 off, u32 val);
- int (*reg_read64)(struct nvme_ctrl *ctrl, u32 off, u64 *val);
+ int (*reg_read32)(struct nvme_ctrl *ctrl, u32 off, u32 *val,
+ int timeout);
+ int (*reg_write32)(struct nvme_ctrl *ctrl, u32 off, u32 val,
+ int timoeut);
+ int (*reg_read64)(struct nvme_ctrl *ctrl, u32 off, u64 *val,
+ int timeout);
void (*free_ctrl)(struct nvme_ctrl *ctrl);
void (*submit_async_event)(struct nvme_ctrl *ctrl);
void (*delete_ctrl)(struct nvme_ctrl *ctrl);
@@ -561,7 +566,8 @@ static inline int nvme_reset_subsystem(struct nvme_ctrl *ctrl)
{
if (!ctrl->subsystem)
return -ENOTTY;
- return ctrl->ops->reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65);
+ return ctrl->ops->reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65,
+ NVME_REG_ACCESS_TIMEOUT);
}
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 66f1eee2509a..b7246fee99ef 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2943,19 +2943,22 @@ static void nvme_remove_dead_ctrl_work(struct work_struct *work)
nvme_put_ctrl(&dev->ctrl);
}
-static int nvme_pci_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val)
+static int nvme_pci_reg_read32(struct nvme_ctrl *ctrl, u32 off, u32 *val,
+ int timeout)
{
*val = readl(to_nvme_dev(ctrl)->bar + off);
return 0;
}
-static int nvme_pci_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val)
+static int nvme_pci_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val,
+ int timeout)
{
writel(val, to_nvme_dev(ctrl)->bar + off);
return 0;
}
-static int nvme_pci_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val)
+static int nvme_pci_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val,
+ int timeout)
{
*val = lo_hi_readq(to_nvme_dev(ctrl)->bar + off);
return 0;
More information about the Linux-nvme
mailing list