[RFC] [PATCH] task_pt_regs for powerpc systems

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 <redacted>


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

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 <redacted>

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

Cheers,
Ben.

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.

On Mon, 2008-07-14 at 08:32 +1000, Paul Mackerras wrote:

Srinivasa D S writes:

quoted

 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.

quoted

+#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.

On Monday 14 July 2008 04:02:41 am Paul Mackerras wrote:

quoted

 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 <redacted>


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


 

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:

quoted

quoted

 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.

quoted

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 <redacted>

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

Thanks !

Ben

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

quoted

Signed-off-by: Srinivasa DS <redacted>

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

Resending the patch against powerpc.git tree.


Signed-off-by: Srinivasa DS <redacted>


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

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:

quoted

quoted

Signed-off-by: Srinivasa DS <redacted>

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.

Benjamin Herrenschmidt wrote:

On Tue, 2008-07-15 at 14:36 +0530, Srinivasa D S wrote:

quoted

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

quoted

quoted

Signed-off-by: Srinivasa DS <redacted>

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

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.

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

Timur Tabi [off-list ref] writes:

Srinivasa D S wrote:

quoted

+#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."

On Monday 14 July 2008 11:06:47 pm Andreas Schwab wrote:

Timur Tabi [off-list ref] writes:

quoted

Srinivasa D S wrote:

quoted

+#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