nvme: Add new detach to a detach context while it is being polled

This update will allow us to use spdk_nvme_detach_async() and
spdk_nvme_detach_poll_async() easier to aggregate multiple detachments.

Previously, we could do:
    spdk_nvme_detach_async()
    spdk_nvme_detach_async()
    spdk_nvme_detach_async()
and then started doing spdk_nvme_detach_poll_async().

Hence aggregating multiple detachments is already supported.

After this patch, the following sequence is possible:
    spdk_nvme_detach_async() = 0
    spdk_nvme_detach_async() = 0
    spdk_nvme_detach_async() = 0
    spdk_nvme_detach_poll_async() = -EAGAIN
    spdk_nvme_detach_async() = 0
    spdk_nvme_detach_async() = 0
    spdk_nvme_detach_poll_async() = -EAGAIN
    spdk_nvme_detach_poll_async() = -EAGAIN
    spdk_nvme_detach_poll_async() = -EAGAIN
    spdk_nvme_detach_poll_async() = 0

The actual changes is to remove the variable polling_started from
struct spdk_nvme_detach_ctx because it is not necessary anymore.

Clarify this change via updating the header file and CHANGELOG.
Verify this change by unit test.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iebdf6c27c5304a2097b7084c315ccc99634ffa1e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8468
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>

CHANGELOG.md

@@ -63,6 +63,9 @@ the spdk_nvme_ctrlr_opts structure prior to controller attach.
 Add a new function `spdk_nvme_detach_poll` to simplify a common use case to continue
 polling until all detachments complete.
 
+An existing function `spdk_nvme_detach_async` was updated to add one or more detachments
+to an active context while it is being polled.
+
 ### rpc
 
 New RPC `bdev_rbd_register_cluster` and `bdev_rbd_unregister_cluster` was added, it allows to create

lib/nvme/nvme.c

@@ -180,9 +180,6 @@ spdk_nvme_detach_async(struct spdk_nvme_ctrlr *ctrlr,
 			return -ENOMEM;
 		}
 		TAILQ_INIT(&detach_ctx->head);
-	} else if (detach_ctx->polling_started) {
-		SPDK_ERRLOG("Busy at polling detachment now.\n");
-		return -EBUSY;
 	}
 
 	rc = nvme_ctrlr_detach_async(ctrlr, &ctx);
@@ -214,8 +211,6 @@ spdk_nvme_detach_poll_async(struct spdk_nvme_detach_ctx *detach_ctx)
 		return -EINVAL;
 	}
 
-	detach_ctx->polling_started = true;
-
 	TAILQ_FOREACH_SAFE(ctx, &detach_ctx->head, link, tmp_ctx) {
 		TAILQ_REMOVE(&detach_ctx->head, ctx, link);
 

lib/nvme/nvme_internal.h

@@ -914,7 +914,6 @@ struct nvme_ctrlr_detach_ctx {
 
 struct spdk_nvme_detach_ctx {
 	TAILQ_HEAD(, nvme_ctrlr_detach_ctx)	head;
-	bool					polling_started;
 };
 
 struct nvme_driver {

test/unit/lib/nvme/nvme.c/nvme_ut.c

@@ -79,12 +79,18 @@ nvme_ctrlr_destruct_async(struct spdk_nvme_ctrlr *ctrlr, struct nvme_ctrlr_detac
 {
 	ut_destruct_called = true;
 	ctrlr->is_destructed = true;
+
+	ctx->shutdown_complete = true;
 }
 
 int
 nvme_ctrlr_destruct_poll_async(struct spdk_nvme_ctrlr *ctrlr,
 			       struct nvme_ctrlr_detach_ctx *ctx)
 {
+	if (!ctx->shutdown_complete) {
+		return -EAGAIN;
+	}
+
 	if (ctx->cb_fn) {
 		ctx->cb_fn(ctrlr);
 	}
@@ -1496,6 +1502,7 @@ test_spdk_nvme_detach_async(void)
 	struct spdk_nvme_ctrlr ctrlr1, ctrlr2;
 	struct nvme_driver test_driver;
 	struct spdk_nvme_detach_ctx *detach_ctx;
+	struct nvme_ctrlr_detach_ctx *ctx;
 
 	detach_ctx = NULL;
 	memset(&ctrlr1, 0, sizeof(ctrlr1));
@@ -1556,6 +1563,49 @@ test_spdk_nvme_detach_async(void)
 	CU_ASSERT(TAILQ_EMPTY(&g_nvme_attached_ctrlrs) == true);
 	CU_ASSERT(TAILQ_EMPTY(&test_driver.shared_attached_ctrlrs) == true);
 
+	/* Test if ctrlr2 can be detached by using the same context that
+	 * ctrlr1 uses while ctrlr1 is being detached.
+	 */
+	detach_ctx = NULL;
+	memset(&ctrlr1, 0, sizeof(ctrlr1));
+	ctrlr1.trid.trtype = SPDK_NVME_TRANSPORT_PCIE;
+	memset(&ctrlr2, 0, sizeof(ctrlr2));
+	ctrlr2.trid.trtype = SPDK_NVME_TRANSPORT_PCIE;
+	TAILQ_INSERT_TAIL(&test_driver.shared_attached_ctrlrs, &ctrlr1, tailq);
+	TAILQ_INSERT_TAIL(&test_driver.shared_attached_ctrlrs, &ctrlr2, tailq);
+
+	rc = spdk_nvme_detach_async(&ctrlr1, &detach_ctx);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(ctrlr1.is_destructed == true);
+	SPDK_CU_ASSERT_FATAL(detach_ctx != NULL);
+
+	ctx = TAILQ_FIRST(&detach_ctx->head);
+	SPDK_CU_ASSERT_FATAL(ctx != NULL);
+	CU_ASSERT(ctx->ctrlr == &ctrlr1);
+	CU_ASSERT(ctx->shutdown_complete == true);
+
+	/* Set ctx->shutdown_complete for ctrlr1 to false to allow ctrlr2 to
+	 * add to detach_ctx while spdk_nvme_detach_poll_async() is being
+	 * executed.
+	 */
+	ctx->shutdown_complete = false;
+
+	rc = spdk_nvme_detach_poll_async(detach_ctx);
+	CU_ASSERT(rc == -EAGAIN);
+
+	rc = spdk_nvme_detach_async(&ctrlr2, &detach_ctx);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(ctrlr2.is_destructed == true);
+
+	/* After ctrlr2 is added to detach_ctx, set ctx->shutdown_complete for
+	 * ctrlr1 to true to complete spdk_nvme_detach_poll_async().
+	 */
+	ctx->shutdown_complete = true;
+
+	rc = spdk_nvme_detach_poll_async(detach_ctx);
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(TAILQ_EMPTY(&test_driver.shared_attached_ctrlrs) == true);
+
 	g_spdk_nvme_driver = NULL;
 	pthread_mutex_destroy(&test_driver.lock);
 	MOCK_CLEAR(nvme_ctrlr_get_ref_count);