Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description

[Josh Triplett <josh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On Sun, Sep 25, 2016 at 09:03:07PM +0200, Michael Kerrisk (man-pages) wrote:
> On 09/04/2016 07:11 AM, Josh Triplett wrote:
> > On Sun, Sep 04, 2016 at 04:30:44PM +1200, Michael Kerrisk (man-pages) wrote:
> >> [CC += Josh]
> >>
> >> Josh, I know you were submitting patches relate to the clone() TLS argument a 
> >> while back. Could you comment on this patch proposal below (also
> >> https://bugzilla.kernel.org/show_bug.cgi?id=118241 is relevant).
> > 
> > Sure.
> > 
> >> On 08/24/2016 03:06 PM, Keno Fischer wrote:
> >>> The prototype for the system call was added in 81f10dad, but looking at the
> >>> kernel's fork.c, I believe the relevant definition is
> >>>
> >>> SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
> >>> int __user *, parent_tidptr,
> >>> int __user *, child_tidptr,
> >>> unsigned long, tls)
> >>>
> >>> so the last argument is the tls argument, not a pt_regs argument.
> >>> I stumbled upon this while trying to understand CLONE_SETTLS, so I expanded
> >>> that description a little to cover other architectures.
> > 
> > This description and patch looks correct to me.
> > Reviewed-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
> 
> Thanks, Josh.
> 
> > Ideally, I'd suggest a follow-up patch to improve the prototypes for the
> > various architectures.  Rather than saying "x86 looks roughly this way,
> > on these architectures swap these arguments, on these architectures
> > ...", I'd suggest explicitly giving each of the four prototypes (normal,
> > CONFIG_CLONE_BACKWARDS, CONFIG_CLONE_BACKWARDS2,
> > CONFIG_CLONE_BACKWARDS3) and the corresponding architectures.
> 
> I applied the patch below. Okay?

Mostly looks good, but one comment below.

> diff --git a/man2/clone.2 b/man2/clone.2
> index 5c1e301..26c6b30 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -54,11 +54,7 @@ clone, __clone2 \- create a child process
>  .BI "          /* pid_t *" ptid ", void *" newtls \
>  ", pid_t *" ctid " */ );"
>  
> -/* Prototype for the raw system call */
> -
> -.BI "long clone(unsigned long " flags ", void *" child_stack ,
> -.BI "          int *" ptid ", int *" ctid ,
> -.BI "          unsigned long " newtls );
> +/* For the prototype of the raw system call, see NOTES */
>  .fi
>  .SH DESCRIPTION
>  .BR clone ()
> @@ -821,16 +817,58 @@ arguments of the
>  .BR clone ()
>  wrapper function are omitted.
>  Furthermore, the argument order changes.
> -The raw system call interface on x86 and many other architectures is roughly:
> +In addition, there are variations across architectures.
> +
> +The raw system call interface on x86-64 and some other architectures
> +(including sh, tile, and alpha) is roughly:
> +
>  .in +4
>  .nf
> +.BI "long clone(unsigned long " flags ", void *" child_stack ,
> +.BI "           int *" ptid ", int *" ctid ,
> +.BI "           unsigned long " newtls );
> +.fi
> +.in
> +
> +On x86-32, and several other common architectures
> +(including score, microblaze, ARM, ARM 64, PA-RISC, arc, Power PC, xtensa,
> +and MIPS),
> +.\" CONFIG_CLONE_BACKWARDS
> +the order of the last two arguments is reversed:
>  
> +.in +4
> +.nf
>  .BI "long clone(unsigned long " flags ", void *" child_stack ,
> +.BI "          int *" ptid ", unsigned long " newtls ,
> +.BI "          int *" ctid );
> +.fi
> +.in
> +
> +On the cris and s390 architectures,
> +.\" CONFIG_CLONE_BACKWARDS2
> +the order of the first two arguments is reversed:
> +
> +.in +4
> +.nf
> +.BI "long clone(void *" child_stack ", unsigned long " flags ,
>  .BI "           int *" ptid ", int *" ctid ,
>  .BI "           unsigned long " newtls );
> +.fi
> +.in
> +
> +On the microblaze architecture,
> +.\" CONFIG_CLONE_BACKWARDS3
> +an additional argument is supplied:

This mentions microblaze twice: one in CONFIG_CLONE_BACKWARDS and again
here.  And I don't see IA-64 mentioned anywhere.

> +.in +4
> +.nf
> +.BI "long clone(unsigned long " flags ", void *" child_stack ,
> +.BI "           int " stack_size , "\fR         /* Size of stack */"
> +.BI "           int *" ptid ", int *" ctid ,
> +.BI "           unsigned long " newtls );
>  .fi
>  .in
> +
>  Another difference for the raw system call is that the
>  .I child_stack
>  argument may be zero, in which case copy-on-write semantics ensure that the
> @@ -839,15 +877,11 @@ the stack.
>  In this case, for correct operation, the
>  .B CLONE_VM
>  option should not be specified.
> -
> -For some architectures, the order of the arguments for the system call
> -differs from that shown above.
> -On the score, microblaze, ARM, ARM 64, PA-RISC, arc, Power PC, xtensa,
> -and MIPS architectures,
> -the order of the fourth and fifth arguments is reversed.
> -On the cris and s390 architectures,
> -the order of the first and second arguments is reversed.
> +.\"
>  .SS blackfin, m68k, and sparc
> +.\" Mike Frysinger noted in a 2013 mail:
> +.\"     these arches don't define __ARCH_WANT_SYS_CLONE:
> +.\"     blackfin ia64 m68k sparc
>  The argument-passing conventions on
>  blackfin, m68k, and sparc are different from the descriptions above.
>  For details, see the kernel (and glibc) source.
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html