[02/11] nptl: Move createthread to pthread_create

Message ID 20210526165728.1772546-3-adhemerval.zanella@linaro.org
State Committed
Headers
Series nptl: pthread cancellation refactor |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella May 26, 2021, 4:57 p.m. UTC
  The 'create_thread' function is moved to pthread_create.c.  It removes
the START_THREAD_DEFN and START_THREAD_SELF macros and make the
lock usage more clear (no need to cross-reference multiple files).

No functional change.
---
 nptl/createthread.c   | 153 ------------------------------------------
 nptl/pthread_create.c | 120 +++++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 160 deletions(-)
 delete mode 100644 nptl/createthread.c
  

Comments

Florian Weimer May 26, 2021, 5:16 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> The 'create_thread' function is moved to pthread_create.c.  It removes
> the START_THREAD_DEFN and START_THREAD_SELF macros and make the
> lock usage more clear (no need to cross-reference multiple files).
>
> No functional change.

Please remove the remaining references to the macros

nptl/pthread_create.c:   unlocks PD->lock which synchronizes-with START_THREAD_DEFN in the
nptl/pthread_create.c:   createthread.c defines the macro START_THREAD_DEFN to the
nptl/pthread_create.c:   START_THREAD_SELF to the expression to optimally deliver the new

and commit this separately.  Thanks.

Florian
  
Adhemerval Zanella May 26, 2021, 5:29 p.m. UTC | #2
On 26/05/2021 14:16, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> The 'create_thread' function is moved to pthread_create.c.  It removes
>> the START_THREAD_DEFN and START_THREAD_SELF macros and make the
>> lock usage more clear (no need to cross-reference multiple files).
>>
>> No functional change.
> 
> Please remove the remaining references to the macros
> 
> nptl/pthread_create.c:   unlocks PD->lock which synchronizes-with START_THREAD_DEFN in the
> nptl/pthread_create.c:   createthread.c defines the macro START_THREAD_DEFN to the
> nptl/pthread_create.c:   START_THREAD_SELF to the expression to optimally deliver the new
> 
> and commit this separately.  Thanks.

Ack, I will remove them.
  

Patch

diff --git a/nptl/createthread.c b/nptl/createthread.c
deleted file mode 100644
index bc3409b326..0000000000
--- a/nptl/createthread.c
+++ /dev/null
@@ -1,153 +0,0 @@ 
-/* Low-level thread creation for NPTL.  Linux version.
-   Copyright (C) 2002-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <setjmp.h>
-#include <signal.h>
-#include <stdlib.h>
-#include <atomic.h>
-#include <ldsodefs.h>
-#include <tls.h>
-#include <stdint.h>
-
-#include <arch-fork.h>
-
-#ifdef __NR_clone2
-# define ARCH_CLONE __clone2
-#else
-# 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
-create_thread (struct pthread *pd, const struct pthread_attr *attr,
-	       bool *stopped_start, STACK_VARIABLES_PARMS, bool *thread_ran)
-{
-  /* Determine whether the newly created threads has to be started
-     stopped since we have to set the scheduling parameters or set the
-     affinity.  */
-  bool need_setaffinity = (attr != NULL && attr->extension != NULL
-			   && attr->extension->cpuset != 0);
-  if (attr != NULL
-      && (__glibc_unlikely (need_setaffinity)
-	  || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)))
-    *stopped_start = true;
-
-  pd->stopped_start = *stopped_start;
-  if (__glibc_unlikely (*stopped_start))
-    /* See CONCURRENCY NOTES in nptl/pthread_creat.c.  */
-    lll_lock (pd->lock, LLL_PRIVATE);
-
-  /* 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_SIGHAND, CLONE_THREAD
-	This flag selects the POSIX signal semantics and various
-	other kinds of sharing (itimers, POSIX timers, etc.).
-
-     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.  */
-  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 (attr != NULL)
-    {
-      int res;
-
-      /* Set the affinity mask if necessary.  */
-      if (need_setaffinity)
-	{
-	  assert (*stopped_start);
-
-	  res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid,
-				       attr->extension->cpusetsize,
-				       attr->extension->cpuset);
-
-	  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res)))
-	  err_out:
-	    {
-	      /* The operation failed.  We have to kill the thread.
-		 We let the normal cancellation mechanism do the work.  */
-
-	      pid_t pid = __getpid ();
-	      INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL);
-
-	      return INTERNAL_SYSCALL_ERRNO (res);
-	    }
-	}
-
-      /* Set the scheduling parameters.  */
-      if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)
-	{
-	  assert (*stopped_start);
-
-	  res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid,
-				       pd->schedpolicy, &pd->schedparam);
-
-	  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res)))
-	    goto err_out;
-	}
-    }
-
-  return 0;
-}
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 5680687efe..b53e3f30a0 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -215,9 +215,6 @@  late_init (void)
 
 /* CREATE THREAD NOTES:
 
-   createthread.c defines the create_thread function, and two macros:
-   START_THREAD_DEFN and START_THREAD_SELF (see below).
-
    create_thread must 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
@@ -242,20 +239,129 @@  late_init (void)
    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
    canceled before calling user code (*PD->start_routine).  */
+
+static int _Noreturn start_thread (void *arg);
+
 static int create_thread (struct pthread *pd, const struct pthread_attr *attr,
 			  bool *stopped_start, STACK_VARIABLES_PARMS,
-			  bool *thread_ran);
+			  bool *thread_ran)
+{
+  /* Determine whether the newly created threads has to be started
+     stopped since we have to set the scheduling parameters or set the
+     affinity.  */
+  bool need_setaffinity = (attr != NULL && attr->extension != NULL
+			   && attr->extension->cpuset != 0);
+  if (attr != NULL
+      && (__glibc_unlikely (need_setaffinity)
+	  || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)))
+    *stopped_start = true;
+
+  pd->stopped_start = *stopped_start;
+  if (__glibc_unlikely (*stopped_start))
+    lll_lock (pd->lock, LLL_PRIVATE);
+
+  /* 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_SIGHAND, CLONE_THREAD
+	This flag selects the POSIX signal semantics and various
+	other kinds of sharing (itimers, POSIX timers, etc.).
+
+     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.  */
+  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);
+
+#ifdef __NR_clone2
+# define ARCH_CLONE __clone2
+#else
+# define ARCH_CLONE __clone
+#endif
+  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;
 
-#include <createthread.c>
+  /* Now we have the possibility to set scheduling parameters etc.  */
+  if (attr != NULL)
+    {
+      int res;
+
+      /* Set the affinity mask if necessary.  */
+      if (need_setaffinity)
+	{
+	  assert (*stopped_start);
+
+	  res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid,
+				       attr->extension->cpusetsize,
+				       attr->extension->cpuset);
+
+	  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res)))
+	  err_out:
+	    {
+	      /* The operation failed.  We have to kill the thread.
+		 We let the normal cancellation mechanism do the work.  */
+
+	      pid_t pid = __getpid ();
+	      INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL);
+
+	      return INTERNAL_SYSCALL_ERRNO (res);
+	    }
+	}
+
+      /* Set the scheduling parameters.  */
+      if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0)
+	{
+	  assert (*stopped_start);
+
+	  res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid,
+				       pd->schedpolicy, &pd->schedparam);
+
+	  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res)))
+	    goto err_out;
+	}
+    }
+
+  return 0;
+}
 
 /* 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
+static int _Noreturn
+start_thread (void *arg)
 {
-  struct pthread *pd = START_THREAD_SELF;
+  struct pthread *pd = arg;
 
   /* Initialize resolver state pointer.  */
   __resp = &pd->res;