From f4e892fe14bd350e3370da44adba5b0dfb89f617 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 29 Nov 2017 15:56:19 +0900 Subject: [PATCH] iscsi: make target management thread-safe by linked list Current fixed sized array for iSCSI target is not thread safe. When newly created targets are inserted into the array, multiple targets may be inserted into the same position. Linked list is better than fixed size array to support dynamic reconfiguration. Change-Id: I4db5a3ec16844db06e0cfcd2aef1f97952f15afa Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/385371 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Daniel Verkamp --- lib/iscsi/iscsi.h | 3 +- lib/iscsi/iscsi_rpc.c | 11 +-- lib/iscsi/iscsi_subsystem.c | 7 +- lib/iscsi/tgt_node.c | 156 ++++++++++++++++++------------------ lib/iscsi/tgt_node.h | 3 +- 5 files changed, 83 insertions(+), 97 deletions(-) diff --git a/lib/iscsi/iscsi.h b/lib/iscsi/iscsi.h index 61f77bac21..7ae45a9b4b 100644 --- a/lib/iscsi/iscsi.h +++ b/lib/iscsi/iscsi.h @@ -61,7 +61,6 @@ #define MAX_NETMASK 256 #define MAX_PORTAL_GROUP 4096 #define MAX_INITIATOR_GROUP 4096 -#define MAX_ISCSI_TARGET_NODE 4096 #define MAX_SESSIONS 1024 #define MAX_ISCSI_CONNECTIONS MAX_SESSIONS #define MAX_FIRSTBURSTLENGTH 16777215 @@ -264,7 +263,7 @@ struct spdk_iscsi_globals { TAILQ_HEAD(, spdk_iscsi_portal_grp) pg_head; TAILQ_HEAD(, spdk_iscsi_init_grp) ig_head; int ntargets; - struct spdk_iscsi_tgt_node *target[MAX_ISCSI_TARGET_NODE]; + TAILQ_HEAD(, spdk_iscsi_tgt_node) target_head; int timeout; int nopininterval; diff --git a/lib/iscsi/iscsi_rpc.c b/lib/iscsi/iscsi_rpc.c index 1a0d22dc6d..272bb46f84 100644 --- a/lib/iscsi/iscsi_rpc.c +++ b/lib/iscsi/iscsi_rpc.c @@ -253,9 +253,8 @@ static void spdk_rpc_get_target_nodes(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) { - struct spdk_iscsi_globals *iscsi = &g_spdk_iscsi; struct spdk_json_write_ctx *w; - size_t tgt_idx; + struct spdk_iscsi_tgt_node *tgtnode; int i; if (params != NULL) { @@ -271,13 +270,7 @@ spdk_rpc_get_target_nodes(struct spdk_jsonrpc_request *request, spdk_json_write_array_begin(w); - for (tgt_idx = 0 ; tgt_idx < MAX_ISCSI_TARGET_NODE; tgt_idx++) { - struct spdk_iscsi_tgt_node *tgtnode = iscsi->target[tgt_idx]; - - if (tgtnode == NULL) { - continue; - } - + TAILQ_FOREACH(tgtnode, &g_spdk_iscsi.target_head, tailq) { spdk_json_write_object_begin(w); spdk_json_write_name(w, "name"); diff --git a/lib/iscsi/iscsi_subsystem.c b/lib/iscsi/iscsi_subsystem.c index 8354e34a27..e62c1e5eaf 100644 --- a/lib/iscsi/iscsi_subsystem.c +++ b/lib/iscsi/iscsi_subsystem.c @@ -228,22 +228,19 @@ static const char *target_nodes_section = \ static void spdk_iscsi_config_dump_target_nodes(FILE *fp) { - int t = 0, l = 0, m = 0; + int l = 0, m = 0; struct spdk_scsi_dev *dev = NULL; struct spdk_iscsi_tgt_node *target = NULL; /* Create target nodes section */ fprintf(fp, "%s", target_nodes_section); - for (t = 0; t < MAX_ISCSI_TARGET_NODE; t++) { + TAILQ_FOREACH(target, &g_spdk_iscsi.target_head, tailq) { int idx; const char *authmethod = "None"; char authgroup[32] = "None"; const char *usedigest = "Auto"; - target = g_spdk_iscsi.target[t]; - if (NULL == target) continue; - dev = target->dev; if (NULL == dev) continue; diff --git a/lib/iscsi/tgt_node.c b/lib/iscsi/tgt_node.c index 1a3f2ec7c6..a5c32de04d 100644 --- a/lib/iscsi/tgt_node.c +++ b/lib/iscsi/tgt_node.c @@ -300,7 +300,7 @@ spdk_iscsi_send_tgts(struct spdk_iscsi_conn *conn, const char *iiqn, int len; int rc; int pg_tag; - int i, j, k; + int i, j; if (conn == NULL) return 0; @@ -321,10 +321,7 @@ spdk_iscsi_send_tgts(struct spdk_iscsi_conn *conn, const char *iiqn, } pthread_mutex_lock(&g_spdk_iscsi.mutex); - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - target = g_spdk_iscsi.target[i]; - if (target == NULL) - continue; + TAILQ_FOREACH(target, &g_spdk_iscsi.target_head, tailq) { if (strcasecmp(tiqn, "ALL") != 0 && strcasecmp(tiqn, target->name) != 0) { continue; @@ -339,11 +336,11 @@ spdk_iscsi_send_tgts(struct spdk_iscsi_conn *conn, const char *iiqn, "TargetName=%s", target->name); total += len + 1; - for (j = 0; j < target->maxmap; j++) { - pg_tag = target->map[j].pg->tag; + for (i = 0; i < target->maxmap; i++) { + pg_tag = target->map[i].pg->tag; /* skip same pg_tag */ - for (k = 0; k < j; k++) { - if (target->map[k].pg->tag == pg_tag) { + for (j = 0; j < i; j++) { + if (target->map[j].pg->tag == pg_tag) { goto skip_pg_tag; } } @@ -393,18 +390,21 @@ skip_pg_tag: return total; } +static void +spdk_iscsi_tgt_node_list_init(void) +{ + g_spdk_iscsi.ntargets = 0; + TAILQ_INIT(&g_spdk_iscsi.target_head); +} + struct spdk_iscsi_tgt_node * spdk_iscsi_find_tgt_node(const char *target_name) { struct spdk_iscsi_tgt_node *target; - int i; if (target_name == NULL) return NULL; - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - target = g_spdk_iscsi.target[i]; - if (target == NULL) - continue; + TAILQ_FOREACH(target, &g_spdk_iscsi.target_head, tailq) { if (strcasecmp(target_name, target->name) == 0) { return target; } @@ -413,6 +413,39 @@ spdk_iscsi_find_tgt_node(const char *target_name) return NULL; } +static int +spdk_iscsi_tgt_node_register(struct spdk_iscsi_tgt_node *target) +{ + pthread_mutex_lock(&g_spdk_iscsi.mutex); + + if (spdk_iscsi_find_tgt_node(target->name) != NULL) { + pthread_mutex_unlock(&g_spdk_iscsi.mutex); + return -EEXIST; + } + + TAILQ_INSERT_TAIL(&g_spdk_iscsi.target_head, target, tailq); + g_spdk_iscsi.ntargets++; + + pthread_mutex_unlock(&g_spdk_iscsi.mutex); + return 0; +} + +static int +spdk_iscsi_tgt_node_unregister(struct spdk_iscsi_tgt_node *target) +{ + struct spdk_iscsi_tgt_node *t; + + TAILQ_FOREACH(t, &g_spdk_iscsi.target_head, tailq) { + if (t == target) { + TAILQ_REMOVE(&g_spdk_iscsi.target_head, t, tailq); + g_spdk_iscsi.ntargets--; + return 0; + } + } + + return -1; +} + static int spdk_check_iscsi_name(const char *name) { @@ -476,17 +509,6 @@ spdk_iscsi_tgt_node_destruct(struct spdk_iscsi_tgt_node *target) free(target); } -static int spdk_get_next_available_tgt_number(void) -{ - int i; - - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - if (g_spdk_iscsi.target[i] == NULL) - break; - } - return i; //Returns MAX_TARGET if none available. -} - static struct spdk_iscsi_tgt_node_map * spdk_iscsi_tgt_node_add_map(struct spdk_iscsi_tgt_node *target, int pg_tag, int ig_tag) @@ -561,19 +583,6 @@ spdk_iscsi_tgt_node_construct(int target_index, return NULL; } - if (target_index == -1) - target_index = spdk_get_next_available_tgt_number(); - - if (target_index >= MAX_ISCSI_TARGET_NODE) { - SPDK_ERRLOG("%d over maximum unit number\n", target_index); - return NULL; - } - - if (g_spdk_iscsi.target[target_index] != NULL) { - SPDK_ERRLOG("tgt_node%d: duplicate unit\n", target_index); - return NULL; - } - if (name == NULL) { SPDK_ERRLOG("TargetName not found\n"); return NULL; @@ -684,10 +693,12 @@ spdk_iscsi_tgt_node_construct(int target_index, } target->queue_depth = queue_depth; - pthread_mutex_lock(&g_spdk_iscsi.mutex); - g_spdk_iscsi.ntargets++; - g_spdk_iscsi.target[target->num] = target; - pthread_mutex_unlock(&g_spdk_iscsi.mutex); + rc = spdk_iscsi_tgt_node_register(target); + if (rc != 0) { + SPDK_ERRLOG("register target is failed\n"); + spdk_iscsi_tgt_node_destruct(target); + return NULL; + } return target; } @@ -912,6 +923,8 @@ int spdk_iscsi_init_tgt_nodes(void) SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "spdk_iscsi_init_tgt_nodes\n"); + spdk_iscsi_tgt_node_list_init(); + sp = spdk_conf_first_section(NULL); while (sp != NULL) { if (spdk_conf_section_match_prefix(sp, "TargetNode")) { @@ -932,50 +945,37 @@ int spdk_iscsi_init_tgt_nodes(void) return 0; } -int +void spdk_iscsi_shutdown_tgt_nodes(void) { - struct spdk_iscsi_tgt_node *target; - int i; + struct spdk_iscsi_tgt_node *target, *tmp; pthread_mutex_lock(&g_spdk_iscsi.mutex); - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - target = g_spdk_iscsi.target[i]; - if (target == NULL) - continue; - spdk_iscsi_tgt_node_destruct(target); + TAILQ_FOREACH_SAFE(target, &g_spdk_iscsi.target_head, tailq, tmp) { + TAILQ_REMOVE(&g_spdk_iscsi.target_head, target, tailq); g_spdk_iscsi.ntargets--; - g_spdk_iscsi.target[i] = NULL; + spdk_iscsi_tgt_node_destruct(target); } pthread_mutex_unlock(&g_spdk_iscsi.mutex); - - return 0; } int spdk_iscsi_shutdown_tgt_node_by_name(const char *target_name) { struct spdk_iscsi_tgt_node *target; - int i = 0; - int ret = -1; pthread_mutex_lock(&g_spdk_iscsi.mutex); - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - target = g_spdk_iscsi.target[i]; - if (target == NULL) - continue; + target = spdk_iscsi_find_tgt_node(target_name); + if (target != NULL) { + spdk_iscsi_tgt_node_unregister(target); + spdk_iscsi_tgt_node_destruct(target); + pthread_mutex_unlock(&g_spdk_iscsi.mutex); - if (strncmp(target_name, target->name, MAX_TMPBUF) == 0) { - spdk_iscsi_tgt_node_destruct(target); - g_spdk_iscsi.ntargets--; - g_spdk_iscsi.target[i] = NULL; - ret = 0; - break; - } + return 0; } pthread_mutex_unlock(&g_spdk_iscsi.mutex); - return ret; + return -ENOENT; } int @@ -1014,33 +1014,29 @@ void spdk_iscsi_tgt_node_delete_map(struct spdk_iscsi_portal_grp *portal_group, struct spdk_iscsi_tgt_node *target; int i = 0; int j = 0; - int k = 0; int flag = 0; - for (i = 0; i < MAX_ISCSI_TARGET_NODE; i++) { - target = g_spdk_iscsi.target[i]; - if (target == NULL) - continue; + TAILQ_FOREACH(target, &g_spdk_iscsi.target_head, tailq) { loop: flag = 0; - for (j = 0; j < target->maxmap; j++) { + for (i = 0; i < target->maxmap; i++) { if (portal_group) { - if (target->map[j].pg->tag == portal_group->tag) { + if (target->map[i].pg->tag == portal_group->tag) { flag = 1; } } if (initiator_group) { - if (target->map[j].ig->tag == initiator_group->tag) { + if (target->map[i].ig->tag == initiator_group->tag) { flag = 1; } } if (flag == 1) { - target->map[j].pg->ref--; - target->map[j].ig->ref--; - for (k = j; k < target->maxmap - 1; k++) { - target->map[k].pg = target->map[k + 1].pg; - target->map[k].ig = target->map[k + 1].ig; + target->map[i].pg->ref--; + target->map[i].ig->ref--; + for (j = i; j < target->maxmap - 1; j++) { + target->map[j].pg = target->map[j + 1].pg; + target->map[j].ig = target->map[j + 1].ig; } target->map[target->maxmap - 1].pg = NULL; target->map[target->maxmap - 1].ig = NULL; diff --git a/lib/iscsi/tgt_node.h b/lib/iscsi/tgt_node.h index 378f0f37a5..ddc0457edf 100644 --- a/lib/iscsi/tgt_node.h +++ b/lib/iscsi/tgt_node.h @@ -75,11 +75,12 @@ struct spdk_iscsi_tgt_node { int maxmap; struct spdk_iscsi_tgt_node_map map[MAX_TARGET_MAP]; + TAILQ_ENTRY(spdk_iscsi_tgt_node) tailq; }; int spdk_iscsi_init_tgt_nodes(void); -int spdk_iscsi_shutdown_tgt_nodes(void); +void spdk_iscsi_shutdown_tgt_nodes(void); int spdk_iscsi_shutdown_tgt_node_by_name(const char *target_name); int spdk_iscsi_send_tgts(struct spdk_iscsi_conn *conn, const char *iiqn, const char *iaddr, const char *tiqn, uint8_t *data, int alloc_len,