[libc-commits] [libc] 004c7b1 - [libc][NFC] Move the mutex implementation into a utility class.

Siva Chandra Reddy via libc-commits libc-commits at lists.llvm.org
Thu Aug 26 11:49:27 PDT 2021


Author: Siva Chandra Reddy
Date: 2021-08-26T18:49:20Z
New Revision: 004c7b1da6cfea1d1f09535607a8c7c2f051e8d5

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

LOG: [libc][NFC] Move the mutex implementation into a utility class.

This allows others parts of the libc to use the mutex types without
actually pulling in public function implementations.

Along the way, few cleanups have been done, like using a uniform type to
refer the linux futex word.

Reviewed By: michaelrj

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

Added: 
    libc/src/threads/linux/Futex.h
    libc/src/threads/linux/Mutex.h
    libc/src/threads/linux/Thread.h

Modified: 
    libc/src/threads/linux/CMakeLists.txt
    libc/src/threads/linux/call_once.cpp
    libc/src/threads/linux/mtx_init.cpp
    libc/src/threads/linux/mtx_lock.cpp
    libc/src/threads/linux/mtx_unlock.cpp
    libc/src/threads/linux/thrd_create.cpp
    libc/src/threads/linux/thrd_join.cpp

Removed: 
    libc/src/threads/linux/thread_utils.h


################################################################################
diff  --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index 767bf34dc36b3..6e8b212612ef7 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/libc/src/threads/linux/CMakeLists.txt
@@ -24,9 +24,14 @@ add_entrypoint_object(
 add_header_library(
   threads_utils
   HDRS
-    thread_utils.h
+    Futex.h
+    Mutex.h
+    Thread.h
   DEPENDS
     .thread_start_args_h
+    libc.config.linux.linux_syscall_h
+    libc.include.sys_syscall
+    libc.include.threads
 )
 
 add_entrypoint_object(
@@ -83,8 +88,6 @@ add_entrypoint_object(
     ../mtx_lock.h
   DEPENDS
     .threads_utils
-    libc.config.linux.linux_syscall_h
-    libc.include.sys_syscall
     libc.include.threads
 )
 
@@ -96,7 +99,5 @@ add_entrypoint_object(
     ../mtx_unlock.h
   DEPENDS
     .threads_utils
-    libc.config.linux.linux_syscall_h
-    libc.include.sys_syscall
     libc.include.threads
 )

diff  --git a/libc/src/threads/linux/thread_utils.h b/libc/src/threads/linux/Futex.h
similarity index 55%
rename from libc/src/threads/linux/thread_utils.h
rename to libc/src/threads/linux/Futex.h
index 202075824068c..e80816f5f5239 100644
--- a/libc/src/threads/linux/thread_utils.h
+++ b/libc/src/threads/linux/Futex.h
@@ -1,4 +1,4 @@
-//===-- Linux specific definitions for threads implementations. --*- C++ -*===//
+//===-- Linux futex related definitions -------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,13 +6,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
-#define LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
-
-#include "thread_start_args.h"
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_FUTEX_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_FUTEX_H
 
 #include <stdatomic.h>
-#include <stdint.h>
 
 namespace __llvm_libc {
 
@@ -20,23 +17,12 @@ namespace __llvm_libc {
 // here as we do not want to use `_Atomic uint32_t` as the _Atomic keyword which
 // is C only. The header stdatomic.h does not define an atomic type
 // corresponding to `uint32_t` or to something which is exactly 4 bytes wide.
-using FutexData = atomic_uint;
-
-// We use a tri-state mutex because we want to avoid making syscalls
-// as much as possible. In `mtx_unlock` a syscall to wake waiting threads is
-// made only if the mutex status is `MutexStatus::Waiting`.
-enum MutexStatus : uint32_t { MS_Free, MS_Locked, MS_Waiting };
-
+using FutexWord = atomic_uint;
 static_assert(sizeof(atomic_uint) == 4,
               "Size of the `atomic_uint` type is not 4 bytes on your platform. "
               "The implementation of the standard threads library for linux "
               "requires that size of `atomic_uint` be 4 bytes.");
 
-struct ThreadParams {
-  static constexpr uintptr_t DefaultStackSize = 1 << 16; // 64 KB
-  static constexpr uint32_t ClearTIDValue = 0xABCD1234;
-};
-
 } // namespace __llvm_libc
 
-#endif // LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_FUTEX_H

diff  --git a/libc/src/threads/linux/Mutex.h b/libc/src/threads/linux/Mutex.h
new file mode 100644
index 0000000000000..454b8ced3054b
--- /dev/null
+++ b/libc/src/threads/linux/Mutex.h
@@ -0,0 +1,121 @@
+//===-- Utility mutex classes -----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_MUTEX_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_MUTEX_H
+
+#include "Futex.h"
+
+#include "config/linux/syscall.h" // For syscall functions.
+#include "include/sys/syscall.h"  // For syscall numbers.
+#include "include/threads.h"      // For values like thrd_success etc.
+
+#include <linux/futex.h> // For futex operations.
+#include <stdatomic.h>
+#include <stdint.h>
+
+namespace __llvm_libc {
+
+struct Mutex {
+  enum Status : uint32_t {
+    MS_Free,
+    MS_Locked,
+    MS_Waiting,
+  };
+
+  FutexWord futex_word;
+  int type;
+
+  static int init(Mutex *mutex, int type) {
+    mutex->futex_word = MS_Free;
+    mutex->type = type;
+    return thrd_success;
+  }
+
+  int lock() {
+    bool was_waiting = false;
+    while (true) {
+      uint32_t mutex_status = MS_Free;
+      uint32_t locked_status = MS_Locked;
+
+      if (atomic_compare_exchange_strong(&futex_word, &mutex_status,
+                                         MS_Locked)) {
+        if (was_waiting)
+          atomic_store(&futex_word, MS_Waiting);
+        return thrd_success;
+      }
+
+      switch (mutex_status) {
+      case MS_Waiting:
+        // If other threads are waiting already, then join them. Note that the
+        // futex syscall will block if the futex data is still `MS_Waiting` (the
+        // 4th argument to the syscall function below.)
+        __llvm_libc::syscall(SYS_futex, &futex_word, FUTEX_WAIT_PRIVATE,
+                             MS_Waiting, 0, 0, 0);
+        was_waiting = true;
+        // Once woken up/unblocked, try everything all over.
+        continue;
+      case MS_Locked:
+        // Mutex has been locked by another thread so set the status to
+        // MS_Waiting.
+        if (atomic_compare_exchange_strong(&futex_word, &locked_status,
+                                           MS_Waiting)) {
+          // If we are able to set the futex data to `MS_Waiting`, then we will
+          // wait for the futex to be woken up. Note again that the following
+          // syscall will block only if the futex data is still `MS_Waiting`.
+          __llvm_libc::syscall(SYS_futex, &futex_word, FUTEX_WAIT_PRIVATE,
+                               MS_Waiting, 0, 0, 0);
+          was_waiting = true;
+        }
+        continue;
+      case MS_Free:
+        // If it was MS_Free, we shouldn't be here at all.
+        [[clang::fallthrough]];
+      default:
+        // Mutex status cannot be anything else. So control should not reach
+        // here at all.
+        return thrd_error;
+      }
+    }
+  }
+
+  int unlock() {
+    while (true) {
+      uint32_t mutex_status = MS_Waiting;
+      if (atomic_compare_exchange_strong(&futex_word, &mutex_status, MS_Free)) {
+        // If any thread is waiting to be woken up, then do it.
+        __llvm_libc::syscall(SYS_futex, &futex_word, FUTEX_WAKE_PRIVATE, 1, 0,
+                             0, 0);
+        return thrd_success;
+      }
+
+      if (mutex_status == MS_Locked) {
+        // If nobody was waiting at this point, just free it.
+        if (atomic_compare_exchange_strong(&futex_word, &mutex_status, MS_Free))
+          return thrd_success;
+      } else {
+        // This can happen, for example if some thread tries to unlock an
+        // already free mutex.
+        return thrd_error;
+      }
+    }
+  }
+};
+
+class MutexLock {
+  Mutex *mutex;
+
+public:
+  explicit MutexLock(Mutex *m) : mutex(m) { mutex->lock(); }
+
+  ~MutexLock() { mutex->unlock(); }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_MUTEX_H

diff  --git a/libc/src/threads/linux/Thread.h b/libc/src/threads/linux/Thread.h
new file mode 100644
index 0000000000000..5adbbfea5f572
--- /dev/null
+++ b/libc/src/threads/linux/Thread.h
@@ -0,0 +1,26 @@
+//===-- Linux specific definitions for threads implementations. --*- C++ -*===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
+
+#include "thread_start_args.h"
+
+#include <stdatomic.h>
+#include <stdint.h>
+
+namespace __llvm_libc {
+
+struct ThreadParams {
+  static constexpr uintptr_t DefaultStackSize = 1 << 16; // 64 KB
+  static constexpr uint32_t ClearTIDValue = 0xABCD1234;
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H

diff  --git a/libc/src/threads/linux/call_once.cpp b/libc/src/threads/linux/call_once.cpp
index ac8aca4cc8d8e..1bcb46f6b0a3b 100644
--- a/libc/src/threads/linux/call_once.cpp
+++ b/libc/src/threads/linux/call_once.cpp
@@ -11,7 +11,7 @@
 #include "include/sys/syscall.h"  // For syscall numbers.
 #include "include/threads.h"      // For call_once related type definition.
 #include "src/__support/common.h"
-#include "src/threads/linux/thread_utils.h"
+#include "src/threads/linux/Futex.h"
 
 #include <limits.h>
 #include <linux/futex.h>
@@ -25,7 +25,7 @@ static constexpr unsigned FINISH = 0x33;
 
 LLVM_LIBC_FUNCTION(void, call_once,
                    (once_flag * flag, __call_once_func_t func)) {
-  FutexData *futex_word = reinterpret_cast<FutexData *>(flag);
+  FutexWord *futex_word = reinterpret_cast<FutexWord *>(flag);
   unsigned int not_called = ONCE_FLAG_INIT;
 
   // The C standard wording says:

diff  --git a/libc/src/threads/linux/mtx_init.cpp b/libc/src/threads/linux/mtx_init.cpp
index 1a77dcb24fe96..7ff606d7d2212 100644
--- a/libc/src/threads/linux/mtx_init.cpp
+++ b/libc/src/threads/linux/mtx_init.cpp
@@ -9,14 +9,13 @@
 #include "src/threads/mtx_init.h"
 #include "include/threads.h" // For mtx_t definition.
 #include "src/__support/common.h"
-#include "src/threads/linux/thread_utils.h"
+#include "src/threads/linux/Mutex.h"
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, mtx_init, (mtx_t * mutex, int type)) {
-  *(reinterpret_cast<uint32_t *>(mutex->__internal_data)) = MS_Free;
-  mutex->__mtx_type = type;
-  return thrd_success;
+  auto *m = reinterpret_cast<Mutex *>(mutex);
+  return Mutex::init(m, type);
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/threads/linux/mtx_lock.cpp b/libc/src/threads/linux/mtx_lock.cpp
index 39bd329e9e9a9..f520601a69732 100644
--- a/libc/src/threads/linux/mtx_lock.cpp
+++ b/libc/src/threads/linux/mtx_lock.cpp
@@ -7,63 +7,16 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/threads/mtx_lock.h"
-#include "config/linux/syscall.h" // For syscall functions.
-#include "include/sys/syscall.h"  // For syscall numbers.
 #include "include/threads.h"      // For mtx_t definition.
 #include "src/__support/common.h"
-#include "src/threads/linux/thread_utils.h"
-
-#include <linux/futex.h> // For futex operations.
-#include <stdatomic.h>   // For atomic_compare_exchange_strong.
+#include "src/threads/linux/Mutex.h"
 
 namespace __llvm_libc {
 
 // The implementation currently handles only plain mutexes.
 LLVM_LIBC_FUNCTION(int, mtx_lock, (mtx_t * mutex)) {
-  FutexData *futex_data = reinterpret_cast<FutexData *>(mutex->__internal_data);
-  bool was_waiting = false;
-  while (true) {
-    uint32_t mutex_status = MS_Free;
-    uint32_t locked_status = MS_Locked;
-
-    if (atomic_compare_exchange_strong(futex_data, &mutex_status, MS_Locked)) {
-      if (was_waiting)
-        atomic_store(futex_data, MS_Waiting);
-      return thrd_success;
-    }
-
-    switch (mutex_status) {
-    case MS_Waiting:
-      // If other threads are waiting already, then join them. Note that the
-      // futex syscall will block if the futex data is still `MS_Waiting` (the
-      // 4th argument to the syscall function below.)
-      __llvm_libc::syscall(SYS_futex, futex_data, FUTEX_WAIT_PRIVATE,
-                           MS_Waiting, 0, 0, 0);
-      was_waiting = true;
-      // Once woken up/unblocked, try everything all over.
-      continue;
-    case MS_Locked:
-      // Mutex has been locked by another thread so set the status to
-      // MS_Waiting.
-      if (atomic_compare_exchange_strong(futex_data, &locked_status,
-                                         MS_Waiting)) {
-        // If we are able to set the futex data to `MS_Waiting`, then we will
-        // wait for the futex to be woken up. Note again that the following
-        // syscall will block only if the futex data is still `MS_Waiting`.
-        __llvm_libc::syscall(SYS_futex, futex_data, FUTEX_WAIT_PRIVATE,
-                             MS_Waiting, 0, 0, 0);
-        was_waiting = true;
-      }
-      continue;
-    case MS_Free:
-      // If it was MS_Free, we shouldn't be here at all.
-      [[clang::fallthrough]];
-    default:
-      // Mutex status cannot be anything else. So control should not reach
-      // here at all.
-      return thrd_error;
-    }
-  }
+  auto *m = reinterpret_cast<Mutex *>(mutex);
+  return m->lock();
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/threads/linux/mtx_unlock.cpp b/libc/src/threads/linux/mtx_unlock.cpp
index 02e2f1a2d1aad..07d13d5cb2acc 100644
--- a/libc/src/threads/linux/mtx_unlock.cpp
+++ b/libc/src/threads/linux/mtx_unlock.cpp
@@ -7,39 +7,16 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/threads/mtx_unlock.h"
-#include "config/linux/syscall.h" // For syscall functions.
-#include "include/sys/syscall.h"  // For syscall numbers.
 #include "include/threads.h"      // For mtx_t definition.
 #include "src/__support/common.h"
-#include "src/threads/linux/thread_utils.h"
-
-#include <linux/futex.h> // For futex operations.
-#include <stdatomic.h>   // for atomic_compare_exchange_strong.
+#include "src/threads/linux/Mutex.h"
 
 namespace __llvm_libc {
 
 // The implementation currently handles only plain mutexes.
 LLVM_LIBC_FUNCTION(int, mtx_unlock, (mtx_t * mutex)) {
-  FutexData *futex_word = reinterpret_cast<FutexData *>(mutex->__internal_data);
-  while (true) {
-    uint32_t mutex_status = MS_Waiting;
-    if (atomic_compare_exchange_strong(futex_word, &mutex_status, MS_Free)) {
-      // If any thread is waiting to be woken up, then do it.
-      __llvm_libc::syscall(SYS_futex, futex_word, FUTEX_WAKE_PRIVATE, 1, 0, 0,
-                           0);
-      return thrd_success;
-    }
-
-    if (mutex_status == MS_Locked) {
-      // If nobody was waiting at this point, just free it.
-      if (atomic_compare_exchange_strong(futex_word, &mutex_status, MS_Free))
-        return thrd_success;
-    } else {
-      // This can happen, for example if some thread tries to unlock an already
-      // free mutex.
-      return thrd_error;
-    }
-  }
+  auto *m = reinterpret_cast<Mutex *>(mutex);
+  return m->unlock();
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/threads/linux/thrd_create.cpp b/libc/src/threads/linux/thrd_create.cpp
index 8b68f2a48688d..5a6bc114e2410 100644
--- a/libc/src/threads/linux/thrd_create.cpp
+++ b/libc/src/threads/linux/thrd_create.cpp
@@ -16,9 +16,9 @@
 #include "src/errno/llvmlibc_errno.h"
 #include "src/sys/mman/mmap.h"
 #include "src/sys/mman/munmap.h"
-#include "src/threads/linux/thread_utils.h"
+#include "src/threads/linux/Futex.h"
+#include "src/threads/linux/Thread.h"
 
-#include <linux/futex.h> // For futex operations.
 #include <linux/sched.h> // For CLONE_* flags.
 #include <stdint.h>
 
@@ -61,8 +61,8 @@ LLVM_LIBC_FUNCTION(int, thrd_create,
   thread->__stack = stack;
   thread->__stack_size = ThreadParams::DefaultStackSize;
   thread->__retval = -1;
-  FutexData *clear_tid_address =
-      reinterpret_cast<FutexData *>(thread->__clear_tid);
+  FutexWord *clear_tid_address =
+      reinterpret_cast<FutexWord *>(thread->__clear_tid);
   *clear_tid_address = ThreadParams::ClearTIDValue;
 
   // When the new thread is spawned by the kernel, the new thread gets the

diff  --git a/libc/src/threads/linux/thrd_join.cpp b/libc/src/threads/linux/thrd_join.cpp
index 7b6cbb762367b..1262a5f17732f 100644
--- a/libc/src/threads/linux/thrd_join.cpp
+++ b/libc/src/threads/linux/thrd_join.cpp
@@ -12,7 +12,8 @@
 #include "include/threads.h"      // For thrd_* type definitions.
 #include "src/__support/common.h"
 #include "src/sys/mman/munmap.h"
-#include "src/threads/linux/thread_utils.h"
+#include "src/threads/linux/Futex.h"
+#include "src/threads/linux/Thread.h"
 
 #include <linux/futex.h> // For futex operations.
 #include <stdatomic.h>   // For atomic_load.
@@ -20,8 +21,8 @@
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, thrd_join, (thrd_t * thread, int *retval)) {
-  FutexData *clear_tid_address =
-      reinterpret_cast<FutexData *>(thread->__clear_tid);
+  FutexWord *clear_tid_address =
+      reinterpret_cast<FutexWord *>(thread->__clear_tid);
 
   // The kernel should set the value at the clear tid address to zero.
   // If not, it is a spurious wake and we should continue to wait on


        


More information about the libc-commits mailing list