From d22497a72c8cb98b9033f18c5955cf47fa3bc88b Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Fri, 5 Mar 2021 13:10:53 +0900 Subject: [PATCH] bdev/nvme: Process pending destruct ctrlr request by adding an new variable The recent refactoring removed the destruct poller and change the reset processing to destruct ctrlr after its completion by conditionally sending message. But differentiating callback function is difficult if we reset multiple ctrlrs. If nvme_bdev_ctrlr->destruct is set, any new reset cannot start. So we can use an new variable and always execute the callback function. Add an new variable pending_destruct to struct nvme_bdev_ctrlr, and set pending_destruct if ctrlr->ref is zero and ctrlr->destruct is true, and then start destruct ctrlr if ctrlr->destruct_after_reset is set after clearing pending resets. Signed-off-by: Shuhei Matsumoto Change-Id: I9f34c42a40c5a5da54611e7871aef8c58117a56a Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6714 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 19 +++++++++++++------ module/bdev/nvme/common.h | 1 + .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 4 ++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 18e025da0..c2a0810c8 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -358,12 +358,20 @@ err: } static void -_bdev_nvme_reset_destruct_ctrlr(struct spdk_io_channel_iter *i, int status) +_bdev_nvme_check_pending_destruct(struct spdk_io_channel_iter *i, int status) { struct nvme_bdev_ctrlr *nvme_bdev_ctrlr = spdk_io_channel_iter_get_io_device(i); - spdk_thread_send_msg(nvme_bdev_ctrlr->thread, nvme_bdev_ctrlr_do_destruct, - nvme_bdev_ctrlr); + pthread_mutex_lock(&nvme_bdev_ctrlr->mutex); + if (nvme_bdev_ctrlr->destruct_after_reset) { + assert(nvme_bdev_ctrlr->ref == 0 && nvme_bdev_ctrlr->destruct); + pthread_mutex_unlock(&nvme_bdev_ctrlr->mutex); + + spdk_thread_send_msg(nvme_bdev_ctrlr->thread, nvme_bdev_ctrlr_do_destruct, + nvme_bdev_ctrlr); + } else { + pthread_mutex_unlock(&nvme_bdev_ctrlr->mutex); + } } static void @@ -395,7 +403,6 @@ _bdev_nvme_reset_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, int rc) /* If it's zero, we succeeded, otherwise, the reset failed. */ void *cb_arg = NULL; struct nvme_bdev_ctrlr_trid *curr_trid; - bool do_destruct = false; if (rc) { cb_arg = (void *)0x1; @@ -416,7 +423,7 @@ _bdev_nvme_reset_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, int rc) if (nvme_bdev_ctrlr->ref == 0 && nvme_bdev_ctrlr->destruct) { /* Destruct ctrlr after clearing pending resets. */ - do_destruct = true; + nvme_bdev_ctrlr->destruct_after_reset = true; } pthread_mutex_unlock(&nvme_bdev_ctrlr->mutex); @@ -425,7 +432,7 @@ _bdev_nvme_reset_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, int rc) spdk_for_each_channel(nvme_bdev_ctrlr, _bdev_nvme_complete_pending_resets, cb_arg, - do_destruct ? _bdev_nvme_reset_destruct_ctrlr : NULL); + _bdev_nvme_check_pending_destruct); } static void diff --git a/module/bdev/nvme/common.h b/module/bdev/nvme/common.h index 6b1678368..407789552 100644 --- a/module/bdev/nvme/common.h +++ b/module/bdev/nvme/common.h @@ -90,6 +90,7 @@ struct nvme_bdev_ctrlr { bool resetting; bool failover_in_progress; bool destruct; + bool destruct_after_reset; /** * PI check flags. This flags is set to NVMe controllers created only * through bdev_nvme_attach_controller RPC or .INI config file. Hot added diff --git a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c index 7ca0f1831..aba5c414d 100644 --- a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c +++ b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c @@ -1056,6 +1056,10 @@ test_race_between_reset_and_destruct_ctrlr(void) CU_ASSERT(nvme_bdev_ctrlr->destruct == true); CU_ASSERT(nvme_bdev_ctrlr->resetting == false); + /* New reset request is rejected. */ + rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + CU_ASSERT(rc == -EBUSY); + /* Additional polling called spdk_io_device_unregister() to ctrlr, * However there are two channels and destruct is not completed yet. */