This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH roland/nptl] NPTL: Don't (re)validate sched_priority in pthread_create.
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Tue, 18 Nov 2014 12:52:13 -0800 (PST)
- Subject: [PATCH roland/nptl] NPTL: Don't (re)validate sched_priority in pthread_create.
- Authentication-results: sourceware.org; auth=none
Now that pthread_attr_setschedparam validates the value, it should no
longer be possible to see an invalid value in pthread_create. I've
verified that in Linux the min/max values depend only on the policy
selection and not on any dynamic factors that might have changed between
pthread_attr_setschedparam and pthread_create.
The one wrinkle is if the user called pthread_attr_setschedparam before
pthread_attr_setschedpolicy and the sched_priority value was valid for the
old policy but invalid for the new one. POSIX does not seem to provide for
pthread_attr_setschedpolicy being able to return an error in this case, or
for it to do something else like reset the sched_param bits to defaults on
pthread_attr_setschedpolicy if the old bits don't make sense for the new
policy. So in this situation, pthread_create failing with EINVAL appears
to be the best we can do--and that's what we do now. But removing this
check in pthread_create doesn't change that behavior observably: the thread
will be started up and then create_thread will cancel it when the
sched_setscheduler syscall fails, propagating the error code (presumably
still EINVAL) back to pthread_create. So this user misuse of the API is
detected with significantly more overhead, but I don't think that matters.
Tested x86_64-linux-gnu.
If nobody objects soon, I'll commit this.
Thanks,
Roland
* nptl/pthread_create.c (__pthread_create_2_1): Don't try to validate
the sched_priority value here. It was already checked when the user
called pthread_attr_setschedparam.
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -605,6 +605,7 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
}
if (iattr->flags & ATTR_FLAG_SCHED_SET)
+ /* The values were validated in pthread_attr_setschedparam. */
memcpy (&pd->schedparam, &iattr->schedparam,
sizeof (struct sched_param));
else if ((pd->flags & ATTR_FLAG_SCHED_SET) == 0)
@@ -612,26 +613,6 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
INTERNAL_SYSCALL (sched_getparam, scerr, 2, 0, &pd->schedparam);
pd->flags |= ATTR_FLAG_SCHED_SET;
}
-
- /* Check for valid priorities. */
- int minprio = INTERNAL_SYSCALL (sched_get_priority_min, scerr, 1,
- iattr->schedpolicy);
- int maxprio = INTERNAL_SYSCALL (sched_get_priority_max, scerr, 1,
- iattr->schedpolicy);
- if (pd->schedparam.sched_priority < minprio
- || pd->schedparam.sched_priority > maxprio)
- {
- /* Perhaps a thread wants to change the IDs and if waiting
- for this stillborn thread. */
- if (__builtin_expect (atomic_exchange_acq (&pd->setxid_futex, 0)
- == -2, 0))
- lll_futex_wake (&pd->setxid_futex, 1, LLL_PRIVATE);
-
- __deallocate_stack (pd);
-
- retval = EINVAL;
- goto out;
- }
}
/* Pass the descriptor to the caller. */