unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Refactor Linux ARCH_FORK implementation
@ 2018-02-07 13:09 Adhemerval Zanella
  2018-02-07 13:09 ` [PATCH 2/3] dynarray: Implement remove function Adhemerval Zanella
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-07 13:09 UTC (permalink / raw)
  To: libc-alpha

This patch refactors the ARCH_FORK macro and the required architecture
specific header to simplify the required architecture definitions
to provide the fork syscall semantic and proper document current
Linux clone ABI variant.

Instead of require the reimplementation of arch-fork.h header, this
patch changes the ARCH_FORK to an inline function with clone ABI
defined by kernel-features.h define.  The generic kernel ABI meant
for newer ports is used as default and redefine if the architecture
requires.

Checked on x86_64-linux-gnu and i686-linux-gnu.  Also with a build
for all the afected ABIs.

	* sysdeps/nptl/fork.c (ARCH_FORK): Replace by auch_fork.
	* sysdeps/unix/sysv/linux/alpha/arch-fork.h: Remove file.
	* sysdeps/unix/sysv/linux/riscv/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/aarch64/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/arm/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/hppa/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/i386/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/ia64/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/m68k/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/microblaze/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/mips/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/nios2/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/powerpc/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/s390/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/sh/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/sparc/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/tile/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/arch-fork.h: Likewise.
	* sysdeps/unix/sysv/linux/arch-fork.h (arch_fork): New function.
	* sysdeps/unix/sysv/linux/aarch64/kernel-features.h: New file.
	* sysdeps/unix/sysv/linux/riscv/kernel-features.h: Likewise.
	* sysdeps/unix/sysv/linux/arm/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS): Define.
	* sysdeps/unix/sysv/linux/createthread.c (ARCH_CLONE): Define to
	__clone2 if __NR_clone2 is defined.
	* sysdeps/unix/sysv/linux/hppa/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS): Likewise.
	* sysdeps/unix/sysv/linux/i386/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS): Likewise.
	* sysdeps/unix/sysv/linux/ia64/kernel-features.h
	(__ASSUME_CLONE2): Likewise.
	* sysdeps/unix/sysv/linux/microblaze/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS3): Likewise.
	* sysdeps/unix/sysv/linux/kernel-features.h: Document possible clone
	variants and the define architecture can use.
	(__ASSUME_CLONE_DEFAULT): Define as default.
	* sysdeps/unix/sysv/linux/mips/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS): Likewise.
	* sysdeps/unix/sysv/linux/s390/kernel-features.h
	(__ASSUME_CLONE_BACKWARDS2): Likewise.
---
 ChangeLog                                          | 45 ++++++++++++++++++++++
 sysdeps/nptl/fork.c                                |  8 +---
 .../arch-fork.h => aarch64/kernel-features.h}      | 15 +++-----
 sysdeps/unix/sysv/linux/alpha/arch-fork.h          | 28 --------------
 sysdeps/unix/sysv/linux/arch-fork.h                | 44 ++++++++++++++++-----
 sysdeps/unix/sysv/linux/arm/arch-fork.h            | 27 -------------
 sysdeps/unix/sysv/linux/arm/kernel-features.h      |  3 ++
 sysdeps/unix/sysv/linux/createthread.c             |  5 ++-
 sysdeps/unix/sysv/linux/hppa/arch-fork.h           | 32 ---------------
 sysdeps/unix/sysv/linux/hppa/kernel-features.h     |  3 ++
 sysdeps/unix/sysv/linux/i386/arch-fork.h           | 27 -------------
 sysdeps/unix/sysv/linux/i386/kernel-features.h     |  3 ++
 sysdeps/unix/sysv/linux/ia64/arch-fork.h           | 31 ---------------
 sysdeps/unix/sysv/linux/ia64/kernel-features.h     |  3 ++
 sysdeps/unix/sysv/linux/kernel-features.h          | 35 +++++++++++++++++
 sysdeps/unix/sysv/linux/m68k/arch-fork.h           | 28 --------------
 sysdeps/unix/sysv/linux/microblaze/arch-fork.h     | 27 -------------
 .../unix/sysv/linux/microblaze/kernel-features.h   |  3 ++
 sysdeps/unix/sysv/linux/mips/arch-fork.h           |  1 -
 sysdeps/unix/sysv/linux/mips/kernel-features.h     |  3 ++
 sysdeps/unix/sysv/linux/nios2/arch-fork.h          | 33 ----------------
 sysdeps/unix/sysv/linux/powerpc/arch-fork.h        |  1 -
 sysdeps/unix/sysv/linux/powerpc/kernel-features.h  |  3 ++
 .../arch-fork.h => riscv/kernel-features.h}        | 17 +++-----
 sysdeps/unix/sysv/linux/s390/arch-fork.h           | 29 --------------
 sysdeps/unix/sysv/linux/s390/kernel-features.h     |  3 ++
 sysdeps/unix/sysv/linux/sh/arch-fork.h             | 28 --------------
 sysdeps/unix/sysv/linux/sparc/arch-fork.h          | 27 -------------
 sysdeps/unix/sysv/linux/tile/arch-fork.h           | 29 --------------
 sysdeps/unix/sysv/linux/x86_64/arch-fork.h         | 27 -------------
 30 files changed, 155 insertions(+), 413 deletions(-)
 rename sysdeps/unix/sysv/linux/{riscv/arch-fork.h => aarch64/kernel-features.h} (67%)
 delete mode 100644 sysdeps/unix/sysv/linux/alpha/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/arm/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/i386/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/ia64/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/m68k/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/microblaze/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/mips/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/nios2/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/arch-fork.h
 rename sysdeps/unix/sysv/linux/{aarch64/arch-fork.h => riscv/kernel-features.h} (66%)
 delete mode 100644 sysdeps/unix/sysv/linux/s390/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/sh/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/tile/arch-fork.h
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/arch-fork.h

diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 846fa49..0061ee0 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -131,13 +131,7 @@ __libc_fork (void)
       call_function_static_weak (__malloc_fork_lock_parent);
     }
 
-#ifdef ARCH_FORK
-  pid = ARCH_FORK ();
-#else
-# error "ARCH_FORK must be defined so that the CLONE_SETTID flag is used"
-  pid = INLINE_SYSCALL (fork, 0);
-#endif
-
+  pid = arch_fork (&THREAD_SELF->tid);
 
   if (pid == 0)
     {
diff --git a/sysdeps/unix/sysv/linux/riscv/arch-fork.h b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
similarity index 67%
rename from sysdeps/unix/sysv/linux/riscv/arch-fork.h
rename to sysdeps/unix/sysv/linux/aarch64/kernel-features.h
index f6f5d73..9cfa514 100644
--- a/sysdeps/unix/sysv/linux/riscv/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
@@ -1,5 +1,6 @@
-/* Internal definitions for thread-friendly fork implementation.  Linux/RISC-V.
-   Copyright (C) 2002-2018 Free Software Foundation, Inc.
+/* Set flags signalling availability of kernel features based on given
+   kernel version number.  AArch64 version.
+   Copyright (C) 2018 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -16,11 +17,7 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <sched.h>
-#include <sysdep.h>
-#include <tls.h>
+#include_next <kernel-features.h>
 
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-		  NULL, NULL, &THREAD_SELF->tid)
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS 1
diff --git a/sysdeps/unix/sysv/linux/alpha/arch-fork.h b/sysdeps/unix/sysv/linux/alpha/arch-fork.h
deleted file mode 100644
index 41897dc..0000000
--- a/sysdeps/unix/sysv/linux/alpha/arch-fork.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  Alpha version.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-
-#define ARCH_FORK()							\
-  INLINE_SYSCALL (clone, 5,						\
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
-		  NULL, NULL, &THREAD_SELF->tid, NULL)
diff --git a/sysdeps/unix/sysv/linux/arch-fork.h b/sysdeps/unix/sysv/linux/arch-fork.h
index d5f5429..62bc23d 100644
--- a/sysdeps/unix/sysv/linux/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/arch-fork.h
@@ -1,4 +1,4 @@
-/* ARCH_FORK definition for Linux fork implementation.  Stub version.
+/* arch_fork definition for Linux fork implementation.
    Copyright (C) 2014-2018 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -16,12 +16,38 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-/* This file should define the function-like macro of no arguments
-   ARCH_FORK to an INLINE_SYSCALL invocation of the clone-like system
-   call, passing the CLONE_CHILD_SETTID and CLONE_CHILD_CLEARTID flags
-   and &THREAD_SELF->tid as the TID address.
+#ifndef __ARCH_FORK_H
+#define __ARCH_FORK_H
 
-   Machines that lack an arch-fork.h header file will hit an #error in
-   fork.c; this stub file doesn't contain an #error itself mainly for
-   the transition period of migrating old machine-specific fork.c files
-   to machine-specific arch-fork.h instead.  */
+#include <unistd.h>
+
+/* Call the clone syscall with fork semantic.  The CTID address is used
+   to both store the child thread ID at its location and and erase it
+   in child memory when the child exits, and do a wakeup on the futex at
+   that address.
+
+   The architecture with non-default kernel abi semantic should correctlly
+   override it with one of the supported calling convention (check generic
+   kernel-features.h for the clone abi variants).  */
+static inline pid_t
+arch_fork (void *ctid)
+{
+  const int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD;
+  long int ret;
+#ifdef __ASSUME_CLONE_BACKWARDS
+  ret = INLINE_SYSCALL_CALL (clone, flags, 0, NULL, 0, ctid);
+#elif defined(__ASSUME_CLONE_BACKWARDS2)
+  ret = INLINE_SYSCALL_CALL (clone, 0, flags, NULL, ctid, 0);
+#elif defined(__ASSUME_CLONE_BACKWARDS3)
+  ret = INLINE_SYSCALL_CALL (clone, flags, 0, 0, NULL, ctid, 0);
+#elif defined(__ASSUME_CLONE2)
+  ret = INLINE_SYSCALL_CALL (clone2, flags, 0, 0, NULL, ctid, 0);
+#elif defined(__ASSUME_CLONE_DEFAULT)
+  ret = INLINE_SYSCALL_CALL (clone, flags, NULL, NULL, ctid, 0);
+#else
+# error "Undefined clone variant"
+#endif
+  return ret;
+}
+
+#endif /* __ARCH_FORK_H  */
diff --git a/sysdeps/unix/sysv/linux/arm/arch-fork.h b/sysdeps/unix/sysv/linux/arm/arch-fork.h
deleted file mode 100644
index ff3bc90..0000000
--- a/sysdeps/unix/sysv/linux/arm/arch-fork.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  ARM version.
-   Copyright (C) 2014-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <sysdep.h>
-#include <tls.h>
-
-
-#define ARCH_FORK()                                                     \
-  INLINE_SYSCALL (clone, 5,                                             \
-                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,  \
-                  NULL, NULL, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/arm/kernel-features.h b/sysdeps/unix/sysv/linux/arm/kernel-features.h
index f13632b..7831ab1 100644
--- a/sysdeps/unix/sysv/linux/arm/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/arm/kernel-features.h
@@ -39,3 +39,6 @@
 
 #define __ASSUME_RECV_SYSCALL   1
 #define __ASSUME_SEND_SYSCALL	1
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS	1
diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c
index 5b5464a..5879e51 100644
--- a/sysdeps/unix/sysv/linux/createthread.c
+++ b/sysdeps/unix/sysv/linux/createthread.c
@@ -28,8 +28,9 @@
 
 #include <arch-fork.h>
 
-
-#ifndef ARCH_CLONE
+#ifdef __NR_clone2
+# define ARCH_CLONE __clone2
+#else
 # define ARCH_CLONE __clone
 #endif
 
diff --git a/sysdeps/unix/sysv/linux/hppa/arch-fork.h b/sysdeps/unix/sysv/linux/hppa/arch-fork.h
deleted file mode 100644
index 7d52994..0000000
--- a/sysdeps/unix/sysv/linux/hppa/arch-fork.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  HPPA version.
-   Copyright (C) 2005-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-/* Argument 1 - Clone flags.
-            2 - Child stack pointer.
-	    3 - Parent tid pointer.
-	    4 - New TLS area pointer.
-	    5 - Child tid pointer. */
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						\
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
-                  NULL, NULL, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/hppa/kernel-features.h b/sysdeps/unix/sysv/linux/hppa/kernel-features.h
index 3f005a5..ef3c4dd 100644
--- a/sysdeps/unix/sysv/linux/hppa/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/hppa/kernel-features.h
@@ -32,3 +32,6 @@
 #if __LINUX_KERNEL_VERSION < 0x040000
 # undef __ASSUME_EXECVEAT
 #endif
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS 1
diff --git a/sysdeps/unix/sysv/linux/i386/arch-fork.h b/sysdeps/unix/sysv/linux/i386/arch-fork.h
deleted file mode 100644
index 0c43e2f..0000000
--- a/sysdeps/unix/sysv/linux/i386/arch-fork.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* Internal definitions for thread-friendly fork implementation.  Linux/i386.
-   Copyright (C) 2002-2018 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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-		  NULL, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/i386/kernel-features.h b/sysdeps/unix/sysv/linux/i386/kernel-features.h
index 8708712..f3cfd48 100644
--- a/sysdeps/unix/sysv/linux/i386/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/i386/kernel-features.h
@@ -48,3 +48,6 @@
 
 /* i686 only supports ipc syscall.  */
 #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS	1
diff --git a/sysdeps/unix/sysv/linux/ia64/arch-fork.h b/sysdeps/unix/sysv/linux/ia64/arch-fork.h
deleted file mode 100644
index 522712e..0000000
--- a/sysdeps/unix/sysv/linux/ia64/arch-fork.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  IA64 version.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone2, 6,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	      \
-		  NULL, 0, NULL, &THREAD_SELF->tid, NULL)
-
-#define ARCH_CLONE __clone2
diff --git a/sysdeps/unix/sysv/linux/ia64/kernel-features.h b/sysdeps/unix/sysv/linux/ia64/kernel-features.h
index cc3fe92..04cdf43 100644
--- a/sysdeps/unix/sysv/linux/ia64/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/ia64/kernel-features.h
@@ -26,4 +26,7 @@
 #define __ASSUME_SEND_SYSCALL		1
 #define __ASSUME_ACCEPT4_SYSCALL	1
 
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE2
+
 #endif /* _KERNEL_FEATURES_H */
diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index 3aa2052..ca18f0b 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -115,3 +115,38 @@
 #if __LINUX_KERNEL_VERSION >= 0x040500
 # define __ASSUME_COPY_FILE_RANGE 1
 #endif
+
+/* Support for clone call used on fork.  The signature varies across the
+   architectures with current 4 different variants:
+
+   1. long int clone (unsigned long flags, unsigned long newsp,
+		      int *parent_tidptr, unsigned long tls,
+		      int *child_tidptr)
+
+   2. long int clone (unsigned long newsp, unsigned long clone_flags,
+		      int *parent_tidptr, int * child_tidptr,
+		      unsigned long tls)
+
+   3. long int clone (unsigned long flags, unsigned long newsp,
+		      int stack_size, int *parent_tidptr,
+		      int *child_tidptr, unsigned long tls)
+
+   4. long int clone (unsigned long flags, unsigned long newsp,
+		      int *parent_tidptr, int *child_tidptr,
+                      unsigned long tls)
+
+   The fourth variant is intended to be used as the default for newer ports,
+   ALso IA64 uses the third variant but with __NR_clone2 instead of
+   __NR_clone.
+
+   The macros names to define the variant used for the architecture is
+   similar to kernel:
+
+   - __ASSUME_CLONE_BACKWARDS: for variant 1.
+   - __ASSUME_CLONE_BACKWARDS2: for variant 2 (s390).
+   - __ASSUME_CLONE_BACKWARDS3: for variant 3 (microblaze).
+   - __ASSUME_CLONE_DEFAULT: for variant 4.
+   - __ASSUME_CLONE2: for clone2 with variant 3 (ia64).
+   */
+
+#define __ASSUME_CLONE_DEFAULT 1
diff --git a/sysdeps/unix/sysv/linux/m68k/arch-fork.h b/sysdeps/unix/sysv/linux/m68k/arch-fork.h
deleted file mode 100644
index 20b6bab..0000000
--- a/sysdeps/unix/sysv/linux/m68k/arch-fork.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  m68k version.
-   Copyright (C) 2010-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Maxim Kuvyrkov <maxim@codesourcery.com>, 2010.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-		  NULL, &THREAD_SELF->tid, NULL)
diff --git a/sysdeps/unix/sysv/linux/microblaze/arch-fork.h b/sysdeps/unix/sysv/linux/microblaze/arch-fork.h
deleted file mode 100644
index 05ed4fa..0000000
--- a/sysdeps/unix/sysv/linux/microblaze/arch-fork.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  MicroBlaze version.
-   Copyright (C) 2014-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK()                                                           \
-  INLINE_SYSCALL (clone, 5,                                                   \
-                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-                  NULL, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
index 745f899..b13b863 100644
--- a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
@@ -57,3 +57,6 @@
 #if __LINUX_KERNEL_VERSION < 0x040A00
 # undef __ASSUME_COPY_FILE_RANGE
 #endif
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS3
diff --git a/sysdeps/unix/sysv/linux/mips/arch-fork.h b/sysdeps/unix/sysv/linux/mips/arch-fork.h
deleted file mode 100644
index 5f94537..0000000
--- a/sysdeps/unix/sysv/linux/mips/arch-fork.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/arch-fork.h>
diff --git a/sysdeps/unix/sysv/linux/mips/kernel-features.h b/sysdeps/unix/sysv/linux/mips/kernel-features.h
index 7756a34..a9009fb 100644
--- a/sysdeps/unix/sysv/linux/mips/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/mips/kernel-features.h
@@ -47,3 +47,6 @@
 #if _MIPS_SIM == _ABIN32
 # define __ASSUME_WORDSIZE64_ILP32	1
 #endif
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS
diff --git a/sysdeps/unix/sysv/linux/nios2/arch-fork.h b/sysdeps/unix/sysv/linux/nios2/arch-fork.h
deleted file mode 100644
index 6dacec2..0000000
--- a/sysdeps/unix/sysv/linux/nios2/arch-fork.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  Nios II version.
-   Copyright (C) 2005-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-/* Argument 1 - Clone flags.
-            2 - Child stack pointer.
-	    3 - Parent tid pointer.
-	    4 - Child tid pointer.
-	    5 - New TLS area pointer. */
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						\
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
-                  NULL, NULL, &THREAD_SELF->tid, NULL)
diff --git a/sysdeps/unix/sysv/linux/powerpc/arch-fork.h b/sysdeps/unix/sysv/linux/powerpc/arch-fork.h
deleted file mode 100644
index 5f94537..0000000
--- a/sysdeps/unix/sysv/linux/powerpc/arch-fork.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/arch-fork.h>
diff --git a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
index f3c02e6..503f562 100644
--- a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
@@ -49,3 +49,6 @@
 
 /* powerpc only supports ipc syscall.  */
 #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS	1
diff --git a/sysdeps/unix/sysv/linux/aarch64/arch-fork.h b/sysdeps/unix/sysv/linux/riscv/kernel-features.h
similarity index 66%
rename from sysdeps/unix/sysv/linux/aarch64/arch-fork.h
rename to sysdeps/unix/sysv/linux/riscv/kernel-features.h
index cab797e..37f4d99 100644
--- a/sysdeps/unix/sysv/linux/aarch64/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/riscv/kernel-features.h
@@ -1,5 +1,6 @@
-/* ARCH_FORK definition for Linux fork implementation.  AArch64 version.
-   Copyright (C) 2005-2018 Free Software Foundation, Inc.
+/* Set flags signalling availability of kernel features based on given
+   kernel version number.  RISC-V version.
+   Copyright (C) 2018 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -16,13 +17,7 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
+#include_next <kernel-features.h>
 
-
-#define ARCH_FORK()							\
-  INLINE_SYSCALL (clone, 5,						\
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
-		  NULL, NULL, NULL, &THREAD_SELF->tid)
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS 1
diff --git a/sysdeps/unix/sysv/linux/s390/arch-fork.h b/sysdeps/unix/sysv/linux/s390/arch-fork.h
deleted file mode 100644
index 152b465..0000000
--- a/sysdeps/unix/sysv/linux/s390/arch-fork.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation. S390 version.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						      \
-		  0, CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,     \
-		  NULL, &THREAD_SELF->tid, NULL)
diff --git a/sysdeps/unix/sysv/linux/s390/kernel-features.h b/sysdeps/unix/sysv/linux/s390/kernel-features.h
index 3caca98..f718264 100644
--- a/sysdeps/unix/sysv/linux/s390/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/s390/kernel-features.h
@@ -50,3 +50,6 @@
 
 /* s390 only supports ipc syscall.  */
 #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
+
+#undef __ASSUME_CLONE_DEFAULT
+#define __ASSUME_CLONE_BACKWARDS2
diff --git a/sysdeps/unix/sysv/linux/sh/arch-fork.h b/sysdeps/unix/sysv/linux/sh/arch-fork.h
deleted file mode 100644
index a29f61c..0000000
--- a/sysdeps/unix/sysv/linux/sh/arch-fork.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  SH version.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-/* TLS pointer argument is passed as the 5-th argument.  */
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 5,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-		  NULL, &THREAD_SELF->tid, NULL)
diff --git a/sysdeps/unix/sysv/linux/sparc/arch-fork.h b/sysdeps/unix/sysv/linux/sparc/arch-fork.h
deleted file mode 100644
index 03ccc9a..0000000
--- a/sysdeps/unix/sysv/linux/sparc/arch-fork.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  SPARC version.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK() \
-  INLINE_CLONE_SYSCALL (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, \
-			0, NULL, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/tile/arch-fork.h b/sysdeps/unix/sysv/linux/tile/arch-fork.h
deleted file mode 100644
index d0526d8..0000000
--- a/sysdeps/unix/sysv/linux/tile/arch-fork.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* ARCH_FORK definition for Linux fork implementation.  Tile* version.
-   Copyright (C) 2011-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-   Based on work 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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 4,						      \
-		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,        \
-		  0, NULL, &THREAD_SELF->tid)
diff --git a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
deleted file mode 100644
index 4471970..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* Internal definitions for thread-friendly fork implementation.  Linux/x86_64.
-   Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <sysdep.h>
-#include <tls.h>
-
-#define ARCH_FORK() \
-  INLINE_SYSCALL (clone, 4,                                                   \
-                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
-                  NULL, &THREAD_SELF->tid)
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/3] dynarray: Implement remove function
  2018-02-07 13:09 [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
@ 2018-02-07 13:09 ` Adhemerval Zanella
  2018-02-07 14:48   ` Alexander Monakov
  2018-02-07 13:09 ` [PATCH 3/3] Refactor atfork handlers Adhemerval Zanella
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-07 13:09 UTC (permalink / raw)
  To: libc-alpha

This patch implements the remove item function for dynarray array.
It is a costly operation, since it requires a memory move operation
possible as large as the array size less one element.

Checked on x86_64-linux-gnu.

	* malloc/dynarray-skeleton.c: List remove as defined function.
	((DYNARRAY_PREFIX##remove): New function.
	* malloc/tst-dynarray.c (test_int): Add tests for remove function.
	(check_int, check_int_array): New functions.
---
 ChangeLog                  |  5 ++++
 malloc/dynarray-skeleton.c | 23 +++++++++++++++++
 malloc/tst-dynarray.c      | 64 ++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/malloc/dynarray-skeleton.c b/malloc/dynarray-skeleton.c
index 5ab4a19..619a750 100644
--- a/malloc/dynarray-skeleton.c
+++ b/malloc/dynarray-skeleton.c
@@ -72,6 +72,7 @@
      DYNARRAY_ELEMENT *DYNARRAY_PREFIX##emplace (struct DYNARRAY_STRUCT *);
      bool DYNARRAY_PREFIX##resize (struct DYNARRAY_STRUCT *, size_t);
      void DYNARRAY_PREFIX##remove_last (struct DYNARRAY_STRUCT *);
+     void DYNARRAY_PREFIX##remove (struct DYNARRAY_STRUCT *, size_t);
      void DYNARRAY_PREFIX##clear (struct DYNARRAY_STRUCT *);
 
    The following functions are provided are provided if the
@@ -428,6 +429,28 @@ DYNARRAY_NAME (remove_last) (struct DYNARRAY_STRUCT *list)
     }
 }
 
+/* Remove the element of index IDX of LIST if it is present.  */
+__attribute__ ((unused, nonnull (1)))
+static void
+DYNARRAY_NAME (remove) (struct DYNARRAY_STRUCT *list, size_t idx)
+{
+  size_t last_pos = list->dynarray_header.used;
+  if (idx + 1 == last_pos)
+    {
+      DYNARRAY_NAME (remove_last) (list);
+      return;
+    }
+  DYNARRAY_ELEMENT *elem = DYNARRAY_NAME (at) (list, idx);
+  DYNARRAY_ELEMENT *last = &list->dynarray_header.array[last_pos];
+
+  ptrdiff_t size_to_move = (uintptr_t)last - (uintptr_t)elem;
+#ifdef DYNARRAY_ELEMENT_FREE
+  DYNARRAY_ELEMENT_FREE (elem);
+#endif
+  memmove (elem, elem + 1, size_to_move);
+  list->dynarray_header.used--;
+}
+
 /* Remove all elements from the list.  The elements are freed, but the
    list itself is not.  */
 __attribute__ ((unused, nonnull (1)))
diff --git a/malloc/tst-dynarray.c b/malloc/tst-dynarray.c
index 0a5716b..c31b73d 100644
--- a/malloc/tst-dynarray.c
+++ b/malloc/tst-dynarray.c
@@ -55,6 +55,40 @@ struct long_array
 
 enum { max_count = 20 };
 
+static void
+check_int (int do_remove, struct dynarray_int *dyn, unsigned int base,
+	   unsigned int final_count, unsigned int to_remove)
+{
+  if (do_remove == 1)
+    for (unsigned int i = 0; i < final_count; ++i)
+      TEST_VERIFY_EXIT (*dynarray_int_at (dyn, i) == base + i);
+  else if (do_remove == 2)
+    {
+      unsigned int i;
+      for (i = 0; i < to_remove; ++i)
+	TEST_VERIFY_EXIT (*dynarray_int_at (dyn, i) == base + i);
+      for (i = to_remove; i < final_count; ++i)
+	TEST_VERIFY_EXIT (*dynarray_int_at (dyn, i) == base + i + 1);
+    }
+}
+
+static void
+check_int_array (int do_remove, struct int_array *result, unsigned int base,
+		 unsigned int final_count, unsigned int to_remove)
+{
+  if (do_remove == 1)
+    for (unsigned int i = 0; i < final_count; ++i)
+      TEST_VERIFY_EXIT (result->array[i] == base + i);
+  else if (do_remove == 2)
+    {
+      unsigned int i;
+      for (i = 0; i < to_remove; ++i)
+	TEST_VERIFY_EXIT (result->array[i] == base + i);
+      for (i = to_remove; i < final_count; ++i)
+	TEST_VERIFY_EXIT (result->array[i] == base + i + 1);
+    }
+}
+
 /* Test dynamic arrays with int elements (no automatic deallocation
    for elements).  */
 static void
@@ -84,15 +118,15 @@ test_int (void)
      do_add: Switch between emplace (false) and add (true).
      do_finalize: Perform finalize call at the end.
      do_clear: Perform clear call at the end.
-     do_remove_last: Perform remove_last call after adding elements.
+     do_remove: Perform remove_last or remove call after adding elements.
      count: Number of elements added to the array.  */
   for (int do_add = 0; do_add < 2; ++do_add)
-    for (int do_finalize = 0; do_finalize < 2; ++do_finalize)
+    for (int do_finalize = 1; do_finalize < 2; ++do_finalize)
       for (int do_clear = 0; do_clear < 2; ++do_clear)
-        for (int do_remove_last = 0; do_remove_last < 2; ++do_remove_last)
-          for (unsigned int count = 0; count < max_count; ++count)
+        for (int do_remove = 1; do_remove < 3; ++do_remove)
+          for (unsigned int count = 2; count < max_count; ++count)
             {
-              if (do_remove_last && count == 0)
+              if (do_remove && count == 0)
                 continue;
               unsigned int base = count * count;
               struct dynarray_int dyn;
@@ -123,7 +157,8 @@ test_int (void)
                 }
               unsigned final_count;
               bool heap_array = dyn.dynarray_header.array != dyn.scratch;
-              if (do_remove_last)
+	      size_t to_remove = dynarray_int_size (&dyn) / 2;
+              if (do_remove == 1)
                 {
                   dynarray_int_remove_last (&dyn);
                   if (count == 0)
@@ -131,7 +166,15 @@ test_int (void)
                   else
                     final_count = count - 1;
                 }
-              else
+              else if (do_remove == 2)
+		{
+                  dynarray_int_remove (&dyn, to_remove);
+                  if (count == 0)
+                    final_count = 0;
+                  else
+                    final_count = count - 1;
+		}
+	      else
                 final_count = count;
               if (final_count > 0)
                 {
@@ -151,8 +194,7 @@ test_int (void)
               TEST_VERIFY_EXIT (dynarray_int_size (&dyn) == final_count);
               TEST_VERIFY_EXIT (dyn.dynarray_header.allocated >= final_count);
               if (!do_clear)
-                for (unsigned int i = 0; i < final_count; ++i)
-                  TEST_VERIFY_EXIT (*dynarray_int_at (&dyn, i) == base + i);
+		check_int (do_remove, &dyn, base, final_count, to_remove);
               if (do_finalize)
                 {
                   struct int_array result = { (int *) (uintptr_t) -1, -1 };
@@ -168,8 +210,8 @@ test_int (void)
                       TEST_VERIFY_EXIT
                         (malloc_usable_size (result.array)
                          >= final_count * sizeof (result.array[0]));
-                      for (unsigned int i = 0; i < final_count; ++i)
-                        TEST_VERIFY_EXIT (result.array[i] == base + i);
+                      check_int_array (do_remove, &result, base, final_count,
+				       to_remove);
                       free (result.array);
                     }
                 }
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/3] Refactor atfork handlers
  2018-02-07 13:09 [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
  2018-02-07 13:09 ` [PATCH 2/3] dynarray: Implement remove function Adhemerval Zanella
@ 2018-02-07 13:09 ` Adhemerval Zanella
  2018-02-07 15:07   ` Florian Weimer
  2018-03-07 16:51 ` [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
  2018-03-08 12:05 ` Florian Weimer
  3 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-07 13:09 UTC (permalink / raw)
  To: libc-alpha

Current implementation (sysdeps/nptl/fork.c) replicates the atfork
handlers list backward to invoke the child handlers after fork/clone
syscall.

The internal atfork handlers is implemented as a single-linked list
so a lock-free algorithm can be used, trading fork mulithread call
performance for some code complexity and dynamic stack allocation
(since the backwards list should not fail).

This patch refactor it to use a dynarary instead of a linked list.
It simplifies the external variables need to be exported and also
the internal atfork handler member definition.

The downside is a serialization of fork call in multithread, since to
operate on the dynarray the internal lock should be used.  However
as noted by Florian, it already acquires external locks for malloc
and libio so it is already hitting some lock contention.  Besides,
posix_spawn should be faster and more scalable to run external programs
in multithread environments.

Checked on x86_64-linux-gnu.

	* nptl/Makefile (routines): Remove unregister-atfork.
	* nptl/register-atfork.c (fork_handler_pool): Remove variable.
	(fork_handler_alloc): Remove function.
	(fork_handlers, fork_handler_init): New variables.
	(__fork_lock): Rename to atfork_lock.
	(__register_atforki, __unregister_atfork, libc_freeres_fn): Rewrite
	to use a dynamic array to add/remove atfork handlers.
	* sysdeps/nptl/fork.c (__libc_fork): Likewise.
	* sysdeps/nptl/fork.h (__fork_lock, __fork_handlers, __linkin_atfork):
	Remove declaration.
	(fork_handler): Remove next, refcntr, and need_signal member.
	(__run_fork_handler_type): New enum.
	(__run_fork_handlers): New prototype.
	* sysdeps/nptl/libc-lockP.h (__libc_atfork): Remove declaration.
---
 ChangeLog                 |  15 +++++
 nptl/Makefile             |   2 +-
 nptl/register-atfork.c    | 146 +++++++++++++++++++---------------------------
 sysdeps/nptl/fork.c       |  96 +-----------------------------
 sysdeps/nptl/fork.h       |  31 +++++-----
 sysdeps/nptl/libc-lockP.h |   2 -
 6 files changed, 97 insertions(+), 195 deletions(-)

diff --git a/nptl/Makefile b/nptl/Makefile
index 6fc2c8b..be7ee3e 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -30,7 +30,7 @@ install-lib-ldscripts := libpthread.so
 
 routines = alloca_cutoff forward libc-lowlevellock libc-cancellation \
 	   libc-cleanup libc_pthread_init libc_multiple_threads \
-	   register-atfork unregister-atfork pthread_self
+	   register-atfork pthread_self
 shared-only-routines = forward
 
 # We need to provide certain routines for compatibility with existing
diff --git a/nptl/register-atfork.c b/nptl/register-atfork.c
index f309cec..0bc2fe9 100644
--- a/nptl/register-atfork.c
+++ b/nptl/register-atfork.c
@@ -22,123 +22,97 @@
 #include <fork.h>
 #include <atomic.h>
 
+#define DYNARRAY_ELEMENT           struct fork_handler
+#define DYNARRAY_STRUCT            fork_handler_list
+#define DYNARRAY_PREFIX            fork_handler_list_
+#define DYNARRAY_INITIAL_SIZE      48
+#include <malloc/dynarray-skeleton.c>
 
-struct fork_handler *__fork_handlers;
-
-/* Lock to protect allocation and deallocation of fork handlers.  */
-int __fork_lock = LLL_LOCK_INITIALIZER;
-
-
-/* Number of pre-allocated handler entries.  */
-#define NHANDLER 48
-
-/* Memory pool for fork handler structures.  */
-static struct fork_handler_pool
-{
-  struct fork_handler_pool *next;
-  struct fork_handler mem[NHANDLER];
-} fork_handler_pool;
-
-
-static struct fork_handler *
-fork_handler_alloc (void)
-{
-  struct fork_handler_pool *runp = &fork_handler_pool;
-  struct fork_handler *result = NULL;
-  unsigned int i;
-
-  do
-    {
-      /* Search for an empty entry.  */
-      for (i = 0; i < NHANDLER; ++i)
-	if (runp->mem[i].refcntr == 0)
-	  goto found;
-    }
-  while ((runp = runp->next) != NULL);
-
-  /* We have to allocate a new entry.  */
-  runp = (struct fork_handler_pool *) calloc (1, sizeof (*runp));
-  if (runp != NULL)
-    {
-      /* Enqueue the new memory pool into the list.  */
-      runp->next = fork_handler_pool.next;
-      fork_handler_pool.next = runp;
-
-      /* We use the last entry on the page.  This means when we start
-	 searching from the front the next time we will find the first
-	 entry unused.  */
-      i = NHANDLER - 1;
-
-    found:
-      result = &runp->mem[i];
-      result->refcntr = 1;
-      result->need_signal = 0;
-    }
-
-  return result;
-}
+static struct fork_handler_list fork_handlers;
+static bool fork_handler_init = false;
 
+static int atfork_lock = LLL_LOCK_INITIALIZER;
 
 int
 __register_atfork (void (*prepare) (void), void (*parent) (void),
 		   void (*child) (void), void *dso_handle)
 {
-  /* Get the lock to not conflict with other allocations.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  struct fork_handler *newp = fork_handler_alloc ();
+  if (!fork_handler_init)
+    {
+      fork_handler_list_init (&fork_handlers);
+      fork_handler_init = true;
+    }
 
+  struct fork_handler *newp = fork_handler_list_emplace (&fork_handlers);
   if (newp != NULL)
     {
-      /* Initialize the new record.  */
       newp->prepare_handler = prepare;
       newp->parent_handler = parent;
       newp->child_handler = child;
       newp->dso_handle = dso_handle;
-
-      __linkin_atfork (newp);
     }
 
   /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 
   return newp == NULL ? ENOMEM : 0;
 }
 libc_hidden_def (__register_atfork)
 
-
 void
-attribute_hidden
-__linkin_atfork (struct fork_handler *newp)
+__unregister_atfork (void *dso_handle)
 {
-  do
-    newp->next = __fork_handlers;
-  while (catomic_compare_and_exchange_bool_acq (&__fork_handlers,
-						newp, newp->next) != 0);
-}
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
+  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers); i++)
+    if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
+      {
+        fork_handler_list_remove (&fork_handlers, i);
+        break;
+      }
 
-libc_freeres_fn (free_mem)
+  lll_unlock (atfork_lock, LLL_PRIVATE);
+}
+
+void
+__run_fork_handlers (enum __run_fork_handler_type who)
 {
-  /* Get the lock to not conflict with running forks.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  struct fork_handler *runp;
 
-  /* No more fork handlers.  */
-  __fork_handlers = NULL;
+  if (who == atfork_run_prepare)
+    {
+      lll_lock (atfork_lock, LLL_PRIVATE);
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = sl; i > 0; i--)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i - 1);
+	  if (runp->prepare_handler != NULL)
+	    runp->prepare_handler ();
+	}
+    }
+  else
+    {
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = 0; i < sl; i++)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i);
+	  if (who == atfork_run_child && runp->child_handler)
+	    runp->child_handler ();
+	  else if (who == atfork_run_parent && runp->parent_handler)
+	    runp->parent_handler ();
+	}
+      lll_unlock (atfork_lock, LLL_PRIVATE);
+    }
+}
 
-  /* Free eventually allocated memory blocks for the object pool.  */
-  struct fork_handler_pool *runp = fork_handler_pool.next;
 
-  memset (&fork_handler_pool, '\0', sizeof (fork_handler_pool));
+libc_freeres_fn (free_mem)
+{
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  fork_handler_list_free (&fork_handlers);
 
-  /* We can free the memory after releasing the lock.  */
-  while (runp != NULL)
-    {
-      struct fork_handler_pool *oldp = runp;
-      runp = runp->next;
-      free (oldp);
-    }
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 }
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 0061ee0..ec56a82 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -48,11 +48,6 @@ pid_t
 __libc_fork (void)
 {
   pid_t pid;
-  struct used_handler
-  {
-    struct fork_handler *handler;
-    struct used_handler *next;
-  } *allp = NULL;
 
   /* Determine if we are running multiple threads.  We skip some fork
      handlers in the single-thread case, to make fork safer to use in
@@ -60,60 +55,7 @@ __libc_fork (void)
      but our current fork implementation is not.  */
   bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
 
-  /* Run all the registered preparation handlers.  In reverse order.
-     While doing this we build up a list of all the entries.  */
-  struct fork_handler *runp;
-  while ((runp = __fork_handlers) != NULL)
-    {
-      /* Make sure we read from the current RUNP pointer.  */
-      atomic_full_barrier ();
-
-      unsigned int oldval = runp->refcntr;
-
-      if (oldval == 0)
-	/* This means some other thread removed the list just after
-	   the pointer has been loaded.  Try again.  Either the list
-	   is empty or we can retry it.  */
-	continue;
-
-      /* Bump the reference counter.  */
-      if (atomic_compare_and_exchange_bool_acq (&__fork_handlers->refcntr,
-						oldval + 1, oldval))
-	/* The value changed, try again.  */
-	continue;
-
-      /* We bumped the reference counter for the first entry in the
-	 list.  That means that none of the following entries will
-	 just go away.  The unloading code works in the order of the
-	 list.
-
-	 While executing the registered handlers we are building a
-	 list of all the entries so that we can go backward later on.  */
-      while (1)
-	{
-	  /* Execute the handler if there is one.  */
-	  if (runp->prepare_handler != NULL)
-	    runp->prepare_handler ();
-
-	  /* Create a new element for the list.  */
-	  struct used_handler *newp
-	    = (struct used_handler *) alloca (sizeof (*newp));
-	  newp->handler = runp;
-	  newp->next = allp;
-	  allp = newp;
-
-	  /* Advance to the next handler.  */
-	  runp = runp->next;
-	  if (runp == NULL)
-	    break;
-
-	  /* Bump the reference counter for the next entry.  */
-	  atomic_increment (&runp->refcntr);
-	}
-
-      /* We are done.  */
-      break;
-    }
+  __run_fork_handlers (atfork_run_prepare);
 
   /* If we are not running multiple threads, we do not have to
      preserve lock state.  If fork runs from a signal handler, only
@@ -192,29 +134,7 @@ __libc_fork (void)
       __rtld_lock_initialize (GL(dl_load_lock));
 
       /* Run the handlers registered for the child.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->child_handler != NULL)
-	    allp->handler->child_handler ();
-
-	  /* Note that we do not have to wake any possible waiter.
-	     This is the only thread in the new process.  The count
-	     may have been bumped up by other threads doing a fork.
-	     We reset it to 1, to avoid waiting for non-existing
-	     thread(s) to release the count.  */
-	  allp->handler->refcntr = 1;
-
-	  /* XXX We could at this point look through the object pool
-	     and mark all objects not on the __fork_handlers list as
-	     unused.  This is necessary in case the fork() happened
-	     while another thread called dlclose() and that call had
-	     to create a new list.  */
-
-	  allp = allp->next;
-	}
-
-      /* Initialize the fork lock.  */
-      __fork_lock = LLL_LOCK_INITIALIZER;
+      __run_fork_handlers (atfork_run_child);
     }
   else
     {
@@ -229,17 +149,7 @@ __libc_fork (void)
 	}
 
       /* Run the handlers registered for the parent.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->parent_handler != NULL)
-	    allp->handler->parent_handler ();
-
-	  if (atomic_decrement_and_test (&allp->handler->refcntr)
-	      && allp->handler->need_signal)
-	    futex_wake (&allp->handler->refcntr, 1, FUTEX_PRIVATE);
-
-	  allp = allp->next;
-	}
+      __run_fork_handlers (atfork_run_parent);
     }
 
   return pid;
diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
index f0330cc..6eab61c 100644
--- a/sysdeps/nptl/fork.h
+++ b/sysdeps/nptl/fork.h
@@ -24,29 +24,37 @@ extern unsigned long int __fork_generation attribute_hidden;
 /* Pointer to the fork generation counter in the thread library.  */
 extern unsigned long int *__fork_generation_pointer attribute_hidden;
 
-/* Lock to protect allocation and deallocation of fork handlers.  */
-extern int __fork_lock attribute_hidden;
-
 /* Elements of the fork handler lists.  */
 struct fork_handler
 {
-  struct fork_handler *next;
   void (*prepare_handler) (void);
   void (*parent_handler) (void);
   void (*child_handler) (void);
   void *dso_handle;
-  unsigned int refcntr;
-  int need_signal;
 };
 
-/* The single linked list of all currently registered for handlers.  */
-extern struct fork_handler *__fork_handlers attribute_hidden;
-
-
 /* Function to call to unregister fork handlers.  */
 extern void __unregister_atfork (void *dso_handle) attribute_hidden;
 #define UNREGISTER_ATFORK(dso_handle) __unregister_atfork (dso_handle)
 
+enum __run_fork_handler_type
+{
+  atfork_run_prepare,
+  atfork_run_child,
+  atfork_run_parent
+};
+
+/* Run the atfork handlers and lock/unlock the internal lock depending
+   of the WHO argument:
+
+   - atfork_run_prepare: run all the PREPARE_HANDLER in reverse order of
+			 insertion and locks the internal lock.
+   - atfork_run_child: run all the CHILD_HANDLER and unlocks the internal
+		       lock.
+   - atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal
+			lock.  */
+extern void __run_fork_handlers (enum __run_fork_handler_type who)
+  attribute_hidden;
 
 /* C library side function to register new fork handlers.  */
 extern int __register_atfork (void (*__prepare) (void),
@@ -54,6 +62,3 @@ extern int __register_atfork (void (*__prepare) (void),
 			      void (*__child) (void),
 			      void *dso_handle);
 libc_hidden_proto (__register_atfork)
-
-/* Add a new element to the fork list.  */
-extern void __linkin_atfork (struct fork_handler *newp) attribute_hidden;
diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h
index 8539bbf..989fefa 100644
--- a/sysdeps/nptl/libc-lockP.h
+++ b/sysdeps/nptl/libc-lockP.h
@@ -319,8 +319,6 @@ __libc_cleanup_routine (struct __pthread_cleanup_frame *f)
 /* Register handlers to execute before and after `fork'.  Note that the
    last parameter is NULL.  The handlers registered by the libc are
    never removed so this is OK.  */
-#define __libc_atfork(PREPARE, PARENT, CHILD) \
-  __register_atfork (PREPARE, PARENT, CHILD, NULL)
 extern int __register_atfork (void (*__prepare) (void),
 			      void (*__parent) (void),
 			      void (*__child) (void),
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] dynarray: Implement remove function
  2018-02-07 13:09 ` [PATCH 2/3] dynarray: Implement remove function Adhemerval Zanella
@ 2018-02-07 14:48   ` Alexander Monakov
  2018-02-07 16:06     ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Alexander Monakov @ 2018-02-07 14:48 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On Wed, 7 Feb 2018, Adhemerval Zanella wrote:

> This patch implements the remove item function for dynarray array.
> It is a costly operation, since it requires a memory move operation
> possible as large as the array size less one element.

If preserving order is not required, then removing an element is as
cheap as moving only the last element to the position of the removed.

If order preservation, is, in fact, part of the intended interface,
then shouldn't the new function be named like '..._ordered_remove'
to reflect that?

Alexander


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-07 13:09 ` [PATCH 3/3] Refactor atfork handlers Adhemerval Zanella
@ 2018-02-07 15:07   ` Florian Weimer
  2018-02-07 17:16     ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-07 15:07 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/07/2018 02:09 PM, Adhemerval Zanella wrote:
> +  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers); i++)
> +    if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
> +      {
> +        fork_handler_list_remove (&fork_handlers, i);
> +        break;
> +      }

I think there can be multiple fork handlers for one dso_handle, and this 
loop only removes one of them.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/3] dynarray: Implement remove function
  2018-02-07 14:48   ` Alexander Monakov
@ 2018-02-07 16:06     ` Adhemerval Zanella
  0 siblings, 0 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-07 16:06 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: libc-alpha



On 07/02/2018 12:48, Alexander Monakov wrote:
> On Wed, 7 Feb 2018, Adhemerval Zanella wrote:
> 
>> This patch implements the remove item function for dynarray array.
>> It is a costly operation, since it requires a memory move operation
>> possible as large as the array size less one element.
> 
> If preserving order is not required, then removing an element is as
> cheap as moving only the last element to the position of the removed.
> 
> If order preservation, is, in fact, part of the intended interface,
> then shouldn't the new function be named like '..._ordered_remove'
> to reflect that?
> 
> Alexander
> 

I see dynarray works similar to c++ vector container and the remove
usage on subsequent patch expects order preservation.  So I would
prefer to use the other way around your suggestion: to add a
unordered_remove if the case.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-07 15:07   ` Florian Weimer
@ 2018-02-07 17:16     ` Adhemerval Zanella
  2018-02-08  8:32       ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-07 17:16 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 07/02/2018 13:07, Florian Weimer wrote:
> On 02/07/2018 02:09 PM, Adhemerval Zanella wrote:
>> +  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers); i++)
>> +    if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
>> +      {
>> +        fork_handler_list_remove (&fork_handlers, i);
>> +        break;
>> +      }
> 
> I think there can be multiple fork handlers for one dso_handle, and this loop only removes one of them.

Indeed, I overlook it.  Below it is an updated patch with a missing removal
I forgo to add (unregister-atfork.c).

--

	* nptl/Makefile (routines): Remove unregister-atfork.
	* nptl/register-atfork.c (fork_handler_pool): Remove variable.
	(fork_handler_alloc): Remove function.
	(fork_handlers, fork_handler_init): New variables.
	(__fork_lock): Rename to atfork_lock.
	(__register_atforki, __unregister_atfork, libc_freeres_fn): Rewrite
	to use a dynamic array to add/remove atfork handlers.
	* sysdeps/nptl/fork.c (__libc_fork): Likewise.
	* sysdeps/nptl/fork.h (__fork_lock, __fork_handlers, __linkin_atfork):
	Remove declaration.
	(fork_handler): Remove next, refcntr, and need_signal member.
	(__run_fork_handler_type): New enum.
	(__run_fork_handlers): New prototype.
	* sysdeps/nptl/libc-lockP.h (__libc_atfork): Remove declaration.

---

diff --git a/nptl/Makefile b/nptl/Makefile
index 6fc2c8b..be7ee3e 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -30,7 +30,7 @@ install-lib-ldscripts := libpthread.so
 
 routines = alloca_cutoff forward libc-lowlevellock libc-cancellation \
 	   libc-cleanup libc_pthread_init libc_multiple_threads \
-	   register-atfork unregister-atfork pthread_self
+	   register-atfork pthread_self
 shared-only-routines = forward
 
 # We need to provide certain routines for compatibility with existing
diff --git a/nptl/register-atfork.c b/nptl/register-atfork.c
index f309cec..72169b8 100644
--- a/nptl/register-atfork.c
+++ b/nptl/register-atfork.c
@@ -22,123 +22,100 @@
 #include <fork.h>
 #include <atomic.h>
 
+#define DYNARRAY_ELEMENT           struct fork_handler
+#define DYNARRAY_STRUCT            fork_handler_list
+#define DYNARRAY_PREFIX            fork_handler_list_
+#define DYNARRAY_INITIAL_SIZE      48
+#include <malloc/dynarray-skeleton.c>
 
-struct fork_handler *__fork_handlers;
-
-/* Lock to protect allocation and deallocation of fork handlers.  */
-int __fork_lock = LLL_LOCK_INITIALIZER;
-
-
-/* Number of pre-allocated handler entries.  */
-#define NHANDLER 48
-
-/* Memory pool for fork handler structures.  */
-static struct fork_handler_pool
-{
-  struct fork_handler_pool *next;
-  struct fork_handler mem[NHANDLER];
-} fork_handler_pool;
-
-
-static struct fork_handler *
-fork_handler_alloc (void)
-{
-  struct fork_handler_pool *runp = &fork_handler_pool;
-  struct fork_handler *result = NULL;
-  unsigned int i;
-
-  do
-    {
-      /* Search for an empty entry.  */
-      for (i = 0; i < NHANDLER; ++i)
-	if (runp->mem[i].refcntr == 0)
-	  goto found;
-    }
-  while ((runp = runp->next) != NULL);
-
-  /* We have to allocate a new entry.  */
-  runp = (struct fork_handler_pool *) calloc (1, sizeof (*runp));
-  if (runp != NULL)
-    {
-      /* Enqueue the new memory pool into the list.  */
-      runp->next = fork_handler_pool.next;
-      fork_handler_pool.next = runp;
-
-      /* We use the last entry on the page.  This means when we start
-	 searching from the front the next time we will find the first
-	 entry unused.  */
-      i = NHANDLER - 1;
-
-    found:
-      result = &runp->mem[i];
-      result->refcntr = 1;
-      result->need_signal = 0;
-    }
-
-  return result;
-}
+static struct fork_handler_list fork_handlers;
+static bool fork_handler_init = false;
 
+static int atfork_lock = LLL_LOCK_INITIALIZER;
 
 int
 __register_atfork (void (*prepare) (void), void (*parent) (void),
 		   void (*child) (void), void *dso_handle)
 {
-  /* Get the lock to not conflict with other allocations.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  struct fork_handler *newp = fork_handler_alloc ();
+  if (!fork_handler_init)
+    {
+      fork_handler_list_init (&fork_handlers);
+      fork_handler_init = true;
+    }
 
+  struct fork_handler *newp = fork_handler_list_emplace (&fork_handlers);
   if (newp != NULL)
     {
-      /* Initialize the new record.  */
       newp->prepare_handler = prepare;
       newp->parent_handler = parent;
       newp->child_handler = child;
       newp->dso_handle = dso_handle;
-
-      __linkin_atfork (newp);
     }
 
   /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 
   return newp == NULL ? ENOMEM : 0;
 }
 libc_hidden_def (__register_atfork)
 
-
 void
-attribute_hidden
-__linkin_atfork (struct fork_handler *newp)
+__unregister_atfork (void *dso_handle)
 {
-  do
-    newp->next = __fork_handlers;
-  while (catomic_compare_and_exchange_bool_acq (&__fork_handlers,
-						newp, newp->next) != 0);
-}
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
+  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers);)
+    {
+      /* dynarray remove maintains element order, so update index iff there is
+	 no removal.  */
+      if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
+        fork_handler_list_remove (&fork_handlers, i);
+      else
+        i++;
+    }
 
-libc_freeres_fn (free_mem)
+  lll_unlock (atfork_lock, LLL_PRIVATE);
+}
+
+void
+__run_fork_handlers (enum __run_fork_handler_type who)
 {
-  /* Get the lock to not conflict with running forks.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  struct fork_handler *runp;
 
-  /* No more fork handlers.  */
-  __fork_handlers = NULL;
+  if (who == atfork_run_prepare)
+    {
+      lll_lock (atfork_lock, LLL_PRIVATE);
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = sl; i > 0; i--)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i - 1);
+	  if (runp->prepare_handler != NULL)
+	    runp->prepare_handler ();
+	}
+    }
+  else
+    {
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = 0; i < sl; i++)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i);
+	  if (who == atfork_run_child && runp->child_handler)
+	    runp->child_handler ();
+	  else if (who == atfork_run_parent && runp->parent_handler)
+	    runp->parent_handler ();
+	}
+      lll_unlock (atfork_lock, LLL_PRIVATE);
+    }
+}
 
-  /* Free eventually allocated memory blocks for the object pool.  */
-  struct fork_handler_pool *runp = fork_handler_pool.next;
 
-  memset (&fork_handler_pool, '\0', sizeof (fork_handler_pool));
+libc_freeres_fn (free_mem)
+{
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  fork_handler_list_free (&fork_handlers);
 
-  /* We can free the memory after releasing the lock.  */
-  while (runp != NULL)
-    {
-      struct fork_handler_pool *oldp = runp;
-      runp = runp->next;
-      free (oldp);
-    }
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 }
diff --git a/nptl/unregister-atfork.c b/nptl/unregister-atfork.c
deleted file mode 100644
index 20411ed..0000000
--- a/nptl/unregister-atfork.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/* Copyright (C) 2002-2018 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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <stdlib.h>
-#include <fork.h>
-#include <atomic.h>
-#include <futex-internal.h>
-
-
-void
-__unregister_atfork (void *dso_handle)
-{
-  /* Check whether there is any entry in the list which we have to
-     remove.  It is likely that this is not the case so don't bother
-     getting the lock.
-
-     We do not worry about other threads adding entries for this DSO
-     right this moment.  If this happens this is a race and we can do
-     whatever we please.  The program will crash anyway seen.  */
-  struct fork_handler *runp = __fork_handlers;
-  struct fork_handler *lastp = NULL;
-
-  while (runp != NULL)
-    if (runp->dso_handle == dso_handle)
-      break;
-    else
-      {
-	lastp = runp;
-	runp = runp->next;
-      }
-
-  if (runp == NULL)
-    /* Nothing to do.  */
-    return;
-
-  /* Get the lock to not conflict with additions or deletions.  Note
-     that there couldn't have been another thread deleting something.
-     The __unregister_atfork function is only called from the
-     dlclose() code which itself serializes the operations.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
-
-  /* We have to create a new list with all the entries we don't remove.  */
-  struct deleted_handler
-  {
-    struct fork_handler *handler;
-    struct deleted_handler *next;
-  } *deleted = NULL;
-
-  /* Remove the entries for the DSO which is unloaded from the list.
-     It's a single linked list so readers are.  */
-  do
-    {
-    again:
-      if (runp->dso_handle == dso_handle)
-	{
-	  if (lastp == NULL)
-	    {
-	      /* We have to use an atomic operation here because
-		 __linkin_atfork also uses one.  */
-	      if (catomic_compare_and_exchange_bool_acq (&__fork_handlers,
-							 runp->next, runp)
-		  != 0)
-		{
-		  runp = __fork_handlers;
-		  goto again;
-		}
-	    }
-	  else
-	    lastp->next = runp->next;
-
-	  /* We cannot overwrite the ->next element now.  Put the deleted
-	     entries in a separate list.  */
-	  struct deleted_handler *newp = alloca (sizeof (*newp));
-	  newp->handler = runp;
-	  newp->next = deleted;
-	  deleted = newp;
-	}
-      else
-	lastp = runp;
-
-      runp = runp->next;
-    }
-  while (runp != NULL);
-
-  /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
-
-  /* Walk the list of all entries which have to be deleted.  */
-  while (deleted != NULL)
-    {
-      /* We need to be informed by possible current users.  */
-      deleted->handler->need_signal = 1;
-      /* Make sure this gets written out first.  */
-      atomic_write_barrier ();
-
-      /* Decrement the reference counter.  If it does not reach zero
-	 wait for the last user.  */
-      atomic_decrement (&deleted->handler->refcntr);
-      unsigned int val;
-      while ((val = deleted->handler->refcntr) != 0)
-	futex_wait_simple (&deleted->handler->refcntr, val, FUTEX_PRIVATE);
-
-      deleted = deleted->next;
-    }
-}
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 0061ee0..ec56a82 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -48,11 +48,6 @@ pid_t
 __libc_fork (void)
 {
   pid_t pid;
-  struct used_handler
-  {
-    struct fork_handler *handler;
-    struct used_handler *next;
-  } *allp = NULL;
 
   /* Determine if we are running multiple threads.  We skip some fork
      handlers in the single-thread case, to make fork safer to use in
@@ -60,60 +55,7 @@ __libc_fork (void)
      but our current fork implementation is not.  */
   bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
 
-  /* Run all the registered preparation handlers.  In reverse order.
-     While doing this we build up a list of all the entries.  */
-  struct fork_handler *runp;
-  while ((runp = __fork_handlers) != NULL)
-    {
-      /* Make sure we read from the current RUNP pointer.  */
-      atomic_full_barrier ();
-
-      unsigned int oldval = runp->refcntr;
-
-      if (oldval == 0)
-	/* This means some other thread removed the list just after
-	   the pointer has been loaded.  Try again.  Either the list
-	   is empty or we can retry it.  */
-	continue;
-
-      /* Bump the reference counter.  */
-      if (atomic_compare_and_exchange_bool_acq (&__fork_handlers->refcntr,
-						oldval + 1, oldval))
-	/* The value changed, try again.  */
-	continue;
-
-      /* We bumped the reference counter for the first entry in the
-	 list.  That means that none of the following entries will
-	 just go away.  The unloading code works in the order of the
-	 list.
-
-	 While executing the registered handlers we are building a
-	 list of all the entries so that we can go backward later on.  */
-      while (1)
-	{
-	  /* Execute the handler if there is one.  */
-	  if (runp->prepare_handler != NULL)
-	    runp->prepare_handler ();
-
-	  /* Create a new element for the list.  */
-	  struct used_handler *newp
-	    = (struct used_handler *) alloca (sizeof (*newp));
-	  newp->handler = runp;
-	  newp->next = allp;
-	  allp = newp;
-
-	  /* Advance to the next handler.  */
-	  runp = runp->next;
-	  if (runp == NULL)
-	    break;
-
-	  /* Bump the reference counter for the next entry.  */
-	  atomic_increment (&runp->refcntr);
-	}
-
-      /* We are done.  */
-      break;
-    }
+  __run_fork_handlers (atfork_run_prepare);
 
   /* If we are not running multiple threads, we do not have to
      preserve lock state.  If fork runs from a signal handler, only
@@ -192,29 +134,7 @@ __libc_fork (void)
       __rtld_lock_initialize (GL(dl_load_lock));
 
       /* Run the handlers registered for the child.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->child_handler != NULL)
-	    allp->handler->child_handler ();
-
-	  /* Note that we do not have to wake any possible waiter.
-	     This is the only thread in the new process.  The count
-	     may have been bumped up by other threads doing a fork.
-	     We reset it to 1, to avoid waiting for non-existing
-	     thread(s) to release the count.  */
-	  allp->handler->refcntr = 1;
-
-	  /* XXX We could at this point look through the object pool
-	     and mark all objects not on the __fork_handlers list as
-	     unused.  This is necessary in case the fork() happened
-	     while another thread called dlclose() and that call had
-	     to create a new list.  */
-
-	  allp = allp->next;
-	}
-
-      /* Initialize the fork lock.  */
-      __fork_lock = LLL_LOCK_INITIALIZER;
+      __run_fork_handlers (atfork_run_child);
     }
   else
     {
@@ -229,17 +149,7 @@ __libc_fork (void)
 	}
 
       /* Run the handlers registered for the parent.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->parent_handler != NULL)
-	    allp->handler->parent_handler ();
-
-	  if (atomic_decrement_and_test (&allp->handler->refcntr)
-	      && allp->handler->need_signal)
-	    futex_wake (&allp->handler->refcntr, 1, FUTEX_PRIVATE);
-
-	  allp = allp->next;
-	}
+      __run_fork_handlers (atfork_run_parent);
     }
 
   return pid;
diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
index f0330cc..6eab61c 100644
--- a/sysdeps/nptl/fork.h
+++ b/sysdeps/nptl/fork.h
@@ -24,29 +24,37 @@ extern unsigned long int __fork_generation attribute_hidden;
 /* Pointer to the fork generation counter in the thread library.  */
 extern unsigned long int *__fork_generation_pointer attribute_hidden;
 
-/* Lock to protect allocation and deallocation of fork handlers.  */
-extern int __fork_lock attribute_hidden;
-
 /* Elements of the fork handler lists.  */
 struct fork_handler
 {
-  struct fork_handler *next;
   void (*prepare_handler) (void);
   void (*parent_handler) (void);
   void (*child_handler) (void);
   void *dso_handle;
-  unsigned int refcntr;
-  int need_signal;
 };
 
-/* The single linked list of all currently registered for handlers.  */
-extern struct fork_handler *__fork_handlers attribute_hidden;
-
-
 /* Function to call to unregister fork handlers.  */
 extern void __unregister_atfork (void *dso_handle) attribute_hidden;
 #define UNREGISTER_ATFORK(dso_handle) __unregister_atfork (dso_handle)
 
+enum __run_fork_handler_type
+{
+  atfork_run_prepare,
+  atfork_run_child,
+  atfork_run_parent
+};
+
+/* Run the atfork handlers and lock/unlock the internal lock depending
+   of the WHO argument:
+
+   - atfork_run_prepare: run all the PREPARE_HANDLER in reverse order of
+			 insertion and locks the internal lock.
+   - atfork_run_child: run all the CHILD_HANDLER and unlocks the internal
+		       lock.
+   - atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal
+			lock.  */
+extern void __run_fork_handlers (enum __run_fork_handler_type who)
+  attribute_hidden;
 
 /* C library side function to register new fork handlers.  */
 extern int __register_atfork (void (*__prepare) (void),
@@ -54,6 +62,3 @@ extern int __register_atfork (void (*__prepare) (void),
 			      void (*__child) (void),
 			      void *dso_handle);
 libc_hidden_proto (__register_atfork)
-
-/* Add a new element to the fork list.  */
-extern void __linkin_atfork (struct fork_handler *newp) attribute_hidden;
diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h
index 8539bbf..989fefa 100644
--- a/sysdeps/nptl/libc-lockP.h
+++ b/sysdeps/nptl/libc-lockP.h
@@ -319,8 +319,6 @@ __libc_cleanup_routine (struct __pthread_cleanup_frame *f)
 /* Register handlers to execute before and after `fork'.  Note that the
    last parameter is NULL.  The handlers registered by the libc are
    never removed so this is OK.  */
-#define __libc_atfork(PREPARE, PARENT, CHILD) \
-  __register_atfork (PREPARE, PARENT, CHILD, NULL)
 extern int __register_atfork (void (*__prepare) (void),
 			      void (*__parent) (void),
 			      void (*__child) (void),
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-07 17:16     ` Adhemerval Zanella
@ 2018-02-08  8:32       ` Florian Weimer
  2018-02-08 12:50         ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-08  8:32 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/07/2018 06:16 PM, Adhemerval Zanella wrote:
> +  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers);)
> +    {
> +      /* dynarray remove maintains element order, so update index iff there is
> +	 no removal.  */
> +      if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
> +        fork_handler_list_remove (&fork_handlers, i);
> +      else
> +        i++;
> +    }

I thought a bit more about this.  Doesn't this lead to cubic run-time as 
DSOs are unloaded (quadratic run-time locally here, multiplied by the 
outer loop for unloading the DSOs)?

I think fork_handler_list_remove is the wrong abstraction here. 
Something like std::remove_if would be better, which moves each array 
element at most once even if multiple elements are removed during the 
scan.  Writing this generically in C is probably not worth the effort, 
so perhaps open-code that here?

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-08  8:32       ` Florian Weimer
@ 2018-02-08 12:50         ` Adhemerval Zanella
  2018-02-20 11:29           ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-08 12:50 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 08/02/2018 06:32, Florian Weimer wrote:
> On 02/07/2018 06:16 PM, Adhemerval Zanella wrote:
>> +  for (size_t i = 0; i < fork_handler_list_size (&fork_handlers);)
>> +    {
>> +      /* dynarray remove maintains element order, so update index iff there is
>> +     no removal.  */
>> +      if (fork_handler_list_at (&fork_handlers, i)->dso_handle == dso_handle)
>> +        fork_handler_list_remove (&fork_handlers, i);
>> +      else
>> +        i++;
>> +    }
> 
> I thought a bit more about this.  Doesn't this lead to cubic run-time as DSOs are unloaded (quadratic run-time locally here, multiplied by the outer loop for unloading the DSOs)?
> 
> I think fork_handler_list_remove is the wrong abstraction here. Something like std::remove_if would be better, which moves each array element at most once even if multiple elements are removed during the scan.  Writing this generically in C is probably not worth the effort, so perhaps open-code that here?
> 
> Thanks,
> Florian

I though about it and I decided use the simplest approach mainly because I assume
the at fork handler number should not that high (current static buffer assumes a
size of 48).  Using a simple benchmark to measure the difference (measures the
time using clock_gettime to remove all elements in the list) I see:

size: 48
  remove    = 1236
  remove_if = 249
size: 1024
  remove    = 313755
  remove_if = 1017
bench: 16384
  remove    = 123934220
  remove_if = 40881

I found 1000 ns and even 30 us negligible, however I do agree if generic usage aims
for high atfork handler remove_if is indeed a better strategy.  I adjusted my patch 
to use instead (and I think we can drop the dynarray remove for now).

--

diff --git a/nptl/Makefile b/nptl/Makefile
index 6fc2c8b..be7ee3e 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -30,7 +30,7 @@ install-lib-ldscripts := libpthread.so
 
 routines = alloca_cutoff forward libc-lowlevellock libc-cancellation \
 	   libc-cleanup libc_pthread_init libc_multiple_threads \
-	   register-atfork unregister-atfork pthread_self
+	   register-atfork pthread_self
 shared-only-routines = forward
 
 # We need to provide certain routines for compatibility with existing
diff --git a/nptl/register-atfork.c b/nptl/register-atfork.c
index f309cec..5826e4c 100644
--- a/nptl/register-atfork.c
+++ b/nptl/register-atfork.c
@@ -22,123 +22,127 @@
 #include <fork.h>
 #include <atomic.h>
 
+#define DYNARRAY_ELEMENT           struct fork_handler
+#define DYNARRAY_STRUCT            fork_handler_list
+#define DYNARRAY_PREFIX            fork_handler_list_
+#define DYNARRAY_INITIAL_SIZE      48
+#include <malloc/dynarray-skeleton.c>
 
-struct fork_handler *__fork_handlers;
-
-/* Lock to protect allocation and deallocation of fork handlers.  */
-int __fork_lock = LLL_LOCK_INITIALIZER;
-
-
-/* Number of pre-allocated handler entries.  */
-#define NHANDLER 48
-
-/* Memory pool for fork handler structures.  */
-static struct fork_handler_pool
-{
-  struct fork_handler_pool *next;
-  struct fork_handler mem[NHANDLER];
-} fork_handler_pool;
-
-
-static struct fork_handler *
-fork_handler_alloc (void)
-{
-  struct fork_handler_pool *runp = &fork_handler_pool;
-  struct fork_handler *result = NULL;
-  unsigned int i;
-
-  do
-    {
-      /* Search for an empty entry.  */
-      for (i = 0; i < NHANDLER; ++i)
-	if (runp->mem[i].refcntr == 0)
-	  goto found;
-    }
-  while ((runp = runp->next) != NULL);
-
-  /* We have to allocate a new entry.  */
-  runp = (struct fork_handler_pool *) calloc (1, sizeof (*runp));
-  if (runp != NULL)
-    {
-      /* Enqueue the new memory pool into the list.  */
-      runp->next = fork_handler_pool.next;
-      fork_handler_pool.next = runp;
-
-      /* We use the last entry on the page.  This means when we start
-	 searching from the front the next time we will find the first
-	 entry unused.  */
-      i = NHANDLER - 1;
-
-    found:
-      result = &runp->mem[i];
-      result->refcntr = 1;
-      result->need_signal = 0;
-    }
-
-  return result;
-}
+static struct fork_handler_list fork_handlers;
+static bool fork_handler_init = false;
 
+static int atfork_lock = LLL_LOCK_INITIALIZER;
 
 int
 __register_atfork (void (*prepare) (void), void (*parent) (void),
 		   void (*child) (void), void *dso_handle)
 {
-  /* Get the lock to not conflict with other allocations.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  struct fork_handler *newp = fork_handler_alloc ();
+  if (!fork_handler_init)
+    {
+      fork_handler_list_init (&fork_handlers);
+      fork_handler_init = true;
+    }
 
+  struct fork_handler *newp = fork_handler_list_emplace (&fork_handlers);
   if (newp != NULL)
     {
-      /* Initialize the new record.  */
       newp->prepare_handler = prepare;
       newp->parent_handler = parent;
       newp->child_handler = child;
       newp->dso_handle = dso_handle;
-
-      __linkin_atfork (newp);
     }
 
   /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 
   return newp == NULL ? ENOMEM : 0;
 }
 libc_hidden_def (__register_atfork)
 
+static struct fork_handler *
+fork_handler_list_find_if (struct fork_handler_list *fork_handlers,
+			   void *dso_handle)
+{
+  for (size_t i = 0; i < fork_handler_list_size (fork_handlers); i++)
+    {
+      struct fork_handler *elem = fork_handler_list_at (fork_handlers, i);
+      if (elem->dso_handle == dso_handle)
+	return elem;
+    }
+  return NULL;
+}
 
 void
-attribute_hidden
-__linkin_atfork (struct fork_handler *newp)
+__unregister_atfork (void *dso_handle)
 {
-  do
-    newp->next = __fork_handlers;
-  while (catomic_compare_and_exchange_bool_acq (&__fork_handlers,
-						newp, newp->next) != 0);
-}
+  lll_lock (atfork_lock, LLL_PRIVATE);
+
+  struct fork_handler *first = fork_handler_list_find_if (&fork_handlers,
+							  dso_handle);
+  /* Removing is done by shifting the elements in the way the elements
+     that are not to be removed appear in the beginning in dynarray.
+     This avoid the quadradic run-time if a naive strategy to remove and
+     shift one element at time.  */
+  if (first != NULL)
+    {
+      struct fork_handler *result = first;
+      first++;
+      for (; first != fork_handler_list_end (&fork_handlers); ++first)
+	{
+	  if (first->dso_handle != dso_handle)
+	    {
+	      memcpy (result, first, sizeof (struct fork_handler));
+	      ++result;
+	    }
+	}
+
+      ptrdiff_t removed = first - result;
+      for (size_t i = 0; i < removed; i++)
+	fork_handler_list_remove_last (&fork_handlers);
+    }
 
+  lll_unlock (atfork_lock, LLL_PRIVATE);
+}
 
-libc_freeres_fn (free_mem)
+void
+__run_fork_handlers (enum __run_fork_handler_type who)
 {
-  /* Get the lock to not conflict with running forks.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
+  struct fork_handler *runp;
 
-  /* No more fork handlers.  */
-  __fork_handlers = NULL;
+  if (who == atfork_run_prepare)
+    {
+      lll_lock (atfork_lock, LLL_PRIVATE);
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = sl; i > 0; i--)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i - 1);
+	  if (runp->prepare_handler != NULL)
+	    runp->prepare_handler ();
+	}
+    }
+  else
+    {
+      size_t sl = fork_handler_list_size (&fork_handlers);
+      for (size_t i = 0; i < sl; i++)
+	{
+	  runp = fork_handler_list_at (&fork_handlers, i);
+	  if (who == atfork_run_child && runp->child_handler)
+	    runp->child_handler ();
+	  else if (who == atfork_run_parent && runp->parent_handler)
+	    runp->parent_handler ();
+	}
+      lll_unlock (atfork_lock, LLL_PRIVATE);
+    }
+}
 
-  /* Free eventually allocated memory blocks for the object pool.  */
-  struct fork_handler_pool *runp = fork_handler_pool.next;
 
-  memset (&fork_handler_pool, '\0', sizeof (fork_handler_pool));
+libc_freeres_fn (free_mem)
+{
+  lll_lock (atfork_lock, LLL_PRIVATE);
 
-  /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
+  fork_handler_list_free (&fork_handlers);
 
-  /* We can free the memory after releasing the lock.  */
-  while (runp != NULL)
-    {
-      struct fork_handler_pool *oldp = runp;
-      runp = runp->next;
-      free (oldp);
-    }
+  lll_unlock (atfork_lock, LLL_PRIVATE);
 }
diff --git a/nptl/unregister-atfork.c b/nptl/unregister-atfork.c
deleted file mode 100644
index 20411ed..0000000
--- a/nptl/unregister-atfork.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/* Copyright (C) 2002-2018 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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <stdlib.h>
-#include <fork.h>
-#include <atomic.h>
-#include <futex-internal.h>
-
-
-void
-__unregister_atfork (void *dso_handle)
-{
-  /* Check whether there is any entry in the list which we have to
-     remove.  It is likely that this is not the case so don't bother
-     getting the lock.
-
-     We do not worry about other threads adding entries for this DSO
-     right this moment.  If this happens this is a race and we can do
-     whatever we please.  The program will crash anyway seen.  */
-  struct fork_handler *runp = __fork_handlers;
-  struct fork_handler *lastp = NULL;
-
-  while (runp != NULL)
-    if (runp->dso_handle == dso_handle)
-      break;
-    else
-      {
-	lastp = runp;
-	runp = runp->next;
-      }
-
-  if (runp == NULL)
-    /* Nothing to do.  */
-    return;
-
-  /* Get the lock to not conflict with additions or deletions.  Note
-     that there couldn't have been another thread deleting something.
-     The __unregister_atfork function is only called from the
-     dlclose() code which itself serializes the operations.  */
-  lll_lock (__fork_lock, LLL_PRIVATE);
-
-  /* We have to create a new list with all the entries we don't remove.  */
-  struct deleted_handler
-  {
-    struct fork_handler *handler;
-    struct deleted_handler *next;
-  } *deleted = NULL;
-
-  /* Remove the entries for the DSO which is unloaded from the list.
-     It's a single linked list so readers are.  */
-  do
-    {
-    again:
-      if (runp->dso_handle == dso_handle)
-	{
-	  if (lastp == NULL)
-	    {
-	      /* We have to use an atomic operation here because
-		 __linkin_atfork also uses one.  */
-	      if (catomic_compare_and_exchange_bool_acq (&__fork_handlers,
-							 runp->next, runp)
-		  != 0)
-		{
-		  runp = __fork_handlers;
-		  goto again;
-		}
-	    }
-	  else
-	    lastp->next = runp->next;
-
-	  /* We cannot overwrite the ->next element now.  Put the deleted
-	     entries in a separate list.  */
-	  struct deleted_handler *newp = alloca (sizeof (*newp));
-	  newp->handler = runp;
-	  newp->next = deleted;
-	  deleted = newp;
-	}
-      else
-	lastp = runp;
-
-      runp = runp->next;
-    }
-  while (runp != NULL);
-
-  /* Release the lock.  */
-  lll_unlock (__fork_lock, LLL_PRIVATE);
-
-  /* Walk the list of all entries which have to be deleted.  */
-  while (deleted != NULL)
-    {
-      /* We need to be informed by possible current users.  */
-      deleted->handler->need_signal = 1;
-      /* Make sure this gets written out first.  */
-      atomic_write_barrier ();
-
-      /* Decrement the reference counter.  If it does not reach zero
-	 wait for the last user.  */
-      atomic_decrement (&deleted->handler->refcntr);
-      unsigned int val;
-      while ((val = deleted->handler->refcntr) != 0)
-	futex_wait_simple (&deleted->handler->refcntr, val, FUTEX_PRIVATE);
-
-      deleted = deleted->next;
-    }
-}
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 0061ee0..ec56a82 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -48,11 +48,6 @@ pid_t
 __libc_fork (void)
 {
   pid_t pid;
-  struct used_handler
-  {
-    struct fork_handler *handler;
-    struct used_handler *next;
-  } *allp = NULL;
 
   /* Determine if we are running multiple threads.  We skip some fork
      handlers in the single-thread case, to make fork safer to use in
@@ -60,60 +55,7 @@ __libc_fork (void)
      but our current fork implementation is not.  */
   bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
 
-  /* Run all the registered preparation handlers.  In reverse order.
-     While doing this we build up a list of all the entries.  */
-  struct fork_handler *runp;
-  while ((runp = __fork_handlers) != NULL)
-    {
-      /* Make sure we read from the current RUNP pointer.  */
-      atomic_full_barrier ();
-
-      unsigned int oldval = runp->refcntr;
-
-      if (oldval == 0)
-	/* This means some other thread removed the list just after
-	   the pointer has been loaded.  Try again.  Either the list
-	   is empty or we can retry it.  */
-	continue;
-
-      /* Bump the reference counter.  */
-      if (atomic_compare_and_exchange_bool_acq (&__fork_handlers->refcntr,
-						oldval + 1, oldval))
-	/* The value changed, try again.  */
-	continue;
-
-      /* We bumped the reference counter for the first entry in the
-	 list.  That means that none of the following entries will
-	 just go away.  The unloading code works in the order of the
-	 list.
-
-	 While executing the registered handlers we are building a
-	 list of all the entries so that we can go backward later on.  */
-      while (1)
-	{
-	  /* Execute the handler if there is one.  */
-	  if (runp->prepare_handler != NULL)
-	    runp->prepare_handler ();
-
-	  /* Create a new element for the list.  */
-	  struct used_handler *newp
-	    = (struct used_handler *) alloca (sizeof (*newp));
-	  newp->handler = runp;
-	  newp->next = allp;
-	  allp = newp;
-
-	  /* Advance to the next handler.  */
-	  runp = runp->next;
-	  if (runp == NULL)
-	    break;
-
-	  /* Bump the reference counter for the next entry.  */
-	  atomic_increment (&runp->refcntr);
-	}
-
-      /* We are done.  */
-      break;
-    }
+  __run_fork_handlers (atfork_run_prepare);
 
   /* If we are not running multiple threads, we do not have to
      preserve lock state.  If fork runs from a signal handler, only
@@ -192,29 +134,7 @@ __libc_fork (void)
       __rtld_lock_initialize (GL(dl_load_lock));
 
       /* Run the handlers registered for the child.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->child_handler != NULL)
-	    allp->handler->child_handler ();
-
-	  /* Note that we do not have to wake any possible waiter.
-	     This is the only thread in the new process.  The count
-	     may have been bumped up by other threads doing a fork.
-	     We reset it to 1, to avoid waiting for non-existing
-	     thread(s) to release the count.  */
-	  allp->handler->refcntr = 1;
-
-	  /* XXX We could at this point look through the object pool
-	     and mark all objects not on the __fork_handlers list as
-	     unused.  This is necessary in case the fork() happened
-	     while another thread called dlclose() and that call had
-	     to create a new list.  */
-
-	  allp = allp->next;
-	}
-
-      /* Initialize the fork lock.  */
-      __fork_lock = LLL_LOCK_INITIALIZER;
+      __run_fork_handlers (atfork_run_child);
     }
   else
     {
@@ -229,17 +149,7 @@ __libc_fork (void)
 	}
 
       /* Run the handlers registered for the parent.  */
-      while (allp != NULL)
-	{
-	  if (allp->handler->parent_handler != NULL)
-	    allp->handler->parent_handler ();
-
-	  if (atomic_decrement_and_test (&allp->handler->refcntr)
-	      && allp->handler->need_signal)
-	    futex_wake (&allp->handler->refcntr, 1, FUTEX_PRIVATE);
-
-	  allp = allp->next;
-	}
+      __run_fork_handlers (atfork_run_parent);
     }
 
   return pid;
diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
index f0330cc..6eab61c 100644
--- a/sysdeps/nptl/fork.h
+++ b/sysdeps/nptl/fork.h
@@ -24,29 +24,37 @@ extern unsigned long int __fork_generation attribute_hidden;
 /* Pointer to the fork generation counter in the thread library.  */
 extern unsigned long int *__fork_generation_pointer attribute_hidden;
 
-/* Lock to protect allocation and deallocation of fork handlers.  */
-extern int __fork_lock attribute_hidden;
-
 /* Elements of the fork handler lists.  */
 struct fork_handler
 {
-  struct fork_handler *next;
   void (*prepare_handler) (void);
   void (*parent_handler) (void);
   void (*child_handler) (void);
   void *dso_handle;
-  unsigned int refcntr;
-  int need_signal;
 };
 
-/* The single linked list of all currently registered for handlers.  */
-extern struct fork_handler *__fork_handlers attribute_hidden;
-
-
 /* Function to call to unregister fork handlers.  */
 extern void __unregister_atfork (void *dso_handle) attribute_hidden;
 #define UNREGISTER_ATFORK(dso_handle) __unregister_atfork (dso_handle)
 
+enum __run_fork_handler_type
+{
+  atfork_run_prepare,
+  atfork_run_child,
+  atfork_run_parent
+};
+
+/* Run the atfork handlers and lock/unlock the internal lock depending
+   of the WHO argument:
+
+   - atfork_run_prepare: run all the PREPARE_HANDLER in reverse order of
+			 insertion and locks the internal lock.
+   - atfork_run_child: run all the CHILD_HANDLER and unlocks the internal
+		       lock.
+   - atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal
+			lock.  */
+extern void __run_fork_handlers (enum __run_fork_handler_type who)
+  attribute_hidden;
 
 /* C library side function to register new fork handlers.  */
 extern int __register_atfork (void (*__prepare) (void),
@@ -54,6 +62,3 @@ extern int __register_atfork (void (*__prepare) (void),
 			      void (*__child) (void),
 			      void *dso_handle);
 libc_hidden_proto (__register_atfork)
-
-/* Add a new element to the fork list.  */
-extern void __linkin_atfork (struct fork_handler *newp) attribute_hidden;
diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h
index 8539bbf..989fefa 100644
--- a/sysdeps/nptl/libc-lockP.h
+++ b/sysdeps/nptl/libc-lockP.h
@@ -319,8 +319,6 @@ __libc_cleanup_routine (struct __pthread_cleanup_frame *f)
 /* Register handlers to execute before and after `fork'.  Note that the
    last parameter is NULL.  The handlers registered by the libc are
    never removed so this is OK.  */
-#define __libc_atfork(PREPARE, PARENT, CHILD) \
-  __register_atfork (PREPARE, PARENT, CHILD, NULL)
 extern int __register_atfork (void (*__prepare) (void),
 			      void (*__parent) (void),
 			      void (*__child) (void),
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-08 12:50         ` Adhemerval Zanella
@ 2018-02-20 11:29           ` Florian Weimer
  2018-02-20 13:00             ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-20 11:29 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/08/2018 01:50 PM, Adhemerval Zanella wrote:
> +static struct fork_handler *
> +fork_handler_list_find_if (struct fork_handler_list *fork_handlers,
> +			   void *dso_handle)

Should be called _find, not find_if (no callback is involved).

> +  struct fork_handler *first = fork_handler_list_find_if (&fork_handlers,
> +							  dso_handle);
> +  /* Removing is done by shifting the elements in the way the elements
> +     that are not to be removed appear in the beginning in dynarray.
> +     This avoid the quadradic run-time if a naive strategy to remove and
> +     shift one element at time.  */
> +  if (first != NULL)
> +    {
> +      struct fork_handler *result = first;

result should probably be called new_end or something like that.

> +      first++;
> +      for (; first != fork_handler_list_end (&fork_handlers); ++first)
> +	{
> +	  if (first->dso_handle != dso_handle)
> +	    {
> +	      memcpy (result, first, sizeof (struct fork_handler));

Wouldn't a simple struct assignment work here?

I think this patch is a step in the right direction, so it should go in 
with these changes.

However, I think we should make a few improvements in follow-up fixes:

Reduce RSS usage for the common case that no atfork handlers are ever 
registered.  This will be the case once we remove the bogus 
__reclaim_stacks function.

Make a temporary copy of the handler array during fork.  This has two 
benefits: We can run the handlers without acquiring the handler lock (to 
avoid application deadlocks).  We also make sure that a handler does not 
run in a child process which did not run in the parent process.  I think 
the old implementation had both properties.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 11:29           ` Florian Weimer
@ 2018-02-20 13:00             ` Adhemerval Zanella
  2018-02-20 13:05               ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-20 13:00 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 20/02/2018 08:29, Florian Weimer wrote:
> On 02/08/2018 01:50 PM, Adhemerval Zanella wrote:
>> +static struct fork_handler *
>> +fork_handler_list_find_if (struct fork_handler_list *fork_handlers,
>> +               void *dso_handle)
> 
> Should be called _find, not find_if (no callback is involved).

Fixed.

> 
>> +  struct fork_handler *first = fork_handler_list_find_if (&fork_handlers,
>> +                              dso_handle);
>> +  /* Removing is done by shifting the elements in the way the elements
>> +     that are not to be removed appear in the beginning in dynarray.
>> +     This avoid the quadradic run-time if a naive strategy to remove and
>> +     shift one element at time.  */
>> +  if (first != NULL)
>> +    {
>> +      struct fork_handler *result = first;
> 
> result should probably be called new_end or something like that.

I changed to new_end.

> 
>> +      first++;
>> +      for (; first != fork_handler_list_end (&fork_handlers); ++first)
>> +    {
>> +      if (first->dso_handle != dso_handle)
>> +        {
>> +          memcpy (result, first, sizeof (struct fork_handler));
> 
> Wouldn't a simple struct assignment work here?

I think so, I changed it to struct assignment.

> 
> I think this patch is a step in the right direction, so it should go in with these changes.

Thanks for the review.

> 
> However, I think we should make a few improvements in follow-up fixes:
> 
> Reduce RSS usage for the common case that no atfork handlers are ever registered.  This will be the case once we remove the bogus __reclaim_stacks function.
> 
> Make a temporary copy of the handler array during fork.  This has two benefits: We can run the handlers without acquiring the handler lock (to avoid application deadlocks).  We also make sure that a handler does not run in a child process which did not run in the parent process.  I think the old implementation had both properties.

The temporary copy is problematic because we either need to allocate on the stack using
vla/alloca (current practice and prone of stack overflow) or by malloc (which requires
locking anyway).  Also, to temporary copy we will need pretty much the same lock-free
algorithm which adds code complexity.

My understanding is current algorithm tries hard to remove any locking on fork generation
mainly because back then posix_spawn was no specified and suboptimal. Now that we have
a faster way to spawn process in multithread environment I think there is no much gain
in trying to optimizing locking in atfork handlers.

Regarding the handler running in child process the proposed implementation does implement
it.  


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:00             ` Adhemerval Zanella
@ 2018-02-20 13:05               ` Florian Weimer
  2018-02-20 13:27                 ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-20 13:05 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/20/2018 02:00 PM, Adhemerval Zanella wrote:
> The temporary copy is problematic because we either need to allocate on the stack using
> vla/alloca (current practice and prone of stack overflow) or by malloc (which requires
> locking anyway).  Also, to temporary copy we will need pretty much the same lock-free
> algorithm which adds code complexity.

I think the lock in malloc is fine, at least for the time being, with 
our non-async-safe fork.

The point is not avoiding the lock, but callbacks when the lock is held. 
  This can easily result in deadlocks.

> My understanding is current algorithm tries hard to remove any locking on fork generation
> mainly because back then posix_spawn was no specified and suboptimal. Now that we have
> a faster way to spawn process in multithread environment I think there is no much gain
> in trying to optimizing locking in atfork handlers.

I think it's also needed to avoid deadlocks .

> Regarding the handler running in child process the proposed implementation does implement
 > it.

I don't see how?  I meant that only those handlers run that ran in the 
parent.  I think there's a window where more fork handlers can be added.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:05               ` Florian Weimer
@ 2018-02-20 13:27                 ` Adhemerval Zanella
  2018-02-20 13:42                   ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-20 13:27 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 20/02/2018 10:05, Florian Weimer wrote:
> On 02/20/2018 02:00 PM, Adhemerval Zanella wrote:
>> The temporary copy is problematic because we either need to allocate on the stack using
>> vla/alloca (current practice and prone of stack overflow) or by malloc (which requires
>> locking anyway).  Also, to temporary copy we will need pretty much the same lock-free
>> algorithm which adds code complexity.
> 
> I think the lock in malloc is fine, at least for the time being, with our non-async-safe fork.
> 
> The point is not avoiding the lock, but callbacks when the lock is held.  This can easily result in deadlocks.

I think it might occur with proposed implementation only if a callback tries to call
pthread_atfork or fork itself.  It these scenario you have in mind? And should we
really support them if this is the case?


> 
>> My understanding is current algorithm tries hard to remove any locking on fork generation
>> mainly because back then posix_spawn was no specified and suboptimal. Now that we have
>> a faster way to spawn process in multithread environment I think there is no much gain
>> in trying to optimizing locking in atfork handlers.
> 
> I think it's also needed to avoid deadlocks .
> 
>> Regarding the handler running in child process the proposed implementation does implement
>> it.
> 
> I don't see how?  I meant that only those handlers run that ran in the parent.  I think there's a window where more fork handlers can be added.
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:27                 ` Adhemerval Zanella
@ 2018-02-20 13:42                   ` Florian Weimer
  2018-02-20 13:48                     ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-20 13:42 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/20/2018 02:27 PM, Adhemerval Zanella wrote:
> I think it might occur with proposed implementation only if a callback tries to call
> pthread_atfork or fork itself.  It these scenario you have in mind? And should we
> really support them if this is the case?

No.

__libc_fork starts like this:

   bool multiple_threads
     = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);

   __run_fork_handlers (atfork_run_prepare);

And then acquires _IO_list_lock.

I don't see anything which prevents concurrent registration of 
additional fork handlers between the first and second call to 
__run_fork_handlers.

As I said, that shouldn't prevent inclusion of the current patch, but we 
need to fix this before 2.28, I think.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:42                   ` Florian Weimer
@ 2018-02-20 13:48                     ` Adhemerval Zanella
  2018-02-20 13:58                       ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-20 13:48 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 20/02/2018 10:42, Florian Weimer wrote:
> On 02/20/2018 02:27 PM, Adhemerval Zanella wrote:
>> I think it might occur with proposed implementation only if a callback tries to call
>> pthread_atfork or fork itself.  It these scenario you have in mind? And should we
>> really support them if this is the case?
> 
> No.
> 
> __libc_fork starts like this:
> 
>   bool multiple_threads
>     = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
> 
>   __run_fork_handlers (atfork_run_prepare);
> 
> And then acquires _IO_list_lock.
> 
> I don't see anything which prevents concurrent registration of additional fork handlers between the first and second call to __run_fork_handlers.

The atfork_run_prepare will instruct __run_fork_handlers to take the internal
atfork_lock handler:

  void
  __run_fork_handlers (enum __run_fork_handler_type who)
  {
    struct fork_handler *runp;

    if (who == atfork_run_prepare)
      {
        lll_lock (atfork_lock, LLL_PRIVATE);

And it will prevent to add new registration until either the parent or the child
call __run_fork_handlers with either 'atfork_run_child' or 'atfork_run_parent'
to release the lock.

> 
> As I said, that shouldn't prevent inclusion of the current patch, but we need to fix this before 2.28, I think.
> 
> Thanks,
> Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:48                     ` Adhemerval Zanella
@ 2018-02-20 13:58                       ` Florian Weimer
  2018-02-20 14:23                         ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-20 13:58 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/20/2018 02:48 PM, Adhemerval Zanella wrote:

> The atfork_run_prepare will instruct __run_fork_handlers to take the internal
> atfork_lock handler:
> 
>    void
>    __run_fork_handlers (enum __run_fork_handler_type who)
>    {
>      struct fork_handler *runp;
> 
>      if (who == atfork_run_prepare)
>        {
>          lll_lock (atfork_lock, LLL_PRIVATE);
> 
> And it will prevent to add new registration until either the parent or the child
> call __run_fork_handlers with either 'atfork_run_child' or 'atfork_run_parent'
> to release the lock.

Oh, sorry, I missed that.  So the patch does not have this problem. 
This does not settle the deadlock issue, though.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 13:58                       ` Florian Weimer
@ 2018-02-20 14:23                         ` Adhemerval Zanella
  2018-02-23 10:41                           ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-20 14:23 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 20/02/2018 10:58, Florian Weimer wrote:
> On 02/20/2018 02:48 PM, Adhemerval Zanella wrote:
> 
>> The atfork_run_prepare will instruct __run_fork_handlers to take the internal
>> atfork_lock handler:
>>
>>    void
>>    __run_fork_handlers (enum __run_fork_handler_type who)
>>    {
>>      struct fork_handler *runp;
>>
>>      if (who == atfork_run_prepare)
>>        {
>>          lll_lock (atfork_lock, LLL_PRIVATE);
>>
>> And it will prevent to add new registration until either the parent or the child
>> call __run_fork_handlers with either 'atfork_run_child' or 'atfork_run_parent'
>> to release the lock.
> 
> Oh, sorry, I missed that.  So the patch does not have this problem. This does not settle the deadlock issue, though.

Aside of the two scenarios (callbacks issuing fork/pthread_atfork), the only
other scenario I see which might trigger a deadlock in this case is a signal
handler issuing fork/pthread_atfork.  

Former is BZ#4737 and my understanding is this should be a EWONTFIX due 
indication future POSIX specification to interpret fork as async-signal-unsafe 
(comment #19 and I am not sure if fork could be made async-signal-safe with 
ticket locks as Rich stated in comment #21).  

Regarding later I think pthread_atfork is inherent async-signal-unsafe due
it might return ENOMEM indicating it might allocate memory and our malloc
is also async-signal-unsafe.

Am I missing a scenario you might be considering?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-20 14:23                         ` Adhemerval Zanella
@ 2018-02-23 10:41                           ` Florian Weimer
  2018-02-23 12:10                             ` Adhemerval Zanella
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-02-23 10:41 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/20/2018 03:23 PM, Adhemerval Zanella wrote:

> Aside of the two scenarios (callbacks issuing fork/pthread_atfork), the only
> other scenario I see which might trigger a deadlock in this case is a signal
> handler issuing fork/pthread_atfork.
> 
> Former is BZ#4737 and my understanding is this should be a EWONTFIX due
> indication future POSIX specification to interpret fork as async-signal-unsafe
> (comment #19 and I am not sure if fork could be made async-signal-safe with
> ticket locks as Rich stated in comment #21).
> 
> Regarding later I think pthread_atfork is inherent async-signal-unsafe due
> it might return ENOMEM indicating it might allocate memory and our malloc
> is also async-signal-unsafe.
> 
> Am I missing a scenario you might be considering?

I looked at the acquired locks during fork, and you are right, the 
corner cases where a deadlock can happen in the upstream sources are 
quite obscure.  However, we do not currently acquire any ld.so locks, 
and I think I've seen patches which change that (because upstream is 
buggy and crash in the new child process).  If any ld.so locks are 
acquired around fork, then we have a lock ordering conflict in case an 
ELF constructor calls pthread_register_atfork (which is an extremely 
natural thing to do), like this:

Fork:

   pthread_register_atfork lock
     rtld load lock

dlopen:

   rtld load lock
     calling ELF constructors, and then:
       pthread_register_atfork lock

The older lock-free code avoids this.  You could do the same even with 
locks if you created a copy of the handler list on the heap.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-23 10:41                           ` Florian Weimer
@ 2018-02-23 12:10                             ` Adhemerval Zanella
  2018-02-27  8:25                               ` Florian Weimer
  0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2018-02-23 12:10 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 23/02/2018 07:41, Florian Weimer wrote:
> On 02/20/2018 03:23 PM, Adhemerval Zanella wrote:
> 
>> Aside of the two scenarios (callbacks issuing fork/pthread_atfork), the only
>> other scenario I see which might trigger a deadlock in this case is a signal
>> handler issuing fork/pthread_atfork.
>>
>> Former is BZ#4737 and my understanding is this should be a EWONTFIX due
>> indication future POSIX specification to interpret fork as async-signal-unsafe
>> (comment #19 and I am not sure if fork could be made async-signal-safe with
>> ticket locks as Rich stated in comment #21).
>>
>> Regarding later I think pthread_atfork is inherent async-signal-unsafe due
>> it might return ENOMEM indicating it might allocate memory and our malloc
>> is also async-signal-unsafe.
>>
>> Am I missing a scenario you might be considering?
> 
> I looked at the acquired locks during fork, and you are right, the corner cases where a deadlock can happen in the upstream sources are quite obscure.  However, we do not currently acquire any ld.so locks, and I think I've seen patches which change that (because upstream is buggy and crash in the new child process).  If any ld.so locks are acquired around fork, then we have a lock ordering conflict in case an ELF constructor calls pthread_register_atfork (which is an extremely natural thing to do), like this:
> 
> Fork:
> 
>   pthread_register_atfork lock
>     rtld load lock
> 
> dlopen:
> 
>   rtld load lock
>     calling ELF constructors, and then:
>       pthread_register_atfork lock
> 
> The older lock-free code avoids this.  You could do the same even with locks if you created a copy of the handler list on the heap.

MY understanding is ld.so locks might be acquired in the callback calls from
__run_fork_handlers:

  fork:
    __run_fork_handlers (atfork_run_prepare)
      lll_lock (atfork_lock)
      <callback>
         rtld load lock

However I do not see who in a different thread dlopen would acquire the same 
lock since it has been already acquired by the callback.  The only way is if 
dlopen is being called by a signal handler, which I think it another obscure 
corner case.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/3] Refactor atfork handlers
  2018-02-23 12:10                             ` Adhemerval Zanella
@ 2018-02-27  8:25                               ` Florian Weimer
  0 siblings, 0 replies; 23+ messages in thread
From: Florian Weimer @ 2018-02-27  8:25 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/23/2018 01:10 PM, Adhemerval Zanella wrote:
> MY understanding is ld.so locks might be acquired in the callback calls from
> __run_fork_handlers:
> 
>    fork:
>      __run_fork_handlers (atfork_run_prepare)
>        lll_lock (atfork_lock)
>        <callback>
>           rtld load lock

Yes, that could happen even with the existing code.

My concern was with certain downstream patches in some distributions 
which acquire the rtld lock around fork, to avoid potentially corrupting 
the dynamic linker state in the child process (because the fork can no 
longer race with rtld data structure updates).

> However I do not see who in a different thread dlopen would acquire the same
> lock since it has been already acquired by the callback.  The only way is if
> dlopen is being called by a signal handler, which I think it another obscure
> corner case.

I meant that one thread would acquire the rtld lock first, and another 
thread would attempt to acquire the atfork lock, and then they proceed 
to acquire the opposite lock, which will deadlock.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] Refactor Linux ARCH_FORK implementation
  2018-02-07 13:09 [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
  2018-02-07 13:09 ` [PATCH 2/3] dynarray: Implement remove function Adhemerval Zanella
  2018-02-07 13:09 ` [PATCH 3/3] Refactor atfork handlers Adhemerval Zanella
@ 2018-03-07 16:51 ` Adhemerval Zanella
  2018-03-08 12:05 ` Florian Weimer
  3 siblings, 0 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2018-03-07 16:51 UTC (permalink / raw)
  To: libc-alpha


If no one opposes I will commit it this shortly.

On 07/02/2018 11:09, Adhemerval Zanella wrote:
> This patch refactors the ARCH_FORK macro and the required architecture
> specific header to simplify the required architecture definitions
> to provide the fork syscall semantic and proper document current
> Linux clone ABI variant.
> 
> Instead of require the reimplementation of arch-fork.h header, this
> patch changes the ARCH_FORK to an inline function with clone ABI
> defined by kernel-features.h define.  The generic kernel ABI meant
> for newer ports is used as default and redefine if the architecture
> requires.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.  Also with a build
> for all the afected ABIs.
> 
> 	* sysdeps/nptl/fork.c (ARCH_FORK): Replace by auch_fork.
> 	* sysdeps/unix/sysv/linux/alpha/arch-fork.h: Remove file.
> 	* sysdeps/unix/sysv/linux/riscv/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/aarch64/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/arm/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/hppa/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/ia64/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/m68k/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/microblaze/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/mips/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/nios2/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/powerpc/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/s390/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/sh/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/sparc/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/tile/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/arch-fork.h: Likewise.
> 	* sysdeps/unix/sysv/linux/arch-fork.h (arch_fork): New function.
> 	* sysdeps/unix/sysv/linux/aarch64/kernel-features.h: New file.
> 	* sysdeps/unix/sysv/linux/riscv/kernel-features.h: Likewise.
> 	* sysdeps/unix/sysv/linux/arm/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS): Define.
> 	* sysdeps/unix/sysv/linux/createthread.c (ARCH_CLONE): Define to
> 	__clone2 if __NR_clone2 is defined.
> 	* sysdeps/unix/sysv/linux/hppa/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS): Likewise.
> 	* sysdeps/unix/sysv/linux/i386/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS): Likewise.
> 	* sysdeps/unix/sysv/linux/ia64/kernel-features.h
> 	(__ASSUME_CLONE2): Likewise.
> 	* sysdeps/unix/sysv/linux/microblaze/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS3): Likewise.
> 	* sysdeps/unix/sysv/linux/kernel-features.h: Document possible clone
> 	variants and the define architecture can use.
> 	(__ASSUME_CLONE_DEFAULT): Define as default.
> 	* sysdeps/unix/sysv/linux/mips/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS): Likewise.
> 	* sysdeps/unix/sysv/linux/powerpc/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS): Likewise.
> 	* sysdeps/unix/sysv/linux/s390/kernel-features.h
> 	(__ASSUME_CLONE_BACKWARDS2): Likewise.
> ---
>  ChangeLog                                          | 45 ++++++++++++++++++++++
>  sysdeps/nptl/fork.c                                |  8 +---
>  .../arch-fork.h => aarch64/kernel-features.h}      | 15 +++-----
>  sysdeps/unix/sysv/linux/alpha/arch-fork.h          | 28 --------------
>  sysdeps/unix/sysv/linux/arch-fork.h                | 44 ++++++++++++++++-----
>  sysdeps/unix/sysv/linux/arm/arch-fork.h            | 27 -------------
>  sysdeps/unix/sysv/linux/arm/kernel-features.h      |  3 ++
>  sysdeps/unix/sysv/linux/createthread.c             |  5 ++-
>  sysdeps/unix/sysv/linux/hppa/arch-fork.h           | 32 ---------------
>  sysdeps/unix/sysv/linux/hppa/kernel-features.h     |  3 ++
>  sysdeps/unix/sysv/linux/i386/arch-fork.h           | 27 -------------
>  sysdeps/unix/sysv/linux/i386/kernel-features.h     |  3 ++
>  sysdeps/unix/sysv/linux/ia64/arch-fork.h           | 31 ---------------
>  sysdeps/unix/sysv/linux/ia64/kernel-features.h     |  3 ++
>  sysdeps/unix/sysv/linux/kernel-features.h          | 35 +++++++++++++++++
>  sysdeps/unix/sysv/linux/m68k/arch-fork.h           | 28 --------------
>  sysdeps/unix/sysv/linux/microblaze/arch-fork.h     | 27 -------------
>  .../unix/sysv/linux/microblaze/kernel-features.h   |  3 ++
>  sysdeps/unix/sysv/linux/mips/arch-fork.h           |  1 -
>  sysdeps/unix/sysv/linux/mips/kernel-features.h     |  3 ++
>  sysdeps/unix/sysv/linux/nios2/arch-fork.h          | 33 ----------------
>  sysdeps/unix/sysv/linux/powerpc/arch-fork.h        |  1 -
>  sysdeps/unix/sysv/linux/powerpc/kernel-features.h  |  3 ++
>  .../arch-fork.h => riscv/kernel-features.h}        | 17 +++-----
>  sysdeps/unix/sysv/linux/s390/arch-fork.h           | 29 --------------
>  sysdeps/unix/sysv/linux/s390/kernel-features.h     |  3 ++
>  sysdeps/unix/sysv/linux/sh/arch-fork.h             | 28 --------------
>  sysdeps/unix/sysv/linux/sparc/arch-fork.h          | 27 -------------
>  sysdeps/unix/sysv/linux/tile/arch-fork.h           | 29 --------------
>  sysdeps/unix/sysv/linux/x86_64/arch-fork.h         | 27 -------------
>  30 files changed, 155 insertions(+), 413 deletions(-)
>  rename sysdeps/unix/sysv/linux/{riscv/arch-fork.h => aarch64/kernel-features.h} (67%)
>  delete mode 100644 sysdeps/unix/sysv/linux/alpha/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/arm/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/hppa/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/i386/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/ia64/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/m68k/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/microblaze/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/mips/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/nios2/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/arch-fork.h
>  rename sysdeps/unix/sysv/linux/{aarch64/arch-fork.h => riscv/kernel-features.h} (66%)
>  delete mode 100644 sysdeps/unix/sysv/linux/s390/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/sh/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/sparc/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/tile/arch-fork.h
>  delete mode 100644 sysdeps/unix/sysv/linux/x86_64/arch-fork.h
> 
> diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
> index 846fa49..0061ee0 100644
> --- a/sysdeps/nptl/fork.c
> +++ b/sysdeps/nptl/fork.c
> @@ -131,13 +131,7 @@ __libc_fork (void)
>        call_function_static_weak (__malloc_fork_lock_parent);
>      }
>  
> -#ifdef ARCH_FORK
> -  pid = ARCH_FORK ();
> -#else
> -# error "ARCH_FORK must be defined so that the CLONE_SETTID flag is used"
> -  pid = INLINE_SYSCALL (fork, 0);
> -#endif
> -
> +  pid = arch_fork (&THREAD_SELF->tid);
>  
>    if (pid == 0)
>      {
> diff --git a/sysdeps/unix/sysv/linux/riscv/arch-fork.h b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> similarity index 67%
> rename from sysdeps/unix/sysv/linux/riscv/arch-fork.h
> rename to sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> index f6f5d73..9cfa514 100644
> --- a/sysdeps/unix/sysv/linux/riscv/arch-fork.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> @@ -1,5 +1,6 @@
> -/* Internal definitions for thread-friendly fork implementation.  Linux/RISC-V.
> -   Copyright (C) 2002-2018 Free Software Foundation, Inc.
> +/* Set flags signalling availability of kernel features based on given
> +   kernel version number.  AArch64 version.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -16,11 +17,7 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <sched.h>
> -#include <sysdep.h>
> -#include <tls.h>
> +#include_next <kernel-features.h>
>  
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -		  NULL, NULL, &THREAD_SELF->tid)
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS 1
> diff --git a/sysdeps/unix/sysv/linux/alpha/arch-fork.h b/sysdeps/unix/sysv/linux/alpha/arch-fork.h
> deleted file mode 100644
> index 41897dc..0000000
> --- a/sysdeps/unix/sysv/linux/alpha/arch-fork.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  Alpha version.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -
> -#define ARCH_FORK()							\
> -  INLINE_SYSCALL (clone, 5,						\
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
> -		  NULL, NULL, &THREAD_SELF->tid, NULL)
> diff --git a/sysdeps/unix/sysv/linux/arch-fork.h b/sysdeps/unix/sysv/linux/arch-fork.h
> index d5f5429..62bc23d 100644
> --- a/sysdeps/unix/sysv/linux/arch-fork.h
> +++ b/sysdeps/unix/sysv/linux/arch-fork.h
> @@ -1,4 +1,4 @@
> -/* ARCH_FORK definition for Linux fork implementation.  Stub version.
> +/* arch_fork definition for Linux fork implementation.
>     Copyright (C) 2014-2018 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -16,12 +16,38 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -/* This file should define the function-like macro of no arguments
> -   ARCH_FORK to an INLINE_SYSCALL invocation of the clone-like system
> -   call, passing the CLONE_CHILD_SETTID and CLONE_CHILD_CLEARTID flags
> -   and &THREAD_SELF->tid as the TID address.
> +#ifndef __ARCH_FORK_H
> +#define __ARCH_FORK_H
>  
> -   Machines that lack an arch-fork.h header file will hit an #error in
> -   fork.c; this stub file doesn't contain an #error itself mainly for
> -   the transition period of migrating old machine-specific fork.c files
> -   to machine-specific arch-fork.h instead.  */
> +#include <unistd.h>
> +
> +/* Call the clone syscall with fork semantic.  The CTID address is used
> +   to both store the child thread ID at its location and and erase it
> +   in child memory when the child exits, and do a wakeup on the futex at
> +   that address.
> +
> +   The architecture with non-default kernel abi semantic should correctlly
> +   override it with one of the supported calling convention (check generic
> +   kernel-features.h for the clone abi variants).  */
> +static inline pid_t
> +arch_fork (void *ctid)
> +{
> +  const int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD;
> +  long int ret;
> +#ifdef __ASSUME_CLONE_BACKWARDS
> +  ret = INLINE_SYSCALL_CALL (clone, flags, 0, NULL, 0, ctid);
> +#elif defined(__ASSUME_CLONE_BACKWARDS2)
> +  ret = INLINE_SYSCALL_CALL (clone, 0, flags, NULL, ctid, 0);
> +#elif defined(__ASSUME_CLONE_BACKWARDS3)
> +  ret = INLINE_SYSCALL_CALL (clone, flags, 0, 0, NULL, ctid, 0);
> +#elif defined(__ASSUME_CLONE2)
> +  ret = INLINE_SYSCALL_CALL (clone2, flags, 0, 0, NULL, ctid, 0);
> +#elif defined(__ASSUME_CLONE_DEFAULT)
> +  ret = INLINE_SYSCALL_CALL (clone, flags, NULL, NULL, ctid, 0);
> +#else
> +# error "Undefined clone variant"
> +#endif
> +  return ret;
> +}
> +
> +#endif /* __ARCH_FORK_H  */
> diff --git a/sysdeps/unix/sysv/linux/arm/arch-fork.h b/sysdeps/unix/sysv/linux/arm/arch-fork.h
> deleted file mode 100644
> index ff3bc90..0000000
> --- a/sysdeps/unix/sysv/linux/arm/arch-fork.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  ARM version.
> -   Copyright (C) 2014-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -
> -#define ARCH_FORK()                                                     \
> -  INLINE_SYSCALL (clone, 5,                                             \
> -                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,  \
> -                  NULL, NULL, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/arm/kernel-features.h b/sysdeps/unix/sysv/linux/arm/kernel-features.h
> index f13632b..7831ab1 100644
> --- a/sysdeps/unix/sysv/linux/arm/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/arm/kernel-features.h
> @@ -39,3 +39,6 @@
>  
>  #define __ASSUME_RECV_SYSCALL   1
>  #define __ASSUME_SEND_SYSCALL	1
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS	1
> diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c
> index 5b5464a..5879e51 100644
> --- a/sysdeps/unix/sysv/linux/createthread.c
> +++ b/sysdeps/unix/sysv/linux/createthread.c
> @@ -28,8 +28,9 @@
>  
>  #include <arch-fork.h>
>  
> -
> -#ifndef ARCH_CLONE
> +#ifdef __NR_clone2
> +# define ARCH_CLONE __clone2
> +#else
>  # define ARCH_CLONE __clone
>  #endif
>  
> diff --git a/sysdeps/unix/sysv/linux/hppa/arch-fork.h b/sysdeps/unix/sysv/linux/hppa/arch-fork.h
> deleted file mode 100644
> index 7d52994..0000000
> --- a/sysdeps/unix/sysv/linux/hppa/arch-fork.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  HPPA version.
> -   Copyright (C) 2005-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -/* Argument 1 - Clone flags.
> -            2 - Child stack pointer.
> -	    3 - Parent tid pointer.
> -	    4 - New TLS area pointer.
> -	    5 - Child tid pointer. */
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						\
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
> -                  NULL, NULL, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/hppa/kernel-features.h b/sysdeps/unix/sysv/linux/hppa/kernel-features.h
> index 3f005a5..ef3c4dd 100644
> --- a/sysdeps/unix/sysv/linux/hppa/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/hppa/kernel-features.h
> @@ -32,3 +32,6 @@
>  #if __LINUX_KERNEL_VERSION < 0x040000
>  # undef __ASSUME_EXECVEAT
>  #endif
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS 1
> diff --git a/sysdeps/unix/sysv/linux/i386/arch-fork.h b/sysdeps/unix/sysv/linux/i386/arch-fork.h
> deleted file mode 100644
> index 0c43e2f..0000000
> --- a/sysdeps/unix/sysv/linux/i386/arch-fork.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* Internal definitions for thread-friendly fork implementation.  Linux/i386.
> -   Copyright (C) 2002-2018 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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -		  NULL, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/i386/kernel-features.h b/sysdeps/unix/sysv/linux/i386/kernel-features.h
> index 8708712..f3cfd48 100644
> --- a/sysdeps/unix/sysv/linux/i386/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/i386/kernel-features.h
> @@ -48,3 +48,6 @@
>  
>  /* i686 only supports ipc syscall.  */
>  #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS	1
> diff --git a/sysdeps/unix/sysv/linux/ia64/arch-fork.h b/sysdeps/unix/sysv/linux/ia64/arch-fork.h
> deleted file mode 100644
> index 522712e..0000000
> --- a/sysdeps/unix/sysv/linux/ia64/arch-fork.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  IA64 version.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone2, 6,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	      \
> -		  NULL, 0, NULL, &THREAD_SELF->tid, NULL)
> -
> -#define ARCH_CLONE __clone2
> diff --git a/sysdeps/unix/sysv/linux/ia64/kernel-features.h b/sysdeps/unix/sysv/linux/ia64/kernel-features.h
> index cc3fe92..04cdf43 100644
> --- a/sysdeps/unix/sysv/linux/ia64/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/ia64/kernel-features.h
> @@ -26,4 +26,7 @@
>  #define __ASSUME_SEND_SYSCALL		1
>  #define __ASSUME_ACCEPT4_SYSCALL	1
>  
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE2
> +
>  #endif /* _KERNEL_FEATURES_H */
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index 3aa2052..ca18f0b 100644
> --- a/sysdeps/unix/sysv/linux/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/kernel-features.h
> @@ -115,3 +115,38 @@
>  #if __LINUX_KERNEL_VERSION >= 0x040500
>  # define __ASSUME_COPY_FILE_RANGE 1
>  #endif
> +
> +/* Support for clone call used on fork.  The signature varies across the
> +   architectures with current 4 different variants:
> +
> +   1. long int clone (unsigned long flags, unsigned long newsp,
> +		      int *parent_tidptr, unsigned long tls,
> +		      int *child_tidptr)
> +
> +   2. long int clone (unsigned long newsp, unsigned long clone_flags,
> +		      int *parent_tidptr, int * child_tidptr,
> +		      unsigned long tls)
> +
> +   3. long int clone (unsigned long flags, unsigned long newsp,
> +		      int stack_size, int *parent_tidptr,
> +		      int *child_tidptr, unsigned long tls)
> +
> +   4. long int clone (unsigned long flags, unsigned long newsp,
> +		      int *parent_tidptr, int *child_tidptr,
> +                      unsigned long tls)
> +
> +   The fourth variant is intended to be used as the default for newer ports,
> +   ALso IA64 uses the third variant but with __NR_clone2 instead of
> +   __NR_clone.
> +
> +   The macros names to define the variant used for the architecture is
> +   similar to kernel:
> +
> +   - __ASSUME_CLONE_BACKWARDS: for variant 1.
> +   - __ASSUME_CLONE_BACKWARDS2: for variant 2 (s390).
> +   - __ASSUME_CLONE_BACKWARDS3: for variant 3 (microblaze).
> +   - __ASSUME_CLONE_DEFAULT: for variant 4.
> +   - __ASSUME_CLONE2: for clone2 with variant 3 (ia64).
> +   */
> +
> +#define __ASSUME_CLONE_DEFAULT 1
> diff --git a/sysdeps/unix/sysv/linux/m68k/arch-fork.h b/sysdeps/unix/sysv/linux/m68k/arch-fork.h
> deleted file mode 100644
> index 20b6bab..0000000
> --- a/sysdeps/unix/sysv/linux/m68k/arch-fork.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  m68k version.
> -   Copyright (C) 2010-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Maxim Kuvyrkov <maxim@codesourcery.com>, 2010.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -		  NULL, &THREAD_SELF->tid, NULL)
> diff --git a/sysdeps/unix/sysv/linux/microblaze/arch-fork.h b/sysdeps/unix/sysv/linux/microblaze/arch-fork.h
> deleted file mode 100644
> index 05ed4fa..0000000
> --- a/sysdeps/unix/sysv/linux/microblaze/arch-fork.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  MicroBlaze version.
> -   Copyright (C) 2014-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK()                                                           \
> -  INLINE_SYSCALL (clone, 5,                                                   \
> -                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -                  NULL, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
> index 745f899..b13b863 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h
> @@ -57,3 +57,6 @@
>  #if __LINUX_KERNEL_VERSION < 0x040A00
>  # undef __ASSUME_COPY_FILE_RANGE
>  #endif
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS3
> diff --git a/sysdeps/unix/sysv/linux/mips/arch-fork.h b/sysdeps/unix/sysv/linux/mips/arch-fork.h
> deleted file mode 100644
> index 5f94537..0000000
> --- a/sysdeps/unix/sysv/linux/mips/arch-fork.h
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <sysdeps/unix/sysv/linux/i386/arch-fork.h>
> diff --git a/sysdeps/unix/sysv/linux/mips/kernel-features.h b/sysdeps/unix/sysv/linux/mips/kernel-features.h
> index 7756a34..a9009fb 100644
> --- a/sysdeps/unix/sysv/linux/mips/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/mips/kernel-features.h
> @@ -47,3 +47,6 @@
>  #if _MIPS_SIM == _ABIN32
>  # define __ASSUME_WORDSIZE64_ILP32	1
>  #endif
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS
> diff --git a/sysdeps/unix/sysv/linux/nios2/arch-fork.h b/sysdeps/unix/sysv/linux/nios2/arch-fork.h
> deleted file mode 100644
> index 6dacec2..0000000
> --- a/sysdeps/unix/sysv/linux/nios2/arch-fork.h
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  Nios II version.
> -   Copyright (C) 2005-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -/* Argument 1 - Clone flags.
> -            2 - Child stack pointer.
> -	    3 - Parent tid pointer.
> -	    4 - Child tid pointer.
> -	    5 - New TLS area pointer. */
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						\
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
> -                  NULL, NULL, &THREAD_SELF->tid, NULL)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/arch-fork.h b/sysdeps/unix/sysv/linux/powerpc/arch-fork.h
> deleted file mode 100644
> index 5f94537..0000000
> --- a/sysdeps/unix/sysv/linux/powerpc/arch-fork.h
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <sysdeps/unix/sysv/linux/i386/arch-fork.h>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
> index f3c02e6..503f562 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/kernel-features.h
> @@ -49,3 +49,6 @@
>  
>  /* powerpc only supports ipc syscall.  */
>  #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS	1
> diff --git a/sysdeps/unix/sysv/linux/aarch64/arch-fork.h b/sysdeps/unix/sysv/linux/riscv/kernel-features.h
> similarity index 66%
> rename from sysdeps/unix/sysv/linux/aarch64/arch-fork.h
> rename to sysdeps/unix/sysv/linux/riscv/kernel-features.h
> index cab797e..37f4d99 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/arch-fork.h
> +++ b/sysdeps/unix/sysv/linux/riscv/kernel-features.h
> @@ -1,5 +1,6 @@
> -/* ARCH_FORK definition for Linux fork implementation.  AArch64 version.
> -   Copyright (C) 2005-2018 Free Software Foundation, Inc.
> +/* Set flags signalling availability of kernel features based on given
> +   kernel version number.  RISC-V version.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -16,13 +17,7 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> +#include_next <kernel-features.h>
>  
> -
> -#define ARCH_FORK()							\
> -  INLINE_SYSCALL (clone, 5,						\
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,	\
> -		  NULL, NULL, NULL, &THREAD_SELF->tid)
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS 1
> diff --git a/sysdeps/unix/sysv/linux/s390/arch-fork.h b/sysdeps/unix/sysv/linux/s390/arch-fork.h
> deleted file mode 100644
> index 152b465..0000000
> --- a/sysdeps/unix/sysv/linux/s390/arch-fork.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation. S390 version.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						      \
> -		  0, CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,     \
> -		  NULL, &THREAD_SELF->tid, NULL)
> diff --git a/sysdeps/unix/sysv/linux/s390/kernel-features.h b/sysdeps/unix/sysv/linux/s390/kernel-features.h
> index 3caca98..f718264 100644
> --- a/sysdeps/unix/sysv/linux/s390/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/s390/kernel-features.h
> @@ -50,3 +50,6 @@
>  
>  /* s390 only supports ipc syscall.  */
>  #undef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> +
> +#undef __ASSUME_CLONE_DEFAULT
> +#define __ASSUME_CLONE_BACKWARDS2
> diff --git a/sysdeps/unix/sysv/linux/sh/arch-fork.h b/sysdeps/unix/sysv/linux/sh/arch-fork.h
> deleted file mode 100644
> index a29f61c..0000000
> --- a/sysdeps/unix/sysv/linux/sh/arch-fork.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  SH version.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -/* TLS pointer argument is passed as the 5-th argument.  */
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 5,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -		  NULL, &THREAD_SELF->tid, NULL)
> diff --git a/sysdeps/unix/sysv/linux/sparc/arch-fork.h b/sysdeps/unix/sysv/linux/sparc/arch-fork.h
> deleted file mode 100644
> index 03ccc9a..0000000
> --- a/sysdeps/unix/sysv/linux/sparc/arch-fork.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  SPARC version.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK() \
> -  INLINE_CLONE_SYSCALL (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, \
> -			0, NULL, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/tile/arch-fork.h b/sysdeps/unix/sysv/linux/tile/arch-fork.h
> deleted file mode 100644
> index d0526d8..0000000
> --- a/sysdeps/unix/sysv/linux/tile/arch-fork.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/* ARCH_FORK definition for Linux fork implementation.  Tile* version.
> -   Copyright (C) 2011-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
> -   Based on work 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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <signal.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 4,						      \
> -		  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD,        \
> -		  0, NULL, &THREAD_SELF->tid)
> diff --git a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
> deleted file mode 100644
> index 4471970..0000000
> --- a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* Internal definitions for thread-friendly fork implementation.  Linux/x86_64.
> -   Copyright (C) 2003-2018 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
> -
> -   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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <sched.h>
> -#include <sysdep.h>
> -#include <tls.h>
> -
> -#define ARCH_FORK() \
> -  INLINE_SYSCALL (clone, 4,                                                   \
> -                  CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0,     \
> -                  NULL, &THREAD_SELF->tid)
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] Refactor Linux ARCH_FORK implementation
  2018-02-07 13:09 [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2018-03-07 16:51 ` [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
@ 2018-03-08 12:05 ` Florian Weimer
  2018-03-08 12:58   ` Adhemerval Zanella
  3 siblings, 1 reply; 23+ messages in thread
From: Florian Weimer @ 2018-03-08 12:05 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

On 02/07/2018 02:09 PM, Adhemerval Zanella wrote:
> 	* sysdeps/nptl/fork.c (ARCH_FORK): Replace by auch_fork.

arch_fork?  Looks good otherwise.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] Refactor Linux ARCH_FORK implementation
  2018-03-08 12:05 ` Florian Weimer
@ 2018-03-08 12:58   ` Adhemerval Zanella
  0 siblings, 0 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2018-03-08 12:58 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha



On 08/03/2018 09:05, Florian Weimer wrote:
> On 02/07/2018 02:09 PM, Adhemerval Zanella wrote:
>>     * sysdeps/nptl/fork.c (ARCH_FORK): Replace by auch_fork.
> 
> arch_fork?  Looks good otherwise.

I will fix it, thanks.

> 
> Thanks,
> Florian


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-03-08 12:56 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 13:09 [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
2018-02-07 13:09 ` [PATCH 2/3] dynarray: Implement remove function Adhemerval Zanella
2018-02-07 14:48   ` Alexander Monakov
2018-02-07 16:06     ` Adhemerval Zanella
2018-02-07 13:09 ` [PATCH 3/3] Refactor atfork handlers Adhemerval Zanella
2018-02-07 15:07   ` Florian Weimer
2018-02-07 17:16     ` Adhemerval Zanella
2018-02-08  8:32       ` Florian Weimer
2018-02-08 12:50         ` Adhemerval Zanella
2018-02-20 11:29           ` Florian Weimer
2018-02-20 13:00             ` Adhemerval Zanella
2018-02-20 13:05               ` Florian Weimer
2018-02-20 13:27                 ` Adhemerval Zanella
2018-02-20 13:42                   ` Florian Weimer
2018-02-20 13:48                     ` Adhemerval Zanella
2018-02-20 13:58                       ` Florian Weimer
2018-02-20 14:23                         ` Adhemerval Zanella
2018-02-23 10:41                           ` Florian Weimer
2018-02-23 12:10                             ` Adhemerval Zanella
2018-02-27  8:25                               ` Florian Weimer
2018-03-07 16:51 ` [PATCH 1/3] Refactor Linux ARCH_FORK implementation Adhemerval Zanella
2018-03-08 12:05 ` Florian Weimer
2018-03-08 12:58   ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).