[libc-commits] [libc] [libc] rework mutex (PR #92168)

[Nick Desaulniers via libc-commits]

================
@@ -9,114 +9,72 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_MUTEX_H
 #define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_MUTEX_H
 
+#include "hdr/types/pid_t.h"
+#include "src/__support/CPP/optional.h"
 #include "src/__support/threads/linux/futex_utils.h"
+#include "src/__support/threads/linux/raw_mutex.h"
 #include "src/__support/threads/mutex_common.h"
 
 namespace LIBC_NAMESPACE {
-struct Mutex {
+
+// TODO: support shared/recursive/robust mutexes.
+class Mutex final : private internal::RawMutex {
+  // reserved timed, may be useful when combined with other flags.
   unsigned char timed;
   unsigned char recursive;
   unsigned char robust;
+  unsigned char pshared;
 
-  void *owner;
+  // TLS address may not work across forked processes. Use thread id instead.
+  pid_t owner;
   unsigned long long lock_count;
 
-  Futex futex_word;
-
-  enum class LockState : FutexWordType {
-    Free,
-    Locked,
-    Waiting,
-  };
-
 public:
-  constexpr Mutex(bool istimed, bool isrecursive, bool isrobust)
-      : timed(istimed), recursive(isrecursive), robust(isrobust),
-        owner(nullptr), lock_count(0),
-        futex_word(FutexWordType(LockState::Free)) {}
-
-  static MutexError init(Mutex *mutex, bool istimed, bool isrecur,
-                         bool isrobust) {
-    mutex->timed = istimed;
+  LIBC_INLINE constexpr Mutex(bool is_timed, bool is_recursive, bool is_robust,
+                              bool is_pshared)
+      : internal::RawMutex(), timed(is_timed), recursive(is_recursive),
+        robust(is_robust), pshared(is_pshared), owner(0), lock_count(0) {}
+
+  LIBC_INLINE static MutexError init(Mutex *mutex, bool is_timed, bool isrecur,
+                                     bool isrobust, bool is_pshared) {
+    internal::RawMutex::init(mutex);
+    mutex->timed = is_timed;
     mutex->recursive = isrecur;
     mutex->robust = isrobust;
-    mutex->owner = nullptr;
+    mutex->pshared = is_pshared;
+    mutex->owner = 0;
     mutex->lock_count = 0;
-    mutex->futex_word.set(FutexWordType(LockState::Free));
     return MutexError::NONE;
   }
 
-  static MutexError destroy(Mutex *) { return MutexError::NONE; }
-
-  MutexError reset();
+  LIBC_INLINE static MutexError destroy(Mutex *) { return MutexError::NONE; }
 
-  MutexError lock() {
-    bool was_waiting = false;
-    while (true) {
-      FutexWordType mutex_status = FutexWordType(LockState::Free);
-      FutexWordType locked_status = FutexWordType(LockState::Locked);
-
-      if (futex_word.compare_exchange_strong(
-              mutex_status, FutexWordType(LockState::Locked))) {
-        if (was_waiting)
-          futex_word = FutexWordType(LockState::Waiting);
-        return MutexError::NONE;
-      }
-
-      switch (LockState(mutex_status)) {
-      case LockState::Waiting:
-        // If other threads are waiting already, then join them. Note that the
-        // futex syscall will block if the futex data is still
-        // `LockState::Waiting` (the 4th argument to the syscall function
-        // below.)
-        futex_word.wait(FutexWordType(LockState::Waiting));
-        was_waiting = true;
-        // Once woken up/unblocked, try everything all over.
-        continue;
-      case LockState::Locked:
-        // Mutex has been locked by another thread so set the status to
-        // LockState::Waiting.
-        if (futex_word.compare_exchange_strong(
-                locked_status, FutexWordType(LockState::Waiting))) {
-          // If we are able to set the futex data to `LockState::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
-          // `LockState::Waiting`.
-          futex_word.wait(FutexWordType(LockState::Waiting));
-          was_waiting = true;
-        }
-        continue;
-      case LockState::Free:
-        // If it was LockState::Free, we shouldn't be here at all.
-        return MutexError::BAD_LOCK_STATE;
-      }
-    }
+  // TODO: record owner and lock count.
+  LIBC_INLINE MutexError lock() {
+    this->internal::RawMutex::lock(
----------------
nickdesaulniers wrote:

Should we check the return value?

https://github.com/llvm/llvm-project/pull/92168