LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
@ 2015-03-13 18:04 Alex Dowad
  2015-03-13 18:04 ` [PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg' Alex Dowad
                   ` (10 more replies)
  0 siblings, 11 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Oleg Nesterov, Peter Zijlstra, Kirill A. Shutemov,
	Rik van Riel, Vladimir Davydov, Thomas Gleixner, David Rientjes,
	Kees Cook, Aaron Tomlin, open list

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


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

* [PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-13 18:04 ` [PATCH 03/32] arc: " Alex Dowad
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner,
	open list:ALPHA PORT, open list

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


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

* [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
  2015-03-13 18:04 ` [PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg' Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-25 11:47   ` Vineet Gupta
  2015-03-13 18:04 ` [PATCH 04/32] arm: copy_thread(): rename 'stk_sz' " Alex Dowad
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Vineet Gupta, open list

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


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

* [PATCH 04/32] arm: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
  2015-03-13 18:04 ` [PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg' Alex Dowad
  2015-03-13 18:04 ` [PATCH 03/32] arc: " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-13 18:04 ` [PATCH 05/32] arm64: " Alex Dowad
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Nathan Lynch, Guenter Roeck, Heiko Stuebner,
	Kees Cook, Sebastian Capella, Stephen Boyd,
	moderated list:ARM PORT, open list

'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


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

* [PATCH 05/32] arm64: copy_thread(): rename 'stk_sz' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (2 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 04/32] arm: copy_thread(): rename 'stk_sz' " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-13 18:04 ` [PATCH 06/32] avr32: copy_thread(): rename 'arg' " Alex Dowad
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Catalin Marinas, Will Deacon, Arun Chandran, Andrew Morton,
	Guenter Roeck, Arun KS, Laura Abbott, linux-arm-kernel,
	open list

'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


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

* [PATCH 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (3 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 05/32] arm64: " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-16  7:03   ` Hans-Christian Egtvedt
  2015-03-13 18:04 ` [PATCH 07/32] blackfin: copy_thread(): rename 'topstk' " Alex Dowad
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Haavard Skinnemoen, Hans-Christian Egtvedt, open list

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


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

* [PATCH 07/32] blackfin: copy_thread(): rename 'topstk' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (4 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 06/32] avr32: copy_thread(): rename 'arg' " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-13 18:04 ` [PATCH 08/32] c6x: copy_thread(): rename 'ustk_size' " Alex Dowad
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Steven Miao, moderated list:BLACKFIN ARCHITEC..., open list

'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


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

* [PATCH 08/32] c6x: copy_thread(): rename 'ustk_size' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (5 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 07/32] blackfin: copy_thread(): rename 'topstk' " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-13 18:04 ` [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' " Alex Dowad
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Salter, Aurelien Jacquiot, open list:C6X ARCHITECTURE, open list

'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


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

* [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (6 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 08/32] c6x: copy_thread(): rename 'ustk_size' " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-25 11:20   ` Jesper Nilsson
  2015-03-13 18:04 ` [PATCH 10/32] cris/arch-v32: " Alex Dowad
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mikael Starvik, Jesper Nilsson, open list:CRIS PORT, open list

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


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

* [PATCH 10/32] cris/arch-v32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (7 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' " Alex Dowad
@ 2015-03-13 18:04 ` Alex Dowad
  2015-03-25 11:20   ` Jesper Nilsson
  2015-03-13 18:18 ` [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Aaron Tomlin
  2015-03-13 23:04 ` josh
  10 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-13 18:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mikael Starvik, Jesper Nilsson, open list:CRIS PORT, open list

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


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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (8 preceding siblings ...)
  2015-03-13 18:04 ` [PATCH 10/32] cris/arch-v32: " Alex Dowad
@ 2015-03-13 18:18 ` Aaron Tomlin
  2015-03-13 23:04 ` josh
  10 siblings, 0 replies; 24+ messages in thread
From: Aaron Tomlin @ 2015-03-13 18:18 UTC (permalink / raw)
  To: Alex Dowad
  Cc: linux-kernel, Andrew Morton, Oleg Nesterov, Peter Zijlstra,
	Kirill A. Shutemov, Rik van Riel, Vladimir Davydov,
	Thomas Gleixner, David Rientjes, Kees Cook, open list

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

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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
                   ` (9 preceding siblings ...)
  2015-03-13 18:18 ` [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Aaron Tomlin
@ 2015-03-13 23:04 ` josh
  2015-03-13 23:21   ` David Rientjes
  2015-03-14 16:19   ` Alex Dowad
  10 siblings, 2 replies; 24+ messages in thread
From: josh @ 2015-03-13 23:04 UTC (permalink / raw)
  To: Alex Dowad
  Cc: linux-kernel, Andrew Morton, Oleg Nesterov, Peter Zijlstra,
	Kirill A. Shutemov, Rik van Riel, Vladimir Davydov

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

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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-13 23:04 ` josh
@ 2015-03-13 23:21   ` David Rientjes
  2015-03-14 17:14     ` Alex Dowad
  2015-03-14 16:19   ` Alex Dowad
  1 sibling, 1 reply; 24+ messages in thread
From: David Rientjes @ 2015-03-13 23:21 UTC (permalink / raw)
  To: josh
  Cc: Alex Dowad, linux-kernel, Andrew Morton, Oleg Nesterov,
	Peter Zijlstra, Kirill A. Shutemov, Rik van Riel,
	Vladimir Davydov

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.

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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-13 23:04 ` josh
  2015-03-13 23:21   ` David Rientjes
@ 2015-03-14 16:19   ` Alex Dowad
  2015-03-14 17:30     ` Geert Uytterhoeven
  1 sibling, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-14 16:19 UTC (permalink / raw)
  To: josh
  Cc: linux-kernel, Andrew Morton, Oleg Nesterov, Peter Zijlstra,
	Kirill A. Shutemov, Rik van Riel, Vladimir Davydov


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

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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-13 23:21   ` David Rientjes
@ 2015-03-14 17:14     ` Alex Dowad
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Dowad @ 2015-03-14 17:14 UTC (permalink / raw)
  To: David Rientjes, josh
  Cc: linux-kernel, Andrew Morton, Oleg Nesterov, Peter Zijlstra,
	Kirill A. Shutemov, Rik van Riel, Vladimir Davydov


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

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

* Re: [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use
  2015-03-14 16:19   ` Alex Dowad
@ 2015-03-14 17:30     ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2015-03-14 17:30 UTC (permalink / raw)
  To: Alex Dowad
  Cc: Josh Triplett, linux-kernel, Andrew Morton, Oleg Nesterov,
	Peter Zijlstra, Kirill A. Shutemov, Rik van Riel,
	Vladimir Davydov

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

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

* Re: [PATCH 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 ` [PATCH 06/32] avr32: copy_thread(): rename 'arg' " Alex Dowad
@ 2015-03-16  7:03   ` Hans-Christian Egtvedt
  2015-03-16  8:18     ` [PATCHv2 " Alex Dowad
  0 siblings, 1 reply; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2015-03-16  7:03 UTC (permalink / raw)
  To: Alex Dowad; +Cc: linux-kernel, Haavard Skinnemoen, open list

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

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

* [PATCHv2 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-16  7:03   ` Hans-Christian Egtvedt
@ 2015-03-16  8:18     ` Alex Dowad
  2015-03-16  9:52       ` Hans-Christian Egtvedt
  0 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-16  8:18 UTC (permalink / raw)
  To: egtvedt; +Cc: hskinnemoen, linux-kernel

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


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

* Re: [PATCHv2 06/32] avr32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-16  8:18     ` [PATCHv2 " Alex Dowad
@ 2015-03-16  9:52       ` Hans-Christian Egtvedt
  0 siblings, 0 replies; 24+ messages in thread
From: Hans-Christian Egtvedt @ 2015-03-16  9:52 UTC (permalink / raw)
  To: Alex Dowad; +Cc: hskinnemoen, linux-kernel

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

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

* Re: [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 ` [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' " Alex Dowad
@ 2015-03-25 11:20   ` Jesper Nilsson
  0 siblings, 0 replies; 24+ messages in thread
From: Jesper Nilsson @ 2015-03-25 11:20 UTC (permalink / raw)
  To: Alex Dowad
  Cc: linux-kernel, Mikael Starvik, Jesper Nilsson, linux-cris-kernel,
	open list

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

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

* Re: [PATCH 10/32] cris/arch-v32: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 ` [PATCH 10/32] cris/arch-v32: " Alex Dowad
@ 2015-03-25 11:20   ` Jesper Nilsson
  0 siblings, 0 replies; 24+ messages in thread
From: Jesper Nilsson @ 2015-03-25 11:20 UTC (permalink / raw)
  To: Alex Dowad
  Cc: linux-kernel, Mikael Starvik, Jesper Nilsson, linux-cris-kernel,
	open list

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

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

* Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-13 18:04 ` [PATCH 03/32] arc: " Alex Dowad
@ 2015-03-25 11:47   ` Vineet Gupta
  2015-03-25 12:34     ` Alex Dowad
  0 siblings, 1 reply; 24+ messages in thread
From: Vineet Gupta @ 2015-03-25 11:47 UTC (permalink / raw)
  To: Alex Dowad, linux-kernel; +Cc: open list

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

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

* Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-25 11:47   ` Vineet Gupta
@ 2015-03-25 12:34     ` Alex Dowad
  2015-03-25 12:37       ` Vineet Gupta
  0 siblings, 1 reply; 24+ messages in thread
From: Alex Dowad @ 2015-03-25 12:34 UTC (permalink / raw)
  To: Vineet Gupta, linux-kernel; +Cc: open list



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?

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

* Re: [PATCH 03/32] arc: copy_thread(): rename 'arg' argument to 'kthread_arg'
  2015-03-25 12:34     ` Alex Dowad
@ 2015-03-25 12:37       ` Vineet Gupta
  0 siblings, 0 replies; 24+ messages in thread
From: Vineet Gupta @ 2015-03-25 12:37 UTC (permalink / raw)
  To: Alex Dowad, Vineet Gupta, linux-kernel; +Cc: open list

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 !

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

end of thread, other threads:[~2015-03-25 12:37 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 18:04 [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Alex Dowad
2015-03-13 18:04 ` [PATCH 02/32] alpha: copy_thread(): rename 'arg' argument to 'kthread_arg' Alex Dowad
2015-03-13 18:04 ` [PATCH 03/32] arc: " Alex Dowad
2015-03-25 11:47   ` Vineet Gupta
2015-03-25 12:34     ` Alex Dowad
2015-03-25 12:37       ` Vineet Gupta
2015-03-13 18:04 ` [PATCH 04/32] arm: copy_thread(): rename 'stk_sz' " Alex Dowad
2015-03-13 18:04 ` [PATCH 05/32] arm64: " Alex Dowad
2015-03-13 18:04 ` [PATCH 06/32] avr32: copy_thread(): rename 'arg' " Alex Dowad
2015-03-16  7:03   ` Hans-Christian Egtvedt
2015-03-16  8:18     ` [PATCHv2 " Alex Dowad
2015-03-16  9:52       ` Hans-Christian Egtvedt
2015-03-13 18:04 ` [PATCH 07/32] blackfin: copy_thread(): rename 'topstk' " Alex Dowad
2015-03-13 18:04 ` [PATCH 08/32] c6x: copy_thread(): rename 'ustk_size' " Alex Dowad
2015-03-13 18:04 ` [PATCH 09/32] cris/arch-v10: copy_thread(): rename 'arg' " Alex Dowad
2015-03-25 11:20   ` Jesper Nilsson
2015-03-13 18:04 ` [PATCH 10/32] cris/arch-v32: " Alex Dowad
2015-03-25 11:20   ` Jesper Nilsson
2015-03-13 18:18 ` [PATCH 01/32] do_fork(): Rename 'stack_size' argument to reflect actual use Aaron Tomlin
2015-03-13 23:04 ` josh
2015-03-13 23:21   ` David Rientjes
2015-03-14 17:14     ` Alex Dowad
2015-03-14 16:19   ` Alex Dowad
2015-03-14 17:30     ` Geert Uytterhoeven

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