commit 8aecba7aa6b7f7b92f69c0d3febef59fdb135f87
author Elliott Hughes <enh@google.com> Tue Oct 17 15:34:41 2017 -0700
committer Elliott Hughes <enh@google.com> Wed Oct 25 14:28:42 2017 -0700
tree fd3d9743a0e282dcbb2a8b2b51906f7da3a84468
parent 435e6384de8f9e35b8878b1ccda5bb5686c15207

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

libc/bionic/pthread_create.cpp

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, &param) == -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, &param) == -1) {
+  }
+  if (need_set) {
+    if (sched_setscheduler(thread->tid, policy, &param) == -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) {