From 4525fc898fb84f85dbac3d09edf7d4cf04923e14 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Wed, 22 Nov 2017 15:51:45 -0700 Subject: [PATCH] nvme/pcie: use common trid -> ctrlr function Simplify the PCIe transport by using an existing function to look up a controller by transport ID. Change-Id: I261865df1ba23069b052ca64944b7637d70c85ba Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/388701 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/nvme/nvme.c | 21 ++++++++---- lib/nvme/nvme_internal.h | 3 ++ lib/nvme/nvme_pcie.c | 33 ++++++++----------- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 6 ++++ 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index 4711d6226..c0dd2d9db 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -416,21 +416,28 @@ nvme_init_controllers(void *cb_ctx, spdk_nvme_attach_cb attach_cb) static struct spdk_nvme_ctrlr * spdk_nvme_get_ctrlr_by_trid(const struct spdk_nvme_transport_id *trid) { - struct spdk_nvme_ctrlr *ctrlr = NULL; - bool found = false; + struct spdk_nvme_ctrlr *ctrlr; nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock); + ctrlr = spdk_nvme_get_ctrlr_by_trid_unsafe(trid); + nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock); + + return ctrlr; +} + +/* This function must be called while holding g_spdk_nvme_driver->lock */ +struct spdk_nvme_ctrlr * +spdk_nvme_get_ctrlr_by_trid_unsafe(const struct spdk_nvme_transport_id *trid) +{ + struct spdk_nvme_ctrlr *ctrlr; TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->attached_ctrlrs, tailq) { if (spdk_nvme_transport_id_compare(&ctrlr->trid, trid) == 0) { - found = true; - break; + return ctrlr; } } - nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock); - - return (found == true) ? ctrlr : NULL; + return NULL; } /* This function must only be called while holding g_spdk_nvme_driver->lock */ diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index d56b402de..17d27a417 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -602,6 +602,9 @@ bool nvme_completion_is_retry(const struct spdk_nvme_cpl *cpl); void nvme_qpair_print_command(struct spdk_nvme_qpair *qpair, struct spdk_nvme_cmd *cmd); void nvme_qpair_print_completion(struct spdk_nvme_qpair *qpair, struct spdk_nvme_cpl *cpl); +struct spdk_nvme_ctrlr *spdk_nvme_get_ctrlr_by_trid_unsafe( + const struct spdk_nvme_transport_id *trid); + /* Transport specific functions */ #define DECLARE_TRANSPORT(name) \ struct spdk_nvme_ctrlr *nvme_ ## name ## _ctrlr_construct(const struct spdk_nvme_transport_id *trid, const struct spdk_nvme_ctrlr_opts *opts, \ diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index 6db0db963..7c0879d6d 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -238,15 +238,14 @@ _nvme_pcie_hotplug_monitor(void *cb_ctx, spdk_nvme_probe_cb probe_cb, } } } else if (event.action == SPDK_NVME_UEVENT_REMOVE) { - bool in_list = false; + struct spdk_nvme_transport_id trid; - TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->attached_ctrlrs, tailq) { - if (strcmp(event.traddr, ctrlr->trid.traddr) == 0) { - in_list = true; - break; - } - } - if (in_list == false) { + memset(&trid, 0, sizeof(trid)); + trid.trtype = SPDK_NVME_TRANSPORT_PCIE; + snprintf(trid.traddr, sizeof(trid.traddr), "%s", event.traddr); + + ctrlr = spdk_nvme_get_ctrlr_by_trid_unsafe(&trid); + if (ctrlr == NULL) { return 0; } SPDK_DEBUGLOG(SPDK_TRACE_NVME, "remove nvme address: %s\n", @@ -578,7 +577,6 @@ pcie_nvme_enum_cb(void *ctx, struct spdk_pci_device *pci_dev) struct spdk_nvme_transport_id trid = {}; struct nvme_pcie_enum_ctx *enum_ctx = ctx; struct spdk_nvme_ctrlr *ctrlr; - int rc = 0; struct spdk_pci_addr pci_addr; pci_addr = spdk_pci_device_get_addr(pci_dev); @@ -587,16 +585,13 @@ pcie_nvme_enum_cb(void *ctx, struct spdk_pci_device *pci_dev) spdk_pci_addr_fmt(trid.traddr, sizeof(trid.traddr), &pci_addr); /* Verify that this controller is not already attached */ - TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->attached_ctrlrs, tailq) { - /* NOTE: In the case like multi-process environment where the device handle is - * different per each process, we compare by BDF to determine whether it is the - * same controller. - */ - if (strcmp(trid.traddr, ctrlr->trid.traddr) == 0) { - if (!spdk_process_is_primary()) { - rc = nvme_ctrlr_add_process(ctrlr, pci_dev); - } - return rc; + ctrlr = spdk_nvme_get_ctrlr_by_trid_unsafe(&trid); + if (ctrlr) { + if (spdk_process_is_primary()) { + /* Already attached */ + return 0; + } else { + return nvme_ctrlr_add_process(ctrlr, pci_dev); } } diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index 81532351f..1bb0cb084 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -268,6 +268,12 @@ nvme_qpair_enable(struct spdk_nvme_qpair *qpair) abort(); } +struct spdk_nvme_ctrlr * +spdk_nvme_get_ctrlr_by_trid_unsafe(const struct spdk_nvme_transport_id *trid) +{ + return NULL; +} + #if 0 /* TODO: update PCIe-specific unit test */ static void