[Openmp-commits] [PATCH] D62251: Fix for third case reported in https://bugs.llvm.org/show_bug.cgi?id=41584

[Andrey Churbanov via Phabricator via Openmp-commits]

AndreyChurbanov created this revision.
AndreyChurbanov added reviewers: tlwilmar, hbae, jlpeyton.
AndreyChurbanov added a project: OpenMP.
Herald added subscribers: openmp-commits, jdoerfert.

The code fragments of recruiting a thread from pool (kmp_runtime.cpp:4297):

  new_thr->th.th_in_pool = FALSE;
  __kmp_lock_suspend_mx(new_thr);
  if (new_thr->th.th_active_in_pool == TRUE) {
    ...
  else
    KMP_DEBUG_ASSERT(new_thr->th.th_active == FALSE);

and its awakening, executed under the mutex (z_Linux_util.cpp:1534):

  th->th.th_active = TRUE;
  if (th->th.th_in_pool) {
    th->th.th_active_in_pool = TRUE;
  }

can be executed in parallel.  Since the "th_in_pool=0" assignment is out of critical section, it is valid to have "th_active==1" and "th_active_in_pool==0" for the worker thread being recruited from common pool (e.g. in case of spurious wakeup).  So the assertion triggered looks wrong.  The patch deletes it.

I actually failed to reproduce the assertion on my server with RHEL7 (probably because it is hard to force spurious wakeup of a thread, and without it I failed to find code path of simultaneous execution of code fragments I referenced), but the patch looks logically correct.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D62251

Files:
  runtime/src/kmp_runtime.cpp


Index: runtime/src/kmp_runtime.cpp
===================================================================
--- runtime/src/kmp_runtime.cpp
+++ runtime/src/kmp_runtime.cpp
@@ -4302,11 +4302,6 @@
       KMP_ATOMIC_DEC(&__kmp_thread_pool_active_nth);
       new_thr->th.th_active_in_pool = FALSE;
     }
-#if KMP_DEBUG
-    else {
-      KMP_DEBUG_ASSERT(new_thr->th.th_active == FALSE);
-    }
-#endif
     __kmp_unlock_suspend_mx(new_thr);
 
     KA_TRACE(20, ("__kmp_allocate_thread: T#%d using thread T#%d\n",


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62251.200710.patch
Type: text/x-patch
Size: 507 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20190522/0c4899c2/attachment.bin>