This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH roland/nptl] NPTL: Don't (re)validate sched_priority in pthread_create.


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.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]