[compiler-rt] e72ad3c - tsan: use semaphores for thread creation synchronization

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2021-08-03T13:47:01+02:00
New Revision: e72ad3c19a15a5716617f8c1d273ca506dc68276

URL: https://github.com/llvm/llvm-project/commit/e72ad3c19a15a5716617f8c1d273ca506dc68276
DIFF: https://github.com/llvm/llvm-project/commit/e72ad3c19a15a5716617f8c1d273ca506dc68276.diff

LOG: tsan: use semaphores for thread creation synchronization

We currently use ad-hoc spin waiting to synchronize thread creation
and thread start both ways. But spinning tend to degrade ungracefully
under high contention (lots of threads are created at the same time).
Use semaphores for synchronization instead.

Reviewed By: melver

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

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index e9766287ecd5..f2a8f7ada749 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -933,14 +933,15 @@ static void thread_finalize(void *v) {
 struct ThreadParam {
   void* (*callback)(void *arg);
   void *param;
-  atomic_uintptr_t tid;
+  Tid tid;
+  Semaphore created;
+  Semaphore started;
 };
 
 extern "C" void *__tsan_thread_start_func(void *arg) {
   ThreadParam *p = (ThreadParam*)arg;
   void* (*callback)(void *arg) = p->callback;
   void *param = p->param;
-  int tid = 0;
   {
     cur_thread_init();
     ThreadState *thr = cur_thread();
@@ -955,12 +956,11 @@ extern "C" void *__tsan_thread_start_func(void *arg) {
     }
     ThreadIgnoreEnd(thr);
 #endif
-    while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
-      internal_sched_yield();
+    p->created.Wait();
     Processor *proc = ProcCreate();
     ProcWire(proc, thr);
-    ThreadStart(thr, tid, GetTid(), ThreadType::Regular);
-    atomic_store(&p->tid, 0, memory_order_release);
+    ThreadStart(thr, p->tid, GetTid(), ThreadType::Regular);
+    p->started.Post();
   }
   void *res = callback(param);
   // Prevent the callback from being tail called,
@@ -999,7 +999,7 @@ TSAN_INTERCEPTOR(int, pthread_create,
   ThreadParam p;
   p.callback = callback;
   p.param = param;
-  atomic_store(&p.tid, 0, memory_order_relaxed);
+  p.tid = kMainTid;
   int res = -1;
   {
     // Otherwise we see false positives in pthread stack manipulation.
@@ -1009,8 +1009,8 @@ TSAN_INTERCEPTOR(int, pthread_create,
     ThreadIgnoreEnd(thr);
   }
   if (res == 0) {
-    Tid tid = ThreadCreate(thr, pc, *(uptr *)th, IsStateDetached(detached));
-    CHECK_NE(tid, kMainTid);
+    p.tid = ThreadCreate(thr, pc, *(uptr *)th, IsStateDetached(detached));
+    CHECK_NE(p.tid, kMainTid);
     // Synchronization on p.tid serves two purposes:
     // 1. ThreadCreate must finish before the new thread starts.
     //    Otherwise the new thread can call pthread_detach, but the pthread_t
@@ -1018,9 +1018,8 @@ TSAN_INTERCEPTOR(int, pthread_create,
     // 2. ThreadStart must finish before this thread continues.
     //    Otherwise, this thread can call pthread_detach and reset thr->sync
     //    before the new thread got a chance to acquire from it in ThreadStart.
-    atomic_store(&p.tid, tid, memory_order_release);
-    while (atomic_load(&p.tid, memory_order_acquire) != 0)
-      internal_sched_yield();
+    p.created.Post();
+    p.started.Wait();
   }
   if (attr == &myattr)
     pthread_attr_destroy(&myattr);