From 4e8f5d39ec5cdda5b63aa6f57ecb436a285a38b9 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 22 Oct 2020 21:03:05 +0900 Subject: [PATCH] bdev/nvme: Fix bdev_nvme_delete() to call ctrlr_get_by_name() and set destruct to true in a lock bdev_nvme_delete() had called nvme_bdev_ctrlr_get_by_name() and then called remove_cb(). But nvme_bdev_ctrlr_get_by_name() had not been guarded by lock and remove_cb() parsed the list again by using spdk_nvme_ctrlr as a key. These were not safe and efficient. This patch refines bdev_nvme_delete() to inline remove_cb() into it and acquire mutex before calling nvme_bdev_ctrlr_get_by_name(). Signed-off-by: Shuhei Matsumoto Change-Id: Ibf39859809efa6f22899353c53b7b1eef58aa470 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4842 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Tomasz Zawadzki Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins --- module/bdev/nvme/bdev_nvme.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index ccf7a21c9..35880fb51 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -1984,29 +1984,53 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, int bdev_nvme_delete(const char *name) { - struct nvme_bdev_ctrlr *nvme_bdev_ctrlr = NULL; + struct nvme_bdev_ctrlr *nvme_bdev_ctrlr; struct nvme_probe_skip_entry *entry; if (name == NULL) { return -EINVAL; } + pthread_mutex_lock(&g_bdev_nvme_mutex); + nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name); if (nvme_bdev_ctrlr == NULL) { + pthread_mutex_unlock(&g_bdev_nvme_mutex); SPDK_ERRLOG("Failed to find NVMe controller\n"); return -ENODEV; } + /* The controller's destruction was already started */ + if (nvme_bdev_ctrlr->destruct) { + pthread_mutex_unlock(&g_bdev_nvme_mutex); + return 0; + } + if (nvme_bdev_ctrlr->connected_trid->trtype == SPDK_NVME_TRANSPORT_PCIE) { entry = calloc(1, sizeof(*entry)); if (!entry) { + pthread_mutex_unlock(&g_bdev_nvme_mutex); return -ENOMEM; } entry->trid = *nvme_bdev_ctrlr->connected_trid; TAILQ_INSERT_TAIL(&g_skipped_nvme_ctrlrs, entry, tailq); } - remove_cb(NULL, nvme_bdev_ctrlr->ctrlr); + nvme_bdev_ctrlr->destruct = true; + pthread_mutex_unlock(&g_bdev_nvme_mutex); + + nvme_ctrlr_depopulate_namespaces(nvme_bdev_ctrlr); + + pthread_mutex_lock(&g_bdev_nvme_mutex); + assert(nvme_bdev_ctrlr->ref > 0); + nvme_bdev_ctrlr->ref--; + if (nvme_bdev_ctrlr->ref == 0) { + pthread_mutex_unlock(&g_bdev_nvme_mutex); + nvme_bdev_ctrlr_destruct(nvme_bdev_ctrlr); + } else { + pthread_mutex_unlock(&g_bdev_nvme_mutex); + } + return 0; }