From f4a4ddd8a113fe4135318a92506138113dee52b0 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Mon, 11 Sep 2017 16:50:29 -0700 Subject: [PATCH] nvmf: add subsystem ctrlr management functions This moves the subsystem->ctrlrs list management fully into the subsystem code, which will help simplify thread safety considerations once we start adding locks. Change-Id: Ibc118923f1bd520f1e524cde5d45ccfcc69aee1e Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/376025 Tested-by: SPDK Automated Test System --- lib/nvmf/ctrlr.c | 29 ++++-------- lib/nvmf/nvmf_internal.h | 7 ++- lib/nvmf/subsystem.c | 65 ++++++++++++++++++--------- test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 16 ++++++- 4 files changed, 74 insertions(+), 43 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index 0115eae8b..d2326a3e5 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -80,15 +80,6 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, return NULL; } - ctrlr->cntlid = spdk_nvmf_subsystem_gen_cntlid(subsystem); - if (ctrlr->cntlid == 0xFFFF) { - /* Unable to get a cntlid */ - SPDK_ERRLOG("Reached max simultaneous ctrlrs\n"); - spdk_nvmf_poll_group_destroy(ctrlr->group); - free(ctrlr); - return NULL; - } - TAILQ_INIT(&ctrlr->qpairs); ctrlr->kato = connect_cmd->kato; ctrlr->async_event_config.raw = 0; @@ -130,13 +121,19 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "cc 0x%x\n", ctrlr->vcprop.cc.raw); SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "csts 0x%x\n", ctrlr->vcprop.csts.raw); - TAILQ_INSERT_TAIL(&subsystem->ctrlrs, ctrlr, link); + if (spdk_nvmf_subsystem_add_ctrlr(subsystem, ctrlr)) { + SPDK_ERRLOG("Unable to add controller to subsystem\n"); + spdk_nvmf_poll_group_destroy(ctrlr->group); + free(ctrlr); + return NULL; + } + return ctrlr; } static void ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr) { - TAILQ_REMOVE(&ctrlr->subsys->ctrlrs, ctrlr, link); + spdk_nvmf_subsystem_remove_ctrlr(ctrlr->subsys, ctrlr); spdk_nvmf_poll_group_destroy(ctrlr->group); free(ctrlr); } @@ -226,18 +223,10 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, return; } } else { - struct spdk_nvmf_ctrlr *tmp; - qpair->type = QPAIR_TYPE_IOQ; SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "Connect I/O Queue for controller id 0x%x\n", data->cntlid); - ctrlr = NULL; - TAILQ_FOREACH(tmp, &subsystem->ctrlrs, link) { - if (tmp->cntlid == data->cntlid) { - ctrlr = tmp; - break; - } - } + ctrlr = spdk_nvmf_subsystem_get_ctrlr(subsystem, data->cntlid); if (ctrlr == NULL) { SPDK_ERRLOG("Unknown controller ID 0x%x\n", data->cntlid); SPDK_NVMF_INVALID_CONNECT_DATA(rsp, cntlid); diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index d5c48b0ce..f06606423 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -240,7 +240,12 @@ int spdk_nvmf_bdev_ctrlr_identify_ns(struct spdk_bdev *bdev, struct spdk_nvme_ns int spdk_nvmf_subsystem_bdev_attach(struct spdk_nvmf_subsystem *subsystem); void spdk_nvmf_subsystem_bdev_detach(struct spdk_nvmf_subsystem *subsystem); -uint16_t spdk_nvmf_subsystem_gen_cntlid(struct spdk_nvmf_subsystem *subsystem); +int spdk_nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem, + struct spdk_nvmf_ctrlr *ctrlr); +void spdk_nvmf_subsystem_remove_ctrlr(struct spdk_nvmf_subsystem *subsystem, + struct spdk_nvmf_ctrlr *ctrlr); +struct spdk_nvmf_ctrlr *spdk_nvmf_subsystem_get_ctrlr(struct spdk_nvmf_subsystem *subsystem, + uint16_t cntlid); static inline struct spdk_nvmf_ns * _spdk_nvmf_subsystem_get_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid) diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index dcb03ba1a..e757908ce 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -497,19 +497,16 @@ spdk_nvmf_subsystem_get_type(struct spdk_nvmf_subsystem *subsystem) return subsystem->subtype; } -uint16_t +static uint16_t spdk_nvmf_subsystem_gen_cntlid(struct spdk_nvmf_subsystem *subsystem) { - struct spdk_nvmf_ctrlr *ctrlr; - uint16_t count; - bool in_use = true; + int count; /* * In the worst case, we might have to try all CNTLID values between 1 and 0xFFF0 - 1 * before we find one that is unused (or find that all values are in use). */ - count = 0xFFF0 - 1; - do { + for (count = 0; count < 0xFFF0 - 1; count++) { subsystem->next_cntlid++; if (subsystem->next_cntlid >= 0xFFF0) { /* The spec reserves cntlid values in the range FFF0h to FFFFh. */ @@ -517,21 +514,49 @@ spdk_nvmf_subsystem_gen_cntlid(struct spdk_nvmf_subsystem *subsystem) } /* Check if a controller with this cntlid currently exists. */ - in_use = false; - TAILQ_FOREACH(ctrlr, &subsystem->ctrlrs, link) { - if (ctrlr->cntlid == subsystem->next_cntlid) { - in_use = true; - break; - } + if (spdk_nvmf_subsystem_get_ctrlr(subsystem, subsystem->next_cntlid) == NULL) { + /* Found unused cntlid */ + return subsystem->next_cntlid; } - - count--; - } while (in_use && count > 0); - - if (count == 0) { - /* All valid cntlid values are in use. */ - return 0xFFFF; } - return subsystem->next_cntlid; + /* All valid cntlid values are in use. */ + return 0xFFFF; +} + +int +spdk_nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_ctrlr *ctrlr) +{ + ctrlr->cntlid = spdk_nvmf_subsystem_gen_cntlid(subsystem); + if (ctrlr->cntlid == 0xFFFF) { + /* Unable to get a cntlid */ + SPDK_ERRLOG("Reached max simultaneous ctrlrs\n"); + return -EBUSY; + } + + TAILQ_INSERT_TAIL(&subsystem->ctrlrs, ctrlr, link); + + return 0; +} + +void +spdk_nvmf_subsystem_remove_ctrlr(struct spdk_nvmf_subsystem *subsystem, + struct spdk_nvmf_ctrlr *ctrlr) +{ + assert(subsystem == ctrlr->subsys); + TAILQ_REMOVE(&subsystem->ctrlrs, ctrlr, link); +} + +struct spdk_nvmf_ctrlr * +spdk_nvmf_subsystem_get_ctrlr(struct spdk_nvmf_subsystem *subsystem, uint16_t cntlid) +{ + struct spdk_nvmf_ctrlr *ctrlr; + + TAILQ_FOREACH(ctrlr, &subsystem->ctrlrs, link) { + if (ctrlr->cntlid == cntlid) { + return ctrlr; + } + } + + return NULL; } diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index db8706de1..0ae4319e0 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -115,12 +115,24 @@ spdk_nvmf_subsystem_get_next_ns(struct spdk_nvmf_subsystem *subsystem, struct sp return NULL; } -uint16_t -spdk_nvmf_subsystem_gen_cntlid(struct spdk_nvmf_subsystem *subsystem) +int +spdk_nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_ctrlr *ctrlr) { return 0; } +void +spdk_nvmf_subsystem_remove_ctrlr(struct spdk_nvmf_subsystem *subsystem, + struct spdk_nvmf_ctrlr *ctrlr) +{ +} + +struct spdk_nvmf_ctrlr * +spdk_nvmf_subsystem_get_ctrlr(struct spdk_nvmf_subsystem *subsystem, uint16_t cntlid) +{ + return NULL; +} + bool spdk_nvmf_ctrlr_dsm_supported(struct spdk_nvmf_ctrlr *ctrlr) {