linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS
@ 2020-06-22 23:43 Christian Brauner
  2020-06-22 23:43 ` [PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork() Christian Brauner
                   ` (14 more replies)
  0 siblings, 15 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Christian Brauner

Hey,

This is a series I had announced some time back (cf. [2]) when we
reworked a chunk of the process creation paths in the kernel and
switched to struct {kernel_}clone_args. I've been sitting on this for a
while due to lack of time and because testing on all architectures that
are non-trivially impacted proved to be interesting. So first, I have to
give a big thank you to Arnd (Bergmann) and Adrian (Glaubitz).
Arnd for the gcc crosstools builds which made it possible to compile for
nearly all supported arches with gcc 10. Adrian for providing me with a
ia64 rack server (that sure made summer in my workspace come a little
earlier) to test the ia64 specific changes, alpha, and other stuff.

This patch series switches all remaining architectures to select
HAVE_COPY_THREAD_TLS and thus to the copy_thread_tls() calling
convention. This split makes the process creation codepaths a bit
convoluted to read. Each architecture has their own copy_thread()
function unless it selects HAVE_COPY_THREAD_TLS then it has a
copy_thread_tls() function. Removing this split cleans up the process
creation codepaths and paves the way for implementing clone3() on those
architectures since it requires the copy_thread_tls() calling
convention. After having made each architectures support
copy_thread_tls() this series simply renames that function back to
copy_thread(). It also switches all architectures that call do_fork()
directly over to _do_fork() and the struct kernel_clone_args calling
convention. This is a corollary of switching the architectures that did
not yet support it over to copy_thread_tls() since do_fork() is
conditional on not supporting copy_thread_tls() (since it lacks an
argument for tls).  But the do_fork() removal is in itself already
useful as it allows to to remove the export of both do_fork() and
_do_fork() we currently have in favor of only _do_fork().  This has
already been discussed back when we added clone3(). The legacy clone()
calling convention is - as most know - somewhat odd or as arch/Kconfig
(cf. [1]) phrases it:

  #
  # ABI hall of shame
  #
  config CLONE_BACKWARDS
  config CLONE_BACKWARDS2
  config CLONE_BACKWARDS3

that is aggravated by the fact that some architectures such as sparc
follow the CLONE_BACKWARDSx calling convention but don't really select
the corresponding config option since thy call do_fork() directly. So
do_fork() enforces a somewhat arbitrary ordering in the first place that
doesn't really help the individual architectures that deviate from it.
They can thus simply be switched to _do_fork() enforcing a single
calling convention. Most architectures have made a similar switch
already (m68k comes to mind). Removing do_fork() is also a corollary of
switching all architectures over to the copy_thread_tls() calling
convention.

Overall this series removes more code then it adds even with a chunk of
added comments (268(+)/274(-)). It simplifies a chunk of arch specific
assembly either by moving the code into C or by simply rewriting the
assembly.

I've done my best to test this. Architectures that have been touched in
non-trivial ways have all been actually boot and stress tested: sparc
and ia64 have been tested with Debian 9 images. nios2 with a custom
build buildroot image. h8300 I couldn't get something bootable but the
changes have been fairly automatic and I'm sure we'll hear people yell
if I broke something there. All other architectures that have been
touched in trivial ways have been compile tested for each single patch
of the series via git rebase -x "make ..." v5.8-rc2. arm and x86 have
been boot tested even though they have just been trivially touched
(removal of the HAVE_COPY_THREAD_TLS macro from their Kconfig) because
well they are basically "core architectures" and since it is trivial to
get your hands on a useable image.

The series is available at:
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=do_fork_removal
https://gitlab.com/brauner/linux/-/tree/do_fork_removal
https://github.com/brauner/linux/tree/do_fork_removal
if anyone with access to weirder architectures wants to test them.

Thanks!
Christian

[1]: d2125043aebf ("generic sys_fork / sys_vfork / sys_clone")
[2]: 606e9ad20094

Christian Brauner (17):
  fork: fold legacy_clone_args_valid() into _do_fork()
  sparc64: enable HAVE_COPY_THREAD_TLS
  sparc: share process creation helpers between sparc and sparc64
  sparc: unconditionally enable HAVE_COPY_THREAD_TLS
  ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  nios2: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  h8300: select HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  fork: remove do_fork()
  alpha: switch to copy_thread_tls()
  c6x: switch to copy_thread_tls()
  hexagon: switch to copy_thread_tls()
  microblaze: switch to copy_thread_tls()
  nds32: switch to copy_thread_tls()
  sh: switch to copy_thread_tls()
  unicore: switch to copy_thread_tls()
  arch: remove HAVE_COPY_THREAD_TLS
  arch: rename copy_thread_tls() back to copy_thread()

 arch/Kconfig                      |   7 --
 arch/alpha/kernel/process.c       |   9 ++-
 arch/arc/Kconfig                  |   1 -
 arch/arc/kernel/process.c         |   2 +-
 arch/arm/Kconfig                  |   1 -
 arch/arm/kernel/process.c         |   2 +-
 arch/arm64/Kconfig                |   1 -
 arch/arm64/kernel/process.c       |   2 +-
 arch/c6x/kernel/process.c         |   4 +-
 arch/csky/Kconfig                 |   1 -
 arch/csky/kernel/process.c        |   2 +-
 arch/h8300/kernel/process.c       |  18 +++--
 arch/hexagon/kernel/process.c     |   4 +-
 arch/ia64/kernel/entry.S          |  32 ++++-----
 arch/ia64/kernel/process.c        |  30 ++++++--
 arch/m68k/Kconfig                 |   1 -
 arch/m68k/kernel/process.c        |   5 +-
 arch/microblaze/kernel/process.c  |   4 +-
 arch/mips/Kconfig                 |   1 -
 arch/mips/kernel/process.c        |   2 +-
 arch/nds32/kernel/process.c       |   5 +-
 arch/nios2/kernel/entry.S         |   7 +-
 arch/nios2/kernel/process.c       |  23 ++++++-
 arch/openrisc/Kconfig             |   1 -
 arch/openrisc/kernel/process.c    |   4 +-
 arch/parisc/Kconfig               |   1 -
 arch/parisc/kernel/process.c      |   2 +-
 arch/powerpc/Kconfig              |   1 -
 arch/powerpc/kernel/process.c     |   2 +-
 arch/riscv/Kconfig                |   1 -
 arch/riscv/kernel/process.c       |   2 +-
 arch/s390/Kconfig                 |   1 -
 arch/s390/kernel/process.c        |   2 +-
 arch/sh/kernel/process_32.c       |   4 +-
 arch/sparc/include/asm/syscalls.h |   7 +-
 arch/sparc/kernel/Makefile        |   1 +
 arch/sparc/kernel/entry.S         |  29 ++------
 arch/sparc/kernel/kernel.h        |  11 ++-
 arch/sparc/kernel/process.c       | 110 ++++++++++++++++++++++++++++++
 arch/sparc/kernel/process_32.c    |  32 +--------
 arch/sparc/kernel/process_64.c    |  39 +----------
 arch/sparc/kernel/syscalls.S      |  23 ++++---
 arch/um/Kconfig                   |   1 -
 arch/um/kernel/process.c          |   2 +-
 arch/unicore32/kernel/process.c   |   8 +--
 arch/x86/Kconfig                  |   1 -
 arch/x86/kernel/process.c         |   2 +-
 arch/x86/kernel/sys_ia32.c        |   3 -
 arch/x86/kernel/unwind_frame.c    |   2 +-
 arch/xtensa/Kconfig               |   1 -
 arch/xtensa/kernel/process.c      |   2 +-
 include/linux/sched/task.h        |  17 +----
 kernel/fork.c                     |  66 +++++-------------
 53 files changed, 268 insertions(+), 274 deletions(-)
 create mode 100644 arch/sparc/kernel/process.c


base-commit: 48778464bb7d346b47157d21ffde2af6b2d39110
-- 
2.27.0


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

* [PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
       [not found]   ` <20200627082748.GM5535@shao2-debian>
  2020-06-22 23:43 ` [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Thomas Gleixner, Ingo Molnar,
	Al Viro, Geert Uytterhoeven, Matthew Wilcox (Oracle),
	Peter Zijlstra (Intel),
	linux-m68k, x86

This separate helper only existed to guarantee the mutual exclusivity of
CLONE_PIDFD and CLONE_PARENT_SETTID for legacy clone since CLONE_PIDFD
abuses the parent_tid field to return the pidfd. But we can actually handle
this uniformely thus removing the helper. For legacy clone we can detect
that CLONE_PIDFD is specified in conjunction with CLONE_PARENT_SETTID
because they will share the same memory which is invalid and for clone3()
setting the separate pidfd and parent_tid fields to the same memory is
bogus as well. So fold that helper directly into _do_fork() by detecting
this case.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: linux-m68k@lists.linux-m68k.org
Cc: x86@kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/m68k/kernel/process.c |  3 ---
 arch/x86/kernel/sys_ia32.c |  3 ---
 include/linux/sched/task.h |  1 -
 kernel/fork.c              | 30 ++++++++++++++----------------
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
index 90ae376b7ab1..0608439ba452 100644
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -125,9 +125,6 @@ asmlinkage int m68k_clone(struct pt_regs *regs)
 		.tls		= regs->d5,
 	};
 
-	if (!legacy_clone_args_valid(&args))
-		return -EINVAL;
-
 	return _do_fork(&args);
 }
 
diff --git a/arch/x86/kernel/sys_ia32.c b/arch/x86/kernel/sys_ia32.c
index f8d65c99feb8..720cde885042 100644
--- a/arch/x86/kernel/sys_ia32.c
+++ b/arch/x86/kernel/sys_ia32.c
@@ -251,9 +251,6 @@ COMPAT_SYSCALL_DEFINE5(ia32_clone, unsigned long, clone_flags,
 		.tls		= tls_val,
 	};
 
-	if (!legacy_clone_args_valid(&args))
-		return -EINVAL;
-
 	return _do_fork(&args);
 }
 #endif /* CONFIG_IA32_EMULATION */
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 38359071236a..ddce0ea515d1 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -96,7 +96,6 @@ extern void exit_files(struct task_struct *);
 extern void exit_itimers(struct signal_struct *);
 
 extern long _do_fork(struct kernel_clone_args *kargs);
-extern bool legacy_clone_args_valid(const struct kernel_clone_args *kargs);
 extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
 struct task_struct *fork_idle(int);
 struct mm_struct *copy_init_mm(void);
diff --git a/kernel/fork.c b/kernel/fork.c
index 142b23645d82..9875aeb2ba41 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2422,6 +2422,20 @@ long _do_fork(struct kernel_clone_args *args)
 	int trace = 0;
 	long nr;
 
+	/*
+	 * For legacy clone() calls, CLONE_PIDFD uses the parent_tid argument
+	 * to return the pidfd. Hence, CLONE_PIDFD and CLONE_PARENT_SETTID are
+	 * mutually exclusive. With clone3() CLONE_PIDFD has grown a separate
+	 * field in struct clone_args and it still doesn't make sense to have
+	 * them both point at the same memory location. Performing this check
+	 * here has the advantage that we don't need to have a separate helper
+	 * to check for legacy clone().
+	 */
+	if ((args->flags & CLONE_PIDFD) &&
+	    (args->flags & CLONE_PARENT_SETTID) &&
+	    (args->pidfd == args->parent_tid))
+		return -EINVAL;
+
 	/*
 	 * Determine whether and which event to report to ptracer.  When
 	 * called from kernel_thread or CLONE_UNTRACED is explicitly
@@ -2479,16 +2493,6 @@ long _do_fork(struct kernel_clone_args *args)
 	return nr;
 }
 
-bool legacy_clone_args_valid(const struct kernel_clone_args *kargs)
-{
-	/* clone(CLONE_PIDFD) uses parent_tidptr to return a pidfd */
-	if ((kargs->flags & CLONE_PIDFD) &&
-	    (kargs->flags & CLONE_PARENT_SETTID))
-		return false;
-
-	return true;
-}
-
 #ifndef CONFIG_HAVE_COPY_THREAD_TLS
 /* For compatibility with architectures that call do_fork directly rather than
  * using the syscall entry points below. */
@@ -2508,9 +2512,6 @@ long do_fork(unsigned long clone_flags,
 		.stack_size	= stack_size,
 	};
 
-	if (!legacy_clone_args_valid(&args))
-		return -EINVAL;
-
 	return _do_fork(&args);
 }
 #endif
@@ -2593,9 +2594,6 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
 		.tls		= tls,
 	};
 
-	if (!legacy_clone_args_valid(&args))
-		return -EINVAL;
-
 	return _do_fork(&args);
 }
 #endif
-- 
2.27.0


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

* [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
  2020-06-22 23:43 ` [PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork() Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-23  3:35   ` David Miller
  2020-06-22 23:43 ` [PATCH 03/17] sparc: share process creation helpers between sparc and sparc64 Christian Brauner
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, David S . Miller,
	Arnd Bergmann, Guo Ren, linux-csky, sparclinux

This is part of a larger series that aims at getting rid of the
copy_thread()/copy_thread_tls() split that makes the process creation
codepaths in the kernel more convoluted and error-prone than they need
to be.
It also unblocks implementing clone3() on architectures not support
copy_thread_tls(). Any architecture that wants to implement clone3()
will need to select HAVE_COPY_THREAD_TLS and thus need to implement
copy_thread_tls(). So both goals are connected but independently
beneficial.

HAVE_COPY_THREAD_TLS means that a given architecture supports
CLONE_SETTLS and not setting it should usually mean that the
architectures doesn't implement it but that's not how things are. In
fact all architectures support CLONE_TLS it's just that they don't
follow the calling convention that HAVE_COPY_THREAD_TLS implies. That
means all architectures can be switched over to select
HAVE_COPY_THREAD_TLS. Once that is done we can remove that macro (yay,
less code), the unnecessary do_fork() export in kernel/fork.c, and also
rename copy_thread_tls() back to copy_thread(). At this point
copy_thread() becomes the main architecture specific part of process
creation but it will be the same layout and calling convention for all
architectures. (Once that is done we can probably cleanup each
copy_thread() function even more but that's for the future.)

Since sparc does support CLONE_SETTLS there's no reason to not select
HAVE_COPY_THREAD_TLS. This brings us one step closer to getting rid of
the copy_thread()/copy_thread_tls() split we still have and ultimately
the HAVE_COPY_THREAD_TLS define in general. A lot of architectures have
already converted and sparc is one of the few hat haven't yet. This also
unblocks implementing the clone3() syscall on sparc which I will follow
up later (if no one gets there before me). Once that is done we can get
of another ARCH_WANTS_* macro.

This patch just switches sparc64 over to HAVE_COPY_THREAD_TLS but not
sparc32 which will be done in the next patch. Once Any architecture that
supports HAVE_COPY_THREAD_TLS cannot call the do_fork() helper anymore.
This is fine and intended since it should be removed in favor of the
new, cleaner _do_fork() calling convention based on struct
kernel_clone_args. In fact, most architectures have already switched.
With this patch, sparc joins the other arches which can't use the
fork(), vfork(), clone(), clone3() syscalls directly and who follow the
new process creation calling convention that is based on struct
kernel_clone_args which we introduced a while back. This means less
custom assembly in the architectures entry path to set up the registers
before calling into the process creation helper and it is easier to to
support new features without having to adapt calling conventions. It
also unifies all process creation paths between fork(), vfork(),
clone(), and clone3(). (We can't fix the ABI nightmare that legacy
clone() is but we can prevent stuff like this happening in the future.)

Note that sparc can't easily call into the syscalls directly because of
its return value conventions when a new process is created which
needs to clobber the UREG_I1 register in copy_thread{_tls()} and it
needs to restore it if process creation fails. That's not a big deal
since the new process creation calling convention makes things simpler.

This removes sparc_do_fork() and replaces it with 3 clean helpers,
sparc_fork(), sparc_vfork(), and sparc_clone(). That means a little more
C code until the next patch unifies sparc 32bit and sparc64. It has the
advantage that we can remove quite a bit of assembler and it makes the
whole syscall.S process creation bits easier to read.
The follow-up patch will remove the custom sparc_do_fork() helper for
32bi sparc and move sparc_fork(), sparc_vfork(), and sparc_clone() into
a common process.c file. This allows us to remove quite a bit of
custom assembly form 32bit sparc's entry.S file too and allows to remove
even more code because now all helpers are shared between 32bit sparc
and sparc64 instead of having to maintain two separate sparc_do_fork()
implementations.

For some more context, please see:
commit 606e9ad20094f6d500166881d301f31a51bc8aa7
Merge: ac61145a725a 457677c70c76
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat Jan 11 15:33:48 2020 -0800

    Merge tag 'clone3-tls-v5.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

    Pull thread fixes from Christian Brauner:
     "This contains a series of patches to fix CLONE_SETTLS when used with
      clone3().

      The clone3() syscall passes the tls argument through struct clone_args
      instead of a register. This means, all architectures that do not
      implement copy_thread_tls() but still support CLONE_SETTLS via
      copy_thread() expecting the tls to be located in a register argument
      based on clone() are currently unfortunately broken. Their tls value
      will be garbage.

      The patch series fixes this on all architectures that currently define
      __ARCH_WANT_SYS_CLONE3. It also adds a compile-time check to ensure
      that any architecture that enables clone3() in the future is forced to
      also implement copy_thread_tls().

      My ultimate goal is to get rid of the copy_thread()/copy_thread_tls()
      split and just have copy_thread_tls() at some point in the not too
      distant future (Maybe even renaming copy_thread_tls() back to simply
      copy_thread() once the old function is ripped from all arches). This
      is dependent now on all arches supporting clone3().

      While all relevant arches do that now there are still four missing:
      ia64, m68k, sh and sparc. They have the system call reserved, but not
      implemented. Once they all implement clone3() we can get rid of
      ARCH_WANT_SYS_CLONE3 and HAVE_COPY_THREAD_TLS.

Note that in the meantime, m68k has already switched to the new calling
convention.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: David S. Miller <davem@davemloft.net>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guo Ren <guoren@kernel.org>
Cc: linux-csky@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sparclinux@vger.kernel.org
See: d95b56c77ef ("openrisc: Cleanup copy_thread_tls docs and comments")
See: 0b9f386c4be ("csky: Implement copy_thread_tls")
Link: https://lore.kernel.org/r/20200512171527.570109-2-christian.brauner@ubuntu.com
---
 arch/sparc/Kconfig                |  1 +
 arch/sparc/include/asm/syscalls.h |  7 +--
 arch/sparc/kernel/process_64.c    | 91 ++++++++++++++++++++++++++-----
 arch/sparc/kernel/syscalls.S      | 23 ++++----
 4 files changed, 93 insertions(+), 29 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 5bf2dc163540..e2bb4956e130 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -94,6 +94,7 @@ config SPARC64
 	select ARCH_HAS_PTE_SPECIAL
 	select PCI_DOMAINS if PCI
 	select ARCH_HAS_GIGANTIC_PAGE
+	select HAVE_COPY_THREAD_TLS
 
 config ARCH_PROC_KCORE_TEXT
 	def_bool y
diff --git a/arch/sparc/include/asm/syscalls.h b/arch/sparc/include/asm/syscalls.h
index 1d819f5e21da..35575fbfb9dc 100644
--- a/arch/sparc/include/asm/syscalls.h
+++ b/arch/sparc/include/asm/syscalls.h
@@ -4,9 +4,8 @@
 
 struct pt_regs;
 
-asmlinkage long sparc_do_fork(unsigned long clone_flags,
-			      unsigned long stack_start,
-			      struct pt_regs *regs,
-			      unsigned long stack_size);
+asmlinkage long sparc_fork(struct pt_regs *regs);
+asmlinkage long sparc_vfork(struct pt_regs *regs);
+asmlinkage long sparc_clone(struct pt_regs *regs);
 
 #endif /* _SPARC64_SYSCALLS_H */
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index 54945eacd3b5..8c400fca4e9f 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -572,31 +572,91 @@ void fault_in_user_windows(struct pt_regs *regs)
 	force_sig(SIGSEGV);
 }
 
-asmlinkage long sparc_do_fork(unsigned long clone_flags,
-			      unsigned long stack_start,
-			      struct pt_regs *regs,
-			      unsigned long stack_size)
+asmlinkage long sparc_fork(struct pt_regs *regs)
 {
-	int __user *parent_tid_ptr, *child_tid_ptr;
 	unsigned long orig_i1 = regs->u_regs[UREG_I1];
 	long ret;
+	struct kernel_clone_args args = {
+		.exit_signal	= SIGCHLD,
+		/* Reuse the parent's stack for the child. */
+		.stack		= regs->u_regs[UREG_FP],
+	};
+
+	ret = _do_fork(&args);
+
+	/* If we get an error and potentially restart the system
+	 * call, we're screwed because copy_thread_tls() clobbered
+	 * the parent's %o1.  So detect that case and restore it
+	 * here.
+	 */
+	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
+		regs->u_regs[UREG_I1] = orig_i1;
+
+	return ret;
+}
+
+asmlinkage long sparc_vfork(struct pt_regs *regs)
+{
+	unsigned long orig_i1 = regs->u_regs[UREG_I1];
+	long ret;
+
+	struct kernel_clone_args args = {
+		.flags		= CLONE_VFORK | CLONE_VM,
+		.exit_signal	= SIGCHLD,
+		/* Reuse the parent's stack for the child. */
+		.stack		= regs->u_regs[UREG_FP],
+	};
+
+	ret = _do_fork(&args);
+
+	/* If we get an error and potentially restart the system
+	 * call, we're screwed because copy_thread_tls() clobbered
+	 * the parent's %o1.  So detect that case and restore it
+	 * here.
+	 */
+	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
+		regs->u_regs[UREG_I1] = orig_i1;
+
+	return ret;
+}
+
+asmlinkage long sparc_clone(struct pt_regs *regs)
+{
+	unsigned long orig_i1 = regs->u_regs[UREG_I1];
+	unsigned int flags = lower_32_bits(regs->u_regs[UREG_I0]);
+	long ret;
+
+	struct kernel_clone_args args = {
+		.flags		= (flags & ~CSIGNAL),
+		.exit_signal	= (flags & CSIGNAL),
+		.tls		= regs->u_regs[UREG_I3],
+	};
 
 #ifdef CONFIG_COMPAT
 	if (test_thread_flag(TIF_32BIT)) {
-		parent_tid_ptr = compat_ptr(regs->u_regs[UREG_I2]);
-		child_tid_ptr = compat_ptr(regs->u_regs[UREG_I4]);
+		args.pidfd	= compat_ptr(regs->u_regs[UREG_I2]);
+		args.child_tid	= compat_ptr(regs->u_regs[UREG_I4]);
+		args.parent_tid	= compat_ptr(regs->u_regs[UREG_I2]);
 	} else
 #endif
 	{
-		parent_tid_ptr = (int __user *) regs->u_regs[UREG_I2];
-		child_tid_ptr = (int __user *) regs->u_regs[UREG_I4];
+		args.pidfd	= (int __user *)regs->u_regs[UREG_I2];
+		args.child_tid	= (int __user *)regs->u_regs[UREG_I4];
+		args.parent_tid	= (int __user *)regs->u_regs[UREG_I2];
 	}
 
-	ret = do_fork(clone_flags, stack_start, stack_size,
-		      parent_tid_ptr, child_tid_ptr);
+	/* Did userspace setup a separate stack for the child or are we
+	 * copying the parent's?
+	 */
+	if (regs->u_regs[UREG_I1])
+		args.stack = regs->u_regs[UREG_I1];
+	else
+		args.stack = regs->u_regs[UREG_FP];
+
+	ret = _do_fork(&args);
 
 	/* If we get an error and potentially restart the system
-	 * call, we're screwed because copy_thread() clobbered
+	 * call, we're screwed because copy_thread_tls() clobbered
 	 * the parent's %o1.  So detect that case and restore it
 	 * here.
 	 */
@@ -611,8 +671,9 @@ asmlinkage long sparc_do_fork(unsigned long clone_flags,
  * Parent -->  %o0 == childs  pid, %o1 == 0
  * Child  -->  %o0 == parents pid, %o1 == 1
  */
-int copy_thread(unsigned long clone_flags, unsigned long sp,
-		unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+		    unsigned long arg, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct thread_info *t = task_thread_info(p);
 	struct pt_regs *regs = current_pt_regs();
@@ -670,7 +731,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
 	regs->u_regs[UREG_I1] = 0;
 
 	if (clone_flags & CLONE_SETTLS)
-		t->kregs->u_regs[UREG_G7] = regs->u_regs[UREG_I3];
+		t->kregs->u_regs[UREG_G7] = tls;
 
 	return 0;
 }
diff --git a/arch/sparc/kernel/syscalls.S b/arch/sparc/kernel/syscalls.S
index db42b4fb3708..192f3a28a2b7 100644
--- a/arch/sparc/kernel/syscalls.S
+++ b/arch/sparc/kernel/syscalls.S
@@ -86,19 +86,22 @@ sys32_rt_sigreturn:
 	 * during system calls...
 	 */
 	.align	32
-sys_vfork: /* Under Linux, vfork and fork are just special cases of clone. */
-	sethi	%hi(0x4000 | 0x0100 | SIGCHLD), %o0
-	or	%o0, %lo(0x4000 | 0x0100 | SIGCHLD), %o0
-	ba,pt	%xcc, sys_clone
+sys_vfork:
+	flushw
+	ba,pt	%xcc, sparc_vfork
+	add	%sp, PTREGS_OFF, %o0
+
+	.align	32
 sys_fork:
-	 clr	%o1
-	mov	SIGCHLD, %o0
+	flushw
+	ba,pt	%xcc, sparc_fork
+	add	%sp, PTREGS_OFF, %o0
+
+	.align	32
 sys_clone:
 	flushw
-	movrz	%o1, %fp, %o1
-	mov	0, %o3
-	ba,pt	%xcc, sparc_do_fork
-	 add	%sp, PTREGS_OFF, %o2
+	ba,pt	%xcc, sparc_clone
+	add	%sp, PTREGS_OFF, %o0
 
 	.globl	ret_from_fork
 ret_from_fork:
-- 
2.27.0


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

* [PATCH 03/17] sparc: share process creation helpers between sparc and sparc64
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
  2020-06-22 23:43 ` [PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork() Christian Brauner
  2020-06-22 23:43 ` [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 04/17] sparc: unconditionally enable HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, David S . Miller,
	Arnd Bergmann, Guo Ren, linux-csky, sparclinux

As promised in the previous patch, this moves the process creation
helpers into a common process.c file that is shared between sparc and
sparc64. It allows us to get rid of quite a bit custom assembler and the
to remove the separe 32bit specific sparc_do_fork() call.

One thing to note, is that when clone() was called with a separate stack
for the child the assembler would align it. But copy_thread() has always
been doing that too so that line wasn't needed and can thus simply be
removed.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: David S. Miller <davem@davemloft.net>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guo Ren <guoren@kernel.org>
Cc: linux-csky@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Link: https://lore.kernel.org/r/20200512171527.570109-3-christian.brauner@ubuntu.com
---
 arch/sparc/kernel/Makefile     |   1 +
 arch/sparc/kernel/entry.S      |  29 +++------
 arch/sparc/kernel/kernel.h     |  11 ++--
 arch/sparc/kernel/process.c    | 110 +++++++++++++++++++++++++++++++++
 arch/sparc/kernel/process_32.c |  27 --------
 arch/sparc/kernel/process_64.c |  94 ----------------------------
 6 files changed, 123 insertions(+), 149 deletions(-)
 create mode 100644 arch/sparc/kernel/process.c

diff --git a/arch/sparc/kernel/Makefile b/arch/sparc/kernel/Makefile
index 97c0e19263d1..d3a0e072ebe8 100644
--- a/arch/sparc/kernel/Makefile
+++ b/arch/sparc/kernel/Makefile
@@ -33,6 +33,7 @@ obj-y                   += irq_$(BITS).o
 obj-$(CONFIG_SPARC32)   += sun4m_irq.o sun4d_irq.o
 
 obj-y                   += process_$(BITS).o
+obj-y                   += process.o
 obj-y                   += signal_$(BITS).o
 obj-y                   += sigutil_$(BITS).o
 obj-$(CONFIG_SPARC32)   += ioport.o
diff --git a/arch/sparc/kernel/entry.S b/arch/sparc/kernel/entry.S
index f636acf3312f..d58940280f8d 100644
--- a/arch/sparc/kernel/entry.S
+++ b/arch/sparc/kernel/entry.S
@@ -869,14 +869,11 @@ flush_patch_two:
 	ld	[%curptr + TI_TASK], %o4
 	rd	%psr, %g4
 	WRITE_PAUSE
-	mov	SIGCHLD, %o0			! arg0:	clone flags
 	rd	%wim, %g5
 	WRITE_PAUSE
-	mov	%fp, %o1			! arg1:	usp
 	std	%g4, [%o4 + AOFF_task_thread + AOFF_thread_fork_kpsr]
-	add	%sp, STACKFRAME_SZ, %o2		! arg2:	pt_regs ptr
-	mov	0, %o3
-	call	sparc_do_fork
+	add	%sp, STACKFRAME_SZ, %o0
+	call	sparc_fork
 	 mov	%l5, %o7
 
 	/* Whee, kernel threads! */
@@ -888,19 +885,11 @@ flush_patch_three:
 	ld	[%curptr + TI_TASK], %o4
 	rd	%psr, %g4
 	WRITE_PAUSE
-
-	/* arg0,1: flags,usp  -- loaded already */
-	cmp	%o1, 0x0			! Is new_usp NULL?
 	rd	%wim, %g5
 	WRITE_PAUSE
-	be,a	1f
-	 mov	%fp, %o1			! yes, use callers usp
-	andn	%o1, 7, %o1			! no, align to 8 bytes
-1:
 	std	%g4, [%o4 + AOFF_task_thread + AOFF_thread_fork_kpsr]
-	add	%sp, STACKFRAME_SZ, %o2		! arg2:	pt_regs ptr
-	mov	0, %o3
-	call	sparc_do_fork
+	add	%sp, STACKFRAME_SZ, %o0
+	call	sparc_clone
 	 mov	%l5, %o7
 
 	/* Whee, real vfork! */
@@ -914,13 +903,9 @@ flush_patch_four:
 	rd	%wim, %g5
 	WRITE_PAUSE
 	std	%g4, [%o4 + AOFF_task_thread + AOFF_thread_fork_kpsr]
-	sethi	%hi(0x4000 | 0x0100 | SIGCHLD), %o0
-	mov	%fp, %o1
-	or	%o0, %lo(0x4000 | 0x0100 | SIGCHLD), %o0
-	sethi	%hi(sparc_do_fork), %l1
-	mov	0, %o3
-	jmpl	%l1 + %lo(sparc_do_fork), %g0
-	 add	%sp, STACKFRAME_SZ, %o2
+	sethi	%hi(sparc_vfork), %l1
+	jmpl	%l1 + %lo(sparc_vfork), %g0
+	 add	%sp, STACKFRAME_SZ, %o0
 
         .align  4
 linux_sparc_ni_syscall:
diff --git a/arch/sparc/kernel/kernel.h b/arch/sparc/kernel/kernel.h
index f6f498ba3198..9cd09a3ef35f 100644
--- a/arch/sparc/kernel/kernel.h
+++ b/arch/sparc/kernel/kernel.h
@@ -14,6 +14,11 @@ extern const char *sparc_pmu_type;
 extern unsigned int fsr_storage;
 extern int ncpus_probed;
 
+/* process{_32,_64}.c */
+asmlinkage long sparc_clone(struct pt_regs *regs);
+asmlinkage long sparc_fork(struct pt_regs *regs);
+asmlinkage long sparc_vfork(struct pt_regs *regs);
+
 #ifdef CONFIG_SPARC64
 /* setup_64.c */
 struct seq_file;
@@ -153,12 +158,6 @@ void floppy_hardint(void);
 extern unsigned long sun4m_cpu_startup;
 extern unsigned long sun4d_cpu_startup;
 
-/* process_32.c */
-asmlinkage int sparc_do_fork(unsigned long clone_flags,
-                             unsigned long stack_start,
-                             struct pt_regs *regs,
-                             unsigned long stack_size);
-
 /* signal_32.c */
 asmlinkage void do_sigreturn(struct pt_regs *regs);
 asmlinkage void do_rt_sigreturn(struct pt_regs *regs);
diff --git a/arch/sparc/kernel/process.c b/arch/sparc/kernel/process.c
new file mode 100644
index 000000000000..8bbe62d77b77
--- /dev/null
+++ b/arch/sparc/kernel/process.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * This file handles the architecture independent parts of process handling..
+ */
+
+#include <linux/compat.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/sched/task.h>
+#include <linux/sched/task_stack.h>
+#include <linux/signal.h>
+
+#include "kernel.h"
+
+asmlinkage long sparc_fork(struct pt_regs *regs)
+{
+	unsigned long orig_i1 = regs->u_regs[UREG_I1];
+	long ret;
+	struct kernel_clone_args args = {
+		.exit_signal	= SIGCHLD,
+		/* Reuse the parent's stack for the child. */
+		.stack		= regs->u_regs[UREG_FP],
+	};
+
+	ret = _do_fork(&args);
+
+	/* If we get an error and potentially restart the system
+	 * call, we're screwed because copy_thread_tls() clobbered
+	 * the parent's %o1.  So detect that case and restore it
+	 * here.
+	 */
+	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
+		regs->u_regs[UREG_I1] = orig_i1;
+
+	return ret;
+}
+
+asmlinkage long sparc_vfork(struct pt_regs *regs)
+{
+	unsigned long orig_i1 = regs->u_regs[UREG_I1];
+	long ret;
+
+	struct kernel_clone_args args = {
+		.flags		= CLONE_VFORK | CLONE_VM,
+		.exit_signal	= SIGCHLD,
+		/* Reuse the parent's stack for the child. */
+		.stack		= regs->u_regs[UREG_FP],
+	};
+
+	ret = _do_fork(&args);
+
+	/* If we get an error and potentially restart the system
+	 * call, we're screwed because copy_thread_tls() clobbered
+	 * the parent's %o1.  So detect that case and restore it
+	 * here.
+	 */
+	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
+		regs->u_regs[UREG_I1] = orig_i1;
+
+	return ret;
+}
+
+asmlinkage long sparc_clone(struct pt_regs *regs)
+{
+	unsigned long orig_i1 = regs->u_regs[UREG_I1];
+	unsigned int flags = lower_32_bits(regs->u_regs[UREG_I0]);
+	long ret;
+
+	struct kernel_clone_args args = {
+		.flags		= (flags & ~CSIGNAL),
+		.exit_signal	= (flags & CSIGNAL),
+		.tls		= regs->u_regs[UREG_I3],
+	};
+
+#ifdef CONFIG_COMPAT
+	if (test_thread_flag(TIF_32BIT)) {
+		args.pidfd	= compat_ptr(regs->u_regs[UREG_I2]);
+		args.child_tid	= compat_ptr(regs->u_regs[UREG_I4]);
+		args.parent_tid	= compat_ptr(regs->u_regs[UREG_I2]);
+	} else
+#endif
+	{
+		args.pidfd	= (int __user *)regs->u_regs[UREG_I2];
+		args.child_tid	= (int __user *)regs->u_regs[UREG_I4];
+		args.parent_tid	= (int __user *)regs->u_regs[UREG_I2];
+	}
+
+	/* Did userspace give setup a separate stack for the child or are we
+	 * reusing the parent's?
+	 */
+	if (regs->u_regs[UREG_I1])
+		args.stack = regs->u_regs[UREG_I1];
+	else
+		args.stack = regs->u_regs[UREG_FP];
+
+	ret = _do_fork(&args);
+
+	/* If we get an error and potentially restart the system
+	 * call, we're screwed because copy_thread_tls() clobbered
+	 * the parent's %o1.  So detect that case and restore it
+	 * here.
+	 */
+	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
+		regs->u_regs[UREG_I1] = orig_i1;
+
+	return ret;
+}
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 13cb5638fab8..229a10bab74a 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -257,33 +257,6 @@ clone_stackframe(struct sparc_stackf __user *dst,
 	return sp;
 }
 
-asmlinkage int sparc_do_fork(unsigned long clone_flags,
-                             unsigned long stack_start,
-                             struct pt_regs *regs,
-                             unsigned long stack_size)
-{
-	unsigned long parent_tid_ptr, child_tid_ptr;
-	unsigned long orig_i1 = regs->u_regs[UREG_I1];
-	long ret;
-
-	parent_tid_ptr = regs->u_regs[UREG_I2];
-	child_tid_ptr = regs->u_regs[UREG_I4];
-
-	ret = do_fork(clone_flags, stack_start, stack_size,
-		      (int __user *) parent_tid_ptr,
-		      (int __user *) child_tid_ptr);
-
-	/* If we get an error and potentially restart the system
-	 * call, we're screwed because copy_thread() clobbered
-	 * the parent's %o1.  So detect that case and restore it
-	 * here.
-	 */
-	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
-		regs->u_regs[UREG_I1] = orig_i1;
-
-	return ret;
-}
-
 /* Copy a Sparc thread.  The fork() return value conventions
  * under SunOS are nothing short of bletcherous:
  * Parent -->  %o0 == childs  pid, %o1 == 0
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index 8c400fca4e9f..278bf287c4be 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -572,100 +572,6 @@ void fault_in_user_windows(struct pt_regs *regs)
 	force_sig(SIGSEGV);
 }
 
-asmlinkage long sparc_fork(struct pt_regs *regs)
-{
-	unsigned long orig_i1 = regs->u_regs[UREG_I1];
-	long ret;
-	struct kernel_clone_args args = {
-		.exit_signal	= SIGCHLD,
-		/* Reuse the parent's stack for the child. */
-		.stack		= regs->u_regs[UREG_FP],
-	};
-
-	ret = _do_fork(&args);
-
-	/* If we get an error and potentially restart the system
-	 * call, we're screwed because copy_thread_tls() clobbered
-	 * the parent's %o1.  So detect that case and restore it
-	 * here.
-	 */
-	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
-		regs->u_regs[UREG_I1] = orig_i1;
-
-	return ret;
-}
-
-asmlinkage long sparc_vfork(struct pt_regs *regs)
-{
-	unsigned long orig_i1 = regs->u_regs[UREG_I1];
-	long ret;
-
-	struct kernel_clone_args args = {
-		.flags		= CLONE_VFORK | CLONE_VM,
-		.exit_signal	= SIGCHLD,
-		/* Reuse the parent's stack for the child. */
-		.stack		= regs->u_regs[UREG_FP],
-	};
-
-	ret = _do_fork(&args);
-
-	/* If we get an error and potentially restart the system
-	 * call, we're screwed because copy_thread_tls() clobbered
-	 * the parent's %o1.  So detect that case and restore it
-	 * here.
-	 */
-	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
-		regs->u_regs[UREG_I1] = orig_i1;
-
-	return ret;
-}
-
-asmlinkage long sparc_clone(struct pt_regs *regs)
-{
-	unsigned long orig_i1 = regs->u_regs[UREG_I1];
-	unsigned int flags = lower_32_bits(regs->u_regs[UREG_I0]);
-	long ret;
-
-	struct kernel_clone_args args = {
-		.flags		= (flags & ~CSIGNAL),
-		.exit_signal	= (flags & CSIGNAL),
-		.tls		= regs->u_regs[UREG_I3],
-	};
-
-#ifdef CONFIG_COMPAT
-	if (test_thread_flag(TIF_32BIT)) {
-		args.pidfd	= compat_ptr(regs->u_regs[UREG_I2]);
-		args.child_tid	= compat_ptr(regs->u_regs[UREG_I4]);
-		args.parent_tid	= compat_ptr(regs->u_regs[UREG_I2]);
-	} else
-#endif
-	{
-		args.pidfd	= (int __user *)regs->u_regs[UREG_I2];
-		args.child_tid	= (int __user *)regs->u_regs[UREG_I4];
-		args.parent_tid	= (int __user *)regs->u_regs[UREG_I2];
-	}
-
-	/* Did userspace setup a separate stack for the child or are we
-	 * copying the parent's?
-	 */
-	if (regs->u_regs[UREG_I1])
-		args.stack = regs->u_regs[UREG_I1];
-	else
-		args.stack = regs->u_regs[UREG_FP];
-
-	ret = _do_fork(&args);
-
-	/* If we get an error and potentially restart the system
-	 * call, we're screwed because copy_thread_tls() clobbered
-	 * the parent's %o1.  So detect that case and restore it
-	 * here.
-	 */
-	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
-		regs->u_regs[UREG_I1] = orig_i1;
-
-	return ret;
-}
-
 /* Copy a Sparc thread.  The fork() return value conventions
  * under SunOS are nothing short of bletcherous:
  * Parent -->  %o0 == childs  pid, %o1 == 0
-- 
2.27.0


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

* [PATCH 04/17] sparc: unconditionally enable HAVE_COPY_THREAD_TLS
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (2 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 03/17] sparc: share process creation helpers between sparc and sparc64 Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 05/17] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args Christian Brauner
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, David S . Miller,
	Arnd Bergmann, Guo Ren, linux-csky, sparclinux

Now that both sparc and sparc64 support copy_thread_tls() and don't rely
on do_fork() anymore, turn on HAVE_COPY_THREAD_TLS unconditionally. Once
all architectures are switched over this macro will be removed and
the old do_fork() calling convention fully abandoned in favor of the
cleaner struct kernel_clone_args one.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: David S. Miller <davem@davemloft.net>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guo Ren <guoren@kernel.org>
Cc: linux-csky@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Link: https://lore.kernel.org/r/20200512171527.570109-4-christian.brauner@ubuntu.com
---
 arch/sparc/Kconfig             | 2 +-
 arch/sparc/kernel/process_32.c | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index e2bb4956e130..66213c0cb557 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -48,6 +48,7 @@ config SPARC
 	select LOCKDEP_SMALL if LOCKDEP
 	select NEED_DMA_MAP_STATE
 	select NEED_SG_DMA_LENGTH
+	select HAVE_COPY_THREAD_TLS
 
 config SPARC32
 	def_bool !64BIT
@@ -94,7 +95,6 @@ config SPARC64
 	select ARCH_HAS_PTE_SPECIAL
 	select PCI_DOMAINS if PCI
 	select ARCH_HAS_GIGANTIC_PAGE
-	select HAVE_COPY_THREAD_TLS
 
 config ARCH_PROC_KCORE_TEXT
 	def_bool y
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 229a10bab74a..3e1f7b639e9a 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -273,8 +273,9 @@ clone_stackframe(struct sparc_stackf __user *dst,
 extern void ret_from_fork(void);
 extern void ret_from_kernel_thread(void);
 
-int copy_thread(unsigned long clone_flags, unsigned long sp,
-		unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+		    unsigned long arg, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct thread_info *ti = task_thread_info(p);
 	struct pt_regs *childregs, *regs = current_pt_regs();
@@ -376,7 +377,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
 	regs->u_regs[UREG_I1] = 0;
 
 	if (clone_flags & CLONE_SETTLS)
-		childregs->u_regs[UREG_G7] = regs->u_regs[UREG_I3];
+		childregs->u_regs[UREG_G7] = tls;
 
 	return 0;
 }
-- 
2.27.0


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

* [PATCH 05/17] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (3 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 04/17] sparc: unconditionally enable HAVE_COPY_THREAD_TLS Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 06/17] nios2: " Christian Brauner
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Tony Luck, Fenghua Yu,
	John Paul Adrian Glaubitz, Thomas Gleixner, Ingo Molnar,
	Sebastian Andrzej Siewior, Peter Zijlstra (Intel),
	Qais Yousef, linux-ia64

This is part of a larger series that aims at getting rid of the
copy_thread()/copy_thread_tls() split that makes the process creation
codepaths in the kernel more convoluted and error-prone than they need
to be.
I'm converting all the remaining arches that haven't yet switched and
am collecting individual acks. Once I have them, I'll send the whole series
removing the copy_thread()/copy_thread_tls() split, the
HAVE_COPY_THREAD_TLS define and the legacy do_fork() helper. The only
kernel-wide process creation entry point for anything not going directly
through the syscall path will then be based on struct kernel_clone_args.
No more danger of weird process creation abi quirks between architectures
hopefully, and easier to maintain overall.
It also unblocks implementing clone3() on architectures not support
copy_thread_tls(). Any architecture that wants to implement clone3()
will need to select HAVE_COPY_THREAD_TLS and thus need to implement
copy_thread_tls(). So both goals are connected but independently
beneficial.

HAVE_COPY_THREAD_TLS means that a given architecture supports
CLONE_SETTLS and not setting it should usually mean that the
architectures doesn't implement it but that's not how things are. In
fact all architectures support CLONE_TLS it's just that they don't
follow the calling convention that HAVE_COPY_THREAD_TLS implies. That
means all architectures can be switched over to select
HAVE_COPY_THREAD_TLS. Once that is done we can remove that macro (yay,
less code), the unnecessary do_fork() export in kernel/fork.c, and also
rename copy_thread_tls() back to copy_thread(). At this point
copy_thread() becomes the main architecture specific part of process
creation but it will be the same layout and calling convention for all
architectures. (Once that is done we can probably cleanup each
copy_thread() function even more but that's for the future.)

Since ia64 does support CLONE_SETTLS there's no reason to not select
HAVE_COPY_THREAD_TLS. This brings us one step closer to getting rid of
the copy_thread()/copy_thread_tls() split we still have and ultimately
the HAVE_COPY_THREAD_TLS define in general. A lot of architectures have
already converted and ia64 is one of the few hat haven't yet. This also
unblocks implementing the clone3() syscall on ia64. Once that is done we
can get of another ARCH_WANTS_* macro.

Once Any architecture that supports HAVE_COPY_THREAD_TLS cannot call the
do_fork() helper anymore. This is fine and intended since it should be
removed in favor of the new, cleaner _do_fork() calling convention based
on struct kernel_clone_args. In fact, most architectures have already
switched.  With this patch, ia64 joins the other arches which can't use
the fork(), vfork(), clone(), clone3() syscalls directly and who follow
the new process creation calling convention that is based on struct
kernel_clone_args which we introduced a while back. This means less
custom assembly in the architectures entry path to set up the registers
before calling into the process creation helper and it is easier to to
support new features without having to adapt calling conventions. It
also unifies all process creation paths between fork(), vfork(),
clone(), and clone3(). (We can't fix the ABI nightmare that legacy
clone() is but we can prevent stuff like this happening in the future.)

Well, the first version I nothing to test this with. I don't know how to
reasonably explain what happened but thanks to Adrian I'm now sitting at
home next to a HP Integrity RX2600. I've done some testing and my initial
version had a bug that became obvious when I took a closer look. The switch
stack logic assumes that ar.pfs is stored in r16 and I changed that to r2.
So with that fixed the following test program runs without any problems:

 #ifndef _GNU_SOURCE
 #define _GNU_SOURCE 1
 #endif
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/sched.h>
 #include <sched.h>
 #include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <syscall.h>
 #include <unistd.h>

 #define IA64_SYSCALL_OFFSET 1024
 #ifndef __NR_clone
 #define __NR_clone (104 + IA64_SYSCALL_OFFSET)
 #endif

 #ifndef __NR_clone2
 #define __NR_clone2 (189 + IA64_SYSCALL_OFFSET)
 #endif

 /*
  * sys_clone(unsigned long flags,
  *	     unsigned long stack,
  *	     int *parent_tidptr,
  *	     int *child_tidptr,
  *	     unsigned long tls)
  */
 static pid_t ia64_raw_clone(void)
 {
 	return syscall(__NR_clone, SIGCHLD, 0, NULL, NULL, 0);
 }

 /*
  * sys_clone2(unsigned long flags,
  *	      unsigned long stack,
  *	      unsigned long stack_size,
  *	      int *parent_tidptr,
  *	      int *child_tidptr,
  *	      unsigned long tls)
  */
 static pid_t ia64_raw_clone2(void)
 {
 	return syscall(__NR_clone2, SIGCHLD, 0, 0, NULL, NULL, 0);
 }

 /*
  * Let's use the "standard stack limit" (i.e. glibc thread size default) for
  * stack sizes: 8MB.
  */
 #define __STACK_SIZE (8 * 1024 * 1024)

 /* This is not always defined in sched.h. */
 extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base,
                      size_t __child_stack_size, int __flags, void *__arg, ...);

 pid_t libc_clone2(int (*fn)(void *), void *arg)
 {
 	pid_t ret;
 	void *stack;

 	stack = malloc(__STACK_SIZE);
 	if (!stack)
 		return -ENOMEM;

 	 return __clone2(fn, stack, __STACK_SIZE, SIGCHLD, arg, NULL, NULL, NULL);
 }

 static int libc_clone2_child(void *data)
 {
 	fprintf(stderr, "I'll just see myself out\n");
 	_exit(EXIT_SUCCESS);
 }

 int main(void)
 {
 	for (int i = 0; i < 1000; i++) {
 		pid_t pid = ia64_raw_clone();
 		if (pid < 0)
 			_exit(EXIT_FAILURE);

 		if (pid == 0)
 			_exit(EXIT_SUCCESS);

 		if (wait(NULL) != pid)
 			_exit(EXIT_FAILURE);
 		fprintf(stderr, "ia64_raw_clone() passed\n");

 		pid = ia64_raw_clone2();
 		if (pid < 0)
 			_exit(EXIT_FAILURE);

 		if (pid == 0)
 			_exit(EXIT_SUCCESS);

 		if (wait(NULL) != pid)
 			_exit(EXIT_FAILURE);
 		fprintf(stderr, "ia64_raw_clone2() passed\n");

 		pid = libc_clone2(libc_clone2_child, NULL);
 		if (pid < 0)
 			_exit(EXIT_FAILURE);

 		if (wait(NULL) != pid)
 			_exit(EXIT_FAILURE);
 		fprintf(stderr, "libc_clone2() passed\n");
 	}

 	_exit(EXIT_SUCCESS);
 }

For some more context, please see:
commit 606e9ad20094f6d500166881d301f31a51bc8aa7
Merge: ac61145a725a 457677c70c76
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat Jan 11 15:33:48 2020 -0800

    Merge tag 'clone3-tls-v5.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

    Pull thread fixes from Christian Brauner:
     "This contains a series of patches to fix CLONE_SETTLS when used with
      clone3().

      The clone3() syscall passes the tls argument through struct clone_args
      instead of a register. This means, all architectures that do not
      implement copy_thread_tls() but still support CLONE_SETTLS via
      copy_thread() expecting the tls to be located in a register argument
      based on clone() are currently unfortunately broken. Their tls value
      will be garbage.

      The patch series fixes this on all architectures that currently define
      __ARCH_WANT_SYS_CLONE3. It also adds a compile-time check to ensure
      that any architecture that enables clone3() in the future is forced to
      also implement copy_thread_tls().

      My ultimate goal is to get rid of the copy_thread()/copy_thread_tls()
      split and just have copy_thread_tls() at some point in the not too
      distant future (Maybe even renaming copy_thread_tls() back to simply
      copy_thread() once the old function is ripped from all arches). This
      is dependent now on all arches supporting clone3().

      While all relevant arches do that now there are still four missing:
      ia64, m68k, sh and sparc. They have the system call reserved, but not
      implemented. Once they all implement clone3() we can get rid of
      ARCH_WANT_SYS_CLONE3 and HAVE_COPY_THREAD_TLS.

Note that in the meantime, m68k has already switched to the new calling
convention. And I've got sparc patches acked by Dave, too.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Qais Yousef <qais.yousef@arm.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: https://lore.kernel.org/r/20200517151635.3085756-1-christian.brauner@ubuntu.com
---
 arch/ia64/Kconfig          |  1 +
 arch/ia64/kernel/entry.S   | 32 +++++++++++++-------------------
 arch/ia64/kernel/process.c | 32 +++++++++++++++++++++++++-------
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 1fa2fe2ef053..1b6034b89a04 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -55,6 +55,7 @@ config IA64
 	select HAVE_ARCH_AUDITSYSCALL
 	select NEED_DMA_MAP_STATE
 	select NEED_SG_DMA_LENGTH
+	select HAVE_COPY_THREAD_TLS
 	select NUMA if !FLATMEM
 	default y
 	help
diff --git a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S
index c5efac285bc3..e98e3dafffd8 100644
--- a/arch/ia64/kernel/entry.S
+++ b/arch/ia64/kernel/entry.S
@@ -112,19 +112,16 @@ GLOBAL_ENTRY(sys_clone2)
 	.prologue ASM_UNW_PRLG_RP|ASM_UNW_PRLG_PFS, ASM_UNW_PRLG_GRSAVE(8)
 	alloc r16=ar.pfs,8,2,6,0
 	DO_SAVE_SWITCH_STACK
-	adds r2=PT(R16)+IA64_SWITCH_STACK_SIZE+16,sp
 	mov loc0=rp
-	mov loc1=r16				// save ar.pfs across do_fork
+	mov loc1=r16                             // save ar.pfs across ia64_clone
 	.body
+	mov out0=in0
 	mov out1=in1
 	mov out2=in2
-	tbit.nz p6,p0=in0,CLONE_SETTLS_BIT
-	mov out3=in3	// parent_tidptr: valid only w/CLONE_PARENT_SETTID
-	;;
-(p6)	st8 [r2]=in5				// store TLS in r16 for copy_thread()
-	mov out4=in4	// child_tidptr:  valid only w/CLONE_CHILD_SETTID or CLONE_CHILD_CLEARTID
-	mov out0=in0				// out0 = clone_flags
-	br.call.sptk.many rp=do_fork
+	mov out3=in3
+	mov out4=in4
+	mov out5=in5
+	br.call.sptk.many rp=ia64_clone
 .ret1:	.restore sp
 	adds sp=IA64_SWITCH_STACK_SIZE,sp	// pop the switch stack
 	mov ar.pfs=loc1
@@ -143,19 +140,16 @@ GLOBAL_ENTRY(sys_clone)
 	.prologue ASM_UNW_PRLG_RP|ASM_UNW_PRLG_PFS, ASM_UNW_PRLG_GRSAVE(8)
 	alloc r16=ar.pfs,8,2,6,0
 	DO_SAVE_SWITCH_STACK
-	adds r2=PT(R16)+IA64_SWITCH_STACK_SIZE+16,sp
 	mov loc0=rp
-	mov loc1=r16				// save ar.pfs across do_fork
+	mov loc1=r16                             // save ar.pfs across ia64_clone
 	.body
+	mov out0=in0
 	mov out1=in1
 	mov out2=16				// stacksize (compensates for 16-byte scratch area)
-	tbit.nz p6,p0=in0,CLONE_SETTLS_BIT
-	mov out3=in2	// parent_tidptr: valid only w/CLONE_PARENT_SETTID
-	;;
-(p6)	st8 [r2]=in4				// store TLS in r13 (tp)
-	mov out4=in3	// child_tidptr:  valid only w/CLONE_CHILD_SETTID or CLONE_CHILD_CLEARTID
-	mov out0=in0				// out0 = clone_flags
-	br.call.sptk.many rp=do_fork
+	mov out3=in3
+	mov out4=in4
+	mov out5=in5
+	br.call.sptk.many rp=ia64_clone
 .ret2:	.restore sp
 	adds sp=IA64_SWITCH_STACK_SIZE,sp	// pop the switch stack
 	mov ar.pfs=loc1
@@ -590,7 +584,7 @@ GLOBAL_ENTRY(ia64_ret_from_clone)
 	nop.i 0
 	/*
 	 * We need to call schedule_tail() to complete the scheduling process.
-	 * Called by ia64_switch_to() after do_fork()->copy_thread().  r8 contains the
+	 * Called by ia64_switch_to() after ia64_clone()->copy_thread().  r8 contains the
 	 * address of the previously executing task.
 	 */
 	br.call.sptk.many rp=ia64_invoke_schedule_tail
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 96dfb9e4b16f..416dca619da5 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -296,7 +296,7 @@ ia64_load_extra (struct task_struct *task)
 		pfm_load_regs(task);
 
 	info = __this_cpu_read(pfm_syst_info);
-	if (info & PFM_CPUINFO_SYST_WIDE) 
+	if (info & PFM_CPUINFO_SYST_WIDE)
 		pfm_syst_wide_update_task(task, info, 1);
 #endif
 }
@@ -310,8 +310,8 @@ ia64_load_extra (struct task_struct *task)
  *
  *	<clone syscall>	        <some kernel call frames>
  *	sys_clone		   :
- *	do_fork			do_fork
- *	copy_thread		copy_thread
+ *	_do_fork		_do_fork
+ *	copy_thread_tls		copy_thread_tls
  *
  * This means that the stack layout is as follows:
  *
@@ -333,9 +333,9 @@ ia64_load_extra (struct task_struct *task)
  * so there is nothing to worry about.
  */
 int
-copy_thread(unsigned long clone_flags,
-	     unsigned long user_stack_base, unsigned long user_stack_size,
-	     struct task_struct *p)
+copy_thread_tls(unsigned long clone_flags, unsigned long user_stack_base,
+		    unsigned long user_stack_size, struct task_struct *p,
+		    unsigned long tls)
 {
 	extern char ia64_ret_from_clone;
 	struct switch_stack *child_stack, *stack;
@@ -416,7 +416,7 @@ copy_thread(unsigned long clone_flags,
 	rbs_size = stack->ar_bspstore - rbs;
 	memcpy((void *) child_rbs, (void *) rbs, rbs_size);
 	if (clone_flags & CLONE_SETTLS)
-		child_ptregs->r13 = regs->r16;	/* see sys_clone2() in entry.S */
+		child_ptregs->r13 = tls;
 	if (user_stack_base) {
 		child_ptregs->r12 = user_stack_base + user_stack_size - 16;
 		child_ptregs->ar_bspstore = user_stack_base;
@@ -441,6 +441,24 @@ copy_thread(unsigned long clone_flags,
 	return retval;
 }
 
+asmlinkage long ia64_clone(unsigned long clone_flags, unsigned long stack_start,
+			   unsigned long stack_size, unsigned long parent_tidptr,
+			   unsigned long child_tidptr, unsigned long tls)
+{
+	struct kernel_clone_args args = {
+		.flags		= (lower_32_bits(clone_flags) & ~CSIGNAL),
+		.pidfd		= (int __user *)parent_tidptr,
+		.child_tid	= (int __user *)child_tidptr,
+		.parent_tid	= (int __user *)parent_tidptr,
+		.exit_signal	= (lower_32_bits(clone_flags) & CSIGNAL),
+		.stack		= stack_start,
+		.stack_size	= stack_size,
+		.tls		= tls,
+	};
+
+	return _do_fork(&args);
+}
+
 static void
 do_copy_task_regs (struct task_struct *task, struct unw_frame_info *info, void *arg)
 {
-- 
2.27.0


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

* [PATCH 06/17] nios2: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (4 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 05/17] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 07/17] h8300: select " Christian Brauner
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Thomas Gleixner,
	Sebastian Andrzej Siewior, Ley Foon Tan

This is part of a larger series that aims at getting rid of the
copy_thread()/copy_thread_tls() split that makes the process creation
codepaths in the kernel more convoluted and error-prone than they need
to be.
I'm converting all the remaining arches that haven't yet switched and
am collecting individual acks. Once I have them, I'll send the whole series
removing the copy_thread()/copy_thread_tls() split, the
HAVE_COPY_THREAD_TLS define and the legacy do_fork() helper. The only
kernel-wide process creation entry point for anything not going directly
through the syscall path will then be based on struct kernel_clone_args.
No more danger of weird process creation abi quirks between architectures
hopefully, and easier to maintain overall.
It also unblocks implementing clone3() on architectures not support
copy_thread_tls(). Any architecture that wants to implement clone3()
will need to select HAVE_COPY_THREAD_TLS and thus need to implement
copy_thread_tls(). So both goals are connected but independently
beneficial.

HAVE_COPY_THREAD_TLS means that a given architecture supports
CLONE_SETTLS and not setting it should usually mean that the
architectures doesn't implement it but that's not how things are. In
fact all architectures support CLONE_TLS it's just that they don't
follow the calling convention that HAVE_COPY_THREAD_TLS implies. That
means all architectures can be switched over to select
HAVE_COPY_THREAD_TLS. Once that is done we can remove that macro (yay,
less code), the unnecessary do_fork() export in kernel/fork.c, and also
rename copy_thread_tls() back to copy_thread(). At this point
copy_thread() becomes the main architecture specific part of process
creation but it will be the same layout and calling convention for all
architectures. (Once that is done we can probably cleanup each
copy_thread() function even more but that's for the future.)

Since nios2 does support CLONE_SETTLS there's no reason to not select
HAVE_COPY_THREAD_TLS. This brings us one step closer to getting rid of
the copy_thread()/copy_thread_tls() split we still have and ultimately
the HAVE_COPY_THREAD_TLS define in general. A lot of architectures have
already converted and nios2 is one of the few hat haven't yet. This also
unblocks implementing the clone3() syscall on nios2. Once that is done we
can get of another ARCH_WANTS_* macro.

Once Any architecture that supports HAVE_COPY_THREAD_TLS cannot call the
do_fork() helper anymore. This is fine and intended since it should be
removed in favor of the new, cleaner _do_fork() calling convention based
on struct kernel_clone_args. In fact, most architectures have already
switched.  With this patch, nios2 joins the other arches which can't use
the fork(), vfork(), clone(), clone3() syscalls directly and who follow
the new process creation calling convention that is based on struct
kernel_clone_args which we introduced a while back. This means less
custom assembly in the architectures entry path to set up the registers
before calling into the process creation helper and it is easier to to
support new features without having to adapt calling conventions. It
also unifies all process creation paths between fork(), vfork(),
clone(), and clone3(). (We can't fix the ABI nightmare that legacy
clone() is but we can prevent stuff like this happening in the future.)

For some more context, please see:
commit 606e9ad20094f6d500166881d301f31a51bc8aa7
Merge: ac61145a725a 457677c70c76
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat Jan 11 15:33:48 2020 -0800

    Merge tag 'clone3-tls-v5.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

    Pull thread fixes from Christian Brauner:
     "This contains a series of patches to fix CLONE_SETTLS when used with
      clone3().

      The clone3() syscall passes the tls argument through struct clone_args
      instead of a register. This means, all architectures that do not
      implement copy_thread_tls() but still support CLONE_SETTLS via
      copy_thread() expecting the tls to be located in a register argument
      based on clone() are currently unfortunately broken. Their tls value
      will be garbage.

      The patch series fixes this on all architectures that currently define
      __ARCH_WANT_SYS_CLONE3. It also adds a compile-time check to ensure
      that any architecture that enables clone3() in the future is forced to
      also implement copy_thread_tls().

      My ultimate goal is to get rid of the copy_thread()/copy_thread_tls()
      split and just have copy_thread_tls() at some point in the not too
      distant future (Maybe even renaming copy_thread_tls() back to simply
      copy_thread() once the old function is ripped from all arches). This
      is dependent now on all arches supporting clone3().

      While all relevant arches do that now there are still four missing:
      ia64, m68k, sh and sparc. They have the system call reserved, but not
      implemented. Once they all implement clone3() we can get rid of
      ARCH_WANT_SYS_CLONE3 and HAVE_COPY_THREAD_TLS.

Note that in the meantime, m68k has already switched to the new calling
convention. And I've got sparc patches acked by Dave and ia64 is already
done too. You can find a link to a booting qemu nios2 system with all the
changes here at [1].

[1]: https://asciinema.org/a/333353
Cc: linux-kernel@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/nios2/Kconfig          |  1 +
 arch/nios2/kernel/entry.S   |  7 +------
 arch/nios2/kernel/process.c | 23 ++++++++++++++++++++---
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
index c6645141bb2a..f9a05957a883 100644
--- a/arch/nios2/Kconfig
+++ b/arch/nios2/Kconfig
@@ -27,6 +27,7 @@ config NIOS2
 	select USB_ARCH_HAS_HCD if USB_SUPPORT
 	select CPU_NO_EFFICIENT_FFS
 	select MMU_GATHER_NO_RANGE if MMU
+	select HAVE_COPY_THREAD_TLS
 
 config GENERIC_CSUM
 	def_bool y
diff --git a/arch/nios2/kernel/entry.S b/arch/nios2/kernel/entry.S
index 3d8d1d0bcb64..da8442450e46 100644
--- a/arch/nios2/kernel/entry.S
+++ b/arch/nios2/kernel/entry.S
@@ -389,12 +389,7 @@ ENTRY(ret_from_interrupt)
  */
 ENTRY(sys_clone)
 	SAVE_SWITCH_STACK
-	addi	sp, sp, -4
-	stw	r7, 0(sp)	/* Pass 5th arg thru stack */
-	mov	r7, r6		/* 4th arg is 3rd of clone() */
-	mov	r6, zero	/* 3rd arg always 0 */
-	call	do_fork
-	addi	sp, sp, 4
+	call	nios2_clone
 	RESTORE_SWITCH_STACK
 	ret
 
diff --git a/arch/nios2/kernel/process.c b/arch/nios2/kernel/process.c
index 509e7855e8dc..3dde4d6d8fbe 100644
--- a/arch/nios2/kernel/process.c
+++ b/arch/nios2/kernel/process.c
@@ -100,8 +100,8 @@ void flush_thread(void)
 {
 }
 
-int copy_thread(unsigned long clone_flags,
-		unsigned long usp, unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long arg, struct task_struct *p, unsigned long tls)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	struct pt_regs *regs;
@@ -140,7 +140,7 @@ int copy_thread(unsigned long clone_flags,
 
 	/* Initialize tls register. */
 	if (clone_flags & CLONE_SETTLS)
-		childstack->r23 = regs->r8;
+		childstack->r23 = tls;
 
 	return 0;
 }
@@ -259,3 +259,20 @@ int dump_fpu(struct pt_regs *regs, elf_fpregset_t *r)
 {
 	return 0; /* Nios2 has no FPU and thus no FPU registers */
 }
+
+asmlinkage int nios2_clone(unsigned long clone_flags, unsigned long newsp,
+			   int __user *parent_tidptr, int __user *child_tidptr,
+			   unsigned long tls)
+{
+	struct kernel_clone_args args = {
+		.flags		= (lower_32_bits(clone_flags) & ~CSIGNAL),
+		.pidfd		= parent_tidptr,
+		.child_tid	= child_tidptr,
+		.parent_tid	= parent_tidptr,
+		.exit_signal	= (lower_32_bits(clone_flags) & CSIGNAL),
+		.stack		= newsp,
+		.tls		= tls,
+	};
+
+	return _do_fork(&args);
+}
-- 
2.27.0


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

* [PATCH 07/17] h8300: select HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (5 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 06/17] nios2: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 08/17] fork: remove do_fork() Christian Brauner
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Yoshinori Sato, uclinux-h8-devel

This is part of a larger series that aims at getting rid of the
copy_thread()/copy_thread_tls() split that makes the process creation
codepaths in the kernel more convoluted and error-prone than they need
to be.
I'm converting all the remaining arches that haven't yet switched and
am collecting individual acks. Once I have them, I'll send the whole series
removing the copy_thread()/copy_thread_tls() split, the
HAVE_COPY_THREAD_TLS define and the legacy do_fork() helper. The only
kernel-wide process creation entry point for anything not going directly
through the syscall path will then be based on struct kernel_clone_args.
No more danger of weird process creation abi quirks between architectures
hopefully, and easier to maintain overall.
It also unblocks implementing clone3() on architectures not support
copy_thread_tls(). Any architecture that wants to implement clone3()
will need to select HAVE_COPY_THREAD_TLS and thus need to implement
copy_thread_tls(). So both goals are connected but independently
beneficial.

HAVE_COPY_THREAD_TLS means that a given architecture supports
CLONE_SETTLS and not setting it should usually mean that the
architectures doesn't implement it but that's not how things are. In
fact all architectures support CLONE_TLS it's just that they don't
follow the calling convention that HAVE_COPY_THREAD_TLS implies. That
means all architectures can be switched over to select
HAVE_COPY_THREAD_TLS. Once that is done we can remove that macro (yay,
less code), the unnecessary do_fork() export in kernel/fork.c, and also
rename copy_thread_tls() back to copy_thread(). At this point
copy_thread() becomes the main architecture specific part of process
creation but it will be the same layout and calling convention for all
architectures. (Once that is done we can probably cleanup each
copy_thread() function even more but that's for the future.)

Though h8300 doesn't not suppor the CLONE_SETTLS flag there's no reason to
not switch to the copy_thread_tls() calling convention. As before
CLONE_SETTLS with legacy clone will just be ignored. This brings us one
step closer to getting rid of the copy_thread()/copy_thread_tls() split we
still have and ultimately the HAVE_COPY_THREAD_TLS define in general. A lot
of architectures have already converted and h8300 is one of the few hat
haven't yet. This also unblocks implementing the clone3() syscall on h8300.
Once that is done we can get of another ARCH_WANTS_* macro.

Once Any architecture that supports HAVE_COPY_THREAD_TLS cannot call the
do_fork() helper anymore. This is fine and intended since it should be
removed in favor of the new, cleaner _do_fork() calling convention based
on struct kernel_clone_args. In fact, most architectures have already
switched.  With this patch, h8300 joins the other arches which can't use
the fork(), vfork(), clone(), clone3() syscalls directly and who follow
the new process creation calling convention that is based on struct
kernel_clone_args which we introduced a while back. This means less
custom assembly in the architectures entry path to set up the registers
before calling into the process creation helper and it is easier to to
support new features without having to adapt calling conventions. It
also unifies all process creation paths between fork(), vfork(),
clone(), and clone3(). (We can't fix the ABI nightmare that legacy
clone() is but we can prevent stuff like this happening in the future.)

For some more context, please see:
commit 606e9ad20094f6d500166881d301f31a51bc8aa7
Merge: ac61145a725a 457677c70c76
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat Jan 11 15:33:48 2020 -0800

    Merge tag 'clone3-tls-v5.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

    Pull thread fixes from Christian Brauner:
     "This contains a series of patches to fix CLONE_SETTLS when used with
      clone3().

      The clone3() syscall passes the tls argument through struct clone_args
      instead of a register. This means, all architectures that do not
      implement copy_thread_tls() but still support CLONE_SETTLS via
      copy_thread() expecting the tls to be located in a register argument
      based on clone() are currently unfortunately broken. Their tls value
      will be garbage.

      The patch series fixes this on all architectures that currently define
      __ARCH_WANT_SYS_CLONE3. It also adds a compile-time check to ensure
      that any architecture that enables clone3() in the future is forced to
      also implement copy_thread_tls().

      My ultimate goal is to get rid of the copy_thread()/copy_thread_tls()
      split and just have copy_thread_tls() at some point in the not too
      distant future (Maybe even renaming copy_thread_tls() back to simply
      copy_thread() once the old function is ripped from all arches). This
      is dependent now on all arches supporting clone3().

      While all relevant arches do that now there are still four missing:
      ia64, m68k, sh and sparc. They have the system call reserved, but not
      implemented. Once they all implement clone3() we can get rid of
      ARCH_WANT_SYS_CLONE3 and HAVE_COPY_THREAD_TLS.

Note that in the meantime, m68k has already switched to the new calling
convention. And I've got sparc patches acked by Dave, patches for ia64, and
nios2 have been sent out and are ready too.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: uclinux-h8-devel@lists.sourceforge.jp
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/h8300/Kconfig          |  1 +
 arch/h8300/kernel/process.c | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index d11666d538fe..de0eb417a0b9 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -26,6 +26,7 @@ config H8300
 	select HAVE_ARCH_HASH
 	select CPU_NO_EFFICIENT_FFS
 	select UACCESS_MEMCPY
+	select HAVE_COPY_THREAD_TLS
 
 config CPU_BIG_ENDIAN
 	def_bool y
diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
index 0ef55e3052c9..ae23de4dcf42 100644
--- a/arch/h8300/kernel/process.c
+++ b/arch/h8300/kernel/process.c
@@ -105,9 +105,9 @@ void flush_thread(void)
 {
 }
 
-int copy_thread(unsigned long clone_flags,
-		unsigned long usp, unsigned long topstk,
-		struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long topstk, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct pt_regs *childregs;
 
@@ -159,11 +159,19 @@ asmlinkage int sys_clone(unsigned long __user *args)
 	unsigned long  newsp;
 	uintptr_t parent_tidptr;
 	uintptr_t child_tidptr;
+	struct kernel_clone_args kargs = {};
 
 	get_user(clone_flags, &args[0]);
 	get_user(newsp, &args[1]);
 	get_user(parent_tidptr, &args[2]);
 	get_user(child_tidptr, &args[3]);
-	return do_fork(clone_flags, newsp, 0,
-		       (int __user *)parent_tidptr, (int __user *)child_tidptr);
+
+	kargs.flags		= (lower_32_bits(clone_flags) & ~CSIGNAL);
+	kargs.pidfd		= (int __user *)parent_tidptr;
+	kargs.child_tid		= (int __user *)child_tidptr;
+	kargs.parent_tid	= (int __user *)parent_tidptr;
+	kargs.exit_signal	= (lower_32_bits(clone_flags) & CSIGNAL);
+	kargs.stack		= newsp;
+
+	return _do_fork(&kargs);
 }
-- 
2.27.0


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

* [PATCH 08/17] fork: remove do_fork()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (6 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 07/17] h8300: select " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 09/17] alpha: switch to copy_thread_tls() Christian Brauner
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Thomas Gleixner, Ingo Molnar,
	Al Viro, Matthew Wilcox (Oracle), Peter Zijlstra (Intel)

Now that all architectures have been switched to use _do_fork() and the new
struct kernel_clone_args calling convention we can remove the legacy
do_fork() helper completely. The calling convention used to be brittle and
do_fork() didn't buy us anything. The only calling convention accepted
should be based on struct kernel_clone_args going forward. It's cleaner and
uniform.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 include/linux/sched/task.h |  1 -
 kernel/fork.c              | 25 +------------------------
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index ddce0ea515d1..9f03c44941fb 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -96,7 +96,6 @@ extern void exit_files(struct task_struct *);
 extern void exit_itimers(struct signal_struct *);
 
 extern long _do_fork(struct kernel_clone_args *kargs);
-extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
 struct task_struct *fork_idle(int);
 struct mm_struct *copy_init_mm(void);
 extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
diff --git a/kernel/fork.c b/kernel/fork.c
index 9875aeb2ba41..0fd7eb1b38f9 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2493,29 +2493,6 @@ long _do_fork(struct kernel_clone_args *args)
 	return nr;
 }
 
-#ifndef CONFIG_HAVE_COPY_THREAD_TLS
-/* For compatibility with architectures that call do_fork directly rather than
- * using the syscall entry points below. */
-long do_fork(unsigned long clone_flags,
-	      unsigned long stack_start,
-	      unsigned long stack_size,
-	      int __user *parent_tidptr,
-	      int __user *child_tidptr)
-{
-	struct kernel_clone_args args = {
-		.flags		= (lower_32_bits(clone_flags) & ~CSIGNAL),
-		.pidfd		= parent_tidptr,
-		.child_tid	= child_tidptr,
-		.parent_tid	= parent_tidptr,
-		.exit_signal	= (lower_32_bits(clone_flags) & CSIGNAL),
-		.stack		= stack_start,
-		.stack_size	= stack_size,
-	};
-
-	return _do_fork(&args);
-}
-#endif
-
 /*
  * Create a kernel thread.
  */
@@ -2923,7 +2900,7 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
 /*
  * unshare allows a process to 'unshare' part of the process
  * context which was originally shared using clone.  copy_*
- * functions used by do_fork() cannot be used here directly
+ * functions used by _do_fork() cannot be used here directly
  * because they modify an inactive task_struct that is being
  * constructed. Here we are modifying the current, active,
  * task_struct.
-- 
2.27.0


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

* [PATCH 09/17] alpha: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (7 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 08/17] fork: remove do_fork() Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 10/17] c6x: " Christian Brauner
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Christian Brauner, linux-alpha

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: linux-alpha@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/alpha/Kconfig          | 1 +
 arch/alpha/kernel/process.c | 9 ++++-----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 10862c5a8c76..b01515c6b2ed 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -38,6 +38,7 @@ config ALPHA
 	select OLD_SIGSUSPEND
 	select CPU_NO_EFFICIENT_FFS if !ALPHA_EV67
 	select MMU_GATHER_NO_RANGE
+	select HAVE_COPY_THREAD_TLS
 	help
 	  The Alpha is a 64-bit general-purpose processor designed and
 	  marketed by the Digital Equipment Corporation of blessed memory,
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index b45f0b0d6511..dfdb6b6ba61c 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -233,10 +233,9 @@ release_thread(struct task_struct *dead_task)
 /*
  * Copy architecture-specific thread state
  */
-int
-copy_thread(unsigned long clone_flags, unsigned long usp,
-	    unsigned long kthread_arg,
-	    struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long kthread_arg, struct task_struct *p,
+		    unsigned long tls)
 {
 	extern void ret_from_fork(void);
 	extern void ret_from_kernel_thread(void);
@@ -267,7 +266,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 	   required for proper operation in the case of a threaded
 	   application calling fork.  */
 	if (clone_flags & CLONE_SETTLS)
-		childti->pcb.unique = regs->r20;
+		childti->pcb.unique = tls;
 	else
 		regs->r20 = 0;	/* OSF/1 has some strange fork() semantics.  */
 	childti->pcb.usp = usp ?: rdusp();
-- 
2.27.0


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

* [PATCH 10/17] c6x: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (8 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 09/17] alpha: switch to copy_thread_tls() Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 11/17] hexagon: " Christian Brauner
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Mark Salter,
	Aurelien Jacquiot, linux-c6x-dev

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

CC: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <jacquiot.aurelien@gmail.com>
Cc: linux-c6x-dev@linux-c6x.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/c6x/Kconfig          | 1 +
 arch/c6x/kernel/process.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig
index 6444ebfd06a6..9cde76a5928e 100644
--- a/arch/c6x/Kconfig
+++ b/arch/c6x/Kconfig
@@ -22,6 +22,7 @@ config C6X
 	select GENERIC_CLOCKEVENTS
 	select MODULES_USE_ELF_RELA
 	select MMU_GATHER_NO_RANGE if MMU
+	select HAVE_COPY_THREAD_TLS
 
 config MMU
 	def_bool n
diff --git a/arch/c6x/kernel/process.c b/arch/c6x/kernel/process.c
index cb9c8b63cddd..afa3ea9a93aa 100644
--- a/arch/c6x/kernel/process.c
+++ b/arch/c6x/kernel/process.c
@@ -104,9 +104,9 @@ void start_thread(struct pt_regs *regs, unsigned int pc, unsigned long usp)
 /*
  * Copy a new thread context in its stack.
  */
-int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long ustk_size,
-		struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long ustk_size, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct pt_regs *childregs;
 
-- 
2.27.0


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

* [PATCH 11/17] hexagon: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (9 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 10/17] c6x: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-23 16:11   ` Brian Cain
  2020-06-22 23:43 ` [PATCH 12/17] microblaze: " Christian Brauner
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Christian Brauner, Brian Cain, linux-hexagon

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: Brian Cain <bcain@codeaurora.org>
Cc: linux-hexagon@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/hexagon/Kconfig          | 1 +
 arch/hexagon/kernel/process.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 667cfc511cf9..19bc2f2ee331 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,6 +31,7 @@ config HEXAGON
 	select GENERIC_CLOCKEVENTS_BROADCAST
 	select MODULES_USE_ELF_RELA
 	select GENERIC_CPU_DEVICES
+	select HAVE_COPY_THREAD_TLS
 	help
 	  Qualcomm Hexagon is a processor architecture designed for high
 	  performance and low power across a wide variety of applications.
diff --git a/arch/hexagon/kernel/process.c b/arch/hexagon/kernel/process.c
index ac07f5f4b76b..d756f9556dd7 100644
--- a/arch/hexagon/kernel/process.c
+++ b/arch/hexagon/kernel/process.c
@@ -50,8 +50,8 @@ void arch_cpu_idle(void)
 /*
  * Copy architecture-specific thread state
  */
-int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long arg, struct task_struct *p, unsigned long tls)
 {
 	struct thread_info *ti = task_thread_info(p);
 	struct hexagon_switch_stack *ss;
@@ -100,7 +100,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 	 * ugp is used to provide TLS support.
 	 */
 	if (clone_flags & CLONE_SETTLS)
-		childregs->ugp = childregs->r04;
+		childregs->ugp = tls;
 
 	/*
 	 * Parent sees new pid -- not necessary, not even possible at
-- 
2.27.0


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

* [PATCH 12/17] microblaze: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (10 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 11/17] hexagon: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 13/17] nds32: " Christian Brauner
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Christian Brauner, Michal Simek

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/microblaze/Kconfig          | 1 +
 arch/microblaze/kernel/process.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index d262ac0c8714..e3a211a41880 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -46,6 +46,7 @@ config MICROBLAZE
 	select CPU_NO_EFFICIENT_FFS
 	select MMU_GATHER_NO_RANGE if MMU
 	select SPARSE_IRQ
+	select HAVE_COPY_THREAD_TLS
 
 # Endianness selection
 choice
diff --git a/arch/microblaze/kernel/process.c b/arch/microblaze/kernel/process.c
index 6527ec22f158..c2ca9c326510 100644
--- a/arch/microblaze/kernel/process.c
+++ b/arch/microblaze/kernel/process.c
@@ -54,8 +54,8 @@ void flush_thread(void)
 {
 }
 
-int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long arg, struct task_struct *p, unsigned long tls)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	struct thread_info *ti = task_thread_info(p);
@@ -114,7 +114,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 	 *  which contains TLS area
 	 */
 	if (clone_flags & CLONE_SETTLS)
-		childregs->r21 = childregs->r10;
+		childregs->r21 = tls;
 
 	return 0;
 }
-- 
2.27.0


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

* [PATCH 13/17] nds32: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (11 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 12/17] microblaze: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 14/17] sh: " Christian Brauner
  2020-06-22 23:43 ` [PATCH 15/17] unicore: " Christian Brauner
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Greentime Hu, Nick Hu, Vincent Chen

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: Greentime Hu <green.hu@gmail.com>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Vincent Chen <deanbo422@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/nds32/Kconfig          | 1 +
 arch/nds32/kernel/process.c | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index e30298e99e1b..7b6eaca81cce 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -48,6 +48,7 @@ config NDS32
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
+	select HAVE_COPY_THREAD_TLS
 	help
 	  Andes(nds32) Linux support.
 
diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
index 9712fd474f2c..7dbb1bf64165 100644
--- a/arch/nds32/kernel/process.c
+++ b/arch/nds32/kernel/process.c
@@ -149,8 +149,9 @@ void flush_thread(void)
 DEFINE_PER_CPU(struct task_struct *, __entry_task);
 
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
-int copy_thread(unsigned long clone_flags, unsigned long stack_start,
-		unsigned long stk_sz, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+		    unsigned long stk_sz, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 
@@ -170,7 +171,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 		childregs->uregs[0] = 0;
 		childregs->osp = 0;
 		if (clone_flags & CLONE_SETTLS)
-			childregs->uregs[25] = childregs->uregs[3];
+			childregs->uregs[25] = tls;
 	}
 	/* cpu context switching  */
 	p->thread.cpu_context.pc = (unsigned long)ret_from_fork;
-- 
2.27.0


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

* [PATCH 14/17] sh: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (12 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 13/17] nds32: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  2020-06-22 23:43 ` [PATCH 15/17] unicore: " Christian Brauner
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Christian Brauner, Rich Felker, Yoshinori Sato, linux-sh

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: Rich Felker <dalias@libc.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/sh/Kconfig             | 1 +
 arch/sh/kernel/process_32.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 9fc2b010e938..e10118d61ce7 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -70,6 +70,7 @@ config SUPERH
 	select ARCH_HIBERNATION_POSSIBLE if MMU
 	select SPARSE_IRQ
 	select HAVE_STACKPROTECTOR
+	select HAVE_COPY_THREAD_TLS
 	help
 	  The SuperH is a RISC processor targeted for use in embedded systems
 	  and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/arch/sh/kernel/process_32.c b/arch/sh/kernel/process_32.c
index 456cc8d171f7..537a82d80616 100644
--- a/arch/sh/kernel/process_32.c
+++ b/arch/sh/kernel/process_32.c
@@ -115,8 +115,8 @@ EXPORT_SYMBOL(dump_fpu);
 asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 
-int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+		    unsigned long arg, struct task_struct *p, unsigned long tls)
 {
 	struct thread_info *ti = task_thread_info(p);
 	struct pt_regs *childregs;
@@ -158,7 +158,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 	ti->addr_limit = USER_DS;
 
 	if (clone_flags & CLONE_SETTLS)
-		childregs->gbr = childregs->regs[0];
+		childregs->gbr = tls;
 
 	childregs->regs[0] = 0; /* Set return value for child */
 	p->thread.pc = (unsigned long) ret_from_fork;
-- 
2.27.0


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

* [PATCH 15/17] unicore: switch to copy_thread_tls()
  2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
                   ` (13 preceding siblings ...)
  2020-06-22 23:43 ` [PATCH 14/17] sh: " Christian Brauner
@ 2020-06-22 23:43 ` Christian Brauner
  14 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Christian Brauner, Guan Xuetao

Use the copy_thread_tls() calling convention which passes tls through a
register. This is required so we can remove the copy_thread{_tls}() split
and remove the HAVE_COPY_THREAD_TLS macro.

Cc: Guan Xuetao <gxt@pku.edu.cn>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 arch/unicore32/Kconfig          | 1 +
 arch/unicore32/kernel/process.c | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/unicore32/Kconfig b/arch/unicore32/Kconfig
index 11ba1839d198..01451cf500d2 100644
--- a/arch/unicore32/Kconfig
+++ b/arch/unicore32/Kconfig
@@ -22,6 +22,7 @@ config UNICORE32
 	select MODULES_USE_ELF_REL
 	select NEED_DMA_MAP_STATE
 	select MMU_GATHER_NO_RANGE if MMU
+	select HAVE_COPY_THREAD_TLS
 	help
 	  UniCore-32 is 32-bit Instruction Set Architecture,
 	  including a series of low-power-consumption RISC chip
diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index b4fd3a604a18..49a305565a53 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -219,9 +219,9 @@ void release_thread(struct task_struct *dead_task)
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread");
 
-int
-copy_thread(unsigned long clone_flags, unsigned long stack_start,
-	    unsigned long stk_sz, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+		    unsigned long stk_sz, struct task_struct *p,
+		    unsigned long tls)
 {
 	struct thread_info *thread = task_thread_info(p);
 	struct pt_regs *childregs = task_pt_regs(p);
@@ -241,7 +241,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,
 			childregs->UCreg_sp = stack_start;
 
 		if (clone_flags & CLONE_SETTLS)
-			childregs->UCreg_16 = childregs->UCreg_03;
+			childregs->UCreg_16 = tls;
 	}
 	return 0;
 }
-- 
2.27.0


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

* Re: [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS
  2020-06-22 23:43 ` [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS Christian Brauner
@ 2020-06-23  3:35   ` David Miller
  2020-06-23  8:42     ` Christian Brauner
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2020-06-23  3:35 UTC (permalink / raw)
  To: christian.brauner
  Cc: linux-kernel, torvalds, arnd, guoren, linux-csky, sparclinux

From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Tue, 23 Jun 2020 01:43:11 +0200

> diff --git a/arch/sparc/kernel/syscalls.S b/arch/sparc/kernel/syscalls.S
> index db42b4fb3708..192f3a28a2b7 100644
> --- a/arch/sparc/kernel/syscalls.S
> +++ b/arch/sparc/kernel/syscalls.S
> @@ -86,19 +86,22 @@ sys32_rt_sigreturn:
>  	 * during system calls...
>  	 */
>  	.align	32
> -sys_vfork: /* Under Linux, vfork and fork are just special cases of clone. */
> -	sethi	%hi(0x4000 | 0x0100 | SIGCHLD), %o0
> -	or	%o0, %lo(0x4000 | 0x0100 | SIGCHLD), %o0
> -	ba,pt	%xcc, sys_clone
> +sys_vfork:
> +	flushw
> +	ba,pt	%xcc, sparc_vfork
> +	add	%sp, PTREGS_OFF, %o0

Please indent branch delay slot instructions with one extra space, as
was done in the code you are changing.

> +	ba,pt	%xcc, sparc_fork
> +	add	%sp, PTREGS_OFF, %o0

Likewise.

> +	ba,pt	%xcc, sparc_clone
> +	add	%sp, PTREGS_OFF, %o0

Likewise.

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

* Re: [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS
  2020-06-23  3:35   ` David Miller
@ 2020-06-23  8:42     ` Christian Brauner
  0 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-23  8:42 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, torvalds, arnd, guoren, linux-csky, sparclinux

On Mon, Jun 22, 2020 at 08:35:16PM -0700, David Miller wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
> Date: Tue, 23 Jun 2020 01:43:11 +0200
> 
> > diff --git a/arch/sparc/kernel/syscalls.S b/arch/sparc/kernel/syscalls.S
> > index db42b4fb3708..192f3a28a2b7 100644
> > --- a/arch/sparc/kernel/syscalls.S
> > +++ b/arch/sparc/kernel/syscalls.S
> > @@ -86,19 +86,22 @@ sys32_rt_sigreturn:
> >  	 * during system calls...
> >  	 */
> >  	.align	32
> > -sys_vfork: /* Under Linux, vfork and fork are just special cases of clone. */
> > -	sethi	%hi(0x4000 | 0x0100 | SIGCHLD), %o0
> > -	or	%o0, %lo(0x4000 | 0x0100 | SIGCHLD), %o0
> > -	ba,pt	%xcc, sys_clone
> > +sys_vfork:
> > +	flushw
> > +	ba,pt	%xcc, sparc_vfork
> > +	add	%sp, PTREGS_OFF, %o0
> 
> Please indent branch delay slot instructions with one extra space, as
> was done in the code you are changing.
> 
> > +	ba,pt	%xcc, sparc_fork
> > +	add	%sp, PTREGS_OFF, %o0
> 
> Likewise.
> 
> > +	ba,pt	%xcc, sparc_clone
> > +	add	%sp, PTREGS_OFF, %o0
> 
> Likewise.

Fixed, thanks for taking a look again! This somehow slipped past me when
applying. 

Christian

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

* RE: [PATCH 11/17] hexagon: switch to copy_thread_tls()
  2020-06-22 23:43 ` [PATCH 11/17] hexagon: " Christian Brauner
@ 2020-06-23 16:11   ` Brian Cain
  0 siblings, 0 replies; 20+ messages in thread
From: Brian Cain @ 2020-06-23 16:11 UTC (permalink / raw)
  To: 'Christian Brauner', linux-kernel
  Cc: 'Linus Torvalds', linux-hexagon

...
> Cc: Brian Cain <bcain@codeaurora.org>
> Cc: linux-hexagon@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  arch/hexagon/Kconfig          | 1 +
>  arch/hexagon/kernel/process.c | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig index
> 667cfc511cf9..19bc2f2ee331 100644
> --- a/arch/hexagon/Kconfig
> +++ b/arch/hexagon/Kconfig
> @@ -31,6 +31,7 @@ config HEXAGON
>  	select GENERIC_CLOCKEVENTS_BROADCAST
>  	select MODULES_USE_ELF_RELA
>  	select GENERIC_CPU_DEVICES
> +	select HAVE_COPY_THREAD_TLS
>  	help
>  	  Qualcomm Hexagon is a processor architecture designed for high
>  	  performance and low power across a wide variety of applications.
> diff --git a/arch/hexagon/kernel/process.c b/arch/hexagon/kernel/process.c
> index ac07f5f4b76b..d756f9556dd7 100644
> --- a/arch/hexagon/kernel/process.c
> +++ b/arch/hexagon/kernel/process.c
> @@ -50,8 +50,8 @@ void arch_cpu_idle(void)
>  /*
>   * Copy architecture-specific thread state
>   */
> -int copy_thread(unsigned long clone_flags, unsigned long usp,
> -		unsigned long arg, struct task_struct *p)
> +int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
> +		    unsigned long arg, struct task_struct *p, unsigned long
tls)
>  {
>  	struct thread_info *ti = task_thread_info(p);
>  	struct hexagon_switch_stack *ss;
> @@ -100,7 +100,7 @@ int copy_thread(unsigned long clone_flags, unsigned
> long usp,
>  	 * ugp is used to provide TLS support.
>  	 */
>  	if (clone_flags & CLONE_SETTLS)
> -		childregs->ugp = childregs->r04;
> +		childregs->ugp = tls;
> 
>  	/*
>  	 * Parent sees new pid -- not necessary, not even possible at
> --
> 2.27.0

Acked-by: Brian Cain <bcain@codeaurora.org>



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

* Re: [fork] 11689456e6: ltp.clone302.fail
       [not found]   ` <20200627082748.GM5535@shao2-debian>
@ 2020-06-27 12:23     ` Christian Brauner
  0 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2020-06-27 12:23 UTC (permalink / raw)
  To: kernel test robot
  Cc: linux-kernel, Linus Torvalds, Thomas Gleixner, Ingo Molnar,
	Al Viro, Geert Uytterhoeven, Matthew Wilcox (Oracle),
	Peter Zijlstra (Intel),
	linux-m68k, x86, lkp

On Sat, Jun 27, 2020 at 04:27:48PM +0800, kernel test robot wrote:
> Greeting,
> 
> FYI, we noticed the following commit (built with gcc-9):
> 
> commit: 11689456e6df828b7917a558a36212e68fa9aa69 ("[PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork()")
> url: https://github.com/0day-ci/linux/commits/Christian-Brauner/arch-remove-do_fork-and-HAVE_COPY_THREAD_TLS/20200623-080105
> base: https://git.kernel.org/cgit/linux/kernel/git/davem/sparc.git master
> 
> in testcase: ltp
> with following parameters:
> 
> 	disk: 1HDD
> 	fs: ext4
> 	test: syscalls_part1
> 
> test-description: The LTP testsuite contains a collection of tools for testing the Linux kernel and related features.
> test-url: http://linux-test-project.github.io/
> 
> 
> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> 
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
> 
> 
> 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> 
> 
> <<<test_start>>>
> tag=clone302 stime=1593153327
> cmdline="clone302"
> contacts=""
> analysis=exit
> <<<test_output>>>
> tst_buffers.c:55: INFO: Test is using guarded buffers
> tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
> clone302.c:92: PASS: invalid args: clone3() failed as expected: EFAULT (14)
> clone302.c:92: PASS: zero size: clone3() failed as expected: EINVAL (22)
> clone302.c:92: PASS: short size: clone3() failed as expected: EINVAL (22)
> clone302.c:92: PASS: extra size: clone3() failed as expected: EFAULT (14)
> clone302.c:92: PASS: sighand-no-VM: clone3() failed as expected: EINVAL (22)
> clone302.c:92: PASS: thread-no-sighand: clone3() failed as expected: EINVAL (22)
> clone302.c:92: PASS: fs-newns: clone3() failed as expected: EINVAL (22)
> clone302.c:92: PASS: invalid pidfd: clone3() failed as expected: EFAULT (14)
> clone302.c:92: PASS: invalid childtid: clone3() failed as expected: EFAULT (14)
> clone302.c:88: FAIL: invalid parenttid: clone3() should fail with EFAULT: EINVAL (22)

In short, this is a change in failure behavior for clone3() I did expect
and am willing to risk. Here's why in the short form:
- clone3() is extremely new
- this failed before
- setting both CLONE_PIDFD and CLONE_PARENT_SETTID is extremely rare
  (haven't seen it in any codebases I know that use clone3())
- setting both CLONE_PIDFD and CLONE_PARENT_SETTID __and__ pointing them
  to the same adress doesn't work
  (haven't seen it in any codebases I know that use clone3() but see
  some more notes on that below)
- the change makes a special case go away and simplifies multiple
  call-sites

So a few notes about the test. I did stare at it for a while and was
confused why you expect EFAULT to be returned when CLONE_PARENT_SETTID
is set to an invalid memory address. Because that doesn't make sense.
When the parent tid is written to the memory location for
CLONE_PARENT_SETTID we're past the point of no return of process
creation, i.e. the return value from put_user() isn't checked and can't
be checked anymore so you'd never receive EFAULT for a bogus parent_tid
memory address. This is not something new. This has been the case since
the introduction of pid namespaces and specifically since commit
30e49c263e36 ("pid namespaces: allow cloning of new namespace").

But then it dawned on me. You're setting CLONE_PIDFD |
CLONE_PARENT_SETTID and you're pointing:
- args->parent_tid	= <invalid-address>
- args->pidfd		= NULL
so the EFAULT you've seen so far in your test-suite has never been for
CLONE_PARENT_SETTID but for CLONE_PIDFD since that value is written
before the point of no return and consequently put_user() is checked and
the EFAULT is surfaced. So independent of that issue here you might want
to adapt that test so it really tests what you want. :) (And maybe it's
worth documenting on the manpage for clone{3}() that failures for
CLONE_PARENT_SETTID and CLONE_CHILD_SETTID are not seen.)

(Also, note that for some reason, args->pidfd and pargs->parent_tid
must've ended up pointing to the same address in your test-suite. So my
guess is that args->pidfd pointed to garbage which got turned into a
useable address after tst_get_bad_addr() returned &invalid_address.
Maybe I'm missing something subtle though.)

Christian

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

end of thread, other threads:[~2020-06-27 12:23 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 23:43 [PATCH 00/17] arch: remove do_fork() and HAVE_COPY_THREAD_TLS Christian Brauner
2020-06-22 23:43 ` [PATCH 01/17] fork: fold legacy_clone_args_valid() into _do_fork() Christian Brauner
     [not found]   ` <20200627082748.GM5535@shao2-debian>
2020-06-27 12:23     ` [fork] 11689456e6: ltp.clone302.fail Christian Brauner
2020-06-22 23:43 ` [PATCH 02/17] sparc64: enable HAVE_COPY_THREAD_TLS Christian Brauner
2020-06-23  3:35   ` David Miller
2020-06-23  8:42     ` Christian Brauner
2020-06-22 23:43 ` [PATCH 03/17] sparc: share process creation helpers between sparc and sparc64 Christian Brauner
2020-06-22 23:43 ` [PATCH 04/17] sparc: unconditionally enable HAVE_COPY_THREAD_TLS Christian Brauner
2020-06-22 23:43 ` [PATCH 05/17] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args Christian Brauner
2020-06-22 23:43 ` [PATCH 06/17] nios2: " Christian Brauner
2020-06-22 23:43 ` [PATCH 07/17] h8300: select " Christian Brauner
2020-06-22 23:43 ` [PATCH 08/17] fork: remove do_fork() Christian Brauner
2020-06-22 23:43 ` [PATCH 09/17] alpha: switch to copy_thread_tls() Christian Brauner
2020-06-22 23:43 ` [PATCH 10/17] c6x: " Christian Brauner
2020-06-22 23:43 ` [PATCH 11/17] hexagon: " Christian Brauner
2020-06-23 16:11   ` Brian Cain
2020-06-22 23:43 ` [PATCH 12/17] microblaze: " Christian Brauner
2020-06-22 23:43 ` [PATCH 13/17] nds32: " Christian Brauner
2020-06-22 23:43 ` [PATCH 14/17] sh: " Christian Brauner
2020-06-22 23:43 ` [PATCH 15/17] unicore: " Christian Brauner

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 NNTP newsgroup(s).