NPTL: Refactor createthread.c
authorRoland McGrath <roland@hack.frob.com>
Tue, 18 Nov 2014 19:03:00 +0000 (11:03 -0800)
committerRoland McGrath <roland@hack.frob.com>
Tue, 18 Nov 2014 19:03:00 +0000 (11:03 -0800)
ChangeLog
nptl/createthread.c
nptl/pthread_create.c

index 2e3359ac038eeb01723724d59c68d6198cef7d0c..a5c08b10711a2b83883218f9bdfb3299969b032c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2014-11-18  Roland McGrath  <roland@hack.frob.com>
+
+       * nptl/createthread.c: Add proper top-line comment.
+       (do_clone): Folded into ...
+       (create_thread): ... here.  Take new arguments STOPPED_START and
+       THREAD_RAN.  Always set PD->stopped_start to something here.  Don't
+       increment __nptl_threads, do event-reporting logic, do
+       CHECK_THREAD_SYSINFO, or set THREAD_SELF->header.multiple_threads
+       here.  Set *THREAD_RAN after ARCH_CLONE call succeeds.  Don't do any
+       resource cleanup if sched_setaffinity or sched_setscheduler fails,
+       just send SIGCANCEL.
+       * nptl/pthread_create.c: Forward-declare create_thread before
+       including createthread.c.
+       (start_thread): Use new macro START_THREAD_DEFN to replace defining
+       declaration, and new macro START_THREAD_SELF to replace argument.
+       Remove return statement.
+       (report_thread_creation): New function.
+       (__pthread_create_2_1): Use it.  Do TD_CREATE reporting,
+       synchronization logic, and __nptl_nthreads increment here, around
+       calling create_thread.  Do CHECK_THREAD_SYSINFO and initialize
+       PD->parent_cancelhandling here, before create_thread.  When
+       create_thread fails, do __nptl_nthreads decrement, setxid_futex wake,
+       __deallocate_stack, and ENOMEM translation here.
+
 2014-11-18  Joseph Myers  <joseph@codesourcery.com>
 
        [BZ #17616]
index 49442d9f237ad9a75a5bf023ed030f3b995feafa..7b05d48954816fdcc1d48d7ed18c49bef275680f 100644 (file)
@@ -1,4 +1,5 @@
-/* Copyright (C) 2002-2014 Free Software Foundation, Inc.
+/* Low-level thread creation for NPTL.  Linux version.
+   Copyright (C) 2002-2014 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
 #include <arch-fork.h>
 
 
-#define CLONE_SIGNAL           (CLONE_SIGHAND | CLONE_THREAD)
-
-
 #ifndef ARCH_CLONE
 # define ARCH_CLONE __clone
 #endif
 
+/* See the comments in pthread_create.c for the requirements for these
+   two macros and the create_thread function.  */
+
+#define START_THREAD_DEFN \
+  static int __attribute__ ((noreturn)) start_thread (void *arg)
+#define START_THREAD_SELF arg
+
+/* pthread_create.c defines this using START_THREAD_DEFN
+   We need a forward declaration here so we can take its address.  */
+static int start_thread (void *arg) __attribute__ ((noreturn));
 
 static int
-do_clone (struct pthread *pd, const struct pthread_attr *attr,
-         int clone_flags, int (*fct) (void *), STACK_VARIABLES_PARMS,
-         int stopped)
+create_thread (struct pthread *pd, const struct pthread_attr *attr,
+              bool stopped_start, STACK_VARIABLES_PARMS, bool *thread_ran)
 {
-  TLS_DEFINE_INIT_TP (tp, pd);
+  /* Determine whether the newly created threads has to be started
+     stopped since we have to set the scheduling parameters or set the
+     affinity.  */
+  if (attr != NULL
+      && (__glibc_unlikely (attr->cpuset != NULL)
+         || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)))
+    stopped_start = true;
 
-  if (__glibc_unlikely (stopped != 0))
+  pd->stopped_start = stopped_start;
+  if (__glibc_unlikely (stopped_start))
     /* We make sure the thread does not run far by forcing it to get a
        lock.  We lock it here too so that the new thread cannot continue
        until we tell it to.  */
     lll_lock (pd->lock, LLL_PRIVATE);
 
-  /* One more thread.  We cannot have the thread do this itself, since it
-     might exist but not have been scheduled yet by the time we've returned
-     and need to check the value to behave correctly.  We must do it before
-     creating the thread, in case it does get scheduled first and then
-     might mistakenly think it was the only thread.  In the failure case,
-     we momentarily store a false value; this doesn't matter because there
-     is no kosher thing a signal handler interrupting us right here can do
-     that cares whether the thread count is correct.  */
-  atomic_increment (&__nptl_nthreads);
+  /* We rely heavily on various flags the CLONE function understands:
 
-  int rc = ARCH_CLONE (fct, STACK_VARIABLES_ARGS, clone_flags,
-                      pd, &pd->tid, tp, &pd->tid);
+     CLONE_VM, CLONE_FS, CLONE_FILES
+       These flags select semantics with shared address space and
+       file descriptors according to what POSIX requires.
 
-  if (__glibc_unlikely (rc == -1))
-    {
-      atomic_decrement (&__nptl_nthreads); /* Oops, we lied for a second.  */
+     CLONE_SIGHAND, CLONE_THREAD
+       This flag selects the POSIX signal semantics and various
+       other kinds of sharing (itimers, POSIX timers, etc.).
 
-      /* 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);
+     CLONE_SETTLS
+       The sixth parameter to CLONE determines the TLS area for the
+       new thread.
 
-      /* Free the resources.  */
-       __deallocate_stack (pd);
+     CLONE_PARENT_SETTID
+       The kernels writes the thread ID of the newly created thread
+       into the location pointed to by the fifth parameters to CLONE.
 
-      /* We have to translate error codes.  */
-      return errno == ENOMEM ? EAGAIN : errno;
-    }
+       Note that it would be semantically equivalent to use
+       CLONE_CHILD_SETTID but it is be more expensive in the kernel.
+
+     CLONE_CHILD_CLEARTID
+       The kernels clears the thread ID of a thread that has called
+       sys_exit() in the location pointed to by the seventh parameter
+       to CLONE.
+
+     The termination signal is chosen to be zero which means no signal
+     is sent.  */
+  const int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM
+                          | CLONE_SIGHAND | CLONE_THREAD
+                          | CLONE_SETTLS | CLONE_PARENT_SETTID
+                          | CLONE_CHILD_CLEARTID
+                          | 0);
+
+  TLS_DEFINE_INIT_TP (tp, pd);
+
+  if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS,
+                                   clone_flags, pd, &pd->tid, tp, &pd->tid)
+                       == -1))
+    return errno;
+
+  /* It's started now, so if we fail below, we'll have to cancel it
+     and let it clean itself up.  */
+  *thread_ran = true;
 
   /* Now we have the possibility to set scheduling parameters etc.  */
-  if (__glibc_unlikely (stopped != 0))
+  if (attr != NULL)
     {
       INTERNAL_SYSCALL_DECL (err);
-      int res = 0;
+      int res;
 
       /* Set the affinity mask if necessary.  */
       if (attr->cpuset != NULL)
        {
+         assert (stopped_start);
+
          res = INTERNAL_SYSCALL (sched_setaffinity, err, 3, pd->tid,
                                  attr->cpusetsize, attr->cpuset);
 
          if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err)))
+         err_out:
            {
-             /* The operation failed.  We have to kill the thread.  First
-                send it the cancellation signal.  */
+             /* The operation failed.  We have to kill the thread.
+                We let the normal cancellation mechanism do the work.  */
+
              INTERNAL_SYSCALL_DECL (err2);
-           err_out:
              (void) INTERNAL_SYSCALL (tgkill, err2, 3,
                                       THREAD_GETMEM (THREAD_SELF, pid),
                                       pd->tid, SIGCANCEL);
 
-             /* We do not free the stack here because the canceled thread
-                itself will do this.  */
-
-             return (INTERNAL_SYSCALL_ERROR_P (res, err)
-                     ? INTERNAL_SYSCALL_ERRNO (res, err)
-                     : 0);
+             return INTERNAL_SYSCALL_ERRNO (res, err);
            }
        }
 
       /* Set the scheduling parameters.  */
       if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)
        {
+         assert (stopped_start);
+
          res = INTERNAL_SYSCALL (sched_setscheduler, err, 3, pd->tid,
                                  pd->schedpolicy, &pd->schedparam);
 
@@ -121,120 +150,5 @@ do_clone (struct pthread *pd, const struct pthread_attr *attr,
        }
     }
 
-  /* We now have for sure more than one thread.  The main thread might
-     not yet have the flag set.  No need to set the global variable
-     again if this is what we use.  */
-  THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1);
-
   return 0;
 }
-
-
-static int
-create_thread (struct pthread *pd, const struct pthread_attr *attr,
-              STACK_VARIABLES_PARMS)
-{
-#if TLS_TCB_AT_TP
-  assert (pd->header.tcb != NULL);
-#endif
-
-  /* We rely heavily on various flags the CLONE function understands:
-
-     CLONE_VM, CLONE_FS, CLONE_FILES
-       These flags select semantics with shared address space and
-       file descriptors according to what POSIX requires.
-
-     CLONE_SIGNAL
-       This flag selects the POSIX signal semantics.
-
-     CLONE_SETTLS
-       The sixth parameter to CLONE determines the TLS area for the
-       new thread.
-
-     CLONE_PARENT_SETTID
-       The kernels writes the thread ID of the newly created thread
-       into the location pointed to by the fifth parameters to CLONE.
-
-       Note that it would be semantically equivalent to use
-       CLONE_CHILD_SETTID but it is be more expensive in the kernel.
-
-     CLONE_CHILD_CLEARTID
-       The kernels clears the thread ID of a thread that has called
-       sys_exit() in the location pointed to by the seventh parameter
-       to CLONE.
-
-     The termination signal is chosen to be zero which means no signal
-     is sent.  */
-  int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGNAL
-                    | CLONE_SETTLS | CLONE_PARENT_SETTID
-                    | CLONE_CHILD_CLEARTID | CLONE_SYSVSEM
-                    | 0);
-
-  if (__glibc_unlikely (THREAD_GETMEM (THREAD_SELF, report_events)))
-    {
-      /* The parent thread is supposed to report events.  Check whether
-        the TD_CREATE event is needed, too.  */
-      const int _idx = __td_eventword (TD_CREATE);
-      const uint32_t _mask = __td_eventmask (TD_CREATE);
-
-      if ((_mask & (__nptl_threads_events.event_bits[_idx]
-                   | pd->eventbuf.eventmask.event_bits[_idx])) != 0)
-       {
-         /* We always must have the thread start stopped.  */
-         pd->stopped_start = true;
-
-         /* Create the thread.  We always create the thread stopped
-            so that it does not get far before we tell the debugger.  */
-         int res = do_clone (pd, attr, clone_flags, start_thread,
-                             STACK_VARIABLES_ARGS, 1);
-         if (res == 0)
-           {
-             /* Now fill in the information about the new thread in
-                the newly created thread's data structure.  We cannot let
-                the new thread do this since we don't know whether it was
-                already scheduled when we send the event.  */
-             pd->eventbuf.eventnum = TD_CREATE;
-             pd->eventbuf.eventdata = pd;
-
-             /* Enqueue the descriptor.  */
-             do
-               pd->nextevent = __nptl_last_event;
-             while (atomic_compare_and_exchange_bool_acq (&__nptl_last_event,
-                                                          pd, pd->nextevent)
-                    != 0);
-
-             /* Now call the function which signals the event.  */
-             __nptl_create_event ();
-
-             /* And finally restart the new thread.  */
-             lll_unlock (pd->lock, LLL_PRIVATE);
-           }
-
-         return res;
-       }
-    }
-
-#ifdef NEED_DL_SYSINFO
-  CHECK_THREAD_SYSINFO (pd);
-#endif
-
-  /* Determine whether the newly created threads has to be started
-     stopped since we have to set the scheduling parameters or set the
-     affinity.  */
-  bool stopped = false;
-  if (attr != NULL && (attr->cpuset != NULL
-                      || (attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))
-    stopped = true;
-  pd->stopped_start = stopped;
-  pd->parent_cancelhandling = THREAD_GETMEM (THREAD_SELF, cancelhandling);
-
-  /* Actually create the thread.  */
-  int res = do_clone (pd, attr, clone_flags, start_thread,
-                     STACK_VARIABLES_ARGS, stopped);
-
-  if (res == 0 && stopped)
-    /* And finally restart the new thread.  */
-    lll_unlock (pd->lock, LLL_PRIVATE);
-
-  return res;
-}
index 0055634cd3706fffa60e9181845368f9c6aa3b4c..da3dc4603f1e5c063286d7315b4246b9dd7831ba 100644 (file)
 #include <stap-probe.h>
 
 
-/* Local function to start thread and handle cleanup.  */
-static int start_thread (void *arg);
-
-
 /* Nozero if debugging mode is enabled.  */
 int __pthread_debug;
 
@@ -56,7 +52,27 @@ unsigned int __nptl_nthreads = 1;
 /* Code to allocate and deallocate a stack.  */
 #include "allocatestack.c"
 
-/* Code to create the thread.  */
+/* createthread.c defines this function, and two macros:
+   START_THREAD_DEFN and START_THREAD_SELF (see below).
+
+   create_thread is obliged to initialize PD->stopped_start.  It
+   should be true if the STOPPED_START parameter is true, or if
+   create_thread needs the new thread to synchronize at startup for
+   some other implementation reason.  If PD->stopped_start will be
+   true, then create_thread is obliged to perform the operation
+   "lll_lock (PD->lock, LLL_PRIVATE)" before starting the thread.
+
+   The return value is zero for success or an errno code for failure.
+   If the return value is ENOMEM, that will be translated to EAGAIN,
+   so create_thread need not do that.  On failure, *THREAD_RAN should
+   be set to true iff the thread actually started up and then got
+   cancelled before calling user code (*PD->start_routine), in which
+   case it is responsible for doing its own cleanup.  */
+
+static int create_thread (struct pthread *pd, const struct pthread_attr *attr,
+                         bool stopped_start, STACK_VARIABLES_PARMS,
+                         bool *thread_ran);
+
 #include <createthread.c>
 
 
@@ -228,10 +244,14 @@ __free_tcb (struct pthread *pd)
 }
 
 
-static int
-start_thread (void *arg)
+/* Local function to start thread and handle cleanup.
+   createthread.c defines the macro START_THREAD_DEFN to the
+   declaration that its create_thread function will refer to, and
+   START_THREAD_SELF to the expression to optimally deliver the new
+   thread's THREAD_SELF value.  */
+START_THREAD_DEFN
 {
-  struct pthread *pd = (struct pthread *) arg;
+  struct pthread *pd = START_THREAD_SELF;
 
 #if HP_TIMING_AVAIL
   /* Remember the time when the thread was started.  */
@@ -439,7 +459,24 @@ start_thread (void *arg)
   __exit_thread ();
 
   /* NOTREACHED */
-  return 0;
+}
+
+
+/* Return true iff obliged to report TD_CREATE events.  */
+static bool
+report_thread_creation (struct pthread *pd)
+{
+  if (__glibc_unlikely (THREAD_GETMEM (THREAD_SELF, report_events)))
+    {
+      /* The parent thread is supposed to report events.
+        Check whether the TD_CREATE event is needed, too.  */
+      const size_t idx = __td_eventword (TD_CREATE);
+      const uint32_t mask = __td_eventmask (TD_CREATE);
+
+      return ((mask & (__nptl_threads_events.event_bits[idx]
+                      | pd->eventbuf.eventmask.event_bits[idx])) != 0);
+    }
+  return false;
 }
 
 
@@ -543,6 +580,15 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
   THREAD_COPY_POINTER_GUARD (pd);
 #endif
 
+  /* Verify the sysinfo bits were copied in allocate_stack if needed.  */
+#ifdef NEED_DL_SYSINFO
+  CHECK_THREAD_SYSINFO (pd);
+#endif
+
+  /* Inform start_thread (above) about cancellation state that might
+     translate into inherited signal state.  */
+  pd->parent_cancelhandling = THREAD_GETMEM (THREAD_SELF, cancelhandling);
+
   /* Determine scheduling parameters for the thread.  */
   if (__builtin_expect ((iattr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0, 0)
       && (iattr->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET)) != 0)
@@ -593,8 +639,95 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
 
   LIBC_PROBE (pthread_create, 4, newthread, attr, start_routine, arg);
 
+  /* One more thread.  We cannot have the thread do this itself, since it
+     might exist but not have been scheduled yet by the time we've returned
+     and need to check the value to behave correctly.  We must do it before
+     creating the thread, in case it does get scheduled first and then
+     might mistakenly think it was the only thread.  In the failure case,
+     we momentarily store a false value; this doesn't matter because there
+     is no kosher thing a signal handler interrupting us right here can do
+     that cares whether the thread count is correct.  */
+  atomic_increment (&__nptl_nthreads);
+
+  bool thread_ran = false;
+
   /* Start the thread.  */
-  retval = create_thread (pd, iattr, STACK_VARIABLES_ARGS);
+  if (__glibc_unlikely (report_thread_creation (pd)))
+    {
+      /* Create the thread.  We always create the thread stopped
+        so that it does not get far before we tell the debugger.  */
+      retval = create_thread (pd, iattr, true, STACK_VARIABLES_ARGS,
+                             &thread_ran);
+      if (retval == 0)
+       {
+         /* create_thread should have set this so that the logic below can
+            test it.  */
+         assert (pd->stopped_start);
+
+         /* Now fill in the information about the new thread in
+            the newly created thread's data structure.  We cannot let
+            the new thread do this since we don't know whether it was
+            already scheduled when we send the event.  */
+         pd->eventbuf.eventnum = TD_CREATE;
+         pd->eventbuf.eventdata = pd;
+
+         /* Enqueue the descriptor.  */
+         do
+           pd->nextevent = __nptl_last_event;
+         while (atomic_compare_and_exchange_bool_acq (&__nptl_last_event,
+                                                      pd, pd->nextevent)
+                != 0);
+
+         /* Now call the function which signals the event.  */
+         __nptl_create_event ();
+       }
+    }
+  else
+    retval = create_thread (pd, iattr, false, STACK_VARIABLES_ARGS,
+                           &thread_ran);
+
+  if (__glibc_unlikely (retval != 0))
+    {
+      /* If thread creation "failed", that might mean that the thread got
+        created and ran a little--short of running user code--but then
+        create_thread cancelled it.  In that case, the thread will do all
+        its own cleanup just like a normal thread exit after a successful
+        creation would do.  */
+
+      if (thread_ran)
+       assert (pd->stopped_start);
+      else
+       {
+         /* Oops, we lied for a second.  */
+         atomic_decrement (&__nptl_nthreads);
+
+         /* Perhaps a thread wants to change the IDs and is waiting for this
+            stillborn thread.  */
+         if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0)
+                               == -2))
+           lll_futex_wake (&pd->setxid_futex, 1, LLL_PRIVATE);
+
+         /* Free the resources.  */
+         __deallocate_stack (pd);
+       }
+
+      /* We have to translate error codes.  */
+      if (retval == ENOMEM)
+       retval = EAGAIN;
+    }
+  else
+    {
+      if (pd->stopped_start)
+       /* The thread blocked on this lock either because we're doing TD_CREATE
+          event reporting, or for some other reason that create_thread chose.
+          Now let it run free.  */
+       lll_unlock (pd->lock, LLL_PRIVATE);
+
+      /* We now have for sure more than one thread.  The main thread might
+        not yet have the flag set.  No need to set the global variable
+        again if this is what we use.  */
+      THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1);
+    }
 
  out:
   if (__glibc_unlikely (free_cpuset))