Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On Sat, Mar 14, 2015 at 5:19 PM, Alex Dowad wrote:
> I've just been searching for history on sys_clone2() but have come up empty.
> "git blame" isn't helping, either... the current code dates back before the
> start of the git history.
There are several trees containing more history (e.g. from bitkeeper).
Search for "full history linux git".
Anyway, it doesn't help much in this case. sys_clone2() was introduced in
commit 0830ad57cf52da22dbb933945fd1bda6b320e0ee
Author: linus1
Date: Thu Jul 13 11:00:00 2000 -0600
Import 2.4.0-test5pre2
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On 14/03/15 01:21, David Rientjes wrote: On Fri, 13 Mar 2015, j...@joshtriplett.org wrote: On Fri, Mar 13, 2015 at 08:04:16PM +0200, Alex Dowad wrote: The 'stack_size' argument is never used to pass a stack size. It's only used when forking a kernel thread, in which case it is an argument which should be passed to the 'main' function which the kernel thread executes. Hence, rename it to 'kthread_arg'. That's not the only use of stack_size. Take a look at the clone2 system call (very minimally documented in the clone manpage) and the implementation of copy_thread on ia64, which does use stack_size in the non-kthread path. Exactly, and it seems like Alex just disregarded this early feedback when this was first raised that suggested it just be named "arg" and to comment the individual usage in the functions that get called with the formal. David, just to clarify: your feedback was much appreciated and has not been disregarded. I am still not convinced that "arg" is the best name for the argument now called "stack_start"; I think there must be a better name, but can't think of what it is. If you or others have more suggestions, that would be helpful. Because of the uncertainty, I have avoided modifying that part of the code, and have focused on what seems like a more clear and unequivocal win for readability: renaming the "stack_size" argument. Josh Triplett kindly pointed out that "stack_size" is in fact used for a stack size when processing one particular syscall on one arch. However, rather than naming the args according to that rare case, it seems like a better idea to name them according to the 99.9% case, and add a comment mentioning the 0.1% case. Or maybe "arg1" and "arg2" are really best. If the other maintainers concur with that, I would be happy to rewrite this set of patches accordingly. Again, I appreciate your feedback and hope to receive more (if you have more to give). Thanks, Alex Dowad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On 14/03/15 01:04, j...@joshtriplett.org wrote: On Fri, Mar 13, 2015 at 08:04:16PM +0200, Alex Dowad wrote: The 'stack_size' argument is never used to pass a stack size. It's only used when forking a kernel thread, in which case it is an argument which should be passed to the 'main' function which the kernel thread executes. Hence, rename it to 'kthread_arg'. That's not the only use of stack_size. Take a look at the clone2 system call (very minimally documented in the clone manpage) and the implementation of copy_thread on ia64, which does use stack_size in the non-kthread path. Thanks for pointing that out. I searched for all uses with cscope but missed sys_clone2 (which is implemented in asm). Won't make that mistake again... I've just been searching for history on sys_clone2() but have come up empty. "git blame" isn't helping, either... the current code dates back before the start of the git history. So out of curiosity, if you are willing to explain more: why does clone2 only exist on IA-64? Is there some characteristic of the architecture that makes being able to specify the size of the user-mode stack especially valuable, as compared to other archs? Is it used much (such as being called from the C library)? Thanks for your feedback, Alex Dowad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On Fri, 13 Mar 2015, j...@joshtriplett.org wrote: > On Fri, Mar 13, 2015 at 08:04:16PM +0200, Alex Dowad wrote: > > The 'stack_size' argument is never used to pass a stack size. It's only > > used when > > forking a kernel thread, in which case it is an argument which should be > > passed > > to the 'main' function which the kernel thread executes. Hence, rename it to > > 'kthread_arg'. > > That's not the only use of stack_size. Take a look at the clone2 system > call (very minimally documented in the clone manpage) and the > implementation of copy_thread on ia64, which does use stack_size in the > non-kthread path. > Exactly, and it seems like Alex just disregarded this early feedback when this was first raised that suggested it just be named "arg" and to comment the individual usage in the functions that get called with the formal. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On Fri, Mar 13, 2015 at 08:04:16PM +0200, Alex Dowad wrote: > The 'stack_size' argument is never used to pass a stack size. It's only used > when > forking a kernel thread, in which case it is an argument which should be > passed > to the 'main' function which the kernel thread executes. Hence, rename it to > 'kthread_arg'. That's not the only use of stack_size. Take a look at the clone2 system call (very minimally documented in the clone manpage) and the implementation of copy_thread on ia64, which does use stack_size in the non-kthread path. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
On Fri 2015-03-13 20:04 +0200, Alex Dowad wrote: > The 'stack_size' argument is never used to pass a stack size. It's only used > when > forking a kernel thread, in which case it is an argument which should be > passed > to the 'main' function which the kernel thread executes. Hence, rename it to > 'kthread_arg'. > > Signed-off-by: Alex Dowad > --- AFAICT this clean up looks OK and should improve readability. Thanks. -- Aaron Tomlin pgp4xNeaAV5NO.pgp Description: PGP signature
[PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
The 'stack_size' argument is never used to pass a stack size. It's only used
when
forking a kernel thread, in which case it is an argument which should be passed
to the 'main' function which the kernel thread executes. Hence, rename it to
'kthread_arg'.
Signed-off-by: Alex Dowad
---
Hi,
The following patches in this series perform a similar cleanup for the
arch-specific
implementations of copy_thread(). Each patch has been sent to the maintainers
for
the relevant arch.
Thanks, AD
kernel/fork.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index cf65139..5a40dfd 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1186,10 +1186,12 @@ init_task_pid(struct task_struct *task, enum pid_type
type, struct pid *pid)
* It copies the registers, and all the appropriate
* parts of the process environment (as per the clone
* flags). The actual kick-off is left to the caller.
*/
static struct task_struct *copy_process(unsigned long clone_flags,
unsigned long stack_start,
- unsigned long stack_size,
+ unsigned long kthread_arg,
int __user *child_tidptr,
struct pid *pid,
int trace)
@@ -1401,7 +1403,7 @@ static struct task_struct *copy_process(unsigned long
clone_flags,
retval = copy_io(clone_flags, p);
if (retval)
goto bad_fork_cleanup_namespaces;
- retval = copy_thread(clone_flags, stack_start, stack_size, p);
+ retval = copy_thread(clone_flags, stack_start, kthread_arg, p);
if (retval)
goto bad_fork_cleanup_io;
@@ -1630,7 +1632,7 @@ struct task_struct *fork_idle(int cpu)
*/
long do_fork(unsigned long clone_flags,
unsigned long stack_start,
- unsigned long stack_size,
+ unsigned long kthread_arg,
int __user *parent_tidptr,
int __user *child_tidptr)
{
@@ -1656,7 +1658,7 @@ long do_fork(unsigned long clone_flags,
trace = 0;
}
- p = copy_process(clone_flags, stack_start, stack_size,
+ p = copy_process(clone_flags, stack_start, kthread_arg,
child_tidptr, NULL, trace);
/*
* Do this prior waking up the new thread - the thread pointer
@@ -1740,7 +1742,7 @@ SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned
long, clone_flags,
int, tls_val)
#elif defined(CONFIG_CLONE_BACKWARDS3)
SYSCALL_DEFINE6(clone, unsigned long, clone_flags, unsigned long, newsp,
- int, stack_size,
+ int, ignored,
int __user *, parent_tidptr,
int __user *, child_tidptr,
int, tls_val)
--
2.0.0.GIT
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
