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

via libc-commits libc-commits at lists.llvm.org
Tue May 7 07:47:45 PDT 2024


Author: Schrodinger ZHU Yifan
Date: 2024-05-07T10:47:41-04:00
New Revision: ab3a9e724d87a4272782f76b90fb0872a6a86939

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

LOG: [libc] clean up futex usage (#91163)

# Motivation

Futex syscalls are widely used in our codebase as synchronization
mechanism. Hence, it may be worthy to abstract out the most common
routines (wait and wake). On the other hand, C++20 also provides
`std::atomic_notify_one/std::atomic_wait/std::atomic_notify_all` which
align with such functionalities. This PR introduces `Futex` as a subtype
of `cpp::Atomic<FutexWordType>` with additional
`notify_one/notify_all/wait` operations.

Providing such wrappers also make future porting easier. For example,
FreeBSD's `_umtx_op` and Darwin's `ulock` can be wrapped in a similar
manner.

### Similar Examples

1. [bionic
futex](https://android.googlesource.com/platform/bionic/+/refs/heads/main/libc/bionic/bionic_futex.cpp)
2. [futex in Rust's
std](https://github.com/rust-lang/rust/blob/8cef37dbb67e9c80702925f19cf298c4203991e4/library/std/src/sys/pal/unix/futex.rs#L21)

Added: 
    libc/src/__support/threads/linux/futex_utils.h

Modified: 
    libc/src/__support/threads/linux/CMakeLists.txt
    libc/src/__support/threads/linux/callonce.cpp
    libc/src/__support/threads/linux/futex_word.h
    libc/src/__support/threads/linux/mutex.h
    libc/src/__support/threads/linux/thread.cpp
    libc/src/__support/threads/mutex.h
    libc/src/__support/threads/thread.cpp
    libc/src/threads/linux/CMakeLists.txt
    libc/src/threads/linux/CndVar.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 87a7a66ac6ea..b277c2a37f2d 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 b6a5ab8c0d07..1c29db5f5c1a 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 000000000000..1fbce4f7bf43
--- /dev/null
+++ b/libc/src/__support/threads/linux/futex_utils.h
@@ -0,0 +1,90 @@
+//===--- 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_UTILS_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_FUTEX_UTILS_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/errno.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_BITSET : FUTEX_WAIT_BITSET_PRIVATE;
+    if (timeout && timeout->is_realtime) {
+      op |= FUTEX_CLOCK_REALTIME;
+    }
+    for (;;) {
+      if (this->load(cpp::MemoryOrder::RELAXED) != expected)
+        return 0;
+
+      long ret = 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);
+
+      // continue waiting if interrupted; otherwise return the result
+      // which should normally be 0 or -ETIMEOUT
+      if (ret == -EINTR)
+        continue;
+
+      return ret;
+    }
+  }
+  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_UTILS_H

diff  --git a/libc/src/__support/threads/linux/futex_word.h b/libc/src/__support/threads/linux/futex_word.h
index 67159b81b561..acdd33bcdaaf 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 618698db0d25..6702de465168 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 fcf87cc587a5..1d986ff38cff 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 fa2bd64b6b51..9dded2e3f952 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 62aa86b7aef7..c1785343671c 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 be5407031aad..d372bd9e18c4 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 b4afdef9f9eb..525a8f0f2b53 100644
--- a/libc/src/threads/linux/CndVar.h
+++ b/libc/src/threads/linux/CndVar.h
@@ -10,8 +10,9 @@
 #define LLVM_LIBC_SRC_THREADS_LINUX_CNDVAR_H
 
 #include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/optional.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 +29,7 @@ struct CndVar {
   };
 
   struct CndWaiter {
-    cpp::Atomic<uint32_t> futex_word = WS_Waiting;
+    Futex futex_word = WS_Waiting;
     CndWaiter *next = nullptr;
   };
 
@@ -84,8 +85,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, cpp::nullopt, true);
 
     // 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 +109,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