Skip to content

Commit 301db3d

Browse files
[libc] add simplified tid cache (#101620)
According to discussions on monthly meeting, we probably don't want to cache `getpid` anymore. glibc removes their cache. bionic is hesitating whether such cache is to be removed. `getpid` is async-signal-safe, so we must make sure it always work. However, for `gettid`, we have more freedom. Moreover, we are using `gettid` to examine deadlock such that the performance penalty is not negligible here. Thus, this patch is separated from previous patch to provide only `tid` caching. It is much more simplified. Hopefully, previous build issues can be resolved easily.
1 parent 982cfae commit 301db3d

File tree

16 files changed

+170
-9
lines changed

16 files changed

+170
-9
lines changed

libc/config/linux/aarch64/entrypoints.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ set(TARGET_LIBC_ENTRYPOINTS
299299
libc.src.unistd.geteuid
300300
libc.src.unistd.getpid
301301
libc.src.unistd.getppid
302+
libc.src.unistd.gettid
302303
libc.src.unistd.getuid
303304
libc.src.unistd.isatty
304305
libc.src.unistd.link

libc/config/linux/riscv/entrypoints.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ set(TARGET_LIBC_ENTRYPOINTS
318318
libc.src.unistd.geteuid
319319
libc.src.unistd.getpid
320320
libc.src.unistd.getppid
321+
libc.src.unistd.gettid
321322
libc.src.unistd.getuid
322323
libc.src.unistd.isatty
323324
libc.src.unistd.link

libc/config/linux/x86_64/entrypoints.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ set(TARGET_LIBC_ENTRYPOINTS
318318
libc.src.unistd.geteuid
319319
libc.src.unistd.getpid
320320
libc.src.unistd.getppid
321+
libc.src.unistd.gettid
321322
libc.src.unistd.getuid
322323
libc.src.unistd.isatty
323324
libc.src.unistd.link

libc/newhdrgen/yaml/unistd.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,3 +307,9 @@ functions:
307307
- type: const void *__restrict
308308
- type: void *
309309
- type: ssize_t
310+
- name: gettid
311+
standards:
312+
- Linux
313+
return_type: pid_t
314+
arguments:
315+
- type: void

libc/spec/posix.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,11 @@ def POSIX : StandardSpec<"POSIX"> {
546546
RetValSpec<PidT>,
547547
[ArgSpec<VoidType>]
548548
>,
549+
FunctionSpec<
550+
"gettid",
551+
RetValSpec<PidT>,
552+
[ArgSpec<VoidType>]
553+
>,
549554
FunctionSpec<
550555
"getuid",
551556
RetValSpec<UidT>,

libc/src/__support/threads/CMakeLists.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,19 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.CndVar)
8989
.${LIBC_TARGET_OS}.CndVar
9090
)
9191
endif()
92+
93+
if (LLVM_LIBC_FULL_BUILD)
94+
set(identifier_dependency_on_thread libc.src.__support.threads.thread)
95+
endif()
96+
97+
add_header_library(
98+
identifier
99+
HDRS
100+
identifier.h
101+
DEPENDS
102+
libc.src.__support.OSUtil.osutil
103+
libc.src.__support.common
104+
libc.include.sys_syscall
105+
libc.hdr.types.pid_t
106+
${identifier_dependency_on_thread}
107+
)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===--- Thread Identifier Header --------------------------------*- C++-*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H
10+
#define LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H
11+
12+
#ifdef LIBC_FULL_BUILD
13+
#include "src/__support/threads/thread.h"
14+
#endif // LIBC_FULL_BUILD
15+
16+
#include "hdr/types/pid_t.h"
17+
#include "src/__support/OSUtil/syscall.h"
18+
#include <sys/syscall.h>
19+
20+
namespace LIBC_NAMESPACE_DECL {
21+
namespace internal {
22+
23+
LIBC_INLINE pid_t *get_tid_cache() {
24+
#ifdef LIBC_FULL_BUILD
25+
return &self.attrib->tid;
26+
#else
27+
// in non-full build mode, we do not control the fork routine. Therefore,
28+
// we do not cache tid at all.
29+
return nullptr;
30+
#endif
31+
}
32+
33+
LIBC_INLINE pid_t gettid() {
34+
pid_t *cache = get_tid_cache();
35+
if (LIBC_UNLIKELY(!cache || *cache <= 0))
36+
return syscall_impl<pid_t>(SYS_gettid);
37+
return *cache;
38+
}
39+
40+
LIBC_INLINE void force_set_tid(pid_t tid) {
41+
pid_t *cache = get_tid_cache();
42+
if (cache)
43+
*cache = tid;
44+
}
45+
46+
} // namespace internal
47+
} // namespace LIBC_NAMESPACE_DECL
48+
49+
#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H

libc/src/__support/threads/linux/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ add_header_library(
5555
libc.src.__support.common
5656
libc.src.__support.OSUtil.osutil
5757
libc.src.__support.CPP.limits
58+
libc.src.__support.threads.identifier
5859
COMPILE_OPTIONS
5960
-DLIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT=${LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT}
6061
${monotonicity_flags}

libc/src/__support/threads/linux/rwlock.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "src/__support/macros/attributes.h"
2020
#include "src/__support/macros/config.h"
2121
#include "src/__support/macros/optimization.h"
22+
#include "src/__support/threads/identifier.h"
2223
#include "src/__support/threads/linux/futex_utils.h"
2324
#include "src/__support/threads/linux/futex_word.h"
2425
#include "src/__support/threads/linux/raw_mutex.h"
@@ -336,8 +337,6 @@ class RwLock {
336337
LIBC_INLINE Role get_preference() const {
337338
return static_cast<Role>(preference);
338339
}
339-
// TODO: use cached thread id once implemented.
340-
LIBC_INLINE static pid_t gettid() { return syscall_impl<pid_t>(SYS_gettid); }
341340

342341
template <Role role> LIBC_INLINE LockResult try_lock(RwState &old) {
343342
if constexpr (role == Role::Reader) {
@@ -359,7 +358,7 @@ class RwLock {
359358
if (LIBC_LIKELY(old.compare_exchange_weak_with(
360359
state, old.set_writer_bit(), cpp::MemoryOrder::ACQUIRE,
361360
cpp::MemoryOrder::RELAXED))) {
362-
writer_tid.store(gettid(), cpp::MemoryOrder::RELAXED);
361+
writer_tid.store(internal::gettid(), cpp::MemoryOrder::RELAXED);
363362
return LockResult::Success;
364363
}
365364
// Notice that old is updated by the compare_exchange_weak_with
@@ -394,7 +393,7 @@ class RwLock {
394393
unsigned spin_count = LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT) {
395394
// Phase 1: deadlock detection.
396395
// A deadlock happens if this is a RAW/WAW lock in the same thread.
397-
if (writer_tid.load(cpp::MemoryOrder::RELAXED) == gettid())
396+
if (writer_tid.load(cpp::MemoryOrder::RELAXED) == internal::gettid())
398397
return LockResult::Deadlock;
399398

400399
#if LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
@@ -520,7 +519,7 @@ class RwLock {
520519
if (old.has_active_writer()) {
521520
// The lock is held by a writer.
522521
// Check if we are the owner of the lock.
523-
if (writer_tid.load(cpp::MemoryOrder::RELAXED) != gettid())
522+
if (writer_tid.load(cpp::MemoryOrder::RELAXED) != internal::gettid())
524523
return LockResult::PermissionDenied;
525524
// clear writer tid.
526525
writer_tid.store(0, cpp::MemoryOrder::RELAXED);

libc/src/unistd/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,13 @@ add_entrypoint_external(
333333
add_entrypoint_external(
334334
opterr
335335
)
336+
337+
add_entrypoint_object(
338+
gettid
339+
SRCS
340+
gettid.cpp
341+
HDRS
342+
gettid.h
343+
DEPENDS
344+
libc.src.__support.threads.identifier
345+
)

libc/src/unistd/gettid.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//===-- Implementation file for gettid --------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "src/unistd/gettid.h"
10+
#include "src/__support/common.h"
11+
#include "src/__support/threads/identifier.h"
12+
13+
namespace LIBC_NAMESPACE_DECL {
14+
15+
LLVM_LIBC_FUNCTION(pid_t, gettid, ()) { return internal::gettid(); }
16+
17+
} // namespace LIBC_NAMESPACE_DECL

libc/src/unistd/gettid.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//===-- Implementation header for gettid ------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_LIBC_SRC_UNISTD_GETTID_H
10+
#define LLVM_LIBC_SRC_UNISTD_GETTID_H
11+
12+
#include "hdr/types/pid_t.h"
13+
#include "src/__support/common.h"
14+
15+
namespace LIBC_NAMESPACE_DECL {
16+
17+
pid_t gettid();
18+
19+
} // namespace LIBC_NAMESPACE_DECL
20+
21+
#endif // LLVM_LIBC_SRC_UNISTD_GETTID_H

libc/src/unistd/linux/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ add_entrypoint_object(
103103
libc.src.__support.OSUtil.osutil
104104
libc.src.__support.threads.thread
105105
libc.src.errno.errno
106+
libc.src.__support.threads.identifier
106107
)
107108

108109
add_entrypoint_object(

libc/src/unistd/linux/fork.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "src/__support/common.h"
1313
#include "src/__support/macros/config.h"
1414
#include "src/__support/threads/fork_callbacks.h"
15+
#include "src/__support/threads/identifier.h"
1516
#include "src/__support/threads/thread.h" // For thread self object
1617

1718
#include "src/errno/libc_errno.h"
@@ -25,19 +26,25 @@ namespace LIBC_NAMESPACE_DECL {
2526

2627
LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
2728
invoke_prepare_callbacks();
29+
pid_t parent_tid = internal::gettid();
30+
// Invalidate parent's tid cache before forking. We cannot do this in child
31+
// process because in the post-fork instruction windows, there may be a signal
32+
// handler triggered which may get the wrong tid.
33+
internal::force_set_tid(0);
2834
#ifdef SYS_fork
29-
pid_t ret = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_fork);
35+
pid_t ret = syscall_impl<pid_t>(SYS_fork);
3036
#elif defined(SYS_clone)
31-
pid_t ret = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_clone, SIGCHLD, 0);
37+
pid_t ret = syscall_impl<pid_t>(SYS_clone, SIGCHLD, 0);
3238
#else
3339
#error "fork and clone syscalls not available."
3440
#endif
41+
3542
if (ret == 0) {
3643
// Return value is 0 in the child process.
3744
// The child is created with a single thread whose self object will be a
3845
// copy of parent process' thread which called fork. So, we have to fix up
3946
// the child process' self object with the new process' tid.
40-
self.attrib->tid = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_gettid);
47+
internal::force_set_tid(syscall_impl<pid_t>(SYS_gettid));
4148
invoke_child_callbacks();
4249
return 0;
4350
}
@@ -47,7 +54,8 @@ LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
4754
libc_errno = static_cast<int>(-ret);
4855
return -1;
4956
}
50-
57+
// recover parent's tid.
58+
internal::force_set_tid(parent_tid);
5159
invoke_parent_callbacks();
5260
return ret;
5361
}

libc/test/integration/src/unistd/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ add_integration_test(
3131
libc.src.sys.wait.wait4
3232
libc.src.sys.wait.waitpid
3333
libc.src.unistd.fork
34+
libc.src.unistd.gettid
35+
libc.src.__support.OSUtil.osutil
36+
libc.src.stdlib.exit
37+
libc.include.sys_syscall
3438
)
3539

3640
if((${LIBC_TARGET_OS} STREQUAL "linux") AND (${LIBC_TARGET_ARCHITECTURE_IS_X86}))

libc/test/integration/src/unistd/fork_test.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,21 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "src/__support/OSUtil/syscall.h"
910
#include "src/pthread/pthread_atfork.h"
1011
#include "src/signal/raise.h"
12+
#include "src/stdlib/exit.h"
1113
#include "src/sys/wait/wait.h"
1214
#include "src/sys/wait/wait4.h"
1315
#include "src/sys/wait/waitpid.h"
1416
#include "src/unistd/fork.h"
17+
#include "src/unistd/gettid.h"
1518

1619
#include "test/IntegrationTest/test.h"
1720

1821
#include <errno.h>
1922
#include <signal.h>
23+
#include <sys/syscall.h>
2024
#include <sys/wait.h>
2125
#include <unistd.h>
2226

@@ -140,7 +144,24 @@ void fork_with_atfork_callbacks() {
140144
ASSERT_NE(child, DONE);
141145
}
142146

147+
void gettid_test() {
148+
// fork and verify tid is consistent with the syscall result.
149+
int pid = LIBC_NAMESPACE::fork();
150+
ASSERT_EQ(LIBC_NAMESPACE::gettid(),
151+
LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_gettid));
152+
if (pid == 0)
153+
LIBC_NAMESPACE::exit(0);
154+
// make sure child process exits normally
155+
int status;
156+
pid_t cpid = LIBC_NAMESPACE::waitpid(pid, &status, 0);
157+
ASSERT_TRUE(cpid > 0);
158+
ASSERT_EQ(cpid, pid);
159+
ASSERT_TRUE(WIFEXITED(status));
160+
ASSERT_EQ(WEXITSTATUS(status), 0);
161+
}
162+
143163
TEST_MAIN(int argc, char **argv, char **envp) {
164+
gettid_test();
144165
fork_and_wait_normal_exit();
145166
fork_and_wait4_normal_exit();
146167
fork_and_waitpid_normal_exit();

0 commit comments

Comments
 (0)