From c06daf9ad3faef636c67e5d3a8a33e34b75b8e55 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Wed, 21 Nov 2018 11:45:31 +0100 Subject: [PATCH] bdev/nvme: delete all controllers on lib finish They used to be deleted together with the last NVMe bdev built on top of them, but that was changed recently. Currently controllers that aren't explicitly deleted are leaked on lib finish. While here, cleanup the destruct flag behavior and add asserts against destroying the same controller twice. Change-Id: I58878664602268398730fa4f619c2acd222317c9 Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/434317 Tested-by: SPDK CI Jenkins Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/bdev/nvme/bdev_nvme.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/bdev/nvme/bdev_nvme.c b/lib/bdev/nvme/bdev_nvme.c index d03b53a22..327270f70 100644 --- a/lib/bdev/nvme/bdev_nvme.c +++ b/lib/bdev/nvme/bdev_nvme.c @@ -263,6 +263,7 @@ bdev_nvme_unregister_cb(void *io_device) static void bdev_nvme_ctrlr_destruct(struct nvme_ctrlr *nvme_ctrlr) { + assert(nvme_ctrlr->destruct); pthread_mutex_lock(&g_bdev_nvme_mutex); TAILQ_REMOVE(&g_nvme_ctrlrs, nvme_ctrlr, tailq); pthread_mutex_unlock(&g_bdev_nvme_mutex); @@ -284,8 +285,6 @@ bdev_nvme_destruct(void *ctx) free(nvme_disk->disk.name); nvme_disk->active = false; if (nvme_ctrlr->ref == 0 && nvme_ctrlr->destruct) { - /* Clear destruct sign in case of reentering controller destruct */ - nvme_ctrlr->destruct = false; pthread_mutex_unlock(&g_bdev_nvme_mutex); bdev_nvme_ctrlr_destruct(nvme_ctrlr); return 0; @@ -1087,7 +1086,6 @@ remove_cb(void *cb_ctx, struct spdk_nvme_ctrlr *ctrlr) pthread_mutex_lock(&g_bdev_nvme_mutex); TAILQ_FOREACH(nvme_ctrlr, &g_nvme_ctrlrs, tailq) { if (nvme_ctrlr->ctrlr == ctrlr) { - nvme_ctrlr->destruct = true; pthread_mutex_unlock(&g_bdev_nvme_mutex); for (i = 0; i < nvme_ctrlr->num_ns; i++) { uint32_t nsid = i + 1; @@ -1100,8 +1098,9 @@ remove_cb(void *cb_ctx, struct spdk_nvme_ctrlr *ctrlr) } pthread_mutex_lock(&g_bdev_nvme_mutex); - if (nvme_ctrlr->ref == 0 && nvme_ctrlr->destruct) { - nvme_ctrlr->destruct = false; + assert(!nvme_ctrlr->destruct); + nvme_ctrlr->destruct = true; + if (nvme_ctrlr->ref == 0) { pthread_mutex_unlock(&g_bdev_nvme_mutex); bdev_nvme_ctrlr_destruct(nvme_ctrlr); } else { @@ -1464,7 +1463,31 @@ end: static void bdev_nvme_library_fini(void) { + struct nvme_ctrlr *nvme_ctrlr, *tmp; + spdk_poller_unregister(&g_hotplug_poller); + + pthread_mutex_lock(&g_bdev_nvme_mutex); + TAILQ_FOREACH_SAFE(nvme_ctrlr, &g_nvme_ctrlrs, tailq, tmp) { + if (nvme_ctrlr->ref > 0) { + SPDK_ERRLOG("Controller %s is still referenced, can't destroy it\n", + nvme_ctrlr->name); + continue; + } + + if (nvme_ctrlr->destruct) { + /* This controller's destruction was already started + * before the application started shutting down + */ + continue; + } + + nvme_ctrlr->destruct = true; + pthread_mutex_unlock(&g_bdev_nvme_mutex); + bdev_nvme_ctrlr_destruct(nvme_ctrlr); + pthread_mutex_lock(&g_bdev_nvme_mutex); + } + pthread_mutex_unlock(&g_bdev_nvme_mutex); } static void