[PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[Alex Dowad]

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 <alexinbeijing@gmail.com>
---

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

[PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/alpha/kernel/process.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index 1941a07..84d1326 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -236,12 +236,11 @@ release_thread(struct task_struct *dead_task)
 }
 
 /*
- * Copy an alpha thread..
+ * Copy architecture-specific thread state
  */
-
 int
 copy_thread(unsigned long clone_flags, unsigned long usp,
-	    unsigned long arg,
+	    unsigned long kthread_arg,
 	    struct task_struct *p)
 {
 	extern void ret_from_fork(void);
@@ -262,7 +261,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 			sizeof(struct switch_stack) + sizeof(struct pt_regs));
 		childstack->r26 = (unsigned long) ret_from_kernel_thread;
 		childstack->r9 = usp;	/* function */
-		childstack->r10 = arg;
+		childstack->r10 = kthread_arg;
 		childregs->hae = alpha_mv.hae_cache,
 		childti->pcb.usp = 0;
 		return 0;
-- 
2.0.0.GIT

[PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/alpha/kernel/process.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index 1941a07..84d1326 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -236,12 +236,11 @@ release_thread(struct task_struct *dead_task)
 }
 
 /*
- * Copy an alpha thread..
+ * Copy architecture-specific thread state
  */
-
 int
 copy_thread(unsigned long clone_flags, unsigned long usp,
-	    unsigned long arg,
+	    unsigned long kthread_arg,
 	    struct task_struct *p)
 {
 	extern void ret_from_fork(void);
@@ -262,7 +261,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 			sizeof(struct switch_stack) + sizeof(struct pt_regs));
 		childstack->r26 = (unsigned long) ret_from_kernel_thread;
 		childstack->r9 = usp;	/* function */
-		childstack->r10 = arg;
+		childstack->r10 = kthread_arg;
 		childregs->hae = alpha_mv.hae_cache,
 		childti->pcb.usp = 0;
 		return 0;
-- 
2.0.0.GIT

[PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/arc/kernel/process.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c
index fdd8971..cf366bd 100644
--- a/arch/arc/kernel/process.c
+++ b/arch/arc/kernel/process.c
@@ -49,7 +49,9 @@ void arch_cpu_idle(void)
 
 asmlinkage void ret_from_fork(void);
 
-/* Layout of Child kernel mode stack as setup at the end of this function is
+/* Copy architecture-specific thread state
+ *
+ * Layout of Child kernel mode stack as setup at the end of this function is
  *
  * |     ...        |
  * |     ...        |
@@ -81,7 +83,7 @@ asmlinkage void ret_from_fork(void);
  * ------------------  <===== END of PAGE
  */
 int copy_thread(unsigned long clone_flags,
-		unsigned long usp, unsigned long arg,
+		unsigned long usp, unsigned long kthread_arg,
 		struct task_struct *p)
 {
 	struct pt_regs *c_regs;        /* child's pt_regs */
@@ -110,9 +112,10 @@ int copy_thread(unsigned long clone_flags,
 	childksp[1] = (unsigned long)ret_from_fork; /* blink */
 
 	if (unlikely(p->flags & PF_KTHREAD)) {
+		/* kernel thread */
 		memset(c_regs, 0, sizeof(struct pt_regs));
 
-		c_callee->r13 = arg; /* argument to kernel thread */
+		c_callee->r13 = kthread_arg;
 		c_callee->r14 = usp;  /* function */
 
 		return 0;
-- 
2.0.0.GIT

Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Vineet Gupta]

On Friday 13 March 2015 11:35 PM, Alex Dowad wrote:
> The 'arg' argument to copy_thread() is only ever used when forking a new
> kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
> with do_fork() and other arch-specific implementations of copy_thread()).
>
> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
> ---
>  arch/arc/kernel/process.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c
> index fdd8971..cf366bd 100644
> --- a/arch/arc/kernel/process.c
> +++ b/arch/arc/kernel/process.c
> @@ -49,7 +49,9 @@ void arch_cpu_idle(void)
>  
>  asmlinkage void ret_from_fork(void);
>  
> -/* Layout of Child kernel mode stack as setup at the end of this function is
> +/* Copy architecture-specific thread state
> + *
> + * Layout of Child kernel mode stack as setup at the end of this function is
>   *
>   * |     ...        |
>   * |     ...        |
> @@ -81,7 +83,7 @@ asmlinkage void ret_from_fork(void);
>   * ------------------  <===== END of PAGE
>   */
>  int copy_thread(unsigned long clone_flags,
> -		unsigned long usp, unsigned long arg,
> +		unsigned long usp, unsigned long kthread_arg,
>  		struct task_struct *p)
>  {
>  	struct pt_regs *c_regs;        /* child's pt_regs */
> @@ -110,9 +112,10 @@ int copy_thread(unsigned long clone_flags,
>  	childksp[1] = (unsigned long)ret_from_fork; /* blink */
>  
>  	if (unlikely(p->flags & PF_KTHREAD)) {
> +		/* kernel thread */

This seems extraneous given that PF_KTHREAD above check makes is obvious that this
is a kernel thread.

>  		memset(c_regs, 0, sizeof(struct pt_regs));
>  
> -		c_callee->r13 = arg; /* argument to kernel thread */
> +		c_callee->r13 = kthread_arg;
>  		c_callee->r14 = usp;  /* function */
>  
>  		return 0;

Applied to for-next after pruning the comment above.

Thx,
-Vineet

Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

On 25/03/15 13:47, Vineet Gupta wrote:
> On Friday 13 March 2015 11:35 PM, Alex Dowad wrote:
>> The 'arg' argument to copy_thread() is only ever used when forking a new
>> kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
>> with do_fork() and other arch-specific implementations of copy_thread()).
>>
>> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
>> ---
>>   arch/arc/kernel/process.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c
>> index fdd8971..cf366bd 100644
>> --- a/arch/arc/kernel/process.c
>> +++ b/arch/arc/kernel/process.c
>> @@ -49,7 +49,9 @@ void arch_cpu_idle(void)
>>   
>>   asmlinkage void ret_from_fork(void);
>>   
>> -/* Layout of Child kernel mode stack as setup at the end of this function is
>> +/* Copy architecture-specific thread state
>> + *
>> + * Layout of Child kernel mode stack as setup at the end of this function is
>>    *
>>    * |     ...        |
>>    * |     ...        |
>> @@ -81,7 +83,7 @@ asmlinkage void ret_from_fork(void);
>>    * ------------------  <===== END of PAGE
>>    */
>>   int copy_thread(unsigned long clone_flags,
>> -		unsigned long usp, unsigned long arg,
>> +		unsigned long usp, unsigned long kthread_arg,
>>   		struct task_struct *p)
>>   {
>>   	struct pt_regs *c_regs;        /* child's pt_regs */
>> @@ -110,9 +112,10 @@ int copy_thread(unsigned long clone_flags,
>>   	childksp[1] = (unsigned long)ret_from_fork; /* blink */
>>   
>>   	if (unlikely(p->flags & PF_KTHREAD)) {
>> +		/* kernel thread */
> This seems extraneous given that PF_KTHREAD above check makes is obvious that this
> is a kernel thread.
>
>>   		memset(c_regs, 0, sizeof(struct pt_regs));
>>   
>> -		c_callee->r13 = arg; /* argument to kernel thread */
>> +		c_callee->r13 = kthread_arg;
>>   		c_callee->r14 = usp;  /* function */
>>   
>>   		return 0;
> Applied to for-next after pruning the comment above.
Thank you. Is it too late for me to tweak the commit comment?

Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Vineet Gupta]

On Wednesday 25 March 2015 06:05 PM, Alex Dowad wrote:
>
> On 25/03/15 13:47, Vineet Gupta wrote:
>> On Friday 13 March 2015 11:35 PM, Alex Dowad wrote:
>>
>> Applied to for-next after pruning the comment above.
> Thank you. Is it too late for me to tweak the commit comment?

I already fixed it up !

[PATCH 04/32] arm: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'

[Alex Dowad]

'stk_sz' is a misnomer: it is never used for a stack size. Rather, it is an
argument which is passed to the main function executed by a kernel thread, when
forking a new kthread. The most appropriate name is 'kthread_arg'.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/arm/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index fdfa3a7..4183ebd 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -342,9 +342,12 @@ void release_thread(struct task_struct *dead_task)
 
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 
+/*
+ * Copy architecture-specific thread state
+ */
 int
 copy_thread(unsigned long clone_flags, unsigned long stack_start,
-	    unsigned long stk_sz, struct task_struct *p)
+	    unsigned long kthread_arg, struct task_struct *p)
 {
 	struct thread_info *thread = task_thread_info(p);
 	struct pt_regs *childregs = task_pt_regs(p);
@@ -352,13 +355,15 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	memset(&thread->cpu_context, 0, sizeof(struct cpu_context_save));
 
 	if (likely(!(p->flags & PF_KTHREAD))) {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		childregs->ARM_r0 = 0;
 		if (stack_start)
 			childregs->ARM_sp = stack_start;
 	} else {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
-		thread->cpu_context.r4 = stk_sz;
+		thread->cpu_context.r4 = kthread_arg;
 		thread->cpu_context.r5 = stack_start;
 		childregs->ARM_cpsr = SVC_MODE;
 	}
-- 
2.0.0.GIT

[PATCH 04/32] arm: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'

[Alex Dowad]

'stk_sz' is a misnomer: it is never used for a stack size. Rather, it is an
argument which is passed to the main function executed by a kernel thread, when
forking a new kthread. The most appropriate name is 'kthread_arg'.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/arm/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index fdfa3a7..4183ebd 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -342,9 +342,12 @@ void release_thread(struct task_struct *dead_task)
 
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 
+/*
+ * Copy architecture-specific thread state
+ */
 int
 copy_thread(unsigned long clone_flags, unsigned long stack_start,
-	    unsigned long stk_sz, struct task_struct *p)
+	    unsigned long kthread_arg, struct task_struct *p)
 {
 	struct thread_info *thread = task_thread_info(p);
 	struct pt_regs *childregs = task_pt_regs(p);
@@ -352,13 +355,15 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	memset(&thread->cpu_context, 0, sizeof(struct cpu_context_save));
 
 	if (likely(!(p->flags & PF_KTHREAD))) {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		childregs->ARM_r0 = 0;
 		if (stack_start)
 			childregs->ARM_sp = stack_start;
 	} else {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
-		thread->cpu_context.r4 = stk_sz;
+		thread->cpu_context.r4 = kthread_arg;
 		thread->cpu_context.r5 = stack_start;
 		childregs->ARM_cpsr = SVC_MODE;
 	}
-- 
2.0.0.GIT

[PATCH 05/32] arm64: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'

[Alex Dowad]

'stk_sz' is a misnomer: it is never used for a stack size. Rather, it is an
argument which is passed to the main function executed by a kernel thread, when
forking a new kthread. The most appropriate name is 'kthread_arg'.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/arm64/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index fde9923..734e2b6 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -242,8 +242,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 
 asmlinkage void ret_from_fork(void) asm("ret_from_fork");
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long stack_start,
-		unsigned long stk_sz, struct task_struct *p)
+		unsigned long kthread_arg, struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	unsigned long tls = p->thread.tp_value;
@@ -251,6 +254,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	memset(&p->thread.cpu_context, 0, sizeof(struct cpu_context));
 
 	if (likely(!(p->flags & PF_KTHREAD))) {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		childregs->regs[0] = 0;
 		if (is_compat_thread(task_thread_info(p))) {
@@ -276,10 +280,11 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 		if (clone_flags & CLONE_SETTLS)
 			tls = childregs->regs[3];
 	} else {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
 		childregs->pstate = PSR_MODE_EL1h;
 		p->thread.cpu_context.x19 = stack_start;
-		p->thread.cpu_context.x20 = stk_sz;
+		p->thread.cpu_context.x20 = kthread_arg;
 	}
 	p->thread.cpu_context.pc = (unsigned long)ret_from_fork;
 	p->thread.cpu_context.sp = (unsigned long)childregs;
-- 
2.0.0.GIT

[PATCH 05/32] arm64: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'

[Alex Dowad]

'stk_sz' is a misnomer: it is never used for a stack size. Rather, it is an
argument which is passed to the main function executed by a kernel thread, when
forking a new kthread. The most appropriate name is 'kthread_arg'.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/arm64/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index fde9923..734e2b6 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -242,8 +242,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 
 asmlinkage void ret_from_fork(void) asm("ret_from_fork");
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long stack_start,
-		unsigned long stk_sz, struct task_struct *p)
+		unsigned long kthread_arg, struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	unsigned long tls = p->thread.tp_value;
@@ -251,6 +254,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	memset(&p->thread.cpu_context, 0, sizeof(struct cpu_context));
 
 	if (likely(!(p->flags & PF_KTHREAD))) {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		childregs->regs[0] = 0;
 		if (is_compat_thread(task_thread_info(p))) {
@@ -276,10 +280,11 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 		if (clone_flags & CLONE_SETTLS)
 			tls = childregs->regs[3];
 	} else {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
 		childregs->pstate = PSR_MODE_EL1h;
 		p->thread.cpu_context.x19 = stack_start;
-		p->thread.cpu_context.x20 = stk_sz;
+		p->thread.cpu_context.x20 = kthread_arg;
 	}
 	p->thread.cpu_context.pc = (unsigned long)ret_from_fork;
 	p->thread.cpu_context.sp = (unsigned long)childregs;
-- 
2.0.0.GIT

[PATCH 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/avr32/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
index 42a53e74..a255bd3 100644
--- a/arch/avr32/kernel/process.c
+++ b/arch/avr32/kernel/process.c
@@ -279,20 +279,25 @@ asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 asmlinkage void syscall_return(void);
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg,
+		unsigned long kthread_arg,
 		struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 
 	if (unlikely(p->flags & PF_KTHREAD)) {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
-		p->thread.cpu_context.r0 = arg;
+		p->thread.cpu_context.r0 = kthread_arg;
 		p->thread.cpu_context.r1 = usp; /* fn */
 		p->thread.cpu_context.r2 = (unsigned long)syscall_return;
 		p->thread.cpu_context.pc = (unsigned long)ret_from_kernel_thread;
 		childregs->sr = MODE_SUPERVISOR;
 	} else {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		if (usp)
 			childregs->sp = usp;
-- 
2.0.0.GIT

Re: [PATCH 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Hans-Christian Egtvedt]

Around Fri 13 Mar 2015 20:04:21 +0200 or thereabout, Alex Dowad wrote:
> The 'arg' argument to copy_thread() is only ever used when forking a new
> kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
> with do_fork() and other arch-specific implementations of copy_thread()).
> 
> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>

In general
Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>

> ---
>  arch/avr32/kernel/process.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
> index 42a53e74..a255bd3 100644
> --- a/arch/avr32/kernel/process.c
> +++ b/arch/avr32/kernel/process.c
> @@ -279,20 +279,25 @@ asmlinkage void ret_from_fork(void);
>  asmlinkage void ret_from_kernel_thread(void);
>  asmlinkage void syscall_return(void);
>  
> +/*
> + * Copy architecture-specific thread state
> + */
>  int copy_thread(unsigned long clone_flags, unsigned long usp,
> -		unsigned long arg,
> +		unsigned long kthread_arg,
>  		struct task_struct *p)
>  {
>  	struct pt_regs *childregs = task_pt_regs(p);
>  
>  	if (unlikely(p->flags & PF_KTHREAD)) {
> +		/* kernel thread */

This is not needed, it is clearly stated on the line above.

>  		memset(childregs, 0, sizeof(struct pt_regs));
> -		p->thread.cpu_context.r0 = arg;
> +		p->thread.cpu_context.r0 = kthread_arg;
>  		p->thread.cpu_context.r1 = usp; /* fn */
>  		p->thread.cpu_context.r2 = (unsigned long)syscall_return;
>  		p->thread.cpu_context.pc = (unsigned long)ret_from_kernel_thread;
>  		childregs->sr = MODE_SUPERVISOR;
>  	} else {
> +		/* user thread */

Likewise, do not add obvious comments.

>  		*childregs = *current_pt_regs();
>  		if (usp)
>  			childregs->sp = usp;
-- 
Hans-Christian Egtvedt

[PATCHv2 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---

Dear Hans-Christian Egtvedt,

Thanks for your feedback! I have removed the unneeded comments.

AD

 arch/avr32/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
index 42a53e74..a255bd3 100644
--- a/arch/avr32/kernel/process.c
+++ b/arch/avr32/kernel/process.c
@@ -279,20 +279,25 @@ asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 asmlinkage void syscall_return(void);
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg,
+		unsigned long kthread_arg,
 		struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 
 	if (unlikely(p->flags & PF_KTHREAD)) {
 		memset(childregs, 0, sizeof(struct pt_regs));
-		p->thread.cpu_context.r0 = arg;
+		p->thread.cpu_context.r0 = kthread_arg;
 		p->thread.cpu_context.r1 = usp; /* fn */
 		p->thread.cpu_context.r2 = (unsigned long)syscall_return;
 		p->thread.cpu_context.pc = (unsigned long)ret_from_kernel_thread;
 		childregs->sr = MODE_SUPERVISOR;
 	} else {
 		*childregs = *current_pt_regs();
 		if (usp)
 			childregs->sp = usp;
-- 
2.0.0.GIT

Re: [PATCHv2 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Hans-Christian Egtvedt]

Around Mon 16 Mar 2015 10:18:29 +0200 or thereabout, Alex Dowad wrote:
> The 'arg' argument to copy_thread() is only ever used when forking a new
> kernel thread. Hence, rename it to 'kthread_arg' for clarity.
> 
> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>

Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>

> ---
> 
> Dear Hans-Christian Egtvedt,
> 
> Thanks for your feedback! I have removed the unneeded comments.
> 
> AD
> 
>  arch/avr32/kernel/process.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/avr32/kernel/process.c b/arch/avr32/kernel/process.c
> index 42a53e74..a255bd3 100644
> --- a/arch/avr32/kernel/process.c
> +++ b/arch/avr32/kernel/process.c
> @@ -279,20 +279,25 @@ asmlinkage void ret_from_fork(void);
>  asmlinkage void ret_from_kernel_thread(void);
>  asmlinkage void syscall_return(void);
>  
> +/*
> + * Copy architecture-specific thread state
> + */
>  int copy_thread(unsigned long clone_flags, unsigned long usp,
> -		unsigned long arg,
> +		unsigned long kthread_arg,
>  		struct task_struct *p)
>  {
>  	struct pt_regs *childregs = task_pt_regs(p);
>  
>  	if (unlikely(p->flags & PF_KTHREAD)) {
>  		memset(childregs, 0, sizeof(struct pt_regs));
> -		p->thread.cpu_context.r0 = arg;
> +		p->thread.cpu_context.r0 = kthread_arg;
>  		p->thread.cpu_context.r1 = usp; /* fn */
>  		p->thread.cpu_context.r2 = (unsigned long)syscall_return;
>  		p->thread.cpu_context.pc = (unsigned long)ret_from_kernel_thread;
>  		childregs->sr = MODE_SUPERVISOR;
>  	} else {
>  		*childregs = *current_pt_regs();
>  		if (usp)
>  			childregs->sp = usp;
-- 
mvh
Hans-Christian Egtvedt

[PATCH 07/32] blackfin: copy_thread(): rename 'topstk' argument to 'kthread_arg'

[Alex Dowad]

'topstk' is a misnomer: it is not a pointer to the top of a stack. Rather, it is
an argument passed to the main function executed by a new kernel thread.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/blackfin/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/blackfin/kernel/process.c b/arch/blackfin/kernel/process.c
index 4aa5545..81cdd5f 100644
--- a/arch/blackfin/kernel/process.c
+++ b/arch/blackfin/kernel/process.c
@@ -109,9 +109,12 @@ asmlinkage int bfin_clone(unsigned long clone_flags, unsigned long newsp)
 	return do_fork(clone_flags, newsp, 0, NULL, NULL);
 }
 
+/*
+ * Copy architecture-specific thread state
+ */
 int
 copy_thread(unsigned long clone_flags,
-	    unsigned long usp, unsigned long topstk,
+	    unsigned long usp, unsigned long kthread_arg,
 	    struct task_struct *p)
 {
 	struct pt_regs *childregs;
@@ -120,14 +123,16 @@ copy_thread(unsigned long clone_flags,
 	childregs = (struct pt_regs *) (task_stack_page(p) + THREAD_SIZE) - 1;
 	v = ((unsigned long *)childregs) - 2;
 	if (unlikely(p->flags & PF_KTHREAD)) {
+		/* kernel thread */
 		memset(childregs, 0, sizeof(struct pt_regs));
 		v[0] = usp;
-		v[1] = topstk;
+		v[1] = kthread_arg;
 		childregs->orig_p0 = -1;
 		childregs->ipend = 0x8000;
 		__asm__ __volatile__("%0 = syscfg;":"=da"(childregs->syscfg):);
 		p->thread.usp = 0;
 	} else {
+		/* user thread */
 		*childregs = *current_pt_regs();
 		childregs->r0 = 0;
 		p->thread.usp = usp ? : rdusp();
-- 
2.0.0.GIT

[PATCH 08/32] c6x: copy_thread(): rename 'ustk_size' argument to 'kthread_arg'

[Alex Dowad]

'ustk_size' is a misnomer: it is never used for the size of the user stack. It is
only used when forking a new kernel thread, as the argument passed to the kthread's
main function.

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/c6x/kernel/process.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/c6x/kernel/process.c b/arch/c6x/kernel/process.c
index 57d2ea8..b519377 100644
--- a/arch/c6x/kernel/process.c
+++ b/arch/c6x/kernel/process.c
@@ -109,10 +109,10 @@ void start_thread(struct pt_regs *regs, unsigned int pc, unsigned long usp)
 }
 
 /*
- * Copy a new thread context in its stack.
+ * Copy architecture-specific thread state
  */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long ustk_size,
+		unsigned long kthread_arg,
 		struct task_struct *p)
 {
 	struct pt_regs *childregs;
@@ -125,9 +125,9 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 		childregs->sp = (unsigned long)(childregs + 1);
 		p->thread.pc = (unsigned long) ret_from_kernel_thread;
 		childregs->a0 = usp;		/* function */
-		childregs->a1 = ustk_size;	/* argument */
+		childregs->a1 = kthread_arg;
 	} else {
-		/* Otherwise use the given stack */
+		/* user thread: use the given stack */
 		*childregs = *current_pt_regs();
 		if (usp)
 			childregs->sp = usp;
-- 
2.0.0.GIT

[PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/cris/arch-v10/kernel/process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/cris/arch-v10/kernel/process.c b/arch/cris/arch-v10/kernel/process.c
index 02b7834..c8c73d3 100644
--- a/arch/cris/arch-v10/kernel/process.c
+++ b/arch/cris/arch-v10/kernel/process.c
@@ -94,8 +94,11 @@ unsigned long thread_saved_pc(struct task_struct *t)
 asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *p)
+		unsigned long kthread_arg, struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	struct switch_stack *swstack = ((struct switch_stack *)childregs) - 1;
@@ -105,10 +108,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 	 */
 
 	if (unlikely(p->flags & PF_KTHREAD)) {
+		/* kernel thread */
 		memset(swstack, 0,
 			sizeof(struct switch_stack) + sizeof(struct pt_regs));
 		swstack->r1 = usp;
-		swstack->r2 = arg;
+		swstack->r2 = kthread_arg;
 		childregs->dccr = 1 << I_DCCR_BITNR;
 		swstack->return_ip = (unsigned long) ret_from_kernel_thread;
 		p->thread.ksp = (unsigned long) swstack;
-- 
2.0.0.GIT

Re: [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Jesper Nilsson]

On Fri, Mar 13, 2015 at 07:04:24PM +0100, Alex Dowad wrote:
> The 'arg' argument to copy_thread() is only ever used when forking a new
> kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
> with do_fork() and other arch-specific implementations of copy_thread()).
> 
> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[PATCH 10/32] cris/arch-v32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Alex Dowad]

The 'arg' argument to copy_thread() is only ever used when forking a new
kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
with do_fork() and other arch-specific implementations of copy_thread()).

Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>
---
 arch/cris/arch-v32/kernel/process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/cris/arch-v32/kernel/process.c b/arch/cris/arch-v32/kernel/process.c
index cebd32e..dc55e6c 100644
--- a/arch/cris/arch-v32/kernel/process.c
+++ b/arch/cris/arch-v32/kernel/process.c
@@ -101,9 +101,12 @@ unsigned long thread_saved_pc(struct task_struct *t)
 extern asmlinkage void ret_from_fork(void);
 extern asmlinkage void ret_from_kernel_thread(void);
 
+/*
+ * Copy architecture-specific thread state
+ */
 int
 copy_thread(unsigned long clone_flags, unsigned long usp,
-	unsigned long arg, struct task_struct *p)
+	unsigned long kthread_arg, struct task_struct *p)
 {
 	struct pt_regs *childregs = task_pt_regs(p);
 	struct switch_stack *swstack = ((struct switch_stack *) childregs) - 1;
@@ -114,10 +117,11 @@ copy_thread(unsigned long clone_flags, unsigned long usp,
 	 * task.
 	 */
 	if (unlikely(p->flags & PF_KTHREAD)) {
+		/* kernel thread */
 		memset(swstack, 0,
 			sizeof(struct switch_stack) + sizeof(struct pt_regs));
 		swstack->r1 = usp;
-		swstack->r2 = arg;
+		swstack->r2 = kthread_arg;
 		childregs->ccs = 1 << (I_CCS_BITNR + CCS_SHIFT);
 		swstack->return_ip = (unsigned long) ret_from_kernel_thread;
 		p->thread.ksp = (unsigned long) swstack;
-- 
2.0.0.GIT

Re: [PATCH 10/32] cris/arch-v32: copy_thread(): rename 'arg' argument to 'kthread_arg'

[Jesper Nilsson]

On Fri, Mar 13, 2015 at 07:04:25PM +0100, Alex Dowad wrote:
> The 'arg' argument to copy_thread() is only ever used when forking a new
> kernel thread. Hence, rename it to 'kthread_arg' for clarity (and consistency
> with do_fork() and other arch-specific implementations of copy_thread()).
> 
> Signed-off-by: Alex Dowad <alexinbeijing@gmail.com>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[Aaron Tomlin]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

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 <alexinbeijing@gmail.com>
> ---

AFAICT this clean up looks OK and should improve readability. Thanks.

-- 
Aaron Tomlin

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[josh]

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

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[David Rientjes]

On Fri, 13 Mar 2015, josh@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.

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[Alex Dowad]

On 14/03/15 01:21, David Rientjes wrote:
> On Fri, 13 Mar 2015, josh@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

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[Alex Dowad]

On 14/03/15 01:04, josh@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

Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use

[Geert Uytterhoeven]

On Sat, Mar 14, 2015 at 5:19 PM, Alex Dowad <alexinbeijing@gmail.com> 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 <torvalds@linuxfoundation.org>
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 -- geert@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