From f1158e4f8e07847f33d3005c8e585cb00d639f18 Mon Sep 17 00:00:00 2001 From: Sai Sunku Date: Thu, 20 Feb 2025 14:33:55 +0000 Subject: [PATCH] include, prov: Change util_av lock to genlock The util_av lock will be held in the fast path in the EFA provider. With the correct threading and progress models (FI_THREAD_DOMAIN and FI_PROGRESS_CONTROL_UNIFIED), this lock should also be a no-op. Similar to the ep_list_lock. This commit changes the mutex to a genlock that becomes a no-op for the correct threading and progress models and a mutex otherwise. Signed-off-by: Sai Sunku --- include/ofi_util.h | 2 +- prov/coll/src/coll_coll.c | 4 ++-- prov/efa/src/efa_av.c | 12 +++++----- prov/rxd/src/rxd_av.c | 8 +++---- prov/shm/src/smr_av.c | 8 +++---- prov/sm2/src/sm2_av.c | 14 ++++++------ prov/tcp/src/xnet_rdm.c | 4 ++-- prov/util/src/rxm_av.c | 42 +++++++++++++++++------------------ prov/util/src/util_av.c | 46 ++++++++++++++++++++++++--------------- 9 files changed, 75 insertions(+), 65 deletions(-) diff --git a/include/ofi_util.h b/include/ofi_util.h index bc590bb4d1a..054e230f80d 100644 --- a/include/ofi_util.h +++ b/include/ofi_util.h @@ -880,7 +880,7 @@ struct util_av { struct fid_av av_fid; struct util_domain *domain; ofi_atomic32_t ref; - ofi_mutex_t lock; + struct ofi_genlock lock; const struct fi_provider *prov; struct util_av_entry *hash; diff --git a/prov/coll/src/coll_coll.c b/prov/coll/src/coll_coll.c index 63fa5b5d7b7..4e0165b50ec 100644 --- a/prov/coll/src/coll_coll.c +++ b/prov/coll/src/coll_coll.c @@ -931,10 +931,10 @@ int coll_join_collective(struct fid_ep *ep, const void *addr, av_set = container_of(set, struct util_av_set, av_set_fid); if (coll_addr == FI_ADDR_NOTAVAIL) { - ofi_mutex_lock(&av_set->av->lock); + ofi_genlock_lock(&av_set->av->lock); assert(av_set->av->av_set); coll_mc = &av_set->av->av_set->coll_mc; - ofi_mutex_unlock(&av_set->av->lock); + ofi_genlock_unlock(&av_set->av->lock); } else { coll_mc = (struct util_coll_mc*) ((uintptr_t) coll_addr); } diff --git a/prov/efa/src/efa_av.c b/prov/efa/src/efa_av.c index 9c574c54121..df2361c3928 100644 --- a/prov/efa/src/efa_av.c +++ b/prov/efa/src/efa_av.c @@ -593,7 +593,7 @@ int efa_av_insert_one(struct efa_av *av, struct efa_ep_addr *addr, if (av->ep_type == FI_EP_DGRAM) addr->qkey = EFA_DGRAM_CONNID; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); memset(raw_gid_str, 0, sizeof(raw_gid_str)); if (!inet_ntop(AF_INET6, addr->raw, raw_gid_str, INET6_ADDRSTRLEN)) { EFA_WARN(FI_LOG_AV, "cannot convert address to string. errno: %d\n", errno); @@ -629,7 +629,7 @@ int efa_av_insert_one(struct efa_av *av, struct efa_ep_addr *addr, raw_gid_str, addr->qpn, addr->qkey, *fi_addr); ret = 0; out: - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return ret; } @@ -742,7 +742,7 @@ static int efa_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, if (av->type != FI_AV_TABLE) return -FI_EINVAL; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); for (i = 0; i < count; i++) { conn = efa_av_addr_to_conn(av, fi_addr[i]); if (!conn) { @@ -758,7 +758,7 @@ static int efa_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, assert(err); } - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return err; } @@ -783,7 +783,7 @@ static void efa_av_close_reverse_av(struct efa_av *av) struct efa_cur_reverse_av *cur_entry, *curtmp; struct efa_prv_reverse_av *prv_entry, *prvtmp; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); HASH_ITER(hh, av->cur_reverse_av, cur_entry, curtmp) { efa_conn_release(av, cur_entry->conn); @@ -793,7 +793,7 @@ static void efa_av_close_reverse_av(struct efa_av *av) efa_conn_release(av, prv_entry->conn); } - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); } static int efa_av_close(struct fid *fid) diff --git a/prov/rxd/src/rxd_av.c b/prov/rxd/src/rxd_av.c index e3adc042778..646bec05cef 100644 --- a/prov/rxd/src/rxd_av.c +++ b/prov/rxd/src/rxd_av.c @@ -209,7 +209,7 @@ static int rxd_av_insert(struct fid_av *av_fid, const void *addr, size_t count, memset(sync_err, 0, sizeof(*sync_err) * count); } - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); if (!av->dg_addrlen) { ret = rxd_av_set_addrlen(av, addr); if (ret) @@ -255,7 +255,7 @@ static int rxd_av_insert(struct fid_av *av_fid, const void *addr, size_t count, } out: av->dg_av_used += success_cnt; - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); for (; i < count; i++) { if (fi_addr) @@ -291,7 +291,7 @@ static int rxd_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, size_t count struct rxd_av *av; av = container_of(av_fid, struct rxd_av, util_av.av_fid); - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); for (i = 0; i < count; i++) { rxd_addr = (intptr_t)ofi_idx_lookup(&av->fi_addr_idx, (int) RXD_IDX_OFFSET(fi_addr[i])); @@ -307,7 +307,7 @@ static int rxd_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, size_t count if (ret) FI_WARN(&rxd_prov, FI_LOG_AV, "Unable to remove address from AV\n"); - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return ret; } diff --git a/prov/shm/src/smr_av.c b/prov/shm/src/smr_av.c index 61e4344bde5..b142d422ebe 100644 --- a/prov/shm/src/smr_av.c +++ b/prov/shm/src/smr_av.c @@ -133,10 +133,10 @@ static int smr_av_insert(struct fid_av *av_fid, const void *addr, size_t count, ret = smr_map_add(&smr_prov, &smr_av->smr_map, addr, &shm_id); if (!ret) { - ofi_mutex_lock(&util_av->lock); + ofi_genlock_lock(&util_av->lock); ret = ofi_av_insert_addr(util_av, &shm_id, &util_addr); - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); } } else { FI_WARN(&smr_prov, FI_LOG_AV, @@ -198,7 +198,7 @@ static int smr_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, size_t count util_av = container_of(av_fid, struct util_av, av_fid); smr_av = container_of(util_av, struct smr_av, util_av); - ofi_mutex_lock(&util_av->lock); + ofi_genlock_lock(&util_av->lock); for (i = 0; i < count; i++) { FI_INFO(&smr_prov, FI_LOG_AV, "%" PRIu64 "\n", fi_addr[i]); id = smr_addr_lookup(util_av, fi_addr[i]); @@ -224,7 +224,7 @@ static int smr_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, size_t count smr_av->used--; } - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); return ret; } diff --git a/prov/sm2/src/sm2_av.c b/prov/sm2/src/sm2_av.c index f2331a77fc6..5641ed59ed8 100644 --- a/prov/sm2/src/sm2_av.c +++ b/prov/sm2/src/sm2_av.c @@ -94,11 +94,11 @@ static int sm2_av_insert(struct fid_av *av_fid, const void *addr, size_t count, if (ret) continue; - ofi_mutex_lock(&util_av->lock); + ofi_genlock_lock(&util_av->lock); ret = ofi_av_insert_addr(util_av, &gid, &util_addr); if (ret) { - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); continue; } @@ -110,7 +110,7 @@ static int sm2_av_insert(struct fid_av *av_fid, const void *addr, size_t count, if (fi_addr) fi_addr[i] = util_addr; - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); succ_count++; } @@ -137,7 +137,7 @@ static int sm2_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, util_av = container_of(av_fid, struct util_av, av_fid); sm2_av = container_of(util_av, struct sm2_av, util_av); - ofi_mutex_lock(&util_av->lock); + ofi_genlock_lock(&util_av->lock); for (i = 0; i < count; i++) { gid = *((sm2_gid_t *) ofi_av_get_addr(util_av, fi_addr[i])); if (gid > 0 && gid < SM2_MAX_UNIVERSE_SIZE) @@ -151,7 +151,7 @@ static int sm2_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, } } - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); return ret; } @@ -169,9 +169,9 @@ static int sm2_av_lookup(struct fid_av *av, fi_addr_t fi_addr, void *addr, util_av = container_of(av, struct util_av, av_fid); sm2_av = container_of(util_av, struct sm2_av, util_av); - ofi_mutex_lock(&util_av->lock); + ofi_genlock_lock(&util_av->lock); gid = *((sm2_gid_t *) ofi_av_get_addr(util_av, fi_addr)); - ofi_mutex_unlock(&util_av->lock); + ofi_genlock_unlock(&util_av->lock); if (gid >= SM2_MAX_UNIVERSE_SIZE) { FI_WARN(&sm2_prov, FI_LOG_EP_DATA, diff --git a/prov/tcp/src/xnet_rdm.c b/prov/tcp/src/xnet_rdm.c index 77456860569..6cec04a3d10 100644 --- a/prov/tcp/src/xnet_rdm.c +++ b/prov/tcp/src/xnet_rdm.c @@ -722,9 +722,9 @@ static int xnet_mplex_av_dup(struct util_ep *ep, struct xnet_mplex_av *mplex_av, if (ret) continue; - ofi_mutex_lock(&subav->lock); + ofi_genlock_lock(&subav->lock); ret = ofi_av_insert_addr_at(subav, addr, i); - ofi_mutex_unlock(&subav->lock); + ofi_genlock_unlock(&subav->lock); if (ret) return ret; } diff --git a/prov/util/src/rxm_av.c b/prov/util/src/rxm_av.c index beb11d0620c..8ed27a456fa 100644 --- a/prov/util/src/rxm_av.c +++ b/prov/util/src/rxm_av.c @@ -38,26 +38,26 @@ size_t rxm_av_max_peers(struct rxm_av *av) { size_t cnt; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); cnt = av->peer_pool->entry_cnt; - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return cnt; } void *rxm_av_alloc_conn(struct rxm_av *av) { void *conn_ctx; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); conn_ctx = ofi_buf_alloc(av->conn_pool); - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return conn_ctx; } void rxm_av_free_conn(struct rxm_av *av, void *conn_ctx) { - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); ofi_buf_free(conn_ctx); - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); } static int rxm_addr_compare(struct ofi_rbmap *map, void *key, void *data) @@ -71,7 +71,7 @@ rxm_alloc_peer(struct rxm_av *av, const void *addr) { struct util_peer_addr *peer; - assert(ofi_mutex_held(&av->util_av.lock)); + assert(ofi_genlock_held(&av->util_av.lock)); peer = ofi_ibuf_alloc(av->peer_pool); if (!peer) return NULL; @@ -92,7 +92,7 @@ rxm_alloc_peer(struct rxm_av *av, const void *addr) static void rxm_free_peer(struct util_peer_addr *peer) { - assert(ofi_mutex_held(&peer->av->util_av.lock)); + assert(ofi_genlock_held(&peer->av->util_av.lock)); assert(!peer->refcnt); ofi_rbmap_delete(&peer->av->addr_map, peer->node); ofi_ibuf_free(peer); @@ -104,7 +104,7 @@ util_get_peer(struct rxm_av *av, const void *addr) struct util_peer_addr *peer; struct ofi_rbnode *node; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); node = ofi_rbmap_find(&av->addr_map, (void *) addr); if (node) { peer = node->data; @@ -113,13 +113,13 @@ util_get_peer(struct rxm_av *av, const void *addr) peer = rxm_alloc_peer(av, addr); } - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return peer; } static void util_deref_peer(struct util_peer_addr *peer) { - assert(ofi_mutex_held(&peer->av->util_av.lock)); + assert(ofi_genlock_held(&peer->av->util_av.lock)); if (--peer->refcnt == 0) rxm_free_peer(peer); } @@ -129,16 +129,16 @@ void util_put_peer(struct util_peer_addr *peer) struct rxm_av *av; av = peer->av; - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); util_deref_peer(peer); - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); } void rxm_ref_peer(struct util_peer_addr *peer) { - ofi_mutex_lock(&peer->av->util_av.lock); + ofi_genlock_lock(&peer->av->util_av.lock); peer->refcnt++; - ofi_mutex_unlock(&peer->av->util_av.lock); + ofi_genlock_unlock(&peer->av->util_av.lock); } static void @@ -201,9 +201,9 @@ rxm_av_add_peers(struct rxm_av *av, const void *addr, size_t count, cur_addr); } if (cur_fi_addr != FI_ADDR_NOTAVAIL) { - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); rxm_put_peer_addr(av, cur_fi_addr); - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); } } return -FI_ENOMEM; @@ -230,7 +230,7 @@ static int rxm_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, * added -- i.e. fi_addr passed in here was also passed into insert. * Thus, we walk through the array backwards. */ - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); for (i = count - 1; i >= 0; i--) { FI_INFO(av->util_av.prov, FI_LOG_AV, "fi_addr %" PRIu64 "\n", fi_addr[i]); @@ -251,7 +251,7 @@ static int rxm_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, */ peer = ofi_av_addr_context(&av->util_av, fi_addr[i]); (*peer)->refcnt++; - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); ofi_genlock_lock(&av->util_av.ep_list_lock); dlist_foreach(&av->util_av.ep_list, item) { @@ -261,7 +261,7 @@ static int rxm_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, } ofi_genlock_unlock(&av->util_av.ep_list_lock); - ofi_mutex_lock(&av->util_av.lock); + ofi_genlock_lock(&av->util_av.lock); util_deref_peer(*peer); } @@ -271,7 +271,7 @@ static int rxm_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, ofi_ibuf_free(av_entry); } } - ofi_mutex_unlock(&av->util_av.lock); + ofi_genlock_unlock(&av->util_av.lock); return 0; } diff --git a/prov/util/src/util_av.c b/prov/util/src/util_av.c index 5594dd3debc..68e16aeea96 100644 --- a/prov/util/src/util_av.c +++ b/prov/util/src/util_av.c @@ -275,7 +275,7 @@ int ofi_av_insert_addr_at(struct util_av *av, const void *addr, fi_addr_t fi_add { struct util_av_entry *entry = NULL; - assert(ofi_mutex_held(&av->lock)); + assert(ofi_genlock_held(&av->lock)); ofi_av_straddr_log(av, FI_LOG_INFO, "inserting addr", addr); HASH_FIND(hh, av->hash, addr, av->addrlen, entry); if (entry) { @@ -302,7 +302,7 @@ int ofi_av_insert_addr(struct util_av *av, const void *addr, fi_addr_t *fi_addr) { struct util_av_entry *entry = NULL; - assert(ofi_mutex_held(&av->lock)); + assert(ofi_genlock_held(&av->lock)); ofi_av_straddr_log(av, FI_LOG_INFO, "inserting addr", addr); HASH_FIND(hh, av->hash, addr, av->addrlen, entry); if (entry) { @@ -334,7 +334,7 @@ int ofi_av_remove_addr(struct util_av *av, fi_addr_t fi_addr) { struct util_av_entry *av_entry; - assert(ofi_mutex_held(&av->lock)); + assert(ofi_genlock_held(&av->lock)); av_entry = ofi_bufpool_get_ibuf(av->av_entry_pool, fi_addr); if (!av_entry) return -FI_ENOENT; @@ -359,9 +359,9 @@ fi_addr_t ofi_av_lookup_fi_addr_unsafe(struct util_av *av, const void *addr) fi_addr_t ofi_av_lookup_fi_addr(struct util_av *av, const void *addr) { fi_addr_t fi_addr; - ofi_mutex_lock(&av->lock); + ofi_genlock_lock(&av->lock); fi_addr = ofi_av_lookup_fi_addr_unsafe(av, addr); - ofi_mutex_unlock(&av->lock); + ofi_genlock_unlock(&av->lock); return fi_addr; } @@ -388,7 +388,7 @@ int ofi_av_close_lightweight(struct util_av *av) ofi_genlock_destroy(&av->ep_list_lock); ofi_atomic_dec32(&av->domain->ref); - ofi_mutex_destroy(&av->lock); + ofi_genlock_destroy(&av->lock); return 0; } @@ -397,16 +397,16 @@ int ofi_av_close(struct util_av *av) { int ret; - ofi_mutex_lock(&av->lock); + ofi_genlock_lock(&av->lock); if (av->av_set) { ret = fi_close(&av->av_set->av_set_fid.fid); if (ret) { - ofi_mutex_unlock(&av->lock); + ofi_genlock_unlock(&av->lock); return ret; } av->av_set = NULL; } - ofi_mutex_unlock(&av->lock); + ofi_genlock_unlock(&av->lock); ret = ofi_av_close_lightweight(av); if (ret) @@ -518,15 +518,14 @@ int ofi_av_init_lightweight(struct util_domain *domain, const struct fi_av_attr struct util_av *av, void *context) { int ret; - enum ofi_lock_type ep_list_lock_type; + enum ofi_lock_type av_lock_type, ep_list_lock_type; ret = util_verify_av_attr(domain, attr); if (ret) return ret; - av->prov = domain->prov; ofi_atomic_initialize32(&av->ref, 0); - ofi_mutex_init(&av->lock); + av->av_fid.fid.fclass = FI_CLASS_AV; /* * ops set by provider @@ -535,10 +534,21 @@ int ofi_av_init_lightweight(struct util_domain *domain, const struct fi_av_attr */ av->context = context; av->domain = domain; + av->prov = domain->prov; + + av_lock_type = domain->threading == FI_THREAD_DOMAIN && + domain->control_progress == + FI_PROGRESS_CONTROL_UNIFIED ? + OFI_LOCK_NOOP : + OFI_LOCK_MUTEX; + + ret = ofi_genlock_init(&av->lock, av_lock_type); + if (ret) + return ret; + ep_list_lock_type = ofi_progress_lock_type(domain->threading, + domain->control_progress); - ep_list_lock_type = ofi_progress_lock_type(av->domain->threading, - av->domain->control_progress); ret = ofi_genlock_init(&av->ep_list_lock, ep_list_lock_type); if (ret) return ret; @@ -580,9 +590,9 @@ static int ip_av_insert_addr(struct util_av *av, const void *addr, int ret; if (ofi_valid_dest_ipaddr(addr)) { - ofi_mutex_lock(&av->lock); + ofi_genlock_lock(&av->lock); ret = ofi_av_insert_addr(av, addr, fi_addr); - ofi_mutex_unlock(&av->lock); + ofi_genlock_unlock(&av->lock); } else { ret = -FI_EADDRNOTAVAIL; if (fi_addr) @@ -881,9 +891,9 @@ int ofi_ip_av_remove(struct fid_av *av_fid, fi_addr_t *fi_addr, * Thus, we walk through the array backwards. */ for (i = count - 1; i >= 0; i--) { - ofi_mutex_lock(&av->lock); + ofi_genlock_lock(&av->lock); ret = ofi_av_remove_addr(av, fi_addr[i]); - ofi_mutex_unlock(&av->lock); + ofi_genlock_unlock(&av->lock); if (ret) { FI_WARN(av->prov, FI_LOG_AV, "removal of fi_addr %"PRIu64" failed\n",