From 04ee899fcfa2257af03f49c333d884c04fe80514 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Fri, 2 Nov 2018 09:08:58 +0100 Subject: [PATCH] nvme: improve probe error handling in MP even further In cases we probe without a specific trid, the underlying rte_bus_probe() in spdk_pci_enumerate() might fail to initialize some devices, but still return with code 0, That's technically correct, as we asked just to probe devices on the bus and that's what it did. Some devices might have been initialized, others not. In secondary process we blindly assumed all devices were probed successfully, which might have eventually led to assert failures, as current process was not on the ctrlr->active_procs list. To fix it, just add an additional check before attaching the controller in secondary process. Change-Id: If015b1e562052a9189ed1a48091b209bd2dd5f2a Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/431727 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/nvme/nvme.c | 5 +++++ test/unit/lib/nvme/nvme.c/nvme_ut.c | 4 ++++ test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c | 3 +++ .../unit/lib/nvme/nvme_ns_ocssd_cmd.c/nvme_ns_ocssd_cmd_ut.c | 2 ++ 4 files changed, 14 insertions(+) diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index fdae2f18a..c5b8ef0af 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -536,6 +536,11 @@ spdk_nvme_probe_internal(const struct spdk_nvme_transport_id *trid, void *cb_ctx continue; } + /* Do not attach if we failed to initialize it in this process */ + if (spdk_nvme_ctrlr_get_current_process(ctrlr) == NULL) { + continue; + } + nvme_ctrlr_proc_get_ref(ctrlr); /* diff --git a/test/unit/lib/nvme/nvme.c/nvme_ut.c b/test/unit/lib/nvme/nvme.c/nvme_ut.c index 6925a2cfd..b8cd6c14e 100644 --- a/test/unit/lib/nvme/nvme.c/nvme_ut.c +++ b/test/unit/lib/nvme/nvme.c/nvme_ut.c @@ -59,6 +59,10 @@ DEFINE_STUB(spdk_pci_device_get_id, struct spdk_pci_id, DEFINE_STUB(spdk_nvme_transport_available, bool, (enum spdk_nvme_transport_type trtype), true) +/* return anything non-NULL, this won't be deferenced anywhere in this test */ +DEFINE_STUB(spdk_nvme_ctrlr_get_current_process, struct spdk_nvme_ctrlr_process *, + (struct spdk_nvme_ctrlr *ctrlr), (struct spdk_nvme_ctrlr_process *)(uintptr_t)0x1) + DEFINE_STUB(nvme_ctrlr_add_process, int, (struct spdk_nvme_ctrlr *ctrlr, void *devhandle), 0) diff --git a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c index f17ffa355..3ccea8ee5 100644 --- a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c @@ -91,6 +91,9 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) { } +DEFINE_STUB(spdk_nvme_ctrlr_get_current_process, struct spdk_nvme_ctrlr_process *, + (struct spdk_nvme_ctrlr *ctrlr), NULL) + int nvme_ctrlr_add_process(struct spdk_nvme_ctrlr *ctrlr, void *devhandle) { diff --git a/test/unit/lib/nvme/nvme_ns_ocssd_cmd.c/nvme_ns_ocssd_cmd_ut.c b/test/unit/lib/nvme/nvme_ns_ocssd_cmd.c/nvme_ns_ocssd_cmd_ut.c index 5d1f42b8d..5d9fc6f9c 100644 --- a/test/unit/lib/nvme/nvme_ns_ocssd_cmd.c/nvme_ns_ocssd_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ns_ocssd_cmd.c/nvme_ns_ocssd_cmd_ut.c @@ -68,6 +68,8 @@ nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) return; } +DEFINE_STUB(spdk_nvme_ctrlr_get_current_process, struct spdk_nvme_ctrlr_process *, + (struct spdk_nvme_ctrlr *ctrlr), NULL) int nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)