From 3cb9bc25933bbc9e71e8fba077242c1a2a5a41f7 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 20 Aug 2021 07:43:52 -0700 Subject: [PATCH] nvme: add spdk_nvme_ctrlr_prepare_for_reset() This new API signals that the ctrlr will soon be reset. This allows the transport to skip unnecessary steps in following calls to the driver prior to the reset - for example, skipping PCIe DELETE_SQ/CQ commands when freeing an IO qpair. Note that if we are deleting a qpair after prepare_for_reset was called, and the qpair is still waiting for a CREATE_IO_CQ or CREATE_IO_SQ, we cannot poll for those commands to complete, but we also cannot free the qpair immediately. So set a flag for this case to defer the destruction until the outstanding CREATE_IO_CQ or CREATE_IO_SQ callback is invoked (typically as an aborted command when the reset happens). Signed-off-by: Jim Harris Change-Id: I34c6276ae71e7d61ad4a3720f1a985b1ee96bd8b Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9249 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Tomasz Zawadzki --- include/spdk/nvme.h | 11 +++++++++++ lib/nvme/nvme_ctrlr.c | 9 +++++++++ lib/nvme/nvme_internal.h | 6 ++++++ lib/nvme/nvme_pcie_common.c | 34 +++++++++++++++++++++++++++++++++- lib/nvme/nvme_pcie_internal.h | 1 + lib/nvme/spdk_nvme.map | 1 + 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 2edbb65315..ca1e2cea4d 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -1039,6 +1039,17 @@ void spdk_nvme_ctrlr_set_remove_cb(struct spdk_nvme_ctrlr *ctrlr, */ int spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr); +/** + * Inform the driver that the application is preparing to reset the specified NVMe controller. + * + * This function allows the driver to make decisions knowing that a reset is about to happen. + * For example, the pcie transport in this case could skip sending DELETE_CQ and DELETE_SQ + * commands to the controller if an io qpair is freed after this function is called. + * + * \param ctrlr Opaque handle to NVMe controller. + */ +void spdk_nvme_ctrlr_prepare_for_reset(struct spdk_nvme_ctrlr *ctrlr); + struct spdk_nvme_ctrlr_reset_ctx; /** diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 9e55b30ffd..54ee102d93 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1440,6 +1440,7 @@ nvme_ctrlr_reset_pre(struct spdk_nvme_ctrlr *ctrlr) struct spdk_nvme_qpair *qpair; nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); + ctrlr->prepare_for_reset = false; if (ctrlr->is_resetting || ctrlr->is_removed) { /* @@ -1629,6 +1630,14 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) return rc; } +void +spdk_nvme_ctrlr_prepare_for_reset(struct spdk_nvme_ctrlr *ctrlr) +{ + nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); + ctrlr->prepare_for_reset = true; + nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); +} + int spdk_nvme_ctrlr_reset_subsystem(struct spdk_nvme_ctrlr *ctrlr) { diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 01bc4fa415..d7a7f7b0b7 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -801,6 +801,12 @@ struct spdk_nvme_ctrlr { bool timeout_enabled; + /* The application is preparing to reset the controller. Transports + * can use this to skip unnecessary parts of the qpair deletion process + * for example, like the DELETE_SQ/CQ commands. + */ + bool prepare_for_reset; + uint16_t max_sges; uint16_t cntlid; diff --git a/lib/nvme/nvme_pcie_common.c b/lib/nvme/nvme_pcie_common.c index f4ff10574a..1e6154526e 100644 --- a/lib/nvme/nvme_pcie_common.c +++ b/lib/nvme/nvme_pcie_common.c @@ -465,6 +465,17 @@ nvme_completion_create_sq_cb(void *arg, const struct spdk_nvme_cpl *cpl) struct nvme_pcie_ctrlr *pctrlr = nvme_pcie_ctrlr(ctrlr); int rc; + if (pqpair->flags.defer_destruction) { + /* This qpair was deleted by the application while the + * connection was still in progress. We had to wait + * to free the qpair resources until this outstanding + * command was completed. Now that we have the completion + * free it now. + */ + nvme_pcie_qpair_destroy(qpair); + return; + } + if (spdk_nvme_cpl_is_error(cpl)) { SPDK_ERRLOG("nvme_create_io_sq failed, deleting cq!\n"); rc = nvme_pcie_ctrlr_cmd_delete_io_cq(qpair->ctrlr, qpair, nvme_completion_sq_error_delete_cq_cb, @@ -502,6 +513,17 @@ nvme_completion_create_cq_cb(void *arg, const struct spdk_nvme_cpl *cpl) struct nvme_pcie_qpair *pqpair = nvme_pcie_qpair(qpair); int rc; + if (pqpair->flags.defer_destruction) { + /* This qpair was deleted by the application while the + * connection was still in progress. We had to wait + * to free the qpair resources until this outstanding + * command was completed. Now that we have the completion + * free it now. + */ + nvme_pcie_qpair_destroy(qpair); + return; + } + if (spdk_nvme_cpl_is_error(cpl)) { pqpair->pcie_state = NVME_PCIE_QPAIR_FAILED; nvme_qpair_set_state(qpair, NVME_QPAIR_DISCONNECTED); @@ -1047,6 +1069,13 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ goto free; } + if (ctrlr->prepare_for_reset) { + if (nvme_qpair_get_state(qpair) == NVME_QPAIR_CONNECTING) { + pqpair->flags.defer_destruction = true; + } + goto clear_shadow_doorbells; + } + /* If attempting to delete a qpair that's still being connected, we have to wait until it's * finished, so that we don't free it while it's waiting for the create cq/sq callbacks. */ @@ -1098,6 +1127,7 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ } free(status); +clear_shadow_doorbells: if (pqpair->flags.has_shadow_doorbell) { *pqpair->shadow_doorbell.sq_tdbl = 0; *pqpair->shadow_doorbell.cq_hdbl = 0; @@ -1110,7 +1140,9 @@ free: nvme_pcie_qpair_abort_trackers(qpair, 1); } - nvme_pcie_qpair_destroy(qpair); + if (!pqpair->flags.defer_destruction) { + nvme_pcie_qpair_destroy(qpair); + } return 0; } diff --git a/lib/nvme/nvme_pcie_internal.h b/lib/nvme/nvme_pcie_internal.h index c3fd27ab94..663cccf827 100644 --- a/lib/nvme/nvme_pcie_internal.h +++ b/lib/nvme/nvme_pcie_internal.h @@ -186,6 +186,7 @@ struct nvme_pcie_qpair { uint8_t delay_cmd_submit : 1; uint8_t has_shadow_doorbell : 1; uint8_t has_pending_vtophys_failures : 1; + uint8_t defer_destruction : 1; } flags; /* diff --git a/lib/nvme/spdk_nvme.map b/lib/nvme/spdk_nvme.map index b5e7b1836d..9a6477ff09 100644 --- a/lib/nvme/spdk_nvme.map +++ b/lib/nvme/spdk_nvme.map @@ -36,6 +36,7 @@ spdk_nvme_ctrlr_set_trid; spdk_nvme_ctrlr_reset_subsystem; spdk_nvme_ctrlr_reset; + spdk_nvme_ctrlr_prepare_for_reset; spdk_nvme_ctrlr_reset_async; spdk_nvme_ctrlr_reset_poll_async; spdk_nvme_ctrlr_fail;