From 2aeb9fb17087434d87497a85077073ea3bf94869 Mon Sep 17 00:00:00 2001 From: Yaniv Kaul Date: Tue, 30 Jul 2019 10:15:42 +0300 Subject: multiple files: reduce minor work under RCU_READ_LOCK 1. Try to unlock faster - in error paths. 2. Remove memory allocations - do them before the lock. Change-Id: I1e9ddd80b99de45ad0f557d62a5f28951dfd54c8 updates: bz#1193929 Signed-off-by: Yaniv Kaul --- xlators/mgmt/glusterd/src/glusterd-peer-utils.c | 254 ++++++++++++------------ 1 file changed, 123 insertions(+), 131 deletions(-) (limited to 'xlators/mgmt/glusterd/src/glusterd-peer-utils.c') diff --git a/xlators/mgmt/glusterd/src/glusterd-peer-utils.c b/xlators/mgmt/glusterd/src/glusterd-peer-utils.c index ce4805b8d6d..1a55a5a5375 100644 --- a/xlators/mgmt/glusterd/src/glusterd-peer-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-peer-utils.c @@ -87,6 +87,107 @@ glusterd_peerinfo_cleanup(glusterd_peerinfo_t *peerinfo) return 0; } +/* gd_peerinfo_find_from_hostname iterates over all the addresses saved for each + * peer and matches it to @hoststr. + * Returns the matched peer if found else returns NULL + */ +static glusterd_peerinfo_t * +gd_peerinfo_find_from_hostname(const char *hoststr) +{ + xlator_t *this = THIS; + glusterd_conf_t *priv = NULL; + glusterd_peerinfo_t *peer = NULL; + glusterd_peerinfo_t *found = NULL; + glusterd_peer_hostname_t *tmphost = NULL; + + GF_ASSERT(this != NULL); + priv = this->private; + GF_VALIDATE_OR_GOTO(this->name, (priv != NULL), out); + + GF_VALIDATE_OR_GOTO(this->name, (hoststr != NULL), out); + + RCU_READ_LOCK; + cds_list_for_each_entry_rcu(peer, &priv->peers, uuid_list) + { + cds_list_for_each_entry_rcu(tmphost, &peer->hostnames, hostname_list) + { + if (!strncasecmp(tmphost->hostname, hoststr, 1024)) { + gf_msg_debug(this->name, 0, "Friend %s found.. state: %d", + tmphost->hostname, peer->state.state); + found = peer; /* Probably needs to be + dereferenced*/ + goto unlock; + } + } + } +unlock: + RCU_READ_UNLOCK; +out: + return found; +} + +/* gd_peerinfo_find_from_addrinfo iterates over all the addresses saved for each + * peer, resolves them and compares them to @addr. + * + * + * NOTE: As getaddrinfo is a blocking call and is being performed multiple times + * in this function, it could lead to the calling thread to be blocked for + * significant amounts of time. + * + * Returns the matched peer if found else returns NULL + */ +static glusterd_peerinfo_t * +gd_peerinfo_find_from_addrinfo(const struct addrinfo *addr) +{ + xlator_t *this = THIS; + glusterd_conf_t *conf = NULL; + glusterd_peerinfo_t *peer = NULL; + glusterd_peerinfo_t *found = NULL; + glusterd_peer_hostname_t *address = NULL; + int ret = 0; + struct addrinfo *paddr = NULL; + struct addrinfo *tmp = NULL; + + GF_ASSERT(this != NULL); + conf = this->private; + GF_VALIDATE_OR_GOTO(this->name, (conf != NULL), out); + + RCU_READ_LOCK; + cds_list_for_each_entry_rcu(peer, &conf->peers, uuid_list) + { + cds_list_for_each_entry_rcu(address, &peer->hostnames, hostname_list) + { + /* TODO: Cache the resolved addrinfos to improve + * performance + */ + ret = getaddrinfo(address->hostname, NULL, NULL, &paddr); + if (ret) { + /* Don't fail if getaddrinfo fails, continue + * onto the next address + */ + gf_msg_trace(this->name, 0, "getaddrinfo for %s failed (%s)", + address->hostname, gai_strerror(ret)); + continue; + } + + for (tmp = paddr; tmp != NULL; tmp = tmp->ai_next) { + if (gf_compare_sockaddr(addr->ai_addr, tmp->ai_addr)) { + found = peer; /* (de)referenced? */ + break; + } + } + + freeaddrinfo(paddr); + if (found) + goto unlock; + } + } +unlock: + RCU_READ_UNLOCK; +out: + return found; +} + /* glusterd_peerinfo_find_by_hostname searches for a peer which matches the * hostname @hoststr and if found returns the pointer to peerinfo object. * Returns NULL otherwise. @@ -101,14 +202,11 @@ glusterd_peerinfo_find_by_hostname(const char *hoststr) int ret = -1; struct addrinfo *addr = NULL; struct addrinfo *p = NULL; - xlator_t *this = NULL; + xlator_t *this = THIS; glusterd_peerinfo_t *peerinfo = NULL; - this = THIS; GF_ASSERT(hoststr); - peerinfo = NULL; - peerinfo = gd_peerinfo_find_from_hostname(hoststr); if (peerinfo) return peerinfo; @@ -178,31 +276,33 @@ glusterd_peerinfo_find_by_uuid(uuid_t uuid) glusterd_conf_t *priv = NULL; glusterd_peerinfo_t *entry = NULL; glusterd_peerinfo_t *found = NULL; - xlator_t *this = NULL; + xlator_t *this = THIS; + glusterd_friend_sm_state_t state; - this = THIS; GF_ASSERT(this); + if (gf_uuid_is_null(uuid)) + return NULL; + priv = this->private; GF_ASSERT(priv); - if (gf_uuid_is_null(uuid)) - return NULL; - RCU_READ_LOCK; cds_list_for_each_entry_rcu(entry, &priv->peers, uuid_list) { if (!gf_uuid_compare(entry->uuid, uuid)) { - gf_msg_debug(this->name, 0, "Friend found... state: %s", - glusterd_friend_sm_state_name_get(entry->state.state)); found = entry; /* Probably should be rcu_dereferenced */ + state = found->state.state; break; } } RCU_READ_UNLOCK; - if (!found) + if (found) + gf_msg_debug(this->name, 0, "Friend found... state: %s", + glusterd_friend_sm_state_name_get(state)); + else gf_msg_debug(this->name, 0, "Friend with uuid: %s, not found", uuid_utoa(uuid)); return found; @@ -216,9 +316,8 @@ glusterd_peerinfo_t * glusterd_peerinfo_find(uuid_t uuid, const char *hostname) { glusterd_peerinfo_t *peerinfo = NULL; - xlator_t *this = NULL; + xlator_t *this = THIS; - this = THIS; GF_ASSERT(this); if (uuid) { @@ -442,9 +541,8 @@ glusterd_are_vol_all_peers_up(glusterd_volinfo_t *volinfo, if (!(peerinfo->connected) || (peerinfo->state.state != GD_FRIEND_STATE_BEFRIENDED)) { *down_peerstr = gf_strdup(peerinfo->hostname); - gf_msg_debug(THIS->name, 0, "Peer %s is down. ", - peerinfo->hostname); RCU_READ_UNLOCK; + gf_msg_debug(THIS->name, 0, "Peer %s is down. ", *down_peerstr); goto out; } } @@ -503,7 +601,6 @@ glusterd_peer_hostname_free(glusterd_peer_hostname_t *name) gf_boolean_t gd_peer_has_address(glusterd_peerinfo_t *peerinfo, const char *address) { - gf_boolean_t ret = _gf_false; glusterd_peer_hostname_t *hostname = NULL; GF_VALIDATE_OR_GOTO("glusterd", (peerinfo != NULL), out); @@ -512,13 +609,12 @@ gd_peer_has_address(glusterd_peerinfo_t *peerinfo, const char *address) cds_list_for_each_entry(hostname, &peerinfo->hostnames, hostname_list) { if (strcmp(hostname->hostname, address) == 0) { - ret = _gf_true; - break; + return _gf_true; } } out: - return ret; + return _gf_false; } int @@ -627,112 +723,6 @@ out: return ret; } -/* gd_peerinfo_find_from_hostname iterates over all the addresses saved for each - * peer and matches it to @hoststr. - * Returns the matched peer if found else returns NULL - */ -glusterd_peerinfo_t * -gd_peerinfo_find_from_hostname(const char *hoststr) -{ - xlator_t *this = NULL; - glusterd_conf_t *priv = NULL; - glusterd_peerinfo_t *peer = NULL; - glusterd_peerinfo_t *found = NULL; - glusterd_peer_hostname_t *tmphost = NULL; - - this = THIS; - GF_ASSERT(this != NULL); - priv = this->private; - GF_VALIDATE_OR_GOTO(this->name, (priv != NULL), out); - - GF_VALIDATE_OR_GOTO(this->name, (hoststr != NULL), out); - - RCU_READ_LOCK; - cds_list_for_each_entry_rcu(peer, &priv->peers, uuid_list) - { - cds_list_for_each_entry_rcu(tmphost, &peer->hostnames, hostname_list) - { - if (!strncasecmp(tmphost->hostname, hoststr, 1024)) { - gf_msg_debug(this->name, 0, "Friend %s found.. state: %d", - tmphost->hostname, peer->state.state); - found = peer; /* Probably needs to be - dereferenced*/ - goto unlock; - } - } - } -unlock: - RCU_READ_UNLOCK; -out: - return found; -} - -/* gd_peerinfo_find_from_addrinfo iterates over all the addresses saved for each - * peer, resolves them and compares them to @addr. - * - * - * NOTE: As getaddrinfo is a blocking call and is being performed multiple times - * in this function, it could lead to the calling thread to be blocked for - * significant amounts of time. - * - * Returns the matched peer if found else returns NULL - */ -glusterd_peerinfo_t * -gd_peerinfo_find_from_addrinfo(const struct addrinfo *addr) -{ - xlator_t *this = NULL; - glusterd_conf_t *conf = NULL; - glusterd_peerinfo_t *peer = NULL; - glusterd_peerinfo_t *found = NULL; - glusterd_peer_hostname_t *address = NULL; - int ret = 0; - struct addrinfo *paddr = NULL; - struct addrinfo *tmp = NULL; - - this = THIS; - GF_ASSERT(this != NULL); - conf = this->private; - GF_VALIDATE_OR_GOTO(this->name, (conf != NULL), out); - - GF_VALIDATE_OR_GOTO(this->name, (addr != NULL), out); - - RCU_READ_LOCK; - cds_list_for_each_entry_rcu(peer, &conf->peers, uuid_list) - { - cds_list_for_each_entry_rcu(address, &peer->hostnames, hostname_list) - { - /* TODO: Cache the resolved addrinfos to improve - * performance - */ - ret = getaddrinfo(address->hostname, NULL, NULL, &paddr); - if (ret) { - /* Don't fail if getaddrinfo fails, continue - * onto the next address - */ - gf_msg_trace(this->name, 0, "getaddrinfo for %s failed (%s)", - address->hostname, gai_strerror(ret)); - ret = 0; - continue; - } - - for (tmp = paddr; tmp != NULL; tmp = tmp->ai_next) { - if (gf_compare_sockaddr(addr->ai_addr, tmp->ai_addr)) { - found = peer; /* (de)referenced? */ - break; - } - } - - freeaddrinfo(paddr); - if (found) - goto unlock; - } - } -unlock: - RCU_READ_UNLOCK; -out: - return found; -} - /* gd_update_peerinfo_from_dict will update the hostnames for @peerinfo from * peer details with @prefix in @dict. * Returns 0 on success and -1 on failure. @@ -986,9 +976,9 @@ glusterd_peerinfo_find_by_generation(uint32_t generation) glusterd_conf_t *priv = NULL; glusterd_peerinfo_t *entry = NULL; glusterd_peerinfo_t *found = NULL; - xlator_t *this = NULL; + xlator_t *this = THIS; + glusterd_friend_sm_state_t state; - this = THIS; GF_ASSERT(this); priv = this->private; @@ -999,15 +989,17 @@ glusterd_peerinfo_find_by_generation(uint32_t generation) cds_list_for_each_entry_rcu(entry, &priv->peers, uuid_list) { if (entry->generation == generation) { - gf_msg_debug(this->name, 0, "Friend found... state: %s", - glusterd_friend_sm_state_name_get(entry->state.state)); found = entry; /* Probably should be rcu_dereferenced */ + state = found->state.state; break; } } RCU_READ_UNLOCK; - if (!found) + if (found) + gf_msg_debug(this->name, 0, "Friend found... state: %s", + glusterd_friend_sm_state_name_get(state)); + else gf_msg_debug(this->name, 0, "Friend with generation: %" PRIu32 ", not found", generation); -- cgit