All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
@ 2016-08-24  3:06 Keno Fischer
       [not found] ` <20160824030607.GA2596-nTuEee01erudBw3G0RLmbfBZMHv189dXZkel5v8DVj8@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Keno Fischer @ 2016-08-24  3:06 UTC (permalink / raw)
  To: Michael Kerrisk-manpages
  Cc: linux-man-u79uwXL29TY76Z2rM5mHXA, pschiffe-H+wXaHxf7aLQT0dZR+AlfA

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.
---
 man2/clone.2 | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/man2/clone.2 b/man2/clone.2
index b867961..7c80e63 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -51,14 +51,14 @@ clone, __clone2 \- create a child process
 
 .BI "int clone(int (*" "fn" ")(void *), void *" child_stack ,
 .BI "          int " flags ", void *" "arg" ", ... "
-.BI "          /* pid_t *" ptid ", struct user_desc *" tls \
+.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 "          void *" ptid ", void *" ctid ,
-.BI "          struct pt_regs *" regs );
+.BI "          unsigned long " newtls );
 .fi
 .SH DESCRIPTION
 .BR clone ()
@@ -544,11 +544,25 @@ then trace the child also (see
 .BR ptrace (2)).
 .TP
 .BR CLONE_SETTLS " (since Linux 2.5.32)"
-The
+The TLS (Thread Local Storage) descriptor is set to
+.I newtls.
+
+The interpretation of
+.I newtls
+and the resulting effect is architecture dependent.
+On x86,
 .I newtls
-argument is the new TLS (Thread Local Storage) descriptor.
+is interpreted as a
+.IR "struct user_desc *"
 (See
-.BR set_thread_area (2).)
+.BR set_thread_area (2)).
+On x86_64 it is the new value to be set for the %fs base register
+(See the
+.I ARCH_SET_FS
+argument to
+.BR arch_prctl (2)).
+On architectures with a dedicated TLS register, it is the new value
+of that register.
 .TP
 .BR CLONE_SIGHAND " (since Linux 2.0)"
 If
@@ -813,7 +827,7 @@ The raw system call interface on x86 and many other architectures is roughly:
 
 .BI "long clone(unsigned long " flags ", void *" child_stack ,
 .BI "           void *" ptid ", void *" ctid ,
-.BI "           struct pt_regs *" regs );
+.BI "           unsigned long " newtls );
 
 .fi
 .in
-- 
2.8.1
--
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

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
       [not found] ` <20160824030607.GA2596-nTuEee01erudBw3G0RLmbfBZMHv189dXZkel5v8DVj8@public.gmane.org>
@ 2016-09-04  4:30   ` Michael Kerrisk (man-pages)
       [not found]     ` <2c257a1e-b7c5-c2bc-09ab-a2a685ae37b7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-09-25 18:06   ` Michael Kerrisk (man-pages)
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-09-04  4:30 UTC (permalink / raw)
  To: Keno Fischer
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA, Josh Triplett

[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).

Thanks,

Michael

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.
> ---
>  man2/clone.2 | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index b867961..7c80e63 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -51,14 +51,14 @@ clone, __clone2 \- create a child process
>  
>  .BI "int clone(int (*" "fn" ")(void *), void *" child_stack ,
>  .BI "          int " flags ", void *" "arg" ", ... "
> -.BI "          /* pid_t *" ptid ", struct user_desc *" tls \
> +.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 "          void *" ptid ", void *" ctid ,
> -.BI "          struct pt_regs *" regs );
> +.BI "          unsigned long " newtls );
>  .fi
>  .SH DESCRIPTION
>  .BR clone ()
> @@ -544,11 +544,25 @@ then trace the child also (see
>  .BR ptrace (2)).
>  .TP
>  .BR CLONE_SETTLS " (since Linux 2.5.32)"
> -The
> +The TLS (Thread Local Storage) descriptor is set to
> +.I newtls.
> +
> +The interpretation of
> +.I newtls
> +and the resulting effect is architecture dependent.
> +On x86,
>  .I newtls
> -argument is the new TLS (Thread Local Storage) descriptor.
> +is interpreted as a
> +.IR "struct user_desc *"
>  (See
> -.BR set_thread_area (2).)
> +.BR set_thread_area (2)).
> +On x86_64 it is the new value to be set for the %fs base register
> +(See the
> +.I ARCH_SET_FS
> +argument to
> +.BR arch_prctl (2)).
> +On architectures with a dedicated TLS register, it is the new value
> +of that register.
>  .TP
>  .BR CLONE_SIGHAND " (since Linux 2.0)"
>  If
> @@ -813,7 +827,7 @@ The raw system call interface on x86 and many other architectures is roughly:
>  
>  .BI "long clone(unsigned long " flags ", void *" child_stack ,
>  .BI "           void *" ptid ", void *" ctid ,
> -.BI "           struct pt_regs *" regs );
> +.BI "           unsigned long " newtls );
>  
>  .fi
>  .in
> 


-- 
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

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
       [not found]     ` <2c257a1e-b7c5-c2bc-09ab-a2a685ae37b7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-09-04  5:11       ` Josh Triplett
  2016-09-25 19:03         ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 7+ messages in thread
From: Josh Triplett @ 2016-09-04  5:11 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Keno Fischer, linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA

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>

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.

> >  man2/clone.2 | 26 ++++++++++++++++++++------
> >  1 file changed, 20 insertions(+), 6 deletions(-)
> > 
> > diff --git a/man2/clone.2 b/man2/clone.2
> > index b867961..7c80e63 100644
> > --- a/man2/clone.2
> > +++ b/man2/clone.2
> > @@ -51,14 +51,14 @@ clone, __clone2 \- create a child process
> >  
> >  .BI "int clone(int (*" "fn" ")(void *), void *" child_stack ,
> >  .BI "          int " flags ", void *" "arg" ", ... "
> > -.BI "          /* pid_t *" ptid ", struct user_desc *" tls \
> > +.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 "          void *" ptid ", void *" ctid ,
> > -.BI "          struct pt_regs *" regs );
> > +.BI "          unsigned long " newtls );
> >  .fi
> >  .SH DESCRIPTION
> >  .BR clone ()
> > @@ -544,11 +544,25 @@ then trace the child also (see
> >  .BR ptrace (2)).
> >  .TP
> >  .BR CLONE_SETTLS " (since Linux 2.5.32)"
> > -The
> > +The TLS (Thread Local Storage) descriptor is set to
> > +.I newtls.
> > +
> > +The interpretation of
> > +.I newtls
> > +and the resulting effect is architecture dependent.
> > +On x86,
> >  .I newtls
> > -argument is the new TLS (Thread Local Storage) descriptor.
> > +is interpreted as a
> > +.IR "struct user_desc *"
> >  (See
> > -.BR set_thread_area (2).)
> > +.BR set_thread_area (2)).
> > +On x86_64 it is the new value to be set for the %fs base register
> > +(See the
> > +.I ARCH_SET_FS
> > +argument to
> > +.BR arch_prctl (2)).
> > +On architectures with a dedicated TLS register, it is the new value
> > +of that register.
> >  .TP
> >  .BR CLONE_SIGHAND " (since Linux 2.0)"
> >  If
> > @@ -813,7 +827,7 @@ The raw system call interface on x86 and many other architectures is roughly:
> >  
> >  .BI "long clone(unsigned long " flags ", void *" child_stack ,
> >  .BI "           void *" ptid ", void *" ctid ,
> > -.BI "           struct pt_regs *" regs );
> > +.BI "           unsigned long " newtls );
> >  
> >  .fi
> >  .in
> > 
> 
> 
> -- 
> 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

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
       [not found] ` <20160824030607.GA2596-nTuEee01erudBw3G0RLmbfBZMHv189dXZkel5v8DVj8@public.gmane.org>
  2016-09-04  4:30   ` Michael Kerrisk (man-pages)
@ 2016-09-25 18:06   ` Michael Kerrisk (man-pages)
  1 sibling, 0 replies; 7+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-09-25 18:06 UTC (permalink / raw)
  To: Keno Fischer
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA, Josh Triplett

Hello Keno,

On 08/24/2016 05:06 AM, 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.

I applied this patch. Further work in progress, following Josh's comments.

Cheers,

Michael

> ---
>  man2/clone.2 | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index b867961..7c80e63 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -51,14 +51,14 @@ clone, __clone2 \- create a child process
>  
>  .BI "int clone(int (*" "fn" ")(void *), void *" child_stack ,
>  .BI "          int " flags ", void *" "arg" ", ... "
> -.BI "          /* pid_t *" ptid ", struct user_desc *" tls \
> +.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 "          void *" ptid ", void *" ctid ,
> -.BI "          struct pt_regs *" regs );
> +.BI "          unsigned long " newtls );
>  .fi
>  .SH DESCRIPTION
>  .BR clone ()
> @@ -544,11 +544,25 @@ then trace the child also (see
>  .BR ptrace (2)).
>  .TP
>  .BR CLONE_SETTLS " (since Linux 2.5.32)"
> -The
> +The TLS (Thread Local Storage) descriptor is set to
> +.I newtls.
> +
> +The interpretation of
> +.I newtls
> +and the resulting effect is architecture dependent.
> +On x86,
>  .I newtls
> -argument is the new TLS (Thread Local Storage) descriptor.
> +is interpreted as a
> +.IR "struct user_desc *"
>  (See
> -.BR set_thread_area (2).)
> +.BR set_thread_area (2)).
> +On x86_64 it is the new value to be set for the %fs base register
> +(See the
> +.I ARCH_SET_FS
> +argument to
> +.BR arch_prctl (2)).
> +On architectures with a dedicated TLS register, it is the new value
> +of that register.
>  .TP
>  .BR CLONE_SIGHAND " (since Linux 2.0)"
>  If
> @@ -813,7 +827,7 @@ The raw system call interface on x86 and many other architectures is roughly:
>  
>  .BI "long clone(unsigned long " flags ", void *" child_stack ,
>  .BI "           void *" ptid ", void *" ctid ,
> -.BI "           struct pt_regs *" regs );
> +.BI "           unsigned long " newtls );
>  
>  .fi
>  .in
> 


-- 
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

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
  2016-09-04  5:11       ` Josh Triplett
@ 2016-09-25 19:03         ` Michael Kerrisk (man-pages)
       [not found]           ` <5b7eb44c-0f9a-f143-9e3d-9927961b2139-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-09-25 19:03 UTC (permalink / raw)
  To: Josh Triplett
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Keno Fischer,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA, Mike Frysinger

[CC += Mike Frysinger, since he also commented on this in the past]

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?

Cheers,

Michael

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:
 
+.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.

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
       [not found]           ` <5b7eb44c-0f9a-f143-9e3d-9927961b2139-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-09-25 19:29             ` Josh Triplett
  2016-09-25 20:20               ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 7+ messages in thread
From: Josh Triplett @ 2016-09-25 19:29 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Keno Fischer, linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA, Mike Frysinger

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

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

* Re: [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description
  2016-09-25 19:29             ` Josh Triplett
@ 2016-09-25 20:20               ` Michael Kerrisk (man-pages)
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-09-25 20:20 UTC (permalink / raw)
  To: Josh Triplett
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Keno Fischer,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	pschiffe-H+wXaHxf7aLQT0dZR+AlfA, Mike Frysinger

On 09/25/2016 09:29 PM, Josh Triplett wrote:
> 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.  

Ahh -- the furst mention was a mistake. Fixed now. Thanks for spotting that.

> And I don't see IA-64 mentioned anywhere.

It was already covered by existing text in the page.

Cheers,

Michael

-- 
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

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

end of thread, other threads:[~2016-09-25 20:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24  3:06 [PATCH] clone.2: Adjust syscall prototype and expand CLONE_SETTLS description Keno Fischer
     [not found] ` <20160824030607.GA2596-nTuEee01erudBw3G0RLmbfBZMHv189dXZkel5v8DVj8@public.gmane.org>
2016-09-04  4:30   ` Michael Kerrisk (man-pages)
     [not found]     ` <2c257a1e-b7c5-c2bc-09ab-a2a685ae37b7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-09-04  5:11       ` Josh Triplett
2016-09-25 19:03         ` Michael Kerrisk (man-pages)
     [not found]           ` <5b7eb44c-0f9a-f143-9e3d-9927961b2139-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-09-25 19:29             ` Josh Triplett
2016-09-25 20:20               ` Michael Kerrisk (man-pages)
2016-09-25 18:06   ` Michael Kerrisk (man-pages)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.