Implement pthread_attr_getinheritsched/pthread_attr_setinheritsched.
Historically, Android defaulted to EXPLICIT but with a special case
because SCHED_NORMAL/priority 0 was awkward. Because the code couldn't
actually tell whether SCHED_NORMAL/priority 0 was a genuine attempt to
explicitly set those attributes (because the parent thread is SCHED_FIFO,
say) or just because the pthread_attr_t was left at its defaults.
Now we support INHERIT, we could call sched_getscheduler to see whether
we actually need to call sched_setscheduler, but since the major cost
is the fixed syscall overhead, we may as well just conservatively
call sched_setscheduler and let the kernel decide whether it's a
no-op. (Especially because we'd then have to add both sched_getscheduler
and sched_setscheduler to any seccomp filter.)
Platform code (or app code that only needs to support >= P) can actually
add a call to pthread_attr_setinheritsched to say that they just want
to inherit (if they know that none of their threads actually mess with
scheduler attributes at all), which will save them a sched_setscheduler
call except in the doubly-special case of SCHED_RESET_ON_FORK (which we
do handle).
An alternative would be "make pthread_attr_setschedparams and
pthread_attr_setschedprio set EXPLICIT and change the platform default
to INHERIT", but even though I can only think of weird pathological
examples where anyone would notice that change, that behavior -- of
pthread_attr_setschedparams/pthread_attr_setschedprio overriding an
earlier call to pthread_attr_setinheritsched -- isn't allowed by POSIX
(whereas defaulting to EXPLICIT is).
If we have a lot of trouble with this change in the app compatibility
testing phase, though, we'll want to reconsider this decision!
-*-
This change also removes a comment about setting the scheduler attributes
in main_thread because we'd have to actually keep them up to date,
and it's not clear that doing so would be worth the trouble.
Also make async_safe_format_log preserve errno so we don't have to be
so careful around it.
Bug: http://b/67471710
Test: ran tests
Change-Id: Idd026c4ce78a536656adcb57aa2e7b2c616eeddf
diff --git a/libc/async_safe/async_safe_log.cpp b/libc/async_safe/async_safe_log.cpp
index 78f62bd..d5aad16 100644
--- a/libc/async_safe/async_safe_log.cpp
+++ b/libc/async_safe/async_safe_log.cpp
@@ -47,6 +47,7 @@
#include <async_safe/log.h>
#include "private/CachedProperty.h"
+#include "private/ErrnoRestorer.h"
#include "private/ScopedPthreadMutexLocker.h"
// Must be kept in sync with frameworks/base/core/java/android/util/EventLog.java.
@@ -508,6 +509,7 @@
}
int async_safe_format_log_va_list(int priority, const char* tag, const char* format, va_list args) {
+ ErrnoRestorer errno_restorer;
char buffer[1024];
BufferOutputStream os(buffer, sizeof(buffer));
out_vformat(os, format, args);
diff --git a/libc/bionic/__libc_init_main_thread.cpp b/libc/bionic/__libc_init_main_thread.cpp
index 9cbff11..6d948ec 100644
--- a/libc/bionic/__libc_init_main_thread.cpp
+++ b/libc/bionic/__libc_init_main_thread.cpp
@@ -86,7 +86,6 @@
pthread_attr_init(&main_thread.attr);
main_thread.attr.guard_size = 0; // The main thread has no guard page.
main_thread.attr.stack_size = 0; // User code should never see this; we'll compute it when asked.
- // TODO: the main thread's sched_policy and sched_priority need to be queried.
// The TLS stack guard is set from the global, so ensure that we've initialized the global
// before we initialize the TLS. Dynamic executables will initialize their copy of the global
diff --git a/libc/bionic/pthread_attr.cpp b/libc/bionic/pthread_attr.cpp
index fc9e74a..0d79374 100644
--- a/libc/bionic/pthread_attr.cpp
+++ b/libc/bionic/pthread_attr.cpp
@@ -54,6 +54,23 @@
return 0;
}
+int pthread_attr_setinheritsched(pthread_attr_t* attr, int flag) {
+ if (flag == PTHREAD_EXPLICIT_SCHED) {
+ attr->flags &= ~PTHREAD_ATTR_FLAG_INHERIT;
+ } else if (flag == PTHREAD_INHERIT_SCHED) {
+ attr->flags |= PTHREAD_ATTR_FLAG_INHERIT;
+ } else {
+ return EINVAL;
+ }
+ return 0;
+}
+
+int pthread_attr_getinheritsched(const pthread_attr_t* attr, int* flag) {
+ *flag = (attr->flags & PTHREAD_ATTR_FLAG_INHERIT) ? PTHREAD_INHERIT_SCHED
+ : PTHREAD_EXPLICIT_SCHED;
+ return 0;
+}
+
int pthread_attr_setdetachstate(pthread_attr_t* attr, int state) {
if (state == PTHREAD_CREATE_DETACHED) {
attr->flags |= PTHREAD_ATTR_FLAG_DETACHED;
diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp
index be0fd1b..11e148c 100644
--- a/libc/bionic/pthread_create.cpp
+++ b/libc/bionic/pthread_create.cpp
@@ -109,7 +109,7 @@
}
int __init_thread(pthread_internal_t* thread) {
- int error = 0;
+ thread->cleanup_stack = nullptr;
if (__predict_true((thread->attr.flags & PTHREAD_ATTR_FLAG_DETACHED) == 0)) {
atomic_init(&thread->join_state, THREAD_NOT_JOINED);
@@ -117,23 +117,43 @@
atomic_init(&thread->join_state, THREAD_DETACHED);
}
- // Set the scheduling policy/priority of the thread.
- if (thread->attr.sched_policy != SCHED_NORMAL) {
- sched_param param;
+ // Set the scheduling policy/priority of the thread if necessary.
+ bool need_set = true;
+ int policy;
+ sched_param param;
+ if (thread->attr.flags & PTHREAD_ATTR_FLAG_INHERIT) {
+ // Unless the parent has SCHED_RESET_ON_FORK set, we've already inherited from the parent.
+ policy = sched_getscheduler(0);
+ need_set = ((policy & SCHED_RESET_ON_FORK) != 0);
+ if (need_set) {
+ if (policy == -1) {
+ async_safe_format_log(ANDROID_LOG_WARN, "libc",
+ "pthread_create sched_getscheduler failed: %s", strerror(errno));
+ return errno;
+ }
+ if (sched_getparam(0, ¶m) == -1) {
+ async_safe_format_log(ANDROID_LOG_WARN, "libc",
+ "pthread_create sched_getparam failed: %s", strerror(errno));
+ return errno;
+ }
+ }
+ } else {
+ policy = thread->attr.sched_policy;
param.sched_priority = thread->attr.sched_priority;
- if (sched_setscheduler(thread->tid, thread->attr.sched_policy, ¶m) == -1) {
+ }
+ if (need_set) {
+ if (sched_setscheduler(thread->tid, policy, ¶m) == -1) {
+ async_safe_format_log(ANDROID_LOG_WARN, "libc",
+ "pthread_create sched_setscheduler(%d, {%d}) call failed: %s", policy,
+ param.sched_priority, strerror(errno));
#if defined(__LP64__)
// For backwards compatibility reasons, we only report failures on 64-bit devices.
- error = errno;
+ return errno;
#endif
- async_safe_format_log(ANDROID_LOG_WARN, "libc",
- "pthread_create sched_setscheduler call failed: %s", strerror(errno));
}
}
- thread->cleanup_stack = NULL;
-
- return error;
+ return 0;
}
static void* __create_thread_mapped_space(size_t mmap_size, size_t stack_guard_size) {
diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index ad8be66..2dd0cff 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -34,12 +34,15 @@
#include "private/bionic_lock.h"
#include "private/bionic_tls.h"
-/* Has the thread been detached by a pthread_join or pthread_detach call? */
+// Has the thread been detached by a pthread_join or pthread_detach call?
#define PTHREAD_ATTR_FLAG_DETACHED 0x00000001
-/* Has the thread been joined by another thread? */
+// Has the thread been joined by another thread?
#define PTHREAD_ATTR_FLAG_JOINED 0x00000002
+// Should we inherit scheduling attributes from the parent on pthread_create?
+#define PTHREAD_ATTR_FLAG_INHERIT 0x00000004
+
class pthread_key_data_t {
public:
uintptr_t seq; // Use uintptr_t just for alignment, as we use pointer below.
diff --git a/libc/include/bits/posix_limits.h b/libc/include/bits/posix_limits.h
index e5846d6..17a39a4 100644
--- a/libc/include/bits/posix_limits.h
+++ b/libc/include/bits/posix_limits.h
@@ -77,7 +77,7 @@
#define _POSIX_THREAD_CPUTIME _POSIX_VERSION /* CLOCK_THREAD_CPUTIME_ID. */
#define _POSIX_THREAD_PRIO_INHERIT __BIONIC_POSIX_FEATURE_MISSING
#define _POSIX_THREAD_PRIO_PROTECT __BIONIC_POSIX_FEATURE_MISSING
-#define _POSIX_THREAD_PRIORITY_SCHEDULING _POSIX_VERSION /* Strictly, pthread_attr_getinheritsched/pthread_attr_setinheritsched are missing. */
+#define _POSIX_THREAD_PRIORITY_SCHEDULING _POSIX_VERSION /* Strictly, pthread_attr_getinheritsched/pthread_attr_setinheritsched arrived in 28. */
#define _POSIX_THREAD_PROCESS_SHARED _POSIX_VERSION
#define _POSIX_THREAD_ROBUST_PRIO_INHERIT __BIONIC_POSIX_FEATURE_MISSING
#define _POSIX_THREAD_ROBUST_PRIO_PROTECT __BIONIC_POSIX_FEATURE_MISSING
diff --git a/libc/include/pthread.h b/libc/include/pthread.h
index 2253e0d..99515da 100644
--- a/libc/include/pthread.h
+++ b/libc/include/pthread.h
@@ -74,20 +74,24 @@
#define PTHREAD_STACK_MIN (2 * PAGE_SIZE)
#endif
-#define PTHREAD_CREATE_DETACHED 0x00000001
-#define PTHREAD_CREATE_JOINABLE 0x00000000
+#define PTHREAD_CREATE_DETACHED 1
+#define PTHREAD_CREATE_JOINABLE 0
-#define PTHREAD_PROCESS_PRIVATE 0
-#define PTHREAD_PROCESS_SHARED 1
+#define PTHREAD_EXPLICIT_SCHED 0
+#define PTHREAD_INHERIT_SCHED 1
-#define PTHREAD_SCOPE_SYSTEM 0
-#define PTHREAD_SCOPE_PROCESS 1
+#define PTHREAD_PROCESS_PRIVATE 0
+#define PTHREAD_PROCESS_SHARED 1
+
+#define PTHREAD_SCOPE_SYSTEM 0
+#define PTHREAD_SCOPE_PROCESS 1
int pthread_atfork(void (*__prepare)(void), void (*__parent)(void), void (*__child)(void)) __INTRODUCED_IN(12);
int pthread_attr_destroy(pthread_attr_t* __attr);
int pthread_attr_getdetachstate(const pthread_attr_t* __attr, int* __state);
int pthread_attr_getguardsize(const pthread_attr_t* __attr, size_t* __size);
+int pthread_attr_getinheritsched(const pthread_attr_t* __attr, int* __flag);
int pthread_attr_getschedparam(const pthread_attr_t* __attr, struct sched_param* __param);
int pthread_attr_getschedpolicy(const pthread_attr_t* __attr, int* __policy);
int pthread_attr_getscope(const pthread_attr_t* __attr, int* __scope);
@@ -96,6 +100,7 @@
int pthread_attr_init(pthread_attr_t* __attr);
int pthread_attr_setdetachstate(pthread_attr_t* __attr, int __state);
int pthread_attr_setguardsize(pthread_attr_t* __attr, size_t __size);
+int pthread_attr_setinheritsched(pthread_attr_t* __attr, int __flag);
int pthread_attr_setschedparam(pthread_attr_t* __attr, const struct sched_param* __param);
int pthread_attr_setschedpolicy(pthread_attr_t* __attr, int __policy);
int pthread_attr_setscope(pthread_attr_t* __attr, int __scope);
diff --git a/libc/libc.arm.map b/libc/libc.arm.map
index 981dd59..3433398 100644
--- a/libc/libc.arm.map
+++ b/libc/libc.arm.map
@@ -1362,6 +1362,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.arm64.map b/libc/libc.arm64.map
index 29c5235..aa82b39 100644
--- a/libc/libc.arm64.map
+++ b/libc/libc.arm64.map
@@ -1282,6 +1282,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.map.txt b/libc/libc.map.txt
index eafbbd7..d3d8e0f 100644
--- a/libc/libc.map.txt
+++ b/libc/libc.map.txt
@@ -1387,6 +1387,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.mips.map b/libc/libc.mips.map
index a32131f..62e0737 100644
--- a/libc/libc.mips.map
+++ b/libc/libc.mips.map
@@ -1346,6 +1346,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.mips64.map b/libc/libc.mips64.map
index 29c5235..aa82b39 100644
--- a/libc/libc.mips64.map
+++ b/libc/libc.mips64.map
@@ -1282,6 +1282,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.x86.map b/libc/libc.x86.map
index f1308ea..5cacee1 100644
--- a/libc/libc.x86.map
+++ b/libc/libc.x86.map
@@ -1344,6 +1344,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;
diff --git a/libc/libc.x86_64.map b/libc/libc.x86_64.map
index 29c5235..aa82b39 100644
--- a/libc/libc.x86_64.map
+++ b/libc/libc.x86_64.map
@@ -1282,6 +1282,8 @@
posix_spawn_file_actions_destroy;
posix_spawn_file_actions_init;
posix_spawnp;
+ pthread_attr_getinheritsched;
+ pthread_attr_setinheritsched;
pthread_setschedprio;
sethostent;
setnetent;