ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bdev/nvme: Inline bdev_nvme_destroy_qpair()

Browse Source 
In the following patches, spdk_nvme_ctrlr_disconnect_io_qpair() will
be changed to be asynchronous, spdk_nvme_ctrlr_disconnect_io_qpair()
will be called first and then spdk_nvme_ctrlr_free_io_qpair() after
the qpair is actually disconnected.

We will not be able to keep the current bdev_nvme_destroy_qpair()
function.

As a preparation, inline bdev_nvme_destroy_qpair() and remove it.

Additionally, this patch has the following changes.

Previously I/O qpair was freed and then I/O path caches were cleared.
Both are SPDK thread local. So there is no dependency for the ordering
of these two operations. However, it will reduce the size of the
following patches if we clear I/O path caches before freeing I/O qpair
when the qpair is disconnected. Hence we clear I/O path caches and then
free I/O qpair.

Remove DTRACE for bdev_nvme_destroy_qpair() for now.
It will be restored in the following patches.

Furthermore, fix potential NULL pointer acces in
bdev_nvme_create_qpair().

Change-Id: I0ab78ccb0d240e56b95b53179341afcd909a31f6
Signed-off-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10746
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
Shuhei Matsumoto 2022-03-09 07:24:35 +09:00 committed by Tomasz Zawadzki
parent 486f46e867
commit d7f0a1820e
2 changed files with 22 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
module/bdev/nvme/bdev_nvme.c
View File

@ -1080,22 +1080,6 @@ nvme_poll_group_get_ctrlr_channel(struct nvme_poll_group *group,
return ctrlr_ch; return ctrlr_ch;
} }
static void
bdev_nvme_destroy_qpair(struct nvme_ctrlr_channel *ctrlr_ch)
{
struct nvme_ctrlr *nvme_ctrlr __attribute__((unused));
if (ctrlr_ch->qpair != NULL) {
nvme_ctrlr = nvme_ctrlr_channel_get_ctrlr(ctrlr_ch);
SPDK_DTRACE_PROBE2(bdev_nvme_destroy_qpair, nvme_ctrlr->nbdev_ctrlr->name,
spdk_nvme_qpair_get_id(ctrlr_ch->qpair));
spdk_nvme_ctrlr_free_io_qpair(ctrlr_ch->qpair);
ctrlr_ch->qpair = NULL;
}
_bdev_nvme_clear_io_path_cache(ctrlr_ch);
}
static void static void
bdev_nvme_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_ctx) bdev_nvme_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_ctx)
{ {
@ -1104,12 +1088,18 @@ bdev_nvme_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_
struct nvme_ctrlr *nvme_ctrlr; struct nvme_ctrlr *nvme_ctrlr;
SPDK_NOTICELOG("qpair %p is disconnected, free the qpair and reset controller.\n", qpair); SPDK_NOTICELOG("qpair %p is disconnected, free the qpair and reset controller.\n", qpair);
/* /*
* Free the I/O qpair and reset the nvme_ctrlr. * Free the I/O qpair and reset the nvme_ctrlr.
*/ */
ctrlr_ch = nvme_poll_group_get_ctrlr_channel(group, qpair); ctrlr_ch = nvme_poll_group_get_ctrlr_channel(group, qpair);
if (ctrlr_ch != NULL) { if (ctrlr_ch != NULL) {
bdev_nvme_destroy_qpair(ctrlr_ch); if (ctrlr_ch->qpair != NULL) {
spdk_nvme_ctrlr_free_io_qpair(ctrlr_ch->qpair);
ctrlr_ch->qpair = NULL;
}
_bdev_nvme_clear_io_path_cache(ctrlr_ch);
nvme_ctrlr = nvme_ctrlr_channel_get_ctrlr(ctrlr_ch); nvme_ctrlr = nvme_ctrlr_channel_get_ctrlr(ctrlr_ch);
bdev_nvme_reset(nvme_ctrlr); bdev_nvme_reset(nvme_ctrlr);
@ -1233,7 +1223,7 @@ bdev_nvme_create_qpair(struct nvme_ctrlr_channel *ctrlr_ch)
} }
SPDK_DTRACE_PROBE3(bdev_nvme_create_qpair, nvme_ctrlr->nbdev_ctrlr->name, SPDK_DTRACE_PROBE3(bdev_nvme_create_qpair, nvme_ctrlr->nbdev_ctrlr->name,
spdk_nvme_qpair_get_id(ctrlr_ch->qpair), spdk_thread_get_id(nvme_ctrlr->thread)); spdk_nvme_qpair_get_id(qpair), spdk_thread_get_id(nvme_ctrlr->thread));
assert(ctrlr_ch->group != NULL); assert(ctrlr_ch->group != NULL);
@ -1504,7 +1494,12 @@ bdev_nvme_reset_destroy_qpair(struct spdk_io_channel_iter *i)
struct spdk_io_channel *ch = spdk_io_channel_iter_get_channel(i); struct spdk_io_channel *ch = spdk_io_channel_iter_get_channel(i);
struct nvme_ctrlr_channel *ctrlr_ch = spdk_io_channel_get_ctx(ch); struct nvme_ctrlr_channel *ctrlr_ch = spdk_io_channel_get_ctx(ch);
bdev_nvme_destroy_qpair(ctrlr_ch); _bdev_nvme_clear_io_path_cache(ctrlr_ch);
if (ctrlr_ch->qpair != NULL) {
spdk_nvme_ctrlr_free_io_qpair(ctrlr_ch->qpair);
ctrlr_ch->qpair = NULL;
}
spdk_for_each_channel_continue(i, 0); spdk_for_each_channel_continue(i, 0);
} }
@ -2096,7 +2091,12 @@ bdev_nvme_destroy_ctrlr_channel_cb(void *io_device, void *ctx_buf)
assert(ctrlr_ch->group != NULL); assert(ctrlr_ch->group != NULL);
bdev_nvme_destroy_qpair(ctrlr_ch); _bdev_nvme_clear_io_path_cache(ctrlr_ch);
if (ctrlr_ch->qpair != NULL) {
spdk_nvme_ctrlr_free_io_qpair(ctrlr_ch->qpair);
ctrlr_ch->qpair = NULL;
}
TAILQ_REMOVE(&ctrlr_ch->group->ctrlr_ch_list, ctrlr_ch, tailq); TAILQ_REMOVE(&ctrlr_ch->group->ctrlr_ch_list, ctrlr_ch, tailq);

5
test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c
View File

@ -4332,9 +4332,8 @@ test_retry_io_for_io_path_error(void)
CU_ASSERT(bdev_io->internal.in_submit_request == true); CU_ASSERT(bdev_io->internal.in_submit_request == true);
CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list)); CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));
bdev_nvme_destroy_qpair(ctrlr_ch1); spdk_nvme_ctrlr_free_io_qpair(ctrlr_ch1->qpair);
ctrlr_ch1->qpair = NULL;
CU_ASSERT(ctrlr_ch1->qpair == NULL);
poll_threads(); poll_threads();