From bc4e31d6b24d08aa20a1166215e0131f72c7c36e Mon Sep 17 00:00:00 2001 From: Seth Howell Date: Thu, 26 Sep 2019 14:56:53 -0700 Subject: [PATCH] nvme: call the remove_cb in nvme_ctrlr_fail. The remove callback is a built in way of alerting the user application that we have removed a controller. Once we fail a controller, we never move it back out of that state so it is in essence removed. Change-Id: Iaad6bef0994e9ddd5a424f6b83502f9191b2de49 Signed-off-by: Seth Howell Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/469637 Tested-by: SPDK CI Jenkins Reviewed-by: Paul Luse Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/nvme/nvme_ctrlr.c | 30 ++++++++++++++++++++++++++++++ lib/nvme/nvme_pcie.c | 13 +------------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 524fbb8a7..fad4dd67f 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -585,6 +585,8 @@ nvme_ctrlr_set_supported_features(struct spdk_nvme_ctrlr *ctrlr) void nvme_ctrlr_fail(struct spdk_nvme_ctrlr *ctrlr, bool hot_remove) { + int had_lock; + /* * Set the flag here and leave the work failure of qpairs to * spdk_nvme_qpair_process_completions(). @@ -592,8 +594,36 @@ nvme_ctrlr_fail(struct spdk_nvme_ctrlr *ctrlr, bool hot_remove) if (hot_remove) { ctrlr->is_removed = true; } + + /* + * Return if we have already set the state to failed since we + * don't want to call the disconnect callback twice. + */ + if (ctrlr->is_failed) { + return; + } ctrlr->is_failed = true; SPDK_ERRLOG("ctrlr %s in failed state.\n", ctrlr->trid.traddr); + + if (ctrlr->remove_cb == NULL) { + return; + } + + /* + * In the pcie hotplug case, this function may be called with the + * global driver lock. In that case, we want to make sure that we + * release it before calling the remove callback and restore the + * old state afterwards. + * robust locks return EPERM if we try to unlock a lock we + * aren't holding. + */ + had_lock = (nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock) == 0); + + ctrlr->remove_cb(ctrlr->cb_ctx, ctrlr); + + if (had_lock) { + nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock); + } } static void diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index d830e4af5..91a598acf 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -296,14 +296,8 @@ _nvme_pcie_hotplug_monitor(struct spdk_nvme_probe_ctx *probe_ctx) SPDK_DEBUGLOG(SPDK_LOG_NVME, "remove nvme address: %s\n", event.traddr); + /* get the user app to clean up and stop I/O. */ nvme_ctrlr_fail(ctrlr, true); - - /* get the user app to clean up and stop I/O */ - if (ctrlr->remove_cb) { - nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock); - ctrlr->remove_cb(probe_ctx->cb_ctx, ctrlr); - nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock); - } } } } @@ -331,11 +325,6 @@ _nvme_pcie_hotplug_monitor(struct spdk_nvme_probe_ctx *probe_ctx) if (do_remove) { nvme_ctrlr_fail(ctrlr, true); - if (ctrlr->remove_cb) { - nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock); - ctrlr->remove_cb(probe_ctx->cb_ctx, ctrlr); - nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock); - } } } return 0;