bdev/nvme: use asynchronous ctrlr detach functions

This patch replaces the synchronous `spdk_nvme_detach()` calls with its
asynchronous counterparts in the controller unregister path.

An additional poller is introduced to periodically poll the NVMe driver
for detach completion.  Once the detach is completed, the poller is
unregistered and the nvme_ctrlr is destroyed.  The poller uses the same
period (1ms) as the async probe poller.

Since reset and detach cannot happen at the same time, reset_poller was
renamed to reset_detach_poller and it can now store the pointer either
to the reset or detach poller, depending on the circumstances.

Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I5eb2dd6383d98d25d1f9748af08c1a13d18acb0e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8729
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
This commit is contained in:
Konrad Sztyber 2021-07-09 10:51:32 +02:00 committed by Tomasz Zawadzki
parent f983acfc3a
commit c5ebb7ff99
4 changed files with 112 additions and 9 deletions

View File

@ -409,7 +409,7 @@ nvme_bdev_ctrlr_delete(struct nvme_bdev_ctrlr *nbdev_ctrlr,
}
static void
nvme_ctrlr_delete(struct nvme_ctrlr *nvme_ctrlr)
_nvme_ctrlr_delete(struct nvme_ctrlr *nvme_ctrlr)
{
struct nvme_ctrlr_trid *trid, *tmp_trid;
uint32_t i;
@ -426,8 +426,6 @@ nvme_ctrlr_delete(struct nvme_ctrlr *nvme_ctrlr)
nvme_bdev_ctrlr_delete(nvme_ctrlr->nbdev_ctrlr, nvme_ctrlr);
}
spdk_nvme_detach(nvme_ctrlr->ctrlr);
spdk_poller_unregister(&nvme_ctrlr->adminq_timer_poller);
for (i = 0; i < nvme_ctrlr->num_ns; i++) {
free(nvme_ctrlr->namespaces[i]);
}
@ -452,6 +450,53 @@ nvme_ctrlr_delete(struct nvme_ctrlr *nvme_ctrlr)
pthread_mutex_unlock(&g_bdev_nvme_mutex);
}
static int
nvme_detach_poller(void *arg)
{
struct nvme_ctrlr *nvme_ctrlr = arg;
int rc;
rc = spdk_nvme_detach_poll_async(nvme_ctrlr->detach_ctx);
if (rc != -EAGAIN) {
spdk_poller_unregister(&nvme_ctrlr->reset_detach_poller);
_nvme_ctrlr_delete(nvme_ctrlr);
}
return SPDK_POLLER_BUSY;
}
static void
nvme_ctrlr_delete(struct nvme_ctrlr *nvme_ctrlr)
{
int rc;
/* First, unregister the adminq poller, as the driver will poll adminq if necessary */
spdk_poller_unregister(&nvme_ctrlr->adminq_timer_poller);
/* If we got here, the reset/detach poller cannot be active */
assert(nvme_ctrlr->reset_detach_poller == NULL);
nvme_ctrlr->reset_detach_poller = SPDK_POLLER_REGISTER(nvme_detach_poller,
nvme_ctrlr, 1000);
if (nvme_ctrlr->reset_detach_poller == NULL) {
SPDK_ERRLOG("Failed to register detach poller\n");
goto error;
}
rc = spdk_nvme_detach_async(nvme_ctrlr->ctrlr, &nvme_ctrlr->detach_ctx);
if (rc != 0) {
SPDK_ERRLOG("Failed to detach the NVMe controller\n");
goto error;
}
return;
error:
/* We don't have a good way to handle errors here, so just do what we can and delete the
* controller without detaching the underlying NVMe device.
*/
spdk_poller_unregister(&nvme_ctrlr->reset_detach_poller);
_nvme_ctrlr_delete(nvme_ctrlr);
}
static void
nvme_ctrlr_unregister_cb(void *io_device)
{
@ -868,7 +913,7 @@ bdev_nvme_ctrlr_reset_poll(void *arg)
return SPDK_POLLER_BUSY;
}
spdk_poller_unregister(&nvme_ctrlr->reset_poller);
spdk_poller_unregister(&nvme_ctrlr->reset_detach_poller);
if (rc == 0) {
/* Recreate all of the I/O queue pairs */
spdk_for_each_channel(nvme_ctrlr,
@ -897,9 +942,9 @@ bdev_nvme_reset_ctrlr(struct spdk_io_channel_iter *i, int status)
SPDK_ERRLOG("Create controller reset context failed\n");
goto err;
}
assert(nvme_ctrlr->reset_poller == NULL);
nvme_ctrlr->reset_poller = SPDK_POLLER_REGISTER(bdev_nvme_ctrlr_reset_poll,
nvme_ctrlr, 0);
assert(nvme_ctrlr->reset_detach_poller == NULL);
nvme_ctrlr->reset_detach_poller = SPDK_POLLER_REGISTER(bdev_nvme_ctrlr_reset_poll,
nvme_ctrlr, 0);
return;

View File

@ -118,7 +118,9 @@ struct nvme_ctrlr {
bdev_nvme_reset_cb reset_cb_fn;
void *reset_cb_arg;
struct spdk_nvme_ctrlr_reset_ctx *reset_ctx;
struct spdk_poller *reset_poller;
/* Poller used to check for reset/detach completion */
struct spdk_poller *reset_detach_poller;
struct spdk_nvme_detach_ctx *detach_ctx;
/** linked list pointer for device list */
TAILQ_ENTRY(nvme_ctrlr) tailq;

View File

@ -38,7 +38,8 @@ $rpc_py bdev_nvme_attach_controller -b $nvme_bdev -t $TEST_TRANSPORT -a $NVMF_FI
# Make sure the reset is also asynchronous
$rpc_py bdev_nvme_reset_controller $nvme_bdev
# TODO: Once the async detach path is functional, send a bdev_nvme_detach_controller here
# Finally, detach the controller to verify the detach path
$rpc_py bdev_nvme_detach_controller $nvme_bdev
trap - SIGINT SIGTERM EXIT
nvmftestfini

View File

@ -565,6 +565,21 @@ spdk_nvme_detach(struct spdk_nvme_ctrlr *ctrlr)
return 0;
}
int
spdk_nvme_detach_async(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_detach_ctx **ctx)
{
SPDK_CU_ASSERT_FATAL(ctx != NULL);
*(struct spdk_nvme_ctrlr **)ctx = ctrlr;
return 0;
}
int
spdk_nvme_detach_poll_async(struct spdk_nvme_detach_ctx *ctx)
{
return spdk_nvme_detach((struct spdk_nvme_ctrlr *)ctx);
}
void
spdk_nvme_ctrlr_get_default_ctrlr_opts(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size)
{
@ -1186,6 +1201,8 @@ test_create_ctrlr(void)
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") != NULL);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1294,6 +1311,8 @@ test_reset_ctrlr(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1369,6 +1388,8 @@ test_race_between_reset_and_destruct_ctrlr(void)
spdk_put_io_channel(ch2);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1508,6 +1529,8 @@ test_failover_ctrlr(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1637,6 +1660,8 @@ test_pending_reset(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1701,6 +1726,8 @@ test_attach_ctrlr(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1735,6 +1762,8 @@ test_attach_ctrlr(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1765,6 +1794,8 @@ test_attach_ctrlr(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -1864,6 +1895,8 @@ test_aer_cb(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2058,6 +2091,8 @@ test_submit_nvme_cmd(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2170,6 +2205,8 @@ test_add_remove_trid(void)
CU_ASSERT(rc == 0);
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == nvme_ctrlr);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2212,6 +2249,8 @@ test_add_remove_trid(void)
CU_ASSERT(rc == 0);
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == nvme_ctrlr);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2388,6 +2427,8 @@ test_abort(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2428,6 +2469,8 @@ test_get_io_qpair(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2492,6 +2535,8 @@ test_bdev_unregister(void)
nvme_ctrlr->destruct = true;
_nvme_ctrlr_destruct(nvme_ctrlr);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2606,6 +2651,8 @@ test_init_ana_log_page(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2767,6 +2814,8 @@ test_reconnect_qpair(void)
rc = bdev_nvme_delete("nvme0", &g_any_trid);
CU_ASSERT(rc == 0);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL);
@ -2851,6 +2900,8 @@ test_create_bdev_ctrlr(void)
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid1) != NULL);
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid2) != NULL);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL);
@ -2892,6 +2943,8 @@ test_create_bdev_ctrlr(void)
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid1) != NULL);
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid2) != NULL);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr);
@ -2905,6 +2958,8 @@ test_create_bdev_ctrlr(void)
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid1) == NULL);
CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid2) != NULL);
poll_threads();
spdk_delay_us(1000);
poll_threads();
CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL);