From b34fb47d8c55c117d21c35a61742283fe5188460 Mon Sep 17 00:00:00 2001 From: Nick Connolly Date: Thu, 28 Jan 2021 21:23:22 +0000 Subject: [PATCH] test/nvme_ctrlr: initialize mutex for portability For correct behaviour, pthread_mutex must be initialized before use and destroyed afterwards. An already initialized mutex should not be re-initialized. Remove the call to mutex_init from setup_qpairs since it will be done in nvme_ctrlr_construct. Add calls to nvme_ctrlr_construct where nvme_ctrlr_destruct is called without a matching construct. Add missing calls to mutex_init and mutex_destroy as required. Tested with a pthreads library that contains debugging code to check the mutex state. Signed-off-by: Nick Connolly Change-Id: I0ee97a70d67157668cd8921fbee03d976d4d607d Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6161 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker --- .../lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 3e2321d80..f3a54a47f 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -1262,8 +1262,6 @@ setup_qpairs(struct spdk_nvme_ctrlr *ctrlr, uint32_t num_io_queues) { uint32_t i; - CU_ASSERT(pthread_mutex_init(&ctrlr->ctrlr_lock, NULL) == 0); - SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(ctrlr) == 0); ctrlr->page_size = 0x1000; @@ -1487,6 +1485,8 @@ test_spdk_nvme_ctrlr_reconnect_io_qpair(void) struct spdk_nvme_qpair qpair = {}; int rc; + CU_ASSERT(pthread_mutex_init(&ctrlr.ctrlr_lock, NULL) == 0); + /* Various states of controller disconnect. */ qpair.id = 1; qpair.ctrlr = &ctrlr; @@ -1533,6 +1533,8 @@ test_spdk_nvme_ctrlr_reconnect_io_qpair(void) rc = spdk_nvme_ctrlr_reconnect_io_qpair(&qpair); CU_ASSERT(g_connect_qpair_called == true); CU_ASSERT(rc == 0) + + CU_ASSERT(pthread_mutex_destroy(&ctrlr.ctrlr_lock) == 0); } static void @@ -1740,6 +1742,8 @@ test_spdk_nvme_ctrlr_update_firmware(void) struct spdk_nvme_status status; enum spdk_nvme_fw_commit_action commit_action = SPDK_NVME_FW_COMMIT_REPLACE_IMG; + CU_ASSERT(pthread_mutex_init(&ctrlr.ctrlr_lock, NULL) == 0); + /* Set invalid size check function return value */ set_size = 5; ret = spdk_nvme_ctrlr_update_firmware(&ctrlr, payload, set_size, slot, commit_action, &status); @@ -1808,6 +1812,7 @@ test_spdk_nvme_ctrlr_update_firmware(void) g_failed_status = NULL; g_wait_for_completion_return_val = 0; + CU_ASSERT(pthread_mutex_destroy(&ctrlr.ctrlr_lock) == 0); set_status_cpl = 0; } @@ -1840,11 +1845,14 @@ test_nvme_ctrlr_test_active_ns(void) { uint32_t nsid, minor; size_t ns_id_count; - struct spdk_nvme_ctrlr ctrlr = {.state = NVME_CTRLR_STATE_READY}; + struct spdk_nvme_ctrlr ctrlr = {}; ctrlr.page_size = 0x1000; for (minor = 0; minor <= 2; minor++) { + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); + ctrlr.state = NVME_CTRLR_STATE_READY; + ctrlr.vs.bits.mjr = 1; ctrlr.vs.bits.mnr = minor; ctrlr.vs.bits.ter = 0; @@ -1983,6 +1991,8 @@ test_spdk_nvme_ctrlr_set_trid(void) struct spdk_nvme_ctrlr ctrlr = {0}; struct spdk_nvme_transport_id new_trid = {{0}}; + CU_ASSERT(pthread_mutex_init(&ctrlr.ctrlr_lock, NULL) == 0); + ctrlr.is_failed = false; ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_RDMA; snprintf(ctrlr.trid.subnqn, SPDK_NVMF_NQN_MAX_LEN, "%s", "nqn.2016-06.io.spdk:cnode1"); @@ -2007,6 +2017,8 @@ test_spdk_nvme_ctrlr_set_trid(void) CU_ASSERT(spdk_nvme_ctrlr_set_trid(&ctrlr, &new_trid) == 0); CU_ASSERT(strncmp(ctrlr.trid.traddr, "192.168.100.9", SPDK_NVMF_TRADDR_MAX_LEN) == 0); CU_ASSERT(strncmp(ctrlr.trid.trsvcid, "4421", SPDK_NVMF_TRSVCID_MAX_LEN) == 0); + + CU_ASSERT(pthread_mutex_destroy(&ctrlr.ctrlr_lock) == 0); } static void @@ -2018,6 +2030,7 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) ctrlr.cdata.nvmf_specific.icdoff = 1; /* Check PCI trtype, */ + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE; ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; @@ -2034,6 +2047,7 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) nvme_ctrlr_destruct(&ctrlr); /* Check RDMA trtype, */ + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_RDMA; ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; @@ -2052,6 +2066,7 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) nvme_ctrlr_destruct(&ctrlr); /* Check TCP trtype, */ + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_TCP; ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; @@ -2070,6 +2085,7 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) nvme_ctrlr_destruct(&ctrlr); /* Check FC trtype, */ + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_FC; ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; @@ -2088,6 +2104,7 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) nvme_ctrlr_destruct(&ctrlr); /* Check CUSTOM trtype, */ + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_CUSTOM; ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; @@ -2109,6 +2126,8 @@ test_nvme_ctrlr_init_set_num_queues(void) { DECLARE_AND_CONSTRUCT_CTRLR(); + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); + ctrlr.state = NVME_CTRLR_STATE_IDENTIFY; CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_IDENTIFY_IOCS_SPECIFIC */ CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC); @@ -2131,6 +2150,8 @@ test_nvme_ctrlr_init_set_keep_alive_timeout(void) { DECLARE_AND_CONSTRUCT_CTRLR(); + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0); + ctrlr.opts.keep_alive_timeout_ms = 60000; ctrlr.cdata.kas = 1; ctrlr.state = NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT;