Commit 89ae0e3c authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

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: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I204f66469f0bf54845c773da5b4ac86f3c8dca60
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3089


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent e685db2c

lib/iscsi/conn.c

+22 −12
Original line number Diff line number Diff line
@@ -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) {

	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 conn;

		}

	}

	pthread_mutex_unlock(&g_conns_mutex);



	return NULL;

	return conn;

}



static void

free_conn(struct spdk_iscsi_conn *conn)

_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)

{

	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);

}

lib/iscsi/conn.h

+2 −0
Original line number Diff line number Diff line
@@ -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;

test/iscsi_tgt/rpc_config/rpc_config.py

+0 −2
Original line number Diff line number Diff line
@@ -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,