bdev/nvme: Complete outstanding reset after canceling pending resets

Previously the NVMe bdev module had completed the outstanding reset and
then canceled pending resets. This was complex.

On the other hand, the generic bdev layer cancels pending resets
and then completes the outstanding reset.

Following the generic bdev layer simplifies the code and makes us easier
to control retry reset, delay retry reset by a few seconds, or stop retry
after repeated failures and then delete ctrlr.

Update unit tests accordingly.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9a68422918ebcb052b3a281316ffba9b3450ecd4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9816
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

module/bdev/nvme/bdev_nvme.c

@@ -883,29 +883,6 @@ err:
 	return rc;
 }
 
-static void
-_bdev_nvme_check_pending_destruct(struct nvme_ctrlr *nvme_ctrlr)
-{
-	pthread_mutex_lock(&nvme_ctrlr->mutex);
-	if (nvme_ctrlr->destruct_after_reset) {
-		assert(nvme_ctrlr->ref == 0 && nvme_ctrlr->destruct);
-		pthread_mutex_unlock(&nvme_ctrlr->mutex);
-
-		spdk_thread_send_msg(nvme_ctrlr->thread, nvme_ctrlr_unregister,
-				     nvme_ctrlr);
-	} else {
-		pthread_mutex_unlock(&nvme_ctrlr->mutex);
-	}
-}
-
-static void
-bdev_nvme_check_pending_destruct(struct spdk_io_channel_iter *i, int status)
-{
-	struct nvme_ctrlr *nvme_ctrlr = spdk_io_channel_iter_get_io_device(i);
-
-	_bdev_nvme_check_pending_destruct(nvme_ctrlr);
-}
-
 static void
 bdev_nvme_complete_pending_resets(struct spdk_io_channel_iter *i)
 {
@@ -928,11 +905,14 @@ bdev_nvme_complete_pending_resets(struct spdk_io_channel_iter *i)
 }
 
 static void
-bdev_nvme_reset_complete(struct nvme_ctrlr *nvme_ctrlr, bool success)
+_bdev_nvme_reset_complete(struct spdk_io_channel_iter *i, int status)
 {
+	struct nvme_ctrlr *nvme_ctrlr = spdk_io_channel_iter_get_io_device(i);
+	bool success = spdk_io_channel_iter_get_ctx(i) == NULL;
 	struct nvme_path_id *path_id;
 	bdev_nvme_reset_cb reset_cb_fn = nvme_ctrlr->reset_cb_fn;
 	void *reset_cb_arg = nvme_ctrlr->reset_cb_arg;
+	bool complete_pending_destruct = false;
 
 	nvme_ctrlr->reset_cb_fn = NULL;
 	nvme_ctrlr->reset_cb_arg = NULL;
@@ -954,8 +934,8 @@ bdev_nvme_reset_complete(struct nvme_ctrlr *nvme_ctrlr, bool success)
 	path_id->is_failed = !success;
 
 	if (nvme_ctrlr->ref == 0 && nvme_ctrlr->destruct) {
-		/* Destruct ctrlr after clearing pending resets. */
+		/* Complete pending destruct after reset completes. */
-		nvme_ctrlr->destruct_after_reset = true;
+		complete_pending_destruct = true;
 	}
 
 	pthread_mutex_unlock(&nvme_ctrlr->mutex);
@@ -964,11 +944,20 @@ bdev_nvme_reset_complete(struct nvme_ctrlr *nvme_ctrlr, bool success)
 		reset_cb_fn(reset_cb_arg, success);
 	}
 
+	if (complete_pending_destruct) {
+		spdk_thread_send_msg(nvme_ctrlr->thread, nvme_ctrlr_unregister,
+				     nvme_ctrlr);
+	}
+}
+
+static void
+bdev_nvme_reset_complete(struct nvme_ctrlr *nvme_ctrlr, bool success)
+{
 	/* Make sure we clear any pending resets before returning. */
 	spdk_for_each_channel(nvme_ctrlr,
 			      bdev_nvme_complete_pending_resets,
 			      success ? NULL : (void *)0x1,
-			      bdev_nvme_check_pending_destruct);
+			      _bdev_nvme_reset_complete);
 }
 
 static void

module/bdev/nvme/bdev_nvme.h

@@ -101,7 +101,6 @@ struct nvme_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

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

@@ -1302,6 +1302,12 @@ test_reset_ctrlr(void)
 	CU_ASSERT(nvme_ctrlr->resetting == true);
 	CU_ASSERT(curr_trid->is_failed == true);
 
+	poll_thread_times(1, 1);
+	CU_ASSERT(nvme_ctrlr->resetting == true);
+	poll_thread_times(0, 1);
+	CU_ASSERT(nvme_ctrlr->resetting == true);
+	poll_thread_times(1, 1);
+	CU_ASSERT(nvme_ctrlr->resetting == true);
 	poll_thread_times(1, 1);
 	CU_ASSERT(nvme_ctrlr->resetting == false);
 	CU_ASSERT(curr_trid->is_failed == false);
@@ -2802,6 +2808,9 @@ test_reconnect_qpair(void)
 	CU_ASSERT(ctrlr_ch2->qpair != NULL);
 	CU_ASSERT(nvme_ctrlr->resetting == true);
 
+	poll_thread_times(1, 1);
+	poll_thread_times(0, 1);
+	poll_thread_times(1, 1);
 	poll_thread_times(1, 1);
 	CU_ASSERT(nvme_ctrlr->resetting == false);
 
@@ -2825,6 +2834,9 @@ test_reconnect_qpair(void)
 	CU_ASSERT(ctrlr_ch2->qpair == NULL);
 	CU_ASSERT(ctrlr->is_failed == true);
 
+	poll_thread_times(1, 1);
+	poll_thread_times(0, 1);
+	poll_thread_times(1, 1);
 	poll_thread_times(1, 1);
 	CU_ASSERT(ctrlr->is_failed == true);
 	CU_ASSERT(nvme_ctrlr->resetting == false);