From 4d48d87a7fd8ac60c3fe5e2407b9a0ba99a6fc40 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 19 Jan 2018 11:15:38 -0700 Subject: [PATCH] iscsi: remove idle connection handling Some upcoming changes will effectively render this moot anyways by adding an epoll/kqueue descriptor to poll on in all cases (not just connections that have been idle for 5ms). The epoll/kqueue code was just ifdef'd out instead of removed - some of this code will be useful and reusable with minimal changes in the upcoming patches. Signed-off-by: Jim Harris Change-Id: I0c354390537e6369cb3c32e78a59c300dec6d098 Reviewed-on: https://review.gerrithub.io/395553 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: --- CHANGELOG.md | 5 + etc/spdk/iscsi.conf.in | 5 - include/spdk/net.h | 1 - lib/iscsi/conn.c | 160 +----------------- lib/iscsi/conn.h | 3 - lib/iscsi/iscsi_rpc.c | 3 - lib/iscsi/iscsi_subsystem.c | 6 - lib/net/net_framework_default.c | 6 - .../idle_migration/build_configuration.sh | 14 -- .../idle_migration/connection_status.py | 74 -------- .../idle_migration/idle_migration.sh | 58 ------- test/iscsi_tgt/idle_migration/iscsi.conf | 10 -- test/iscsi_tgt/iscsi_tgt.sh | 1 - 13 files changed, 14 insertions(+), 332 deletions(-) delete mode 100755 test/iscsi_tgt/idle_migration/build_configuration.sh delete mode 100755 test/iscsi_tgt/idle_migration/connection_status.py delete mode 100755 test/iscsi_tgt/idle_migration/idle_migration.sh delete mode 100644 test/iscsi_tgt/idle_migration/iscsi.conf diff --git a/CHANGELOG.md b/CHANGELOG.md index 00025112ac..1bc1f13bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,11 @@ Previously a part of the bdev_virtio module, now a separate library. Virtio is n via `spdk_internal/virtio.h` file. This is an internal interface to be used when implementing new Virtio backends, namely Virtio-BLK. +### iSCSI + +The MinConnectionIdleInterval parameter has been removed, and connections are no longer migrated +to an epoll/kqueue descriptor on the master core when idle. + ## v17.10: Logical Volumes ### New dependencies diff --git a/etc/spdk/iscsi.conf.in b/etc/spdk/iscsi.conf.in index 58e1907eb6..d5d3a3e106 100644 --- a/etc/spdk/iscsi.conf.in +++ b/etc/spdk/iscsi.conf.in @@ -37,11 +37,6 @@ AuthFile /usr/local/etc/spdk/auth.conf MinConnectionsPerCore 4 - # Power saving related variable, this parameter defines how long an iSCSI - # connection must be idle before moving it to a state where it will consume - # less power. This variable is defined in terms of microseconds. We set default - # value as 5ms. - MinConnectionIdleInterval 5000 # Socket I/O timeout sec. (0 is infinite) Timeout 30 diff --git a/include/spdk/net.h b/include/spdk/net.h index 39f29c1b68..fa3bf922e7 100644 --- a/include/spdk/net.h +++ b/include/spdk/net.h @@ -55,7 +55,6 @@ const char *spdk_net_framework_get_name(void); int spdk_net_framework_start(void); void spdk_net_framework_clear_socket_association(int sock); void spdk_net_framework_fini(void); -int spdk_net_framework_idle_time(void); #define SPDK_IFNAMSIZE 32 #define SPDK_MAX_IP_PER_IFC 32 diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 691740eb9d..1b9e5139c5 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -61,9 +61,6 @@ memset(&(conn)->portal, 0, sizeof(*(conn)) - \ offsetof(struct spdk_iscsi_conn, portal)); -#define MICROSECOND_TO_TSC(x) ((x) * spdk_get_ticks_hz()/1000000) -static int64_t g_conn_idle_interval_in_tsc = -1; - #define DEFAULT_CONNECTIONS_PER_LCORE 4 #define SPDK_MAX_POLLERS_PER_CORE 4096 static int g_connections_per_lcore = DEFAULT_CONNECTIONS_PER_LCORE; @@ -77,16 +74,9 @@ static pthread_mutex_t g_conns_mutex; static struct spdk_poller *g_shutdown_timer = NULL; static uint32_t spdk_iscsi_conn_allocate_reactor(uint64_t cpumask); -static void __add_idle_conn(void *arg1, void *arg2); - -/** Global variables used for managing idle connections. */ -static int g_poll_fd = 0; -static struct spdk_poller *g_idle_conn_poller; -static STAILQ_HEAD(idle_list, spdk_iscsi_conn) g_idle_conn_list_head; void spdk_iscsi_conn_login_do_work(void *arg); void spdk_iscsi_conn_full_feature_do_work(void *arg); -void spdk_iscsi_conn_idle_do_work(void *arg); static void spdk_iscsi_conn_full_feature_migrate(void *arg1, void *arg2); static struct spdk_event *spdk_iscsi_conn_get_migrate_event(struct spdk_iscsi_conn *conn, @@ -94,11 +84,6 @@ static struct spdk_event *spdk_iscsi_conn_get_migrate_event(struct spdk_iscsi_co static void spdk_iscsi_conn_stop_poller(struct spdk_iscsi_conn *conn, spdk_event_fn fn_after_stop, int lcore); -void spdk_iscsi_set_min_conn_idle_interval(int interval_in_us) -{ - g_conn_idle_interval_in_tsc = MICROSECOND_TO_TSC(interval_in_us); -} - static struct spdk_iscsi_conn * allocate_conn(void) { @@ -138,6 +123,11 @@ spdk_find_iscsi_connection_by_id(int cid) } } +/* + * Some of this code may be useful once we add back an epoll/kqueue descriptor + * for normal processing. So just #if 0 it out for now. + */ +#if 0 #if defined(__FreeBSD__) static int @@ -335,6 +325,7 @@ check_idle_conns(void) } } +#endif #endif int spdk_initialize_iscsi_conns(void) @@ -383,17 +374,6 @@ int spdk_initialize_iscsi_conns(void) return -1; } - if (g_conn_idle_interval_in_tsc == -1) { - spdk_iscsi_set_min_conn_idle_interval(spdk_net_framework_idle_time()); - } - - STAILQ_INIT(&g_idle_conn_list_head); - if (init_idle_conns() < 0) { - return -1; - } - - g_idle_conn_poller = spdk_poller_register(spdk_iscsi_conn_idle_do_work, NULL, 0); - return 0; } @@ -504,11 +484,9 @@ error_return: free_conn(conn); return -1; } - conn->is_idle = 0; conn->logout_timer = NULL; conn->shutdown_timer = NULL; SPDK_NOTICELOG("Launching connection on acceptor thread\n"); - conn->last_activity_tsc = spdk_get_ticks(); conn->pending_task_cnt = 0; conn->pending_activate_event = false; @@ -845,18 +823,8 @@ spdk_iscsi_conn_stop_poller(struct spdk_iscsi_conn *conn, spdk_event_fn fn_after void spdk_shutdown_iscsi_conns(void) { - struct spdk_iscsi_conn *conn, *tmp; - int i; - - /* cleanup - move conns from list back into ring - where they will get cleaned up - */ - STAILQ_FOREACH_SAFE(conn, &g_idle_conn_list_head, link, tmp) { - STAILQ_REMOVE(&g_idle_conn_list_head, conn, spdk_iscsi_conn, link); - spdk_event_call(spdk_iscsi_conn_get_migrate_event(conn, NULL)); - conn->is_idle = 0; - del_idle_conn(conn); - } + struct spdk_iscsi_conn *conn; + int i; pthread_mutex_lock(&g_conns_mutex); @@ -989,7 +957,6 @@ spdk_iscsi_task_mgmt_cpl(struct spdk_scsi_task *scsi_task) { struct spdk_iscsi_task *task = spdk_iscsi_task_from_scsi_task(scsi_task); - task->conn->last_activity_tsc = spdk_get_ticks(); spdk_iscsi_task_mgmt_response(task->conn, task); spdk_iscsi_task_put(task); } @@ -1063,7 +1030,6 @@ spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task) struct spdk_iscsi_conn *conn = task->conn; spdk_trace_record(TRACE_ISCSI_TASK_DONE, conn->id, 0, (uintptr_t)task, 0); - conn->last_activity_tsc = spdk_get_ticks(); primary = spdk_iscsi_task_get_primary(task); @@ -1361,19 +1327,6 @@ spdk_iscsi_conn_handle_incoming_pdus(struct spdk_iscsi_conn *conn) return i; } -static void spdk_iscsi_conn_handle_idle(struct spdk_iscsi_conn *conn) -{ - uint64_t current_tsc = spdk_get_ticks(); - - if (g_conn_idle_interval_in_tsc > 0 && - ((int64_t)(current_tsc - conn->last_activity_tsc)) >= g_conn_idle_interval_in_tsc && - conn->pending_task_cnt == 0) { - - spdk_trace_record(TRACE_ISCSI_CONN_IDLE, conn->id, 0, 0, 0); - spdk_iscsi_conn_stop_poller(conn, __add_idle_conn, spdk_env_get_first_core()); - } -} - static int spdk_iscsi_conn_execute(struct spdk_iscsi_conn *conn) { @@ -1471,103 +1424,8 @@ void spdk_iscsi_conn_full_feature_do_work(void *arg) { struct spdk_iscsi_conn *conn = arg; - int rc = 0; - rc = spdk_iscsi_conn_execute(conn); - if (rc < 0) { - return; - } else if (rc > 0) { - conn->last_activity_tsc = spdk_get_ticks(); - } - - /* Check if the session was idle during this access pass. If it was, - and it was idle longer than the configured timeout, migrate this - session to the first core. */ - spdk_iscsi_conn_handle_idle(conn); -} - -/** - * \brief This is the main routine for the iSCSI 'idle' connection - * work item. - * - * This function handles processing of connecitons whose state have - * been determined as 'idle' for lack of activity. These connections - * no longer reside in the reactor's poller ring, instead they have - * been staged into an idle list. This function utilizes the use of - * epoll as a non-blocking means to test for new socket connection - * events that indicate the connection should be moved back into the - * active ring. - * - * While in the idle list, this function must scan these connections - * to process required timer based actions that must be maintained - * even though the connection is considered 'idle'. - */ -void spdk_iscsi_conn_idle_do_work(void *arg) -{ - uint64_t tsc; - struct spdk_iscsi_conn *tconn; - - check_idle_conns(); - - /* Now walk the idle list to process timer based actions */ - STAILQ_FOREACH(tconn, &g_idle_conn_list_head, link) { - - assert(tconn->is_idle == 1); - - if (tconn->pending_activate_event == false) { - tsc = spdk_get_ticks(); - if (tsc - tconn->last_nopin > tconn->nopininterval) { - tconn->pending_activate_event = true; - } - } - - if (tconn->pending_activate_event) { - int lcore; - - spdk_trace_record(TRACE_ISCSI_CONN_ACTIVE, tconn->id, 0, 0, 0); - - /* remove connection from idle list */ - STAILQ_REMOVE(&g_idle_conn_list_head, tconn, spdk_iscsi_conn, link); - tconn->last_activity_tsc = spdk_get_ticks(); - tconn->pending_activate_event = false; - tconn->is_idle = 0; - del_idle_conn(tconn); - /* migrate work item to new core */ - spdk_net_framework_clear_socket_association(tconn->sock); - spdk_event_call(spdk_iscsi_conn_get_migrate_event(tconn, &lcore)); - __sync_fetch_and_add(&g_num_connections[lcore], 1); - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "add conn id = %d, cid = %d poller = %p to lcore = %d active\n", - tconn->id, tconn->cid, &tconn->poller, lcore); - } - } /* for each conn in idle list */ -} - -static void -__add_idle_conn(void *arg1, void *arg2) -{ - struct spdk_iscsi_conn *conn = arg1; - int rc; - - /* - * The iSCSI target may have started shutting down when this connection was - * determined as idle. In that case, do not append the connection to the - * idle list - just start the work item again so it can start its shutdown - * process. - */ - if (conn->state == ISCSI_CONN_STATE_EXITING) { - spdk_event_call(spdk_iscsi_conn_get_migrate_event(conn, NULL)); - return; - } - - rc = add_idle_conn(conn); - if (rc == 0) { - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "add conn id = %d, cid = %d poller = %p to idle\n", - conn->id, conn->cid, conn->poller); - conn->is_idle = 1; - STAILQ_INSERT_TAIL(&g_idle_conn_list_head, conn, link); - } else { - SPDK_ERRLOG("add_idle_conn() failed\n"); - } + spdk_iscsi_conn_execute(conn); } void diff --git a/lib/iscsi/conn.h b/lib/iscsi/conn.h index ea3cec32f7..ce8e56d1ed 100644 --- a/lib/iscsi/conn.h +++ b/lib/iscsi/conn.h @@ -70,7 +70,6 @@ struct spdk_poller; struct spdk_iscsi_conn { int id; int is_valid; - int is_idle; /* * All fields below this point are reinitialized each time the * connection object is allocated. Make sure to update the @@ -136,7 +135,6 @@ struct spdk_iscsi_conn { int authenticated; int req_auth; int req_mutual; - uint64_t last_activity_tsc; uint32_t pending_task_cnt; uint32_t data_out_cnt; uint32_t data_in_cnt; @@ -177,7 +175,6 @@ void spdk_iscsi_conn_logout(struct spdk_iscsi_conn *conn); int spdk_iscsi_drop_conns(struct spdk_iscsi_conn *conn, const char *conn_match, int drop_all); void spdk_iscsi_conn_set_min_per_core(int count); -void spdk_iscsi_set_min_conn_idle_interval(int interval_in_us); int spdk_iscsi_conn_read_data(struct spdk_iscsi_conn *conn, int len, void *buf); diff --git a/lib/iscsi/iscsi_rpc.c b/lib/iscsi/iscsi_rpc.c index 9bd10c9d55..b63cb63089 100644 --- a/lib/iscsi/iscsi_rpc.c +++ b/lib/iscsi/iscsi_rpc.c @@ -970,9 +970,6 @@ spdk_rpc_get_iscsi_connections(struct spdk_jsonrpc_request *request, spdk_json_write_name(w, "tsih"); spdk_json_write_int32(w, tsih); - spdk_json_write_name(w, "is_idle"); - spdk_json_write_int32(w, c->is_idle); - spdk_json_write_name(w, "lcore_id"); spdk_json_write_int32(w, c->lcore); diff --git a/lib/iscsi/iscsi_subsystem.c b/lib/iscsi/iscsi_subsystem.c index 0cd5471e5f..0cb05d4251 100644 --- a/lib/iscsi/iscsi_subsystem.c +++ b/lib/iscsi/iscsi_subsystem.c @@ -577,7 +577,6 @@ spdk_iscsi_read_parameters_from_config_file(struct spdk_conf_section *sp) int timeout; int nopininterval; int min_conn_per_core = 0; - int conn_idle_interval = 0; const char *ag_tag; int ag_tag_i; @@ -733,11 +732,6 @@ spdk_iscsi_read_parameters_from_config_file(struct spdk_conf_section *sp) if (min_conn_per_core >= 0) { spdk_iscsi_conn_set_min_per_core(min_conn_per_core); } - - conn_idle_interval = spdk_conf_section_get_intval(sp, "MinConnectionIdleInterval"); - if (conn_idle_interval > 0) { - spdk_iscsi_set_min_conn_idle_interval(conn_idle_interval); - } } static int diff --git a/lib/net/net_framework_default.c b/lib/net/net_framework_default.c index 77317b097a..e18f98fc07 100644 --- a/lib/net/net_framework_default.c +++ b/lib/net/net_framework_default.c @@ -54,9 +54,3 @@ __attribute__((weak)) void spdk_net_framework_clear_socket_association(int sock) { } - -__attribute__((weak)) -int spdk_net_framework_idle_time(void) -{ - return IDLE_INTERVAL_TIME_IN_US; -} diff --git a/test/iscsi_tgt/idle_migration/build_configuration.sh b/test/iscsi_tgt/idle_migration/build_configuration.sh deleted file mode 100755 index b86506af94..0000000000 --- a/test/iscsi_tgt/idle_migration/build_configuration.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/usr/bin/env bash - -set -xe -rootdir=$(readlink -f $(dirname $0))/../../.. - -rpc_py=$rootdir/scripts/rpc.py - -"$rpc_py" add_initiator_group 1 "ANY" "127.0.0.1/32" -"$rpc_py" add_portal_group 1 '127.0.0.1:3260' - -for i in $(seq 0 15); do - "$rpc_py" construct_malloc_bdev 32 512 - "$rpc_py" construct_target_node "Target$i" "Target_alias$i" "Malloc$i:0" "1:1" 64 1 0 0 0 -done diff --git a/test/iscsi_tgt/idle_migration/connection_status.py b/test/iscsi_tgt/idle_migration/connection_status.py deleted file mode 100755 index e9ebafa0ee..0000000000 --- a/test/iscsi_tgt/idle_migration/connection_status.py +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/env python - -import json -import os -import sys -from time import sleep -from subprocess import check_output - -rpc_py = os.path.dirname(os.path.realpath(__file__)) + '/../../../scripts/rpc.py' - -class spdk_rpc(object): - - def __init__(self, rpc_py): - self.rpc_py = rpc_py - - def __getattr__(self, name): - def call(*args): - cmd = "python {} {}".format(self.rpc_py, name) - for arg in args: - cmd += " {}".format(arg) - return check_output(cmd, shell=True) - return call - -if __name__ == '__main__': - - if (len(sys.argv) < 2) or (sys.argv[1] != "idle" and sys.argv[1] != "active"): - print "must specify \"idle\" or \"active\"" - sys.exit(1) - - rpc = spdk_rpc(rpc_py) - - idle = 0 - active = 0 - - # capture connection state 10 times, 10 ms apart and keep a - # a running count of how many connections were found idle - # and active - for i in range(10): - - conns = json.loads(rpc.get_iscsi_connections()) - num_conns = len(conns) - - for conn in conns: - if conn['is_idle'] == 1: - idle += 1 - else: - active += 1 - - # sleep 10ms - sleep(0.01) - - active_pct = float(active) / (idle + active) - - # even when there is no active I/O on a connection, there could be - # a nopin/nopout being processed which causes a connection to - # temporarily go active; also even when fio is actively running - # there could be a brief period of time where the initiator has - # no active I/O to some connection - # - # so do not enforce that *all* connections must be idle or active; - # allow for some percentage of anomalies - anomaly_pct_allowed = 0.10 - - print "active = {}".format(active) - print "idle = {}".format(idle) - print "active_pct = {}".format(active_pct) - - if sys.argv[1] == "idle" and active_pct > anomaly_pct_allowed: - sys.exit(1) - - if sys.argv[1] == "active" and active_pct < (1.00 - anomaly_pct_allowed): - sys.exit(1) - - sys.exit(0) diff --git a/test/iscsi_tgt/idle_migration/idle_migration.sh b/test/iscsi_tgt/idle_migration/idle_migration.sh deleted file mode 100755 index 5f57b2393b..0000000000 --- a/test/iscsi_tgt/idle_migration/idle_migration.sh +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/env bash - -testdir=$(readlink -f $(dirname $0)) -rootdir=$(readlink -f $testdir/../../..) -source $rootdir/scripts/autotest_common.sh -source $rootdir/test/iscsi_tgt/common.sh - -timing_enter idle_migration - -# iSCSI target configuration -PORT=3260 - -fio_py="python $rootdir/scripts/fio.py" - -timing_enter start_iscsi_tgt - -$ISCSI_APP -c $testdir/iscsi.conf -m $ISCSI_TEST_CORE_MASK & -pid=$! -echo "Process pid: $pid" - -trap "killprocess $pid; exit 1" SIGINT SIGTERM EXIT - -waitforlisten $pid -echo "iscsi_tgt is listening. Running tests..." - -timing_exit start_iscsi_tgt - -$testdir/build_configuration.sh - -sleep 1 - -iscsiadm -m discovery -t sendtargets -p $TARGET_IP:$PORT -iscsiadm -m node --login -p $TARGET_IP:$PORT - -trap "iscsicleanup; killprocess $pid; exit 1" SIGINT SIGTERM EXIT - -sleep 5 - -# verify that ids has connections in idle state -python $testdir/connection_status.py idle - -# start fio in background - while it is running, verify that connections are active -$fio_py 4096 16 randrw 15 & -fiopid=$! -sleep 5 -python $testdir/connection_status.py active -kill $fiopid -wait $fiopid || true -sleep 1 - -# verify again that ids has connections in idle state -python $testdir/connection_status.py idle - -trap - SIGINT SIGTERM EXIT - -iscsicleanup -killprocess $pid -timing_exit idle_migration diff --git a/test/iscsi_tgt/idle_migration/iscsi.conf b/test/iscsi_tgt/idle_migration/iscsi.conf deleted file mode 100644 index 0546b93a71..0000000000 --- a/test/iscsi_tgt/idle_migration/iscsi.conf +++ /dev/null @@ -1,10 +0,0 @@ -[Global] - -[iSCSI] - NodeBase "iqn.2016-06.io.spdk" - AuthFile /usr/local/etc/spdk/auth.conf - Timeout 30 - DiscoveryAuthMethod Auto - MaxSessions 64 - ImmediateData Yes - ErrorRecoveryLevel 0 diff --git a/test/iscsi_tgt/iscsi_tgt.sh b/test/iscsi_tgt/iscsi_tgt.sh index 0014888c74..5338a49436 100755 --- a/test/iscsi_tgt/iscsi_tgt.sh +++ b/test/iscsi_tgt/iscsi_tgt.sh @@ -37,7 +37,6 @@ if [ $RUN_NIGHTLY -eq 1 ]; then if [ $SPDK_TEST_NVML -eq 1 ]; then run_test ./test/iscsi_tgt/pmem/iscsi_pmem.sh 4096 10 fi - run_test ./test/iscsi_tgt/idle_migration/idle_migration.sh run_test ./test/iscsi_tgt/ip_migration/ip_migration.sh run_test ./test/iscsi_tgt/ext4test/ext4test.sh run_test ./test/iscsi_tgt/digests/digests.sh