[PATCH 16/32] metag: copy_thread(): rename 'arg' argument to 'kthread_arg'

[PATCH 16/32] metag: 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/metag/kernel/process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
index 483dff9..dcb4609 100644
--- a/arch/metag/kernel/process.c
+++ b/arch/metag/kernel/process.c
@@ -174,8 +174,11 @@ void show_regs(struct pt_regs *regs)
 	show_trace(NULL, (unsigned long *)regs->ctx.AX[0].U0, regs);
 }
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *tsk)
+		unsigned long kthread_arg, struct task_struct *tsk)
 {
 	struct pt_regs *childregs = task_pt_regs(tsk);
 	void *kernel_context = ((void *) childregs +
@@ -204,10 +207,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 		childregs->ctx.AX[0].U0 = (unsigned long) kernel_context;
 		/* Set D1Ar1=arg and D1RtP=usp (fn) */
 		childregs->ctx.DX[4].U1 = usp;
-		childregs->ctx.DX[3].U1 = arg;
+		childregs->ctx.DX[3].U1 = kthread_arg;
 		tsk->thread.int_depth = 2;
 		return 0;
 	}
+
 	/*
 	 * Get a pointer to where the new child's register block should have
 	 * been pushed.
-- 
2.0.0.GIT

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

[James Hogan]

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

On 13/03/15 18:14, 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/metag/kernel/process.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
> index 483dff9..dcb4609 100644
> --- a/arch/metag/kernel/process.c
> +++ b/arch/metag/kernel/process.c
> @@ -174,8 +174,11 @@ void show_regs(struct pt_regs *regs)
>  	show_trace(NULL, (unsigned long *)regs->ctx.AX[0].U0, regs);
>  }
>  
> +/*
> + * Copy architecture-specific thread state
> + */
>  int copy_thread(unsigned long clone_flags, unsigned long usp,
> -		unsigned long arg, struct task_struct *tsk)
> +		unsigned long kthread_arg, struct task_struct *tsk)
>  {
>  	struct pt_regs *childregs = task_pt_regs(tsk);
>  	void *kernel_context = ((void *) childregs +
> @@ -204,10 +207,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
>  		childregs->ctx.AX[0].U0 = (unsigned long) kernel_context;
>  		/* Set D1Ar1=arg and D1RtP=usp (fn) */

Please update this comment too (ctx.DX[3].U1 stores the D1Ar1 register).

Other than that:
Acked-by: James Hogan <james.hogan@imgtec.com>

I'll assume you plan to get all these patches merged together rather
than via individual arch trees unless you say otherwise.

Cheers
James

>  		childregs->ctx.DX[4].U1 = usp;
> -		childregs->ctx.DX[3].U1 = arg;
> +		childregs->ctx.DX[3].U1 = kthread_arg;
>  		tsk->thread.int_depth = 2;
>  		return 0;
>  	}
> +
>  	/*
>  	 * Get a pointer to where the new child's register block should have
>  	 * been pushed.
> 


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

[PATCHv2 16/32] metag: 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>
---

James Hogan,

Thanks for reviewing this patch. The comment has been updated as you suggested.

You said: "I'll assume you plan to get all these patches merged together rather than
via individual arch trees..." The truth is that I am as green as green can be and have
no idea how I plan to get these patches merged. I was just reading the Linux source and
saw what looked like an opportunity to make the code a tiny bit easier to read. If you
have any suggestions on how to proceed, please let me know.

Kind regards, Alex Dowad

 arch/metag/kernel/process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
index 483dff9..dcb4609 100644
--- a/arch/metag/kernel/process.c
+++ b/arch/metag/kernel/process.c
@@ -174,8 +174,11 @@ void show_regs(struct pt_regs *regs)
 	show_trace(NULL, (unsigned long *)regs->ctx.AX[0].U0, regs);
 }
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *tsk)
+		unsigned long kthread_arg, struct task_struct *tsk)
 {
 	struct pt_regs *childregs = task_pt_regs(tsk);
 	void *kernel_context = ((void *) childregs +
@@ -204,10 +207,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 		childregs->ctx.AX[0].U0 = (unsigned long) kernel_context;
-   /* Set D1Ar1=arg and D1RtP=usp (fn) */
+ 	/* Set D1Ar1=kthread_arg and D1RtP=usp (fn) */
 		childregs->ctx.DX[4].U1 = usp;
-		childregs->ctx.DX[3].U1 = arg;
+		childregs->ctx.DX[3].U1 = kthread_arg;
 		tsk->thread.int_depth = 2;
 		return 0;
 	}
+
 	/*
 	 * Get a pointer to where the new child's register block should have
 	 * been pushed.
-- 
2.0.0.GIT

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

[James Hogan]

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

Hi Alex,

On 16/03/15 13:13, 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>
> ---
> 
> James Hogan,
> 
> Thanks for reviewing this patch. The comment has been updated as you suggested.
> 
> You said: "I'll assume you plan to get all these patches merged together rather than
> via individual arch trees..." The truth is that I am as green as green can be and have
> no idea how I plan to get these patches merged. I was just reading the Linux source and
> saw what looked like an opportunity to make the code a tiny bit easier to read. If you
> have any suggestions on how to proceed, please let me know.

I guess there are 3 main paths:
1) Ask individual arch maintainers to apply the patches if possible,
since it doesn't have dependencies on other patches you're submitting.
2) Gather acks from maintainers for the remaining patches and ask Andrew
Morton or another relevant maintainer to apply them (Andrew often picks
up misc patches like this I believe).
3) Or gather acks for remaining patches and send a pull request to Linus
yourself during the next merge window.


Btw, the whitespace seems to be corrupted here, so the patch won't apply:

> -   /* Set D1Ar1=arg and D1RtP=usp (fn) */

^^^ currently this is indented with tabs not spaces

> + 	/* Set D1Ar1=kthread_arg and D1RtP=usp (fn) */

^^^ and this has a space before a tab

Cheers
James


>  		childregs->ctx.DX[4].U1 = usp;
> -		childregs->ctx.DX[3].U1 = arg;
> +		childregs->ctx.DX[3].U1 = kthread_arg;
>  		tsk->thread.int_depth = 2;
>  		return 0;
>  	}
> +
>  	/*
>  	 * Get a pointer to where the new child's register block should have
>  	 * been pushed.
> 


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

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

[Alex Dowad]

On 16/03/15 16:31, James Hogan wrote:
> Hi Alex,
>
> On 16/03/15 13:13, 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>
>> ---
>>
>> James Hogan,
>>
>> Thanks for reviewing this patch. The comment has been updated as you suggested.
>>
>> You said: "I'll assume you plan to get all these patches merged together rather than
>> via individual arch trees..." The truth is that I am as green as green can be and have
>> no idea how I plan to get these patches merged. I was just reading the Linux source and
>> saw what looked like an opportunity to make the code a tiny bit easier to read. If you
>> have any suggestions on how to proceed, please let me know.
> I guess there are 3 main paths:
> 1) Ask individual arch maintainers to apply the patches if possible,
> since it doesn't have dependencies on other patches you're submitting.
> 2) Gather acks from maintainers for the remaining patches and ask Andrew
> Morton or another relevant maintainer to apply them (Andrew often picks
> up misc patches like this I believe).
> 3) Or gather acks for remaining patches and send a pull request to Linus
> yourself during the next merge window.
>
>
> Btw, the whitespace seems to be corrupted here, so the patch won't apply:
Grrr. Sending again.

Thanks for the pointers on merge workflow. I doubt that I will get the 
attention of each and every arch maintainer, but let's see what will 
happen. Thanks again.

>
>> -   /* Set D1Ar1=arg and D1RtP=usp (fn) */
> ^^^ currently this is indented with tabs not spaces
>
>> + 	/* Set D1Ar1=kthread_arg and D1RtP=usp (fn) */
> ^^^ and this has a space before a tab
>
> Cheers
> James

[PATCHv3 16/32] metag: 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>
---

Fixed problem with whitespace, sending again. AD

 arch/metag/kernel/process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
index 483dff9..dcb4609 100644
--- a/arch/metag/kernel/process.c
+++ b/arch/metag/kernel/process.c
@@ -174,8 +174,11 @@ void show_regs(struct pt_regs *regs)
 	show_trace(NULL, (unsigned long *)regs->ctx.AX[0].U0, regs);
 }
 
+/*
+ * Copy architecture-specific thread state
+ */
 int copy_thread(unsigned long clone_flags, unsigned long usp,
-		unsigned long arg, struct task_struct *tsk)
+		unsigned long kthread_arg, struct task_struct *tsk)
 {
 	struct pt_regs *childregs = task_pt_regs(tsk);
 	void *kernel_context = ((void *) childregs +
@@ -204,10 +207,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 		childregs->ctx.AX[0].U0 = (unsigned long) kernel_context;
-		/* Set D1Ar1=arg and D1RtP=usp (fn) */
+ 	/* Set D1Ar1=kthread_arg and D1RtP=usp (fn) */
 		childregs->ctx.DX[4].U1 = usp;
-		childregs->ctx.DX[3].U1 = arg;
+		childregs->ctx.DX[3].U1 = kthread_arg;
 		tsk->thread.int_depth = 2;
 		return 0;
 	}
+
 	/*
 	 * Get a pointer to where the new child's register block should have
 	 * been pushed.
-- 
2.0.0.GIT

Re: [PATCHv3 16/32] metag: copy_thread(): rename 'arg' argument to 'kthread_arg'

[James Hogan]

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

On 16/03/15 14:38, 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> 
> Fixed problem with whitespace, sending again. AD
> 
>  arch/metag/kernel/process.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/metag/kernel/process.c b/arch/metag/kernel/process.c
> index 483dff9..dcb4609 100644
> --- a/arch/metag/kernel/process.c
> +++ b/arch/metag/kernel/process.c
> @@ -174,8 +174,11 @@ void show_regs(struct pt_regs *regs)
>  	show_trace(NULL, (unsigned long *)regs->ctx.AX[0].U0, regs);
>  }
>  
> +/*
> + * Copy architecture-specific thread state
> + */
>  int copy_thread(unsigned long clone_flags, unsigned long usp,
> -		unsigned long arg, struct task_struct *tsk)
> +		unsigned long kthread_arg, struct task_struct *tsk)
>  {
>  	struct pt_regs *childregs = task_pt_regs(tsk);
>  	void *kernel_context = ((void *) childregs +
> @@ -204,10 +207,11 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
>  		childregs->ctx.AX[0].U0 = (unsigned long) kernel_context;
> -		/* Set D1Ar1=arg and D1RtP=usp (fn) */
> + 	/* Set D1Ar1=kthread_arg and D1RtP=usp (fn) */

FYI this still has a space before the tab, and isn't indented to the
same level as the surrounding code, but I'll just fix that up when I
apply it.

Thanks
James

>  		childregs->ctx.DX[4].U1 = usp;
> -		childregs->ctx.DX[3].U1 = arg;
> +		childregs->ctx.DX[3].U1 = kthread_arg;
>  		tsk->thread.int_depth = 2;
>  		return 0;
>  	}
> +
>  	/*
>  	 * Get a pointer to where the new child's register block should have
>  	 * been pushed.
> 


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