[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