From cb3d98d59604b16a2fcc8d9a15c4a6887d7b5e4a Mon Sep 17 00:00:00 2001 From: John Levon Date: Mon, 7 Feb 2022 21:17:00 +0000 Subject: [PATCH] nvmf: fix nvmf_tgt_create_poll_group() cleanup On failure, we weren't cleaning up the poll group data properly, and in one place, we were trying to remove ourselves from the tgt-> list prior to being on it. Signed-off-by: John Levon Change-Id: I9bbe5847b3703eba1ee1d762392ad3159a74ac8b Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10717 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/nvmf/nvmf.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/nvmf/nvmf.c b/lib/nvmf/nvmf.c index 12f629353..cc743d849 100644 --- a/lib/nvmf/nvmf.c +++ b/lib/nvmf/nvmf.c @@ -106,21 +106,17 @@ nvmf_poll_group_poll(void *ctx) return count > 0 ? SPDK_POLLER_BUSY : SPDK_POLLER_IDLE; } +/* + * Reset and clean up the poll group (I/O channel code will actually free the + * group). + */ static void -nvmf_tgt_destroy_poll_group(void *io_device, void *ctx_buf) +nvmf_tgt_cleanup_poll_group(struct spdk_nvmf_poll_group *group) { - struct spdk_nvmf_tgt *tgt = io_device; - struct spdk_nvmf_poll_group *group = ctx_buf; struct spdk_nvmf_transport_poll_group *tgroup, *tmp; struct spdk_nvmf_subsystem_poll_group *sgroup; uint32_t sid, nsid; - SPDK_DTRACE_PROBE1(nvmf_destroy_poll_group, spdk_thread_get_id(group->thread)); - - pthread_mutex_lock(&tgt->mutex); - TAILQ_REMOVE(&tgt->poll_groups, group, link); - pthread_mutex_unlock(&tgt->mutex); - TAILQ_FOREACH_SAFE(tgroup, &group->tgroups, link, tmp) { TAILQ_REMOVE(&group->tgroups, tgroup, link); nvmf_transport_poll_group_destroy(tgroup); @@ -129,6 +125,8 @@ nvmf_tgt_destroy_poll_group(void *io_device, void *ctx_buf) for (sid = 0; sid < group->num_sgroups; sid++) { sgroup = &group->sgroups[sid]; + assert(sgroup != NULL); + for (nsid = 0; nsid < sgroup->num_ns; nsid++) { if (sgroup->ns_info[nsid].channel) { spdk_put_io_channel(sgroup->ns_info[nsid].channel); @@ -148,6 +146,24 @@ nvmf_tgt_destroy_poll_group(void *io_device, void *ctx_buf) } } +/* + * Callback to unregister a poll group from the target, and clean up its state. + */ +static void +nvmf_tgt_destroy_poll_group(void *io_device, void *ctx_buf) +{ + struct spdk_nvmf_tgt *tgt = io_device; + struct spdk_nvmf_poll_group *group = ctx_buf; + + SPDK_DTRACE_PROBE1(nvmf_destroy_poll_group, spdk_thread_get_id(group->thread)); + + pthread_mutex_lock(&tgt->mutex); + TAILQ_REMOVE(&tgt->poll_groups, group, link); + pthread_mutex_unlock(&tgt->mutex); + + nvmf_tgt_cleanup_poll_group(group); +} + static int nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) { @@ -165,6 +181,7 @@ nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) TAILQ_FOREACH(transport, &tgt->transports, link) { rc = nvmf_poll_group_add_transport(group, transport); if (rc != 0) { + nvmf_tgt_cleanup_poll_group(group); return rc; } } @@ -172,6 +189,7 @@ nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) group->num_sgroups = tgt->max_subsystems; group->sgroups = calloc(tgt->max_subsystems, sizeof(struct spdk_nvmf_subsystem_poll_group)); if (!group->sgroups) { + nvmf_tgt_cleanup_poll_group(group); return -ENOMEM; } @@ -184,7 +202,7 @@ nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) } if (nvmf_poll_group_add_subsystem(group, subsystem, NULL, NULL) != 0) { - nvmf_tgt_destroy_poll_group(io_device, ctx_buf); + nvmf_tgt_cleanup_poll_group(group); return -1; } }