prov/util: Make ep_list_lock noop for FI_PROGRESS_CONTROL_UNIFIED

This patch makes ep_list_lock a noop when user has set FI_PROGRESS_CONTROL_UNIFIED and the threading model is FI_THREAD_DOMAIN or FI_THREAD_COMPLETION. The util_cntr and util_cq ep_list_locks are used on the critical path for data transfer, so this change should provide a performance improvement. The util_av ep_list_lock should not be used on the performance critical path, but it is changed for simiplicty/consistency for the reader. Signed-off-by: Seth Zegelstein <szegel@amazon.com>
ofiwg · Mar 5, 2024 · f63359d · f63359d
1 parent 72ac584
commit f63359d
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 8 deletions.
diff --git a/include/ofi_util.h b/include/ofi_util.h
@@ -1370,6 +1370,13 @@ static inline void ofi_cq_err_memcpy(uint32_t api_version,
 	}
 }
 
+static inline enum ofi_lock_type
+ofi_progress_lock_type(enum fi_threading threading, enum fi_progress control)
+{
+	return (threading == FI_THREAD_DOMAIN || threading == FI_THREAD_COMPLETION) &&
+		control == FI_PROGRESS_CONTROL_UNIFIED ? OFI_LOCK_NOOP : OFI_LOCK_MUTEX;
+}
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/prov/util/src/util_av.c b/prov/util/src/util_av.c
@@ -536,6 +536,7 @@ 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;
 
 	ret = util_verify_av_attr(domain, attr);
 	if (ret)
@@ -553,7 +554,10 @@ int ofi_av_init_lightweight(struct util_domain *domain, const struct fi_av_attr
 	av->context = context;
 	av->domain = domain;
 
-	ret = ofi_genlock_init(&av->ep_list_lock, OFI_LOCK_MUTEX);
+
+	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;
 

diff --git a/prov/util/src/util_cntr.c b/prov/util/src/util_cntr.c
@@ -336,6 +336,7 @@ int ofi_cntr_init(const struct fi_provider *prov, struct fid_domain *domain,
 	int ret;
 	struct fi_wait_attr wait_attr;
 	struct fid_wait *wait;
+	enum ofi_lock_type ep_list_lock_type;
 
 	assert(progress);
 	ret = ofi_check_cntr_attr(prov, attr);
@@ -390,7 +391,9 @@ int ofi_cntr_init(const struct fi_provider *prov, struct fid_domain *domain,
 			goto errout_close_wait;
 	}
 
-	ret = ofi_genlock_init(&cntr->ep_list_lock, OFI_LOCK_MUTEX);
+	ep_list_lock_type = ofi_progress_lock_type(cntr->domain->threading,
+						   cntr->domain->control_progress);
+	ret = ofi_genlock_init(&cntr->ep_list_lock, ep_list_lock_type);
 	if (ret)
 		goto errout_close_wait;
 

diff --git a/prov/util/src/util_cq.c b/prov/util/src/util_cq.c
@@ -708,7 +708,8 @@ int ofi_cq_init(const struct fi_provider *prov, struct fid_domain *domain,
 {
 	struct fi_wait_attr wait_attr;
 	struct fid_wait *wait;
-	enum ofi_lock_type lock_type;
+	enum ofi_lock_type cq_lock_type;
+	enum ofi_lock_type ep_list_lock_type;
 	int ret;
 
 	assert(progress);
@@ -728,16 +729,17 @@ int ofi_cq_init(const struct fi_provider *prov, struct fid_domain *domain,
 
 	if (cq->domain->threading == FI_THREAD_COMPLETION ||
 	    cq->domain->threading == FI_THREAD_DOMAIN)
-		lock_type = OFI_LOCK_NOOP;
+		cq_lock_type = OFI_LOCK_NOOP;
 	else
-		lock_type = cq->domain->lock.lock_type;
+		cq_lock_type = cq->domain->lock.lock_type;
 
-	ret = ofi_genlock_init(&cq->cq_lock, lock_type);
+	ret = ofi_genlock_init(&cq->cq_lock, cq_lock_type);
 	if (ret)
 		return ret;
 
-	/* TODO Figure out how to optimize this lock for rdm and msg endpoints */
-	ret = ofi_genlock_init(&cq->ep_list_lock, OFI_LOCK_MUTEX);
+	ep_list_lock_type = ofi_progress_lock_type(cq->domain->threading,
+						   cq->domain->control_progress);
+	ret = ofi_genlock_init(&cq->ep_list_lock, ep_list_lock_type);
 	if (ret)
 		goto destroy1;