From 7bcb08c02be656bc4a18fc089ce47e07f2285c58 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 15 Nov 2017 08:57:01 +0900 Subject: [PATCH] iscsi: Load balancing of iSCSI target is not working When an iSCSI connection is closed, spdk_iscsi_conn_stop_poller() is called in spdk_iscsi_conn_destruct() or spdk_iscsi_conn_check_shutdown(). However, through both paths, g_num_connections[] is decremented twice: (1) in spdk_iscsi_conn_stop_poller() (2) in spdk_iscsi_conn_free() This behavior makes g_num_connections[] negative unexpectedly and for some cases load balancing does not work and the same CPU is always selected. How to fix: spdk_iscsi_conn_stop_poller() is called in the following functions: - spdk_iscsi_conn_check_shutdown() - spdk_iscsi_conn_destruct() - spdk_iscsi_conn_handle_idle(). [Idea 1] Remove the code to decrement g_num_connections[] from spdk_iscsi_conn_free(). [Idea 2] Remove the code to decrement g_num_connections[] from spdk_iscsi_conn_stop_poller(). Add the code to decrement g_num_connections[] to __add_idle_conn(). [Idea 1] is simple because just only one line is deleted. [Idea 2] may be more symmetric than [Idea 1] but to find the right place to add the code in __add_idle_conn() is not clear and difficult. Hence [Idea 1] is proposed. [Idea 1] may not be the optimal one and more refactoring may be done. Change-Id: I81050f4a0e7b3ddd40896f46ab2eb8de14bbcb3a Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/384026 Reviewed-by: Cunyin Chang Reviewed-by: Ben Walker Reviewed-by: Jim Harris Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp --- lib/iscsi/conn.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index c1a13c481..49145b590 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -678,8 +678,6 @@ _spdk_iscsi_conn_free(void *arg1, void *arg2) pthread_mutex_lock(&g_conns_mutex); spdk_iscsi_remove_conn(conn); pthread_mutex_unlock(&g_conns_mutex); - - __sync_fetch_and_sub(&g_num_connections[spdk_env_get_current_core()], 1); } static void