[libc-commits] [libc] [libc] refactor futex and remove dependency cycle (PR #117923)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Wed Nov 27 12:41:42 PST 2024
https://github.com/SchrodingerZhu created https://github.com/llvm/llvm-project/pull/117923
On Linux, we have the following dependency chain:
```
clock_gettime -> vDSO -> callonce -> futex -> abs_timeout -> clock_gettime
```
This was previously solved by separating out the implementation of `abs_timeout` related implementation out of the its definition header, which is quite ugly. Recently, I started working on Windows implementations and such hidden cyclic dependency bring back some headache when I was refactoring the timeout to support both absolution timeout and relative timeout.
We should address the problem in a saner approach.
So, the real problem is that `callonce` does not need timeout at all and we should not pull in the timing APIs via `callonce` which is used everywhere in low-level initialization operations. That is to make everything depending on timing APIs.
This PR separates the timeout support from the main futex implementation. Additionally, it lifts futex to the upper-level, making it a general platform abstraction rather a linux-specific abstraction.
Windows support for futex will be added very soon.
>From 8b5bb48d200e2abb1b613a3174fc7e60f4269f14 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 27 Nov 2024 15:33:05 -0500
Subject: [PATCH] [libc] refactor futex and remove dependency cycle
---
libc/src/__support/threads/CMakeLists.txt | 17 +++++
libc/src/__support/threads/futex_timeout.h | 18 ++++++
libc/src/__support/threads/futex_utils.h | 28 +++++++++
.../__support/threads/linux/CMakeLists.txt | 12 +++-
libc/src/__support/threads/linux/CndVar.cpp | 4 +-
.../__support/threads/linux/futex_timeout.h | 62 +++++++++++++++++++
.../src/__support/threads/linux/futex_utils.h | 11 +---
libc/src/__support/threads/linux/raw_mutex.h | 8 +--
libc/src/__support/threads/linux/rwlock.h | 12 ++--
libc/src/__support/threads/linux/thread.cpp | 2 +-
libc/src/threads/linux/CMakeLists.txt | 2 +-
11 files changed, 151 insertions(+), 25 deletions(-)
create mode 100644 libc/src/__support/threads/futex_timeout.h
create mode 100644 libc/src/__support/threads/futex_utils.h
create mode 100644 libc/src/__support/threads/linux/futex_timeout.h
diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index bd49bbb5ad2fe7..5a39c9dff9216c 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/libc/src/__support/threads/CMakeLists.txt
@@ -105,3 +105,20 @@ add_header_library(
libc.hdr.types.pid_t
${identifier_dependency_on_thread}
)
+
+# TODO: implement windows "futex" with WaitOnAddress
+add_header_library(
+ futex_utils
+ HDRS
+ futex_utils.h
+ DEPENDS
+ .${LIBC_TARGET_OS}.futex_utils
+)
+
+add_header_library(
+ futex_timeout
+ HDRS
+ futex_timeout.h
+ DEPENDS
+ .${LIBC_TARGET_OS}.futex_timeout
+)
diff --git a/libc/src/__support/threads/futex_timeout.h b/libc/src/__support/threads/futex_timeout.h
new file mode 100644
index 00000000000000..7adfbc594bfca6
--- /dev/null
+++ b/libc/src/__support/threads/futex_timeout.h
@@ -0,0 +1,18 @@
+//===--- Futex With Timeout Support -----------------------------*- 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_FUTEX_TIMEOUT_H
+#define LLVM_LIBC_SRC_SUPPORT_THREADS_FUTEX_TIMEOUT_H
+
+// TODO: implement futex for other platforms.
+#ifdef __linux__
+#include "src/__support/threads/linux/futex_timeout.h"
+#else
+#error "Unsupported platform"
+#endif
+
+#endif // LLVM_LIBC_SRC_SUPPORT_THREADS_FUTEX_TIMEOUT_H
diff --git a/libc/src/__support/threads/futex_utils.h b/libc/src/__support/threads/futex_utils.h
new file mode 100644
index 00000000000000..61ccabeadc85b9
--- /dev/null
+++ b/libc/src/__support/threads/futex_utils.h
@@ -0,0 +1,28 @@
+//===--- Futex Utilities ----------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+//===--- Futex Wrapper ------------------------------------------*- C++ -*-===//
+// We take the name "futex" from Linux system. This library provides a general
+// wrapper for waiting and notifying on atomic words. Various platforms support
+// futex-like operations.
+// - Windows: WaitOnAddress and WakeByAddressSingle/WakeByAddressAll
+// (Windows futex cannot be used in inter-process synchronization)
+// - MacOS: os_sync_wait_on_address or __ulock_wait/__ulock_wake
+// - FreeBSD: _umtx_op
+
+#ifndef LLVM_LIBC_SRC_SUPPORT_THREADS_FUTEX_UTILS_H
+#define LLVM_LIBC_SRC_SUPPORT_THREADS_FUTEX_UTILS_H
+
+// TODO: implement futex for other platforms.
+#ifdef __linux__
+#include "src/__support/threads/linux/futex_utils.h"
+#else
+#error "Unsupported platform"
+#endif
+
+#endif // LLVM_LIBC_SRC_SUPPORT_THREADS_FUTEX_UTILS_H
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 47598d98c98863..19ce75ce9cd6b7 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -19,6 +19,14 @@ add_header_library(
libc.src.__support.CPP.atomic
libc.src.__support.CPP.limits
libc.src.__support.CPP.optional
+)
+
+add_header_library(
+ futex_timeout
+ HDRS
+ futex_timeout.h
+ DEPENDS
+ .futex_utils
libc.src.__support.time.linux.abs_timeout
)
@@ -34,7 +42,7 @@ add_header_library(
HDRS
mutex.h
DEPENDS
- .futex_utils
+ .futex_timeout
libc.src.__support.threads.sleep
libc.src.__support.time.linux.abs_timeout
libc.src.__support.time.linux.monotonicity
@@ -50,7 +58,7 @@ add_header_library(
HDRS
rwlock.h
DEPENDS
- .futex_utils
+ .futex_timeout
.raw_mutex
libc.src.__support.common
libc.src.__support.OSUtil.osutil
diff --git a/libc/src/__support/threads/linux/CndVar.cpp b/libc/src/__support/threads/linux/CndVar.cpp
index be74c18dddf31a..2f7297b7433959 100644
--- a/libc/src/__support/threads/linux/CndVar.cpp
+++ b/libc/src/__support/threads/linux/CndVar.cpp
@@ -8,7 +8,7 @@
#include "src/__support/threads/CndVar.h"
#include "src/__support/CPP/mutex.h"
-#include "src/__support/OSUtil/syscall.h" // syscall_impl
+#include "src/__support/OSUtil/syscall.h" // syscall_impl
#include "src/__support/macros/config.h"
#include "src/__support/threads/linux/futex_word.h" // FutexWordType
#include "src/__support/threads/linux/raw_mutex.h" // RawMutex
@@ -56,7 +56,7 @@ int CndVar::wait(Mutex *m) {
}
}
- waiter.futex_word.wait(WS_Waiting, cpp::nullopt, true);
+ waiter.futex_word.wait(WS_Waiting, /*is_shared=*/true);
// At this point, if locking |m| fails, we can simply return as the
// queued up waiter would have been removed from the queue.
diff --git a/libc/src/__support/threads/linux/futex_timeout.h b/libc/src/__support/threads/linux/futex_timeout.h
new file mode 100644
index 00000000000000..ba12cab7d80660
--- /dev/null
+++ b/libc/src/__support/threads/linux/futex_timeout.h
@@ -0,0 +1,62 @@
+//===--- 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_TIMEOUT_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_FUTEX_TIMEOUT_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/macros/config.h"
+#include "src/__support/threads/linux/futex_utils.h"
+#include "src/__support/time/linux/abs_timeout.h"
+
+namespace LIBC_NAMESPACE_DECL {
+class TimedFutex : public Futex {
+public:
+ using Timeout = internal::AbsTimeout;
+ using Futex::Futex;
+ using Futex::operator=;
+ 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->get_timespec() : 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;
+ }
+ }
+};
+
+static_assert(__is_standard_layout(TimedFutex),
+ "Futex must be a standard layout type.");
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_FUTEX_TIMEOUT_H
diff --git a/libc/src/__support/threads/linux/futex_utils.h b/libc/src/__support/threads/linux/futex_utils.h
index 943a99ab38c8ca..0d36eb92232faa 100644
--- a/libc/src/__support/threads/linux/futex_utils.h
+++ b/libc/src/__support/threads/linux/futex_utils.h
@@ -16,28 +16,21 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/threads/linux/futex_word.h"
-#include "src/__support/time/linux/abs_timeout.h"
#include <linux/errno.h>
#include <linux/futex.h>
namespace LIBC_NAMESPACE_DECL {
class Futex : public cpp::Atomic<FutexWordType> {
public:
- using Timeout = internal::AbsTimeout;
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) {
+ LIBC_INLINE long wait(FutexWordType expected, 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;
@@ -47,7 +40,7 @@ class Futex : public cpp::Atomic<FutexWordType> {
/* futex address */ this,
/* futex operation */ op,
/* expected value */ expected,
- /* timeout */ timeout ? &timeout->get_timespec() : nullptr,
+ /* timeout */ nullptr,
/* ignored */ nullptr,
/* bitset */ FUTEX_BITSET_MATCH_ANY);
diff --git a/libc/src/__support/threads/linux/raw_mutex.h b/libc/src/__support/threads/linux/raw_mutex.h
index 47f0aa70f1c46f..717238b2c263ec 100644
--- a/libc/src/__support/threads/linux/raw_mutex.h
+++ b/libc/src/__support/threads/linux/raw_mutex.h
@@ -14,7 +14,7 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h"
-#include "src/__support/threads/linux/futex_utils.h"
+#include "src/__support/threads/linux/futex_timeout.h"
#include "src/__support/threads/linux/futex_word.h"
#include "src/__support/threads/sleep.h"
#include "src/__support/time/linux/abs_timeout.h"
@@ -38,7 +38,7 @@ namespace LIBC_NAMESPACE_DECL {
// critical sections.
class RawMutex {
protected:
- Futex futex;
+ TimedFutex futex;
LIBC_INLINE_VAR static constexpr FutexWordType UNLOCKED = 0b00;
LIBC_INLINE_VAR static constexpr FutexWordType LOCKED = 0b01;
LIBC_INLINE_VAR static constexpr FutexWordType IN_CONTENTION = 0b10;
@@ -63,7 +63,7 @@ class RawMutex {
// Return true if the lock is acquired. Return false if timeout happens before
// the lock is acquired.
- LIBC_INLINE bool lock_slow(cpp::optional<Futex::Timeout> timeout,
+ LIBC_INLINE bool lock_slow(cpp::optional<TimedFutex::Timeout> timeout,
bool is_pshared, unsigned spin_count) {
FutexWordType state = spin(spin_count);
// Before go into contention state, try to grab the lock.
@@ -102,7 +102,7 @@ class RawMutex {
expected, LOCKED, cpp::MemoryOrder::ACQUIRE, cpp::MemoryOrder::RELAXED);
}
LIBC_INLINE bool
- lock(cpp::optional<Futex::Timeout> timeout = cpp::nullopt,
+ lock(cpp::optional<TimedFutex::Timeout> timeout = cpp::nullopt,
bool is_shared = false,
unsigned spin_count = LIBC_COPT_RAW_MUTEX_DEFAULT_SPIN_COUNT) {
// Timeout will not be checked if immediate lock is possible.
diff --git a/libc/src/__support/threads/linux/rwlock.h b/libc/src/__support/threads/linux/rwlock.h
index 57fcc7bb67a6a0..6d3e2f2ff85eaa 100644
--- a/libc/src/__support/threads/linux/rwlock.h
+++ b/libc/src/__support/threads/linux/rwlock.h
@@ -55,9 +55,9 @@ class WaitingQueue final : private RawMutex {
// Pending writer count (protected by the mutex)
FutexWordType pending_writers;
// Reader serialization (increases on each reader-waking operation)
- Futex reader_serialization;
+ TimedFutex reader_serialization;
// Writer serialization (increases on each writer-waking operation)
- Futex writer_serialization;
+ TimedFutex writer_serialization;
public:
// RAII guard to lock and unlock the waiting queue.
@@ -98,7 +98,7 @@ class WaitingQueue final : private RawMutex {
template <Role role>
LIBC_INLINE long wait(FutexWordType expected,
- cpp::optional<Futex::Timeout> timeout,
+ cpp::optional<TimedFutex::Timeout> timeout,
bool is_pshared) {
if constexpr (role == Role::Reader)
return reader_serialization.wait(expected, timeout, is_pshared);
@@ -389,7 +389,7 @@ class RwLock {
private:
template <Role role>
LIBC_INLINE LockResult
- lock_slow(cpp::optional<Futex::Timeout> timeout = cpp::nullopt,
+ lock_slow(cpp::optional<TimedFutex::Timeout> timeout = cpp::nullopt,
unsigned spin_count = LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT) {
// Phase 1: deadlock detection.
// A deadlock happens if this is a RAW/WAW lock in the same thread.
@@ -468,7 +468,7 @@ class RwLock {
public:
[[nodiscard]]
LIBC_INLINE LockResult
- read_lock(cpp::optional<Futex::Timeout> timeout = cpp::nullopt,
+ read_lock(cpp::optional<TimedFutex::Timeout> timeout = cpp::nullopt,
unsigned spin_count = LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT) {
LockResult result = try_read_lock();
if (LIBC_LIKELY(result != LockResult::Busy))
@@ -477,7 +477,7 @@ class RwLock {
}
[[nodiscard]]
LIBC_INLINE LockResult
- write_lock(cpp::optional<Futex::Timeout> timeout = cpp::nullopt,
+ write_lock(cpp::optional<TimedFutex::Timeout> timeout = cpp::nullopt,
unsigned spin_count = LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT) {
LockResult result = try_write_lock();
if (LIBC_LIKELY(result != LockResult::Busy))
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index c531d74c533550..d55dc453f8279e 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -377,7 +377,7 @@ void Thread::wait() {
// 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);
+ clear_tid->wait(CLEAR_TID_VALUE, /*is_shared=*/true);
}
bool Thread::operator==(const Thread &thread) const {
diff --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index 6c8e0845faf4c4..cf50e0ac560127 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/libc/src/threads/linux/CMakeLists.txt
@@ -10,7 +10,7 @@ add_header_library(
libc.src.__support.OSUtil.osutil
libc.src.__support.threads.mutex
libc.src.__support.threads.linux.raw_mutex
- libc.src.__support.threads.linux.futex_utils
+ libc.src.__support.threads.futex_utils
)
add_entrypoint_object(
More information about the libc-commits
mailing list