set explicit scheduling for new thread from calling thread, not self
authorRich Felker <dalias@aerifal.cx>
Fri, 6 Sep 2019 19:26:44 +0000 (15:26 -0400)
committerRich Felker <dalias@aerifal.cx>
Fri, 6 Sep 2019 19:54:32 +0000 (15:54 -0400)
if setting scheduling properties succeeds, the new thread may end up
with lower priority than the caller, and may be unable to continue
running due to another intermediate-priority thread. this produces a
priority inversion situation for the thread calling pthread_create,
since it cannot return until the new thread reports success.

originally, the parent was responsible for setting the new thread's
priority; commits b8742f32602add243ee2ce74d804015463726899 and
40bae2d32fd6f3ffea437fa745ad38a1fe77b27e changed it as part of
trimming down the pthread structure. since then, commit
04335d9260c076cf4d9264bd93dd3b06c237a639 partly reversed the changes,
but did not switch responsibilities back. do that now.

src/thread/pthread_create.c

index edaf9a6e2022acfec53dff13b6b535acebfacbea..edcdf041bf7b241d18da3361085648d3c8768b9f 100644 (file)
@@ -172,22 +172,19 @@ void __do_cleanup_pop(struct __ptcb *cb)
 struct start_args {
        void *(*start_func)(void *);
        void *start_arg;
-       pthread_attr_t *attr;
-       volatile int *perr;
+       volatile int control;
        unsigned long sig_mask[_NSIG/8/sizeof(long)];
 };
 
 static int start(void *p)
 {
        struct start_args *args = p;
-       if (args->attr) {
-               pthread_t self = __pthread_self();
-               int ret = -__syscall(SYS_sched_setscheduler, self->tid,
-                       args->attr->_a_policy, &args->attr->_a_prio);
-               if (a_swap(args->perr, ret)==-2)
-                       __wake(args->perr, 1, 1);
-               if (ret) {
-                       self->detach_state = DT_DETACHED;
+       int state = args->control;
+       if (state) {
+               if (a_cas(&args->control, 1, 2)==1)
+                       __wait(&args->control, 0, 2, 1);
+               if (args->control) {
+                       __pthread_self()->detach_state = DT_DETACHED;
                        __pthread_exit(0);
                }
        }
@@ -233,7 +230,6 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
                | CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID | CLONE_DETACHED;
        pthread_attr_t attr = { 0 };
        sigset_t set;
-       volatile int err = -1;
 
        if (!libc.can_do_threads) return ENOSYS;
        self = __pthread_self();
@@ -325,13 +321,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
        struct start_args *args = (void *)stack;
        args->start_func = entry;
        args->start_arg = arg;
-       if (attr._a_sched) {
-               args->attr = &attr;
-               args->perr = &err;
-       } else {
-               args->attr = 0;
-               args->perr = 0;
-       }
+       args->control = attr._a_sched ? 1 : 0;
 
        /* Application signals (but not the synccall signal) must be
         * blocked before the thread list lock can be taken, to ensure
@@ -369,9 +359,10 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
        }
 
        if (attr._a_sched) {
-               if (a_cas(&err, -1, -2)==-1)
-                       __wait(&err, 0, -2, 1);
-               ret = err;
+               int ret = -__syscall(SYS_sched_setscheduler, new->tid,
+                       attr._a_policy, &attr._a_prio);
+               if (a_swap(&args->control, ret ? 3 : 0)==2)
+                       __wake(&args->control, 1, 1);
                if (ret) return ret;
        }