[RFC] [PATCH] task_pt_regs for powerpc systems

[RFC] [PATCH] task_pt_regs for powerpc systems

[Srinivasa D S]

Hi
 task_pt_regs() macro defines pt_regs for the given task,  this macro is  
currently not defined for powerpc arch. We need this macro for 
upcoming utrace features. 
 Below attached patch  defines this macro for powerpc arch. Please let 
me know your comments on this.

Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>


---
 include/asm-powerpc/processor.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6.26-rc9/include/asm-powerpc/processor.h
===================================================================
--- linux-2.6.26-rc9.orig/include/asm-powerpc/processor.h
+++ linux-2.6.26-rc9/include/asm-powerpc/processor.h
@@ -214,6 +214,8 @@ struct thread_struct {
 #define thread_saved_pc(tsk)    \
         ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)
 
+#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)
+
 unsigned long get_wchan(struct task_struct *p);
 
 #define KSTK_EIP(tsk)  ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Benjamin Herrenschmidt]

On Mon, 2008-07-07 at 19:52 +0530, Srinivasa D S wrote:
> Hi
>  task_pt_regs() macro defines pt_regs for the given task,  this macro is  
> currently not defined for powerpc arch. We need this macro for 
> upcoming utrace features. 
>  Below attached patch  defines this macro for powerpc arch. Please let 
> me know your comments on this.
> 
> Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>

Looks good to me. I'll include it in the next batch.

Cheers,
Ben.

> ---
>  include/asm-powerpc/processor.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-2.6.26-rc9/include/asm-powerpc/processor.h
> ===================================================================
> --- linux-2.6.26-rc9.orig/include/asm-powerpc/processor.h
> +++ linux-2.6.26-rc9/include/asm-powerpc/processor.h
> @@ -214,6 +214,8 @@ struct thread_struct {
>  #define thread_saved_pc(tsk)    \
>          ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)
>  
> +#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)
> +
>  unsigned long get_wchan(struct task_struct *p);
>  
>  #define KSTK_EIP(tsk)  ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Paul Mackerras]

Srinivasa D S writes:

>  task_pt_regs() macro defines pt_regs for the given task,  this macro is  
> currently not defined for powerpc arch. We need this macro for 
> upcoming utrace features. 
>  Below attached patch  defines this macro for powerpc arch. Please let 
> me know your comments on this.

> +#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)

The cast is unnecessary since tsk->thread.regs is already a struct
pt_regs *.  Also note that tsk->thread.regs will be NULL for a kernel
thread.

Paul.

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Benjamin Herrenschmidt]

On Mon, 2008-07-14 at 08:32 +1000, Paul Mackerras wrote:
> Srinivasa D S writes:
> 
> >  task_pt_regs() macro defines pt_regs for the given task,  this macro is  
> > currently not defined for powerpc arch. We need this macro for 
> > upcoming utrace features. 
> >  Below attached patch  defines this macro for powerpc arch. Please let 
> > me know your comments on this.
> 
> > +#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)
> 
> The cast is unnecessary since tsk->thread.regs is already a struct
> pt_regs *.  Also note that tsk->thread.regs will be NULL for a kernel
> thread.

Hrm.. I stuck that one in powerpc master, but not yet in next. Wonder if
I should back it out, sounds like a minor issue.

Ben.

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Srinivasa D S]

On Monday 14 July 2008 04:02:41 am Paul Mackerras wrote:
> >  Below attached patch  defines this macro for powerpc arch. Please let
> > me know your comments on this.
> >
> > +#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)
>
> The cast is unnecessary since tsk->thread.regs is already a struct
> pt_regs *. 

True.

> Also note that tsk->thread.regs will be NULL for a kernel 
> thread.

Yes, users of task_pt_regs will verify for NULL.  Updated patch attached 
below.

Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>


---
 include/asm-powerpc/processor.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6.26-rc9/include/asm-powerpc/processor.h
===================================================================
--- linux-2.6.26-rc9.orig/include/asm-powerpc/processor.h
+++ linux-2.6.26-rc9/include/asm-powerpc/processor.h
@@ -214,6 +214,8 @@ struct thread_struct {
 #define thread_saved_pc(tsk)    \
         ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)
 
+#define task_pt_regs(tsk)	(tsk)->thread.regs
+
 unsigned long get_wchan(struct task_struct *p);
 
 #define KSTK_EIP(tsk)  ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)


 

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Benjamin Herrenschmidt]

On Mon, 2008-07-14 at 14:01 +0530, Srinivasa D S wrote:
> On Monday 14 July 2008 04:02:41 am Paul Mackerras wrote:
> > >  Below attached patch  defines this macro for powerpc arch. Please let
> > > me know your comments on this.
> > >
> > > +#define task_pt_regs(tsk)	((struct pt_regs *)(tsk)->thread.regs)
> >
> > The cast is unnecessary since tsk->thread.regs is already a struct
> > pt_regs *. 
> 
> True.
> 
> > Also note that tsk->thread.regs will be NULL for a kernel 
> > thread.
> 
> Yes, users of task_pt_regs will verify for NULL.  Updated patch attached 
> below.
> 
> Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>

Can you send a cleanup patch against powerpc.git instead ?

Thanks !

Ben

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Timur Tabi]

Srinivasa D S wrote:

> +#define task_pt_regs(tsk)	(tsk)->thread.regs

Shouldn't this be:

	#define task_pt_regs(tsk)	((tsk)->thread.regs)

just to be safe?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Andreas Schwab]

Timur Tabi <timur@freescale.com> writes:

> Srinivasa D S wrote:
>
>> +#define task_pt_regs(tsk)	(tsk)->thread.regs
>
> Shouldn't this be:
>
> 	#define task_pt_regs(tsk)	((tsk)->thread.regs)
>
> just to be safe?

Both -> and . have already highest precedence as postfix operators.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Srinivasa D S]

On Monday 14 July 2008 02:36:57 pm Benjamin Herrenschmidt wrote:

> > Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>
>
> Can you send a cleanup patch against powerpc.git instead ?
>

Resending the patch against powerpc.git tree.


Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>


---
 include/asm-powerpc/processor.h |    2 ++
 1 file changed, 2 insertions(+)

Index: powerpc.git/include/asm-powerpc/processor.h
===================================================================
--- powerpc.git.orig/include/asm-powerpc/processor.h
+++ powerpc.git/include/asm-powerpc/processor.h
@@ -234,6 +234,8 @@ struct thread_struct {
 #define thread_saved_pc(tsk)    \
         ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)
 
+#define task_pt_regs(tsk)	((tsk)->thread.regs)
+
 unsigned long get_wchan(struct task_struct *p);
 
 #define KSTK_EIP(tsk)  ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Benjamin Herrenschmidt]

On Tue, 2008-07-15 at 14:36 +0530, Srinivasa D S wrote:
> On Monday 14 July 2008 02:36:57 pm Benjamin Herrenschmidt wrote:
> 
> > > Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>
> >
> > Can you send a cleanup patch against powerpc.git instead ?
> >
> 
> Resending the patch against powerpc.git tree.

Nah, your initial patch is there already :-) I'm just asking for a
cleanup one that removes the useless cast.

Cheers,
Ben.

> 
> Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>
> 
> 
> ---
>  include/asm-powerpc/processor.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: powerpc.git/include/asm-powerpc/processor.h
> ===================================================================
> --- powerpc.git.orig/include/asm-powerpc/processor.h
> +++ powerpc.git/include/asm-powerpc/processor.h
> @@ -234,6 +234,8 @@ struct thread_struct {
>  #define thread_saved_pc(tsk)    \
>          ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)
>  
> +#define task_pt_regs(tsk)	((tsk)->thread.regs)
> +
>  unsigned long get_wchan(struct task_struct *p);
>  
>  #define KSTK_EIP(tsk)  ((tsk)->thread.regs? (tsk)->thread.regs->nip: 0)

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Srinivasa D S]

On Monday 14 July 2008 11:06:47 pm Andreas Schwab wrote:
> Timur Tabi <timur@freescale.com> writes:
> > Srinivasa D S wrote:
> >> +#define task_pt_regs(tsk)	(tsk)->thread.regs
> >
> > Shouldn't this be:
> >
> > 	#define task_pt_regs(tsk)	((tsk)->thread.regs)
> >
> > just to be safe?
>
> Both -> and . have already highest precedence as postfix operators.
>

Thanks for the comments, For safer side I have used "()"  and sent the updated 
patch.

Thanks
 Srinivasa DS

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Srinivasa DS]

Benjamin Herrenschmidt wrote:
> On Tue, 2008-07-15 at 14:36 +0530, Srinivasa D S wrote:
>> On Monday 14 July 2008 02:36:57 pm Benjamin Herrenschmidt wrote:
>>
>>>> Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>
>>> Can you send a cleanup patch against powerpc.git instead ?
>>>
>> Resending the patch against powerpc.git tree.
> 
> Nah, your initial patch is there already :-) I'm just asking for a
> cleanup one that removes the useless cast.
> 

Sorry, I got it wrong, But I dont find my patch in your latest powerpc 
git tree(git.kernel.org/?p=linux/kernel/git/benh/powerpc.git).

Thanks
  Srinivasa DS

Re: [RFC] [PATCH] task_pt_regs for powerpc systems

[Benjamin Herrenschmidt]

On Tue, 2008-07-15 at 16:00 +0530, Srinivasa DS wrote:
> Sorry, I got it wrong, But I dont find my patch in your latest
> powerpc 
> git tree(git.kernel.org/?p=linux/kernel/git/benh/powerpc.git).

Hrm... I thought I merged it. I'll check that tomorrow.

Cheers,
Ben.