From 89ae0e3c623206a14a0403523d9b0b731683b1e1 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 23 Jun 2020 15:54:37 +0900 Subject: [PATCH] lib/iscsi: Manage free connections not by array but by TAILQ Previously free connections had been managed by g_conns_array, and allocate_conn() gets the lowest free connection. This had worked almost as LIFO, and the just freed connection had been reused immediately to the new connection. Using TAILQ makes management of free connections FIFO, and this will be more intuitive and simpler, and avoid potential issues due to the fact that we do not know the state INVALID is the current connection or the current connection is exited and the new connection is allocated. This patch includes following updates. Remove the test condition that the connection ID should be zero. Connection ID is used as Target Transfer Tag (TTT) and TTT is opaque number. Hence requiring connection ID to be zero is not meaningful. iscsi_conn_free() calls free_conn() while holding g_conns_mutex, but iscsi_conn_construct() does not call free_conn() without holding g_conns_mutex. Hence add g_conns_mutex to the latter. Signed-off-by: Shuhei Matsumoto Change-Id: I204f66469f0bf54845c773da5b4ac86f3c8dca60 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3089 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/iscsi/conn.c | 38 ++++++++++++++++--------- lib/iscsi/conn.h | 2 ++ test/iscsi_tgt/rpc_config/rpc_config.py | 2 -- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 6716c20f8..c433e18ab 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -66,6 +66,8 @@ struct spdk_iscsi_conn *g_conns_array = MAP_FAILED; static int g_conns_array_fd = -1; static char g_shm_name[64]; +static TAILQ_HEAD(, spdk_iscsi_conn) g_free_conns = TAILQ_HEAD_INITIALIZER(g_free_conns); + static pthread_mutex_t g_conns_mutex = PTHREAD_MUTEX_INITIALIZER; static struct spdk_poller *g_shutdown_timer = NULL; @@ -77,29 +79,36 @@ static struct spdk_iscsi_conn * allocate_conn(void) { struct spdk_iscsi_conn *conn; - int i; pthread_mutex_lock(&g_conns_mutex); - for (i = 0; i < MAX_ISCSI_CONNECTIONS; i++) { - conn = &g_conns_array[i]; - if (!conn->is_valid) { - SPDK_ISCSI_CONNECTION_MEMSET(conn); - conn->is_valid = 1; - pthread_mutex_unlock(&g_conns_mutex); - return conn; - } + conn = TAILQ_FIRST(&g_free_conns); + if (conn != NULL) { + assert(!conn->is_valid); + TAILQ_REMOVE(&g_free_conns, conn, conn_link); + SPDK_ISCSI_CONNECTION_MEMSET(conn); + conn->is_valid = 1; } pthread_mutex_unlock(&g_conns_mutex); - return NULL; + return conn; +} + +static void +_free_conn(struct spdk_iscsi_conn *conn) +{ + memset(conn->portal_host, 0, sizeof(conn->portal_host)); + memset(conn->portal_port, 0, sizeof(conn->portal_port)); + conn->is_valid = 0; + + TAILQ_INSERT_TAIL(&g_free_conns, conn, conn_link); } static void free_conn(struct spdk_iscsi_conn *conn) { - memset(conn->portal_host, 0, sizeof(conn->portal_host)); - memset(conn->portal_port, 0, sizeof(conn->portal_port)); - conn->is_valid = 0; + pthread_mutex_lock(&g_conns_mutex); + _free_conn(conn); + pthread_mutex_unlock(&g_conns_mutex); } static struct spdk_iscsi_conn * @@ -158,6 +167,7 @@ int initialize_iscsi_conns(void) for (i = 0; i < MAX_ISCSI_CONNECTIONS; i++) { g_conns_array[i].id = i; + TAILQ_INSERT_TAIL(&g_free_conns, &g_conns_array[i], conn_link); } return 0; @@ -435,7 +445,7 @@ iscsi_conn_free(struct spdk_iscsi_conn *conn) end: SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "cleanup free conn\n"); iscsi_param_free(conn->params); - free_conn(conn); + _free_conn(conn); pthread_mutex_unlock(&g_conns_mutex); } diff --git a/lib/iscsi/conn.h b/lib/iscsi/conn.h index 08aaf2d05..8c072025f 100644 --- a/lib/iscsi/conn.h +++ b/lib/iscsi/conn.h @@ -196,6 +196,8 @@ struct spdk_iscsi_conn { TAILQ_HEAD(queued_datain_tasks, spdk_iscsi_task) queued_datain_tasks; struct spdk_iscsi_lun *luns[SPDK_SCSI_DEV_MAX_LUN]; + + TAILQ_ENTRY(spdk_iscsi_conn) conn_link; }; extern struct spdk_iscsi_conn *g_conns_array; diff --git a/test/iscsi_tgt/rpc_config/rpc_config.py b/test/iscsi_tgt/rpc_config/rpc_config.py index 1d9c4275e..9f8e32909 100755 --- a/test/iscsi_tgt/rpc_config/rpc_config.py +++ b/test/iscsi_tgt/rpc_config/rpc_config.py @@ -116,8 +116,6 @@ def verify_iscsi_connection_rpc_methods(rpc_py): jsonvalues = json.loads(output) verify(jsonvalues[0]['target_node_name'] == rpc_param['target_name'], 1, "target node name vaule is {}, expected {}".format(jsonvalues[0]['target_node_name'], rpc_param['target_name'])) - verify(jsonvalues[0]['id'] == 0, 1, - "device id value is {}, expected 0".format(jsonvalues[0]['id'])) verify(jsonvalues[0]['initiator_addr'] == rpc_param['initiator_ip'], 1, "initiator address values is {}, expected {}".format(jsonvalues[0]['initiator_addr'], rpc_param['initiator_ip'])) verify(jsonvalues[0]['target_addr'] == rpc_param['target_ip'], 1,