nvmf: Rework qpair disconnect when subsystem is removed

When we iterate qpairs that belong to a subsystem
and try to disconnect them, there is a chance that
some qpair can be disconnected on transport level,
e.g. the initiator may receive a disconnect for
the first qpair and disconnect others. That may lead
to a dead loop when we call spdk_nvmf_qpair_disconnect
with a callback, the callback is called immediatelly
and tries to disconnect the qpair again.

To solve this problem, move part of nvmf_poll_group_remove_subsystem
function to another function nvmf_poll_group_remove_subsystem_msg
which disconnects all qpair at once without any callback
and calls itself via thread_send_msg untill all qpairs are
disconnected.

Fixes github issue #1780

Change-Id: I1000cda73e6164917fc13f7f374366af90571b99
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6597
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: <dongx.yi@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
This commit is contained in:
Alexey Marchuk 2021-02-26 18:27:23 +03:00 committed by Tomasz Zawadzki
parent a5b784fb2a
commit 95fc1ac759

View File

@ -2,7 +2,7 @@
* BSD LICENSE * BSD LICENSE
* *
* Copyright (c) Intel Corporation. All rights reserved. * Copyright (c) Intel Corporation. All rights reserved.
* Copyright (c) 2018-2019 Mellanox Technologies LTD. All rights reserved. * Copyright (c) 2018-2019, 2021 Mellanox Technologies LTD. All rights reserved.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
@ -1361,49 +1361,54 @@ fini:
} }
static void static void
_nvmf_subsystem_disconnect_next_qpair(void *ctx) nvmf_poll_group_remove_subsystem_msg(void *ctx)
{ {
struct spdk_nvmf_qpair *qpair; struct spdk_nvmf_qpair *qpair, *qpair_tmp;
struct nvmf_qpair_disconnect_many_ctx *qpair_ctx = ctx;
struct spdk_nvmf_subsystem *subsystem; struct spdk_nvmf_subsystem *subsystem;
struct spdk_nvmf_poll_group *group; struct spdk_nvmf_poll_group *group;
struct nvmf_qpair_disconnect_many_ctx *qpair_ctx = ctx;
int rc = 0; int rc = 0;
bool have_qpairs = false;
group = qpair_ctx->group; group = qpair_ctx->group;
subsystem = qpair_ctx->subsystem; subsystem = qpair_ctx->subsystem;
TAILQ_FOREACH(qpair, &group->qpairs, link) { TAILQ_FOREACH_SAFE(qpair, &group->qpairs, link, qpair_tmp) {
if ((qpair->ctrlr != NULL) && (qpair->ctrlr->subsys == subsystem)) { if ((qpair->ctrlr != NULL) && (qpair->ctrlr->subsys == subsystem)) {
/* Use another variable to check if there were any qpairs disconnected in this call since
* we can loop over all qpairs and iterator will be NULL in the end */
have_qpairs = true;
rc = spdk_nvmf_qpair_disconnect(qpair, NULL, NULL);
if (rc) {
break; break;
} }
} }
if (qpair) {
rc = spdk_nvmf_qpair_disconnect(qpair, _nvmf_subsystem_disconnect_next_qpair, qpair_ctx);
} }
if (!qpair || rc != 0) { if (!have_qpairs || rc) {
_nvmf_poll_group_remove_subsystem_cb(ctx, rc); _nvmf_poll_group_remove_subsystem_cb(ctx, rc);
}
return; return;
} }
spdk_thread_send_msg(spdk_get_thread(), nvmf_poll_group_remove_subsystem_msg, ctx);
}
void void
nvmf_poll_group_remove_subsystem(struct spdk_nvmf_poll_group *group, nvmf_poll_group_remove_subsystem(struct spdk_nvmf_poll_group *group,
struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_subsystem *subsystem,
spdk_nvmf_poll_group_mod_done cb_fn, void *cb_arg) spdk_nvmf_poll_group_mod_done cb_fn, void *cb_arg)
{ {
struct spdk_nvmf_qpair *qpair;
struct spdk_nvmf_subsystem_poll_group *sgroup; struct spdk_nvmf_subsystem_poll_group *sgroup;
struct nvmf_qpair_disconnect_many_ctx *ctx; struct nvmf_qpair_disconnect_many_ctx *ctx;
int rc = 0;
uint32_t i; uint32_t i;
ctx = calloc(1, sizeof(struct nvmf_qpair_disconnect_many_ctx)); ctx = calloc(1, sizeof(struct nvmf_qpair_disconnect_many_ctx));
if (!ctx) { if (!ctx) {
SPDK_ERRLOG("Unable to allocate memory for context to remove poll subsystem\n"); SPDK_ERRLOG("Unable to allocate memory for context to remove poll subsystem\n");
goto fini; if (cb_fn) {
cb_fn(cb_arg, -1);
}
return;
} }
ctx->group = group; ctx->group = group;
@ -1418,29 +1423,7 @@ nvmf_poll_group_remove_subsystem(struct spdk_nvmf_poll_group *group,
sgroup->ns_info[i].state = SPDK_NVMF_SUBSYSTEM_INACTIVE; sgroup->ns_info[i].state = SPDK_NVMF_SUBSYSTEM_INACTIVE;
} }
TAILQ_FOREACH(qpair, &group->qpairs, link) { nvmf_poll_group_remove_subsystem_msg(ctx);
if ((qpair->ctrlr != NULL) && (qpair->ctrlr->subsys == subsystem)) {
break;
}
}
if (qpair) {
rc = spdk_nvmf_qpair_disconnect(qpair, _nvmf_subsystem_disconnect_next_qpair, ctx);
} else {
/* call the callback immediately. It will handle any channel iteration */
_nvmf_poll_group_remove_subsystem_cb(ctx, 0);
}
if (rc != 0 && rc != -EINPROGRESS) {
free(ctx);
goto fini;
}
return;
fini:
if (cb_fn) {
cb_fn(cb_arg, rc);
}
} }
void void