[Openmp-commits] [openmp] r360919 - Fixed second issue reported in https://bugs.llvm.org/show_bug.cgi?id=41584.

[Andrey Churbanov via Openmp-commits]

Author: achurbanov
Date: Thu May 16 10:52:53 2019
New Revision: 360919

URL: http://llvm.org/viewvc/llvm-project?rev=360919&view=rev
Log:
Fixed second issue reported in https://bugs.llvm.org/show_bug.cgi?id=41584.

Added synchronization for possible concurrent initialization of mutexes
by multiple threads. The need of synchronization caused by commit r357927
which added the use of mutexes at threads movement to/from common pool
(earlier the mutexes were used only at suspend/resume).

Patch by Johnny Peyton.

Differential Revision: https://reviews.llvm.org/D61995

Modified:
    openmp/trunk/runtime/src/kmp.h
    openmp/trunk/runtime/src/z_Linux_util.cpp
    openmp/trunk/runtime/src/z_Windows_NT_util.cpp

Modified: openmp/trunk/runtime/src/kmp.h
URL: http://llvm.org/viewvc/llvm-project/openmp/trunk/runtime/src/kmp.h?rev=360919&r1=360918&r2=360919&view=diff
==============================================================================
--- openmp/trunk/runtime/src/kmp.h (original)
+++ openmp/trunk/runtime/src/kmp.h Thu May 16 10:52:53 2019
@@ -2616,12 +2616,12 @@ typedef struct KMP_ALIGN_CACHE kmp_base_
 #if KMP_OS_WINDOWS
   kmp_win32_cond_t th_suspend_cv;
   kmp_win32_mutex_t th_suspend_mx;
-  int th_suspend_init;
+  std::atomic<int> th_suspend_init;
 #endif
 #if KMP_OS_UNIX
   kmp_cond_align_t th_suspend_cv;
   kmp_mutex_align_t th_suspend_mx;
-  int th_suspend_init_count;
+  std::atomic<int> th_suspend_init_count;
 #endif
 
 #if USE_ITT_BUILD

Modified: openmp/trunk/runtime/src/z_Linux_util.cpp
URL: http://llvm.org/viewvc/llvm-project/openmp/trunk/runtime/src/z_Linux_util.cpp?rev=360919&r1=360918&r2=360919&view=diff
==============================================================================
--- openmp/trunk/runtime/src/z_Linux_util.cpp (original)
+++ openmp/trunk/runtime/src/z_Linux_util.cpp Thu May 16 10:52:53 2019
@@ -1354,9 +1354,20 @@ void __kmp_suspend_initialize(void) {
 
 void __kmp_suspend_initialize_thread(kmp_info_t *th) {
   ANNOTATE_HAPPENS_AFTER(&th->th.th_suspend_init_count);
-  if (th->th.th_suspend_init_count <= __kmp_fork_count) {
-    /* this means we haven't initialized the suspension pthread objects for this
-       thread in this instance of the process */
+  int old_value = KMP_ATOMIC_LD_RLX(&th->th.th_suspend_init_count);
+  int new_value = __kmp_fork_count + 1;
+  // Return if already initialized
+  if (old_value == new_value)
+    return;
+  // Wait, then return if being initialized
+  if (old_value == -1 ||
+      !__kmp_atomic_compare_store(&th->th.th_suspend_init_count, old_value,
+                                  -1)) {
+    while (KMP_ATOMIC_LD_ACQ(&th->th.th_suspend_init_count) != new_value) {
+      KMP_CPU_PAUSE();
+    }
+  } else {
+    // Claim to be the initializer and do initializations
     int status;
     status = pthread_cond_init(&th->th.th_suspend_cv.c_cond,
                                &__kmp_suspend_cond_attr);
@@ -1364,13 +1375,13 @@ void __kmp_suspend_initialize_thread(kmp
     status = pthread_mutex_init(&th->th.th_suspend_mx.m_mutex,
                                 &__kmp_suspend_mutex_attr);
     KMP_CHECK_SYSFAIL("pthread_mutex_init", status);
-    *(volatile int *)&th->th.th_suspend_init_count = __kmp_fork_count + 1;
+    KMP_ATOMIC_ST_REL(&th->th.th_suspend_init_count, new_value);
     ANNOTATE_HAPPENS_BEFORE(&th->th.th_suspend_init_count);
   }
 }
 
 void __kmp_suspend_uninitialize_thread(kmp_info_t *th) {
-  if (th->th.th_suspend_init_count > __kmp_fork_count) {
+  if (KMP_ATOMIC_LD_ACQ(&th->th.th_suspend_init_count) > __kmp_fork_count) {
     /* this means we have initialize the suspension pthread objects for this
        thread in this instance of the process */
     int status;
@@ -1384,7 +1395,8 @@ void __kmp_suspend_uninitialize_thread(k
       KMP_SYSFAIL("pthread_mutex_destroy", status);
     }
     --th->th.th_suspend_init_count;
-    KMP_DEBUG_ASSERT(th->th.th_suspend_init_count == __kmp_fork_count);
+    KMP_DEBUG_ASSERT(KMP_ATOMIC_LD_RLX(&th->th.th_suspend_init_count) ==
+                     __kmp_fork_count);
   }
 }
 

Modified: openmp/trunk/runtime/src/z_Windows_NT_util.cpp
URL: http://llvm.org/viewvc/llvm-project/openmp/trunk/runtime/src/z_Windows_NT_util.cpp?rev=360919&r1=360918&r2=360919&view=diff
==============================================================================
--- openmp/trunk/runtime/src/z_Windows_NT_util.cpp (original)
+++ openmp/trunk/runtime/src/z_Windows_NT_util.cpp Thu May 16 10:52:53 2019
@@ -310,22 +310,32 @@ void __kmp_suspend_initialize(void) { /*
 }
 
 void __kmp_suspend_initialize_thread(kmp_info_t *th) {
-  if (!TCR_4(th->th.th_suspend_init)) {
-    /* this means we haven't initialized the suspension pthread objects for this
-       thread in this instance of the process */
+  int old_value = KMP_ATOMIC_LD_RLX(&th->th.th_suspend_init);
+  int new_value = TRUE;
+  // Return if already initialized
+  if (old_value == new_value)
+    return;
+  // Wait, then return if being initialized
+  if (old_value == -1 ||
+      !__kmp_atomic_compare_store(&th->th.th_suspend_init, old_value, -1)) {
+    while (KMP_ATOMIC_LD_ACQ(&th->th.th_suspend_init) != new_value) {
+      KMP_CPU_PAUSE();
+    }
+  } else {
+    // Claim to be the initializer and do initializations
     __kmp_win32_cond_init(&th->th.th_suspend_cv);
     __kmp_win32_mutex_init(&th->th.th_suspend_mx);
-    TCW_4(th->th.th_suspend_init, TRUE);
+    KMP_ATOMIC_ST_REL(&th->th.th_suspend_init, new_value);
   }
 }
 
 void __kmp_suspend_uninitialize_thread(kmp_info_t *th) {
-  if (TCR_4(th->th.th_suspend_init)) {
+  if (KMP_ATOMIC_LD_ACQ(&th->th.th_suspend_init)) {
     /* this means we have initialize the suspension pthread objects for this
        thread in this instance of the process */
     __kmp_win32_cond_destroy(&th->th.th_suspend_cv);
     __kmp_win32_mutex_destroy(&th->th.th_suspend_mx);
-    TCW_4(th->th.th_suspend_init, FALSE);
+    KMP_ATOMIC_ST_REL(&th->th.th_suspend_init, FALSE);
   }
 }