[PATCH 2/2] nvme: don't set iosqes,iocqes for discovery controllers
Sagi Grimberg
sagi at grimberg.me
Thu Feb 11 05:43:27 EST 2021
>>> So if we change this we break interaction with all old Linux (and maybe
>>> other?) targets.
>>
>> Well yes.. Do you think that we should do a retry in case it fails for
>> backward compatibility?
>
> I think we have to.
The annoying thing here is that this issue will manifest in the host
waiting on nvme_wait_ready for a long 7.5 seconds to understand that
its maybe incompatible and re-attempt (nvme_enable_ctrl writes to cap
and then polls CSTS)...
What I have now looks like:
--
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 02579f4f776c..ee65c89a991c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2457,8 +2457,14 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
}
EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
+static inline bool nvme_discovery_ctrl(struct nvme_ctrl *ctrl)
+{
+ return ctrl->opts && ctrl->opts->discovery_nqn;
+}
+
int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
{
+ bool compat_disc_ctrl_config = false;
unsigned dev_page_min;
int ret;
@@ -2482,13 +2488,33 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
ctrl->ctrl_config = NVME_CC_CSS_NVM;
ctrl->ctrl_config |= (NVME_CTRL_PAGE_SHIFT - 12) <<
NVME_CC_MPS_SHIFT;
ctrl->ctrl_config |= NVME_CC_AMS_RR | NVME_CC_SHN_NONE;
- ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
+again:
+ if (!nvme_discovery_ctrl(ctrl) || compat_disc_ctrl_config)
+ 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);
if (ret)
return ret;
- return nvme_wait_ready(ctrl, ctrl->cap, true);
+
+ ret = nvme_wait_ready(ctrl, ctrl->cap, true);
+ if (ret) {
+ if (!compat_disc_ctrl_config) {
+ pr_err("retrying...\n");
+ /*
+ * Backward compatibility work-around: some targets
+ * (i.e. older Linux targets) may incorrectly verify
+ * iosqes,iocqes are non-zero for discovery
+ * controllers. So in order not to break them, we
+ * attempt once again with the incorrect settings.
+ */
+ if (nvme_disable_ctrl(ctrl))
+ return ret;
+ compat_disc_ctrl_config = true;
+ goto again;
+ }
+ }
+ return ret;
}
EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
@@ -2902,11 +2928,6 @@ static const struct attribute_group
*nvme_subsys_attrs_groups[] = {
NULL,
};
-static inline bool nvme_discovery_ctrl(struct nvme_ctrl *ctrl)
-{
- return ctrl->opts && ctrl->opts->discovery_nqn;
-}
-
static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
{
--
More information about the Linux-nvme
mailing list