From 2910ba6c53d4f2f23e78a4921fa4c37406a7a20b Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Mon, 22 Feb 2021 13:08:02 +0000 Subject: [PATCH] nvme: simplify controller statemachine For the following nvme controller statemachine states: NVME_CTRLR_STATE_IDENTIFY_NS NVME_CTRLR_STATE_IDENTIFY_ID_DESCS NVME_CTRLR_STATE_IDENTIFY_NS_IOCS_SPECIFIC The statemachine can either: - Jump to succeeding state - If active ns list is empty, jump directly to NVME_CTRLR_STATE_CONFIGURE_AER - In the unlikely case if NVMe completion error, jump to NVME_CTRLR_STATE_ERROR Simply this such that we either: - Jump to succeeding state - In the unlikely case if NVMe completion error, jump to NVME_CTRLR_STATE_ERROR This will help to reduce the complexity of the nvme controller statemachine, especially considering that there are new additional states (NVME_CTRLR_STATE_IDENTIFY_NS_DIRECTIVE and NVME_CTRLR_STATE_CONFIGURE_NS_STREAMS) currently on review that would continue with the bad habit of having three possible jump states instead of just two. Signed-off-by: Niklas Cassel Change-Id: I3242052b1108afcd8adbe6d0378b1358fef58ec8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6521 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: sunshihao Reviewed-by: Jim Harris --- lib/nvme/nvme_ctrlr.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 7814d6ad6..269177570 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -2010,7 +2010,7 @@ nvme_ctrlr_identify_namespaces(struct spdk_nvme_ctrlr *ctrlr) ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); if (ns == NULL) { /* No active NS, move on to the next state */ - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_ID_DESCS, ctrlr->opts.admin_timeout_ms); return 0; } @@ -2042,6 +2042,7 @@ nvme_ctrlr_identify_namespaces_iocs_specific_next(struct spdk_nvme_ctrlr *ctrlr, ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); if (ns == NULL) { + /* No first/next active NS, move on to the next state */ nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, ctrlr->opts.admin_timeout_ms); return 0; @@ -2126,6 +2127,7 @@ static int nvme_ctrlr_identify_namespaces_iocs_specific(struct spdk_nvme_ctrlr *ctrlr) { if (!nvme_ctrlr_multi_iocs_enabled(ctrlr)) { + /* Multi IOCS not supported/enabled, move on to the next state */ nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, ctrlr->opts.admin_timeout_ms); return 0; @@ -2143,7 +2145,19 @@ nvme_ctrlr_identify_id_desc_async_done(void *arg, const struct spdk_nvme_cpl *cp int rc; if (spdk_nvme_cpl_is_error(cpl)) { - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, + /* + * Many controllers claim to be compatible with NVMe 1.3, however, + * they do not implement NS ID Desc List. Therefore, instead of setting + * the state to NVME_CTRLR_STATE_ERROR, silently ignore the completion + * error and move on to the next state. + * + * The proper way is to create a new quirk for controllers that violate + * the NVMe 1.3 spec by not supporting NS ID Desc List. + * (Re-using the NVME_QUIRK_IDENTIFY_CNS quirk is not possible, since + * it is too generic and was added in order to handle controllers that + * violate the NVMe 1.1 spec by not supporting ACTIVE LIST). + */ + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_NS_IOCS_SPECIFIC, ctrlr->opts.admin_timeout_ms); return; } @@ -2190,7 +2204,8 @@ nvme_ctrlr_identify_id_desc_namespaces(struct spdk_nvme_ctrlr *ctrlr) !(ctrlr->cap.bits.css & SPDK_NVME_CAP_CSS_IOCS)) || (ctrlr->quirks & NVME_QUIRK_IDENTIFY_CNS)) { SPDK_DEBUGLOG(nvme, "Version < 1.3; not attempting to retrieve NS ID Descriptor List\n"); - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, + /* NS ID Desc List not supported, move on to the next state */ + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_NS_IOCS_SPECIFIC, ctrlr->opts.admin_timeout_ms); return 0; } @@ -2199,7 +2214,7 @@ nvme_ctrlr_identify_id_desc_namespaces(struct spdk_nvme_ctrlr *ctrlr) ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); if (ns == NULL) { /* No active NS, move on to the next state */ - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER, + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_NS_IOCS_SPECIFIC, ctrlr->opts.admin_timeout_ms); return 0; }