[libc] clean up futex usage by SchrodingerZhu · Pull Request #91163 · llvm/llvm-project (original) (raw)
@llvm/pr-subscribers-libc
Author: Schrodinger ZHU Yifan (SchrodingerZhu)
Changes
Full diff: https://github.com/llvm/llvm-project/pull/91163.diff
10 Files Affected:
- (modified) libc/src/__support/threads/linux/CMakeLists.txt (+16-8)
- (modified) libc/src/__support/threads/linux/callonce.cpp (+5-19)
- (added) libc/src/__support/threads/linux/futex_utils.h (+77)
- (modified) libc/src/__support/threads/linux/futex_word.h (-1)
- (modified) libc/src/__support/threads/linux/mutex.h (+5-17)
- (modified) libc/src/__support/threads/linux/thread.cpp (+10-15)
- (modified) libc/src/__support/threads/mutex.h (+2-2)
- (modified) libc/src/__support/threads/thread.cpp (+2-2)
- (modified) libc/src/threads/linux/CMakeLists.txt (+1-1)
- (modified) libc/src/threads/linux/CndVar.h (+4-4)
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 *>(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(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( - 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 { +public: + struct Timeout { + timespec abs_time; + bool is_realtime; + }; + LIBC_INLINE constexpr Futex(FutexWordType value) + : cpp::Atomic(value) {} + LIBC_INLINE Futex &operator=(FutexWordType value) { + cpp::Atomic::store(value); + return this; + } + LIBC_INLINE long wait(FutexWordType expected, + cpp::optional 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( + / 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( + / 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( + / syscall number / FUTEX_SYSCALL_ID, + / futex address / this, + / futex operation / is_shared ? FUTEX_WAKE : FUTEX_WAKE_PRIVATE, + / wake up limit / cpp::numeric_limits::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 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>);
// 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. Ifsizeof(StartArgs) + sizeof(ThreadAttributes) + sizeof(Futex);
@@ -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 *>(
- 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(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 futex_word = WS_Waiting;
- Futex futex_word = WS_Waiting; CndWaiter *next = nullptr; };
@@ -84,8 +84,7 @@ struct CndVar { } }
- LIBC_NAMESPACE::syscall_impl(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( FUTEX_SYSCALL_ID, &qmtx.futex_word.val, FUTEX_WAKE_OP, 1, 1, &first->futex_word.val,