[libc-commits] [libc] [libc] clean up futex usage (PR #91163)

Schrodinger ZHU Yifan via libc-commits libc-commits at lists.llvm.org
Sun May 5 20:43:17 PDT 2024


https://github.com/SchrodingerZhu created https://github.com/llvm/llvm-project/pull/91163

None

>From 29db7f0e1a0c33c3d5fbe1ae57b6fda508be75cb Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Sun, 5 May 2024 23:42:36 -0400
Subject: [PATCH] [libc] clean up futex usage

---
 .../__support/threads/linux/CMakeLists.txt    | 24 ++++--
 libc/src/__support/threads/linux/callonce.cpp | 24 ++----
 .../src/__support/threads/linux/futex_utils.h | 77 +++++++++++++++++++
 libc/src/__support/threads/linux/futex_word.h |  1 -
 libc/src/__support/threads/linux/mutex.h      | 22 ++----
 libc/src/__support/threads/linux/thread.cpp   | 25 +++---
 libc/src/__support/threads/mutex.h            |  4 +-
 libc/src/__support/threads/thread.cpp         |  4 +-
 libc/src/threads/linux/CMakeLists.txt         |  2 +-
 libc/src/threads/linux/CndVar.h               |  8 +-
 10 files changed, 122 insertions(+), 69 deletions(-)
 create mode 100644 libc/src/__support/threads/linux/futex_utils.h

diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 87a7a66ac6ea57..b277c2a37f2d0f 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -9,14 +9,25 @@ if(NOT TARGET libc.src.__support.OSUtil.osutil)
 endif()
 
 add_header_library(
-  mutex
+  futex_utils
   HDRS
-    mutex.h
+    futex_utils.h
   DEPENDS
     .futex_word_type
     libc.include.sys_syscall
-    libc.src.__support.CPP.atomic
     libc.src.__support.OSUtil.osutil
+    libc.src.__support.CPP.atomic
+    libc.src.__support.CPP.limits
+    libc.src.__support.CPP.optional
+    libc.hdr.types.struct_timespec
+)
+
+add_header_library(
+  mutex
+  HDRS
+    mutex.h
+  DEPENDS
+    .futex
     libc.src.__support.threads.mutex_common
 )
 
@@ -25,7 +36,7 @@ add_object_library(
   SRCS
     thread.cpp
   DEPENDS
-    .futex_word_type
+    .futex_utils
     libc.config.linux.app_h
     libc.include.sys_syscall
     libc.src.errno.errno
@@ -50,8 +61,5 @@ add_object_library(
   HDRS
     ../callonce.h
   DEPENDS
-    libc.include.sys_syscall
-    libc.src.__support.CPP.atomic
-    libc.src.__support.CPP.limits
-    libc.src.__support.OSUtil.osutil
+    .futex_utils
 )
diff --git a/libc/src/__support/threads/linux/callonce.cpp b/libc/src/__support/threads/linux/callonce.cpp
index b6a5ab8c0d07a9..1c29db5f5c1a11 100644
--- a/libc/src/__support/threads/linux/callonce.cpp
+++ b/libc/src/__support/threads/linux/callonce.cpp
@@ -6,15 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "futex_word.h"
-
-#include "src/__support/CPP/atomic.h"
-#include "src/__support/CPP/limits.h"     // INT_MAX
-#include "src/__support/OSUtil/syscall.h" // For syscall functions.
 #include "src/__support/threads/callonce.h"
-
-#include <linux/futex.h>
-#include <sys/syscall.h> // For syscall numbers.
+#include "src/__support/threads/linux/futex_utils.h"
 
 namespace LIBC_NAMESPACE {
 
@@ -24,7 +17,7 @@ static constexpr FutexWordType WAITING = 0x22;
 static constexpr FutexWordType FINISH = 0x33;
 
 int callonce(CallOnceFlag *flag, CallOnceCallback *func) {
-  auto *futex_word = reinterpret_cast<cpp::Atomic<FutexWordType> *>(flag);
+  auto *futex_word = reinterpret_cast<Futex *>(flag);
 
   FutexWordType not_called = NOT_CALLED;
 
@@ -33,22 +26,15 @@ int callonce(CallOnceFlag *flag, CallOnceCallback *func) {
   if (futex_word->compare_exchange_strong(not_called, START)) {
     func();
     auto status = futex_word->exchange(FINISH);
-    if (status == WAITING) {
-      LIBC_NAMESPACE::syscall_impl<long>(FUTEX_SYSCALL_ID, &futex_word->val,
-                                         FUTEX_WAKE_PRIVATE,
-                                         INT_MAX, // Wake all waiters.
-                                         0, 0, 0);
-    }
+    if (status == WAITING)
+      futex_word->notify_all();
     return 0;
   }
 
   FutexWordType status = START;
   if (futex_word->compare_exchange_strong(status, WAITING) ||
       status == WAITING) {
-    LIBC_NAMESPACE::syscall_impl<long>(
-        FUTEX_SYSCALL_ID, &futex_word->val, FUTEX_WAIT_PRIVATE,
-        WAITING, // Block only if status is still |WAITING|.
-        0, 0, 0);
+    futex_word->wait(WAITING);
   }
 
   return 0;
diff --git a/libc/src/__support/threads/linux/futex_utils.h b/libc/src/__support/threads/linux/futex_utils.h
new file mode 100644
index 00000000000000..b245f4b17d9dbd
--- /dev/null
+++ b/libc/src/__support/threads/linux/futex_utils.h
@@ -0,0 +1,77 @@
+//===--- Futex Wrapper ------------------------------------------*- 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___SUPPORT_THREADS_LINUX_FUTEX_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_FUTEX_H
+
+#include "hdr/types/struct_timespec.h"
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/limits.h"
+#include "src/__support/CPP/optional.h"
+#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/macros/attributes.h"
+#include "src/__support/threads/linux/futex_word.h"
+#include <linux/futex.h>
+
+namespace LIBC_NAMESPACE {
+class Futex : public cpp::Atomic<FutexWordType> {
+public:
+  struct Timeout {
+    timespec abs_time;
+    bool is_realtime;
+  };
+  LIBC_INLINE constexpr Futex(FutexWordType value)
+      : cpp::Atomic<FutexWordType>(value) {}
+  LIBC_INLINE Futex &operator=(FutexWordType value) {
+    cpp::Atomic<FutexWordType>::store(value);
+    return *this;
+  }
+  LIBC_INLINE long wait(FutexWordType expected,
+                        cpp::optional<Timeout> timeout = cpp::nullopt,
+                        bool is_shared = false) {
+    // use bitset variants to enforce abs_time
+    uint32_t op = is_shared ? FUTEX_WAIT : FUTEX_WAIT_PRIVATE;
+    if (timeout && timeout->is_realtime) {
+      op |= FUTEX_CLOCK_REALTIME;
+    }
+    return syscall_impl<long>(
+        /* syscall number */ FUTEX_SYSCALL_ID,
+        /* futex address */ this,
+        /* futex operation  */ op,
+        /* expected value */ expected,
+        /* timeout */ timeout ? &timeout->abs_time : nullptr,
+        /* ignored */ nullptr,
+        /* bitset */ FUTEX_BITSET_MATCH_ANY);
+  }
+  LIBC_INLINE long notify_one(bool is_shared = false) {
+    return syscall_impl<long>(
+        /* syscall number */ FUTEX_SYSCALL_ID,
+        /* futex address */ this,
+        /* futex operation  */ is_shared ? FUTEX_WAKE : FUTEX_WAKE_PRIVATE,
+        /* wake up limit */ 1,
+        /* ignored */ nullptr,
+        /* ignored */ nullptr,
+        /* ignored */ 0);
+  }
+  LIBC_INLINE long notify_all(bool is_shared = false) {
+    return syscall_impl<long>(
+        /* syscall number */ FUTEX_SYSCALL_ID,
+        /* futex address */ this,
+        /* futex operation  */ is_shared ? FUTEX_WAKE : FUTEX_WAKE_PRIVATE,
+        /* wake up limit */ cpp::numeric_limits<int>::max(),
+        /* ignored */ nullptr,
+        /* ignored */ nullptr,
+        /* ignored */ 0);
+  }
+};
+
+static_assert(__is_standard_layout(Futex),
+              "Futex must be a standard layout type.");
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_FUTEX_H
diff --git a/libc/src/__support/threads/linux/futex_word.h b/libc/src/__support/threads/linux/futex_word.h
index 67159b81b56132..acdd33bcdaafa1 100644
--- a/libc/src/__support/threads/linux/futex_word.h
+++ b/libc/src/__support/threads/linux/futex_word.h
@@ -11,7 +11,6 @@
 
 #include <stdint.h>
 #include <sys/syscall.h>
-
 namespace LIBC_NAMESPACE {
 
 // Futexes are 32 bits in size on all platforms, including 64-bit platforms.
diff --git a/libc/src/__support/threads/linux/mutex.h b/libc/src/__support/threads/linux/mutex.h
index 618698db0d25b3..6702de46516869 100644
--- a/libc/src/__support/threads/linux/mutex.h
+++ b/libc/src/__support/threads/linux/mutex.h
@@ -9,17 +9,10 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_MUTEX_H
 #define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_MUTEX_H
 
-#include "src/__support/CPP/atomic.h"
-#include "src/__support/OSUtil/syscall.h" // For syscall functions.
-#include "src/__support/threads/linux/futex_word.h"
+#include "src/__support/threads/linux/futex_utils.h"
 #include "src/__support/threads/mutex_common.h"
 
-#include <linux/futex.h>
-#include <stdint.h>
-#include <sys/syscall.h> // For syscall numbers.
-
 namespace LIBC_NAMESPACE {
-
 struct Mutex {
   unsigned char timed;
   unsigned char recursive;
@@ -28,7 +21,7 @@ struct Mutex {
   void *owner;
   unsigned long long lock_count;
 
-  cpp::Atomic<FutexWordType> futex_word;
+  Futex futex_word;
 
   enum class LockState : FutexWordType {
     Free,
@@ -76,9 +69,7 @@ struct Mutex {
         // futex syscall will block if the futex data is still
         // `LockState::Waiting` (the 4th argument to the syscall function
         // below.)
-        LIBC_NAMESPACE::syscall_impl<long>(
-            FUTEX_SYSCALL_ID, &futex_word.val, FUTEX_WAIT_PRIVATE,
-            FutexWordType(LockState::Waiting), 0, 0, 0);
+        futex_word.wait(FutexWordType(LockState::Waiting));
         was_waiting = true;
         // Once woken up/unblocked, try everything all over.
         continue;
@@ -91,9 +82,7 @@ struct Mutex {
           // 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`.
-          LIBC_NAMESPACE::syscall_impl<long>(
-              FUTEX_SYSCALL_ID, &futex_word, FUTEX_WAIT_PRIVATE,
-              FutexWordType(LockState::Waiting), 0, 0, 0);
+          futex_word.wait(FutexWordType(LockState::Waiting));
           was_waiting = true;
         }
         continue;
@@ -110,8 +99,7 @@ struct Mutex {
       if (futex_word.compare_exchange_strong(mutex_status,
                                              FutexWordType(LockState::Free))) {
         // If any thread is waiting to be woken up, then do it.
-        LIBC_NAMESPACE::syscall_impl<long>(FUTEX_SYSCALL_ID, &futex_word,
-                                           FUTEX_WAKE_PRIVATE, 1, 0, 0, 0);
+        futex_word.notify_one();
         return MutexError::NONE;
       }
 
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index fcf87cc587a509..1d986ff38cfffc 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -14,15 +14,14 @@
 #include "src/__support/OSUtil/syscall.h" // For syscall functions.
 #include "src/__support/common.h"
 #include "src/__support/error_or.h"
-#include "src/__support/threads/linux/futex_word.h" // For FutexWordType
-#include "src/errno/libc_errno.h"                   // For error macros
+#include "src/__support/threads/linux/futex_utils.h" // For FutexWordType
+#include "src/errno/libc_errno.h"                    // For error macros
 
 #ifdef LIBC_TARGET_ARCH_IS_AARCH64
 #include <arm_acle.h>
 #endif
 
 #include <fcntl.h>
-#include <linux/futex.h>
 #include <linux/param.h> // For EXEC_PAGESIZE.
 #include <linux/prctl.h> // For PR_SET_NAME
 #include <linux/sched.h> // For CLONE_* flags.
@@ -247,8 +246,7 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
   // stack memory.
 
   static constexpr size_t INTERNAL_STACK_DATA_SIZE =
-      sizeof(StartArgs) + sizeof(ThreadAttributes) +
-      sizeof(cpp::Atomic<FutexWordType>);
+      sizeof(StartArgs) + sizeof(ThreadAttributes) + sizeof(Futex);
 
   // This is pretty arbitrary, but at the moment we don't adjust user provided
   // stacksize (or default) to account for this data as its assumed minimal. If
@@ -288,9 +286,9 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
   start_args->runner = runner;
   start_args->arg = arg;
 
-  auto clear_tid = reinterpret_cast<cpp::Atomic<FutexWordType> *>(
+  auto clear_tid = reinterpret_cast<Futex *>(
       adjusted_stack + sizeof(StartArgs) + sizeof(ThreadAttributes));
-  clear_tid->val = CLEAR_TID_VALUE;
+  clear_tid->set(CLEAR_TID_VALUE);
   attrib->platform_data = clear_tid;
 
   // The clone syscall takes arguments in an architecture specific order.
@@ -374,14 +372,11 @@ void Thread::wait() {
   // 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
   // the futex.
-  auto *clear_tid =
-      reinterpret_cast<cpp::Atomic<FutexWordType> *>(attrib->platform_data);
-  while (clear_tid->load() != 0) {
-    // We cannot do a FUTEX_WAIT_PRIVATE here as the kernel does a
-    // FUTEX_WAKE and not a FUTEX_WAKE_PRIVATE.
-    LIBC_NAMESPACE::syscall_impl<long>(FUTEX_SYSCALL_ID, &clear_tid->val,
-                                       FUTEX_WAIT, CLEAR_TID_VALUE, nullptr);
-  }
+  auto *clear_tid = reinterpret_cast<Futex *>(attrib->platform_data);
+  // We cannot do a FUTEX_WAIT_PRIVATE here as the kernel does a
+  // FUTEX_WAKE and not a FUTEX_WAKE_PRIVATE.
+  while (clear_tid->load() != 0)
+    clear_tid->wait(CLEAR_TID_VALUE, cpp::nullopt, true);
 }
 
 bool Thread::operator==(const Thread &thread) const {
diff --git a/libc/src/__support/threads/mutex.h b/libc/src/__support/threads/mutex.h
index fa2bd64b6b51cf..9dded2e3f952a1 100644
--- a/libc/src/__support/threads/mutex.h
+++ b/libc/src/__support/threads/mutex.h
@@ -38,9 +38,9 @@
 // want the constructors of the Mutex classes to be constexprs.
 
 #if defined(__linux__)
-#include "linux/mutex.h"
+#include "src/__support/threads/linux/mutex.h"
 #elif defined(LIBC_TARGET_ARCH_IS_GPU)
-#include "gpu/mutex.h"
+#include "src/__support/threads/gpu/mutex.h"
 #endif // __linux__
 
 namespace LIBC_NAMESPACE {
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index 62aa86b7aef708..c1785343671c5d 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/libc/src/__support/threads/thread.cpp
@@ -6,8 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "thread.h"
-#include "mutex.h"
+#include "src/__support/threads/thread.h"
+#include "src/__support/threads/mutex.h"
 
 #include "src/__support/CPP/array.h"
 #include "src/__support/CPP/optional.h"
diff --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index be5407031aaddb..d372bd9e18c4a8 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/libc/src/threads/linux/CMakeLists.txt
@@ -9,7 +9,7 @@ add_header_library(
     libc.src.__support.CPP.atomic
     libc.src.__support.OSUtil.osutil
     libc.src.__support.threads.mutex
-    libc.src.__support.threads.linux.futex_word_type
+    libc.src.__support.threads.linux.futex_utils
 )
 
 add_entrypoint_object(
diff --git a/libc/src/threads/linux/CndVar.h b/libc/src/threads/linux/CndVar.h
index b4afdef9f9eba7..fae4d868a72577 100644
--- a/libc/src/threads/linux/CndVar.h
+++ b/libc/src/threads/linux/CndVar.h
@@ -11,7 +11,7 @@
 
 #include "src/__support/CPP/atomic.h"
 #include "src/__support/OSUtil/syscall.h" // For syscall functions.
-#include "src/__support/threads/linux/futex_word.h"
+#include "src/__support/threads/linux/futex_utils.h"
 #include "src/__support/threads/mutex.h"
 
 #include <linux/futex.h> // For futex operations.
@@ -28,7 +28,7 @@ struct CndVar {
   };
 
   struct CndWaiter {
-    cpp::Atomic<uint32_t> futex_word = WS_Waiting;
+    Futex futex_word = WS_Waiting;
     CndWaiter *next = nullptr;
   };
 
@@ -84,8 +84,7 @@ struct CndVar {
       }
     }
 
-    LIBC_NAMESPACE::syscall_impl<long>(FUTEX_SYSCALL_ID, &waiter.futex_word.val,
-                                       FUTEX_WAIT, WS_Waiting, 0, 0, 0);
+    waiter.futex_word.wait(WS_Waiting);
 
     // At this point, if locking |m| fails, we can simply return as the
     // queued up waiter would have been removed from the queue.
@@ -109,6 +108,7 @@ struct CndVar {
 
     qmtx.futex_word = FutexWordType(Mutex::LockState::Free);
 
+    // this is a special WAKE_OP, so we use syscall directly
     LIBC_NAMESPACE::syscall_impl<long>(
         FUTEX_SYSCALL_ID, &qmtx.futex_word.val, FUTEX_WAKE_OP, 1, 1,
         &first->futex_word.val,



More information about the libc-commits mailing list