From b7b498657685d7a305987b9140253523e77fd4e1 Mon Sep 17 00:00:00 2001 From: Jonathan Peyton Date: Thu, 5 May 2022 09:15:41 -0500 Subject: [PATCH] [OpenMP][libomp] Hold old __kmp_threads arrays until library shutdown When many nested teams are formed, __kmp_threads may be reallocated to accommodate new threads. This reallocation causes a data race when another existing team's thread simultaneously references __kmp_threads. This patch keeps the old thread arrays around until library shutdown so these lingering references can complete without issue and access to __kmp_threads remains a simple array reference. Fixes: https://github.com/llvm/llvm-project/issues/54708 Differential Revision: https://reviews.llvm.org/D125013 --- openmp/runtime/src/kmp.h | 11 +++++++++++ openmp/runtime/src/kmp_global.cpp | 1 + openmp/runtime/src/kmp_runtime.cpp | 18 ++++++++++++++++-- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h index 57798e2edc67..c90931d8a996 100644 --- a/openmp/runtime/src/kmp.h +++ b/openmp/runtime/src/kmp.h @@ -2989,6 +2989,15 @@ struct fortran_inx_info { kmp_int32 data; }; +// This list type exists to hold old __kmp_threads arrays so that +// old references to them may complete while reallocation takes place when +// expanding the array. The items in this list are kept alive until library +// shutdown. +typedef struct kmp_old_threads_list_t { + kmp_info_t **threads; + struct kmp_old_threads_list_t *next; +} kmp_old_threads_list_t; + /* ------------------------------------------------------------------------ */ extern int __kmp_settings; @@ -3270,6 +3279,8 @@ extern int __kmp_teams_thread_limit; /* the following are protected by the fork/join lock */ /* write: lock read: anytime */ extern kmp_info_t **__kmp_threads; /* Descriptors for the threads */ +/* Holds old arrays of __kmp_threads until library shutdown */ +extern kmp_old_threads_list_t *__kmp_old_threads_list; /* read/write: lock */ extern volatile kmp_team_t *__kmp_team_pool; extern volatile kmp_info_t *__kmp_thread_pool; diff --git a/openmp/runtime/src/kmp_global.cpp b/openmp/runtime/src/kmp_global.cpp index a82c3da8ca6b..525873646125 100644 --- a/openmp/runtime/src/kmp_global.cpp +++ b/openmp/runtime/src/kmp_global.cpp @@ -442,6 +442,7 @@ kmp_uint64 __kmp_pause_init = 1; // for tpause KMP_ALIGN_CACHE kmp_info_t **__kmp_threads = NULL; kmp_root_t **__kmp_root = NULL; +kmp_old_threads_list_t *__kmp_old_threads_list = NULL; /* data read/written to often by primary threads */ KMP_ALIGN_CACHE diff --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp index b509a5d7e663..a44b4d88bc0b 100644 --- a/openmp/runtime/src/kmp_runtime.cpp +++ b/openmp/runtime/src/kmp_runtime.cpp @@ -3669,11 +3669,16 @@ static int __kmp_expand_threads(int nNeed) { __kmp_threads_capacity * sizeof(kmp_info_t *)); KMP_MEMCPY(newRoot, __kmp_root, __kmp_threads_capacity * sizeof(kmp_root_t *)); + // Put old __kmp_threads array on a list. Any ongoing references to the old + // list will be valid. This list is cleaned up at library shutdown. + kmp_old_threads_list_t *node = + (kmp_old_threads_list_t *)__kmp_allocate(sizeof(kmp_old_threads_list_t)); + node->threads = __kmp_threads; + node->next = __kmp_old_threads_list; + __kmp_old_threads_list = node; - kmp_info_t **temp_threads = __kmp_threads; *(kmp_info_t * *volatile *)&__kmp_threads = newThreads; *(kmp_root_t * *volatile *)&__kmp_root = newRoot; - __kmp_free(temp_threads); added += newCapacity - __kmp_threads_capacity; *(volatile int *)&__kmp_threads_capacity = newCapacity; @@ -8101,6 +8106,15 @@ void __kmp_cleanup(void) { __kmp_root = NULL; __kmp_threads_capacity = 0; + // Free old __kmp_threads arrays if they exist. + kmp_old_threads_list_t *ptr = __kmp_old_threads_list; + while (ptr) { + kmp_old_threads_list_t *next = ptr->next; + __kmp_free(ptr->threads); + __kmp_free(ptr); + ptr = next; + } + #if KMP_USE_DYNAMIC_LOCK __kmp_cleanup_indirect_user_locks(); #else