Skip to content

Commit 81c2e23

Browse files
author
Tomislav Janjusic
committed
oshmem:ucx, fix race condition and add context recycling
1) Race condition: Do not add private contexts to active list. Private contexts are only visible to the user. 2) Recycled contexts: Destroyed contexts are put on an idle list until finalize, continuous context creation will lead to oom condition. Instead, check if context from idle list meets new context requirements and reuse it. Co-authored with: Artem Y. Polyakov <[email protected]>, Manjunath Gorentla Venkata <[email protected]> Signed-off-by: Tomislav Janjusic <[email protected]>
1 parent 119b1c3 commit 81c2e23

File tree

1 file changed

+28
-11
lines changed

1 file changed

+28
-11
lines changed

oshmem/mca/spml/ucx/spml_ucx.c

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -573,11 +573,9 @@ static inline void _ctx_add(mca_spml_ucx_ctx_array_t *array, mca_spml_ucx_ctx_t
573573
array->ctxs_count++;
574574
}
575575

576-
static inline void _ctx_remove(mca_spml_ucx_ctx_array_t *array, mca_spml_ucx_ctx_t *ctx)
576+
static inline void _ctx_remove(mca_spml_ucx_ctx_array_t *array, mca_spml_ucx_ctx_t *ctx, int i)
577577
{
578-
int i;
579-
580-
for (i = 0; i < array->ctxs_count; i++) {
578+
for (; i < array->ctxs_count; i++) {
581579
if (array->ctxs[i] == ctx) {
582580
array->ctxs[i] = array->ctxs[array->ctxs_count-1];
583581
array->ctxs[array->ctxs_count-1] = NULL;
@@ -681,14 +679,29 @@ static int mca_spml_ucx_ctx_create_common(long options, mca_spml_ucx_ctx_t **ucx
681679

682680
int mca_spml_ucx_ctx_create(long options, shmem_ctx_t *ctx)
683681
{
684-
mca_spml_ucx_ctx_t *ucx_ctx;
685-
int rc;
682+
mca_spml_ucx_ctx_t *ucx_ctx = NULL;
683+
mca_spml_ucx_ctx_array_t *idle_array = &mca_spml_ucx.idle_array;
684+
mca_spml_ucx_ctx_array_t *active_array = &mca_spml_ucx.active_array;
685+
int i, rc;
686686

687687
/* Take a lock controlling context creation. AUX context may set specific
688688
* UCX parameters affecting worker creation, which are not needed for
689689
* regular contexts. */
690690
pthread_mutex_lock(&mca_spml_ucx.ctx_create_mutex);
691-
rc = mca_spml_ucx_ctx_create_common(options, &ucx_ctx);
691+
692+
/* Check if we have an idle context to reuse */
693+
for (i = 0; i < idle_array->ctxs_count; i++) {
694+
if (idle_array->ctxs[i]->options & options) {
695+
ucx_ctx = idle_array->ctxs[i];
696+
_ctx_remove(idle_array, ucx_ctx, i);
697+
}
698+
}
699+
700+
/* If we cannot reuse, create new ctx */
701+
if (ucx_ctx == NULL) {
702+
rc = mca_spml_ucx_ctx_create_common(options, &ucx_ctx);
703+
}
704+
692705
pthread_mutex_unlock(&mca_spml_ucx.ctx_create_mutex);
693706
if (rc != OSHMEM_SUCCESS) {
694707
return rc;
@@ -698,9 +711,11 @@ int mca_spml_ucx_ctx_create(long options, shmem_ctx_t *ctx)
698711
opal_progress_register(spml_ucx_ctx_progress);
699712
}
700713

701-
SHMEM_MUTEX_LOCK(mca_spml_ucx.internal_mutex);
702-
_ctx_add(&mca_spml_ucx.active_array, ucx_ctx);
703-
SHMEM_MUTEX_UNLOCK(mca_spml_ucx.internal_mutex);
714+
if (!(options & SHMEM_CTX_PRIVATE)) {
715+
SHMEM_MUTEX_LOCK(mca_spml_ucx.internal_mutex);
716+
_ctx_add(&mca_spml_ucx.active_array, ucx_ctx);
717+
SHMEM_MUTEX_UNLOCK(mca_spml_ucx.internal_mutex);
718+
}
704719

705720
(*ctx) = (shmem_ctx_t)ucx_ctx;
706721
return OSHMEM_SUCCESS;
@@ -711,7 +726,9 @@ void mca_spml_ucx_ctx_destroy(shmem_ctx_t ctx)
711726
MCA_SPML_CALL(quiet(ctx));
712727

713728
SHMEM_MUTEX_LOCK(mca_spml_ucx.internal_mutex);
714-
_ctx_remove(&mca_spml_ucx.active_array, (mca_spml_ucx_ctx_t *)ctx);
729+
if (!(((mca_spml_ucx_ctx_t *)ctx)->options & SHMEM_CTX_PRIVATE)) {
730+
_ctx_remove(&mca_spml_ucx.active_array, (mca_spml_ucx_ctx_t *)ctx, 0);
731+
}
715732
_ctx_add(&mca_spml_ucx.idle_array, (mca_spml_ucx_ctx_t *)ctx);
716733
SHMEM_MUTEX_UNLOCK(mca_spml_ucx.internal_mutex);
717734

0 commit comments

Comments
 (0)