public inbox for linux-arch@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] Kprobes: Define retval helper
       [not found]     ` <20060809094311.GA20050@in.ibm.com>
@ 2006-08-09  9:45       ` Christoph Hellwig
  2006-08-09  9:55         ` Andi Kleen
  2006-08-09 13:16         ` David Howells
  0 siblings, 2 replies; 6+ messages in thread
From: Christoph Hellwig @ 2006-08-09  9:45 UTC (permalink / raw)
  To: Ananth N Mavinakayanahalli
  Cc: Christoph Hellwig, linux-kernel, Prasanna S Panchamukhi,
	Anil S Keshavamurthy, Jim Keniston, linux-arch

On Wed, Aug 09, 2006 at 03:13:11PM +0530, Ananth N Mavinakayanahalli wrote:
> > Good idea.  You should add parentheses around regs, otherwise the C
> > preprocessor might bite users.  Also the shouting name is quite ugly.
> > In fact it should probably go to asm/system.h or similar and not have
> > a kprobes name - it just extracts the return value from a struct pt_regs
> > after all.
> 
> Done! How does this look? I added it to asm/ptrace.h so it lives along
> with the instruction_pointer() definition.

Looks good, but it would be much better if we had it for every single
architecture.  I've cc'ed linux-arch so the arch maintainers can comments.

Even if we don't manage to get every architecture maintainer to help out
you should at least add sparc and s390 to have full coverage of
architectures with kprobes support

> Add the "get_retval" macro that just extracts the return value given the
> pt_regs. Useful in situations such as while using function-return
> probes.
> 
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> ---
>  include/asm-i386/ptrace.h    |    3 +++
>  include/asm-ia64/ptrace.h    |    3 +++
>  include/asm-powerpc/ptrace.h |    2 ++
>  include/asm-x86_64/ptrace.h  |    2 ++
>  4 files changed, 10 insertions(+)
> 
> Index: linux-2.6.18-rc3/include/asm-i386/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-i386/ptrace.h
> +++ linux-2.6.18-rc3/include/asm-i386/ptrace.h
> @@ -79,7 +79,10 @@ static inline int user_mode_vm(struct pt
>  {
>  	return ((regs->xcs & 3) | (regs->eflags & VM_MASK)) != 0;
>  }
> +
>  #define instruction_pointer(regs) ((regs)->eip)
> +#define get_retval(regs) ((regs)->eax)
> +
>  #if defined(CONFIG_SMP) && defined(CONFIG_FRAME_POINTER)
>  extern unsigned long profile_pc(struct pt_regs *regs);
>  #else
> Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-ia64/ptrace.h
> +++ linux-2.6.18-rc3/include/asm-ia64/ptrace.h
> @@ -237,6 +237,9 @@ struct switch_stack {
>   * the canonical representation by adding to instruction pointer.
>   */
>  # define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)
> +
> +#define get_retval(regs) ((regs)->r8)
> +
>  /* Conserve space in histogram by encoding slot bits in address
>   * bits 2 and 3 rather than bits 0 and 1.
>   */
> Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-powerpc/ptrace.h
> +++ linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
> @@ -73,6 +73,8 @@ struct pt_regs {
>  #ifndef __ASSEMBLY__
>  
>  #define instruction_pointer(regs) ((regs)->nip)
> +#define get_retval(regs) ((regs)->gpr[3])
> +
>  #ifdef CONFIG_SMP
>  extern unsigned long profile_pc(struct pt_regs *regs);
>  #else
> Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
> ===================================================================
> --- linux-2.6.18-rc3.orig/include/asm-x86_64/ptrace.h
> +++ linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
> @@ -84,6 +84,8 @@ struct pt_regs {
>  #define user_mode(regs) (!!((regs)->cs & 3))
>  #define user_mode_vm(regs) user_mode(regs)
>  #define instruction_pointer(regs) ((regs)->rip)
> +#define get_retval(regs) ((regs)->rax)
> +
>  extern unsigned long profile_pc(struct pt_regs *regs);
>  void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
>  
---end quoted text---

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

* Re: [PATCH 2/3] Kprobes: Define retval helper
  2006-08-09  9:45       ` [PATCH 2/3] Kprobes: Define retval helper Christoph Hellwig
@ 2006-08-09  9:55         ` Andi Kleen
  2006-08-09 10:19           ` Ananth N Mavinakayanahalli
  2006-08-09 13:16         ` David Howells
  1 sibling, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2006-08-09  9:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ananth N Mavinakayanahalli, linux-kernel, Prasanna S Panchamukhi,
	Anil S Keshavamurthy, Jim Keniston, linux-arch


> >  #define instruction_pointer(regs) ((regs)->eip)
> > +#define get_retval(regs) ((regs)->eax)

return_value() would match the names of the existing macro better

-Andi


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

* Re: [PATCH 2/3] Kprobes: Define retval helper
  2006-08-09  9:55         ` Andi Kleen
@ 2006-08-09 10:19           ` Ananth N Mavinakayanahalli
  2006-08-09 10:28             ` Martin Schwidefsky
  0 siblings, 1 reply; 6+ messages in thread
From: Ananth N Mavinakayanahalli @ 2006-08-09 10:19 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Christoph Hellwig, linux-kernel, Prasanna S Panchamukhi,
	Anil S Keshavamurthy, Jim Keniston, linux-arch

On Wed, Aug 09, 2006 at 11:55:50AM +0200, Andi Kleen wrote:
> 
> > >  #define instruction_pointer(regs) ((regs)->eip)
> > > +#define get_retval(regs) ((regs)->eax)
> 
> return_value() would match the names of the existing macro better
 
Updated patch ...

Add the "return_value" macro that just extracts the return value given
the pt_regs. Useful in situations such as while using function-return
probes.


Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

---
 include/asm-i386/ptrace.h    |    3 +++
 include/asm-ia64/ptrace.h    |    3 +++
 include/asm-powerpc/ptrace.h |    2 ++
 include/asm-x86_64/ptrace.h  |    2 ++
 4 files changed, 10 insertions(+)

Index: linux-2.6.18-rc3/include/asm-i386/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-i386/ptrace.h
+++ linux-2.6.18-rc3/include/asm-i386/ptrace.h
@@ -79,7 +79,10 @@ static inline int user_mode_vm(struct pt
 {
 	return ((regs->xcs & 3) | (regs->eflags & VM_MASK)) != 0;
 }
+
 #define instruction_pointer(regs) ((regs)->eip)
+#define return_value(regs) ((regs)->eax)
+
 #if defined(CONFIG_SMP) && defined(CONFIG_FRAME_POINTER)
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
Index: linux-2.6.18-rc3/include/asm-ia64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-ia64/ptrace.h
+++ linux-2.6.18-rc3/include/asm-ia64/ptrace.h
@@ -237,6 +237,9 @@ struct switch_stack {
  * the canonical representation by adding to instruction pointer.
  */
 # define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)
+
+#define return_value(regs) ((regs)->r8)
+
 /* Conserve space in histogram by encoding slot bits in address
  * bits 2 and 3 rather than bits 0 and 1.
  */
Index: linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-powerpc/ptrace.h
+++ linux-2.6.18-rc3/include/asm-powerpc/ptrace.h
@@ -73,6 +73,8 @@ struct pt_regs {
 #ifndef __ASSEMBLY__
 
 #define instruction_pointer(regs) ((regs)->nip)
+#define return_value(regs) ((regs)->gpr[3])
+
 #ifdef CONFIG_SMP
 extern unsigned long profile_pc(struct pt_regs *regs);
 #else
Index: linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
===================================================================
--- linux-2.6.18-rc3.orig/include/asm-x86_64/ptrace.h
+++ linux-2.6.18-rc3/include/asm-x86_64/ptrace.h
@@ -84,6 +84,8 @@ struct pt_regs {
 #define user_mode(regs) (!!((regs)->cs & 3))
 #define user_mode_vm(regs) user_mode(regs)
 #define instruction_pointer(regs) ((regs)->rip)
+#define return_value(regs) ((regs)->rax)
+
 extern unsigned long profile_pc(struct pt_regs *regs);
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 

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

* Re: [PATCH 2/3] Kprobes: Define retval helper
  2006-08-09 10:19           ` Ananth N Mavinakayanahalli
@ 2006-08-09 10:28             ` Martin Schwidefsky
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Schwidefsky @ 2006-08-09 10:28 UTC (permalink / raw)
  To: ananth
  Cc: Andi Kleen, Christoph Hellwig, linux-kernel,
	Prasanna S Panchamukhi, Anil S Keshavamurthy, Jim Keniston,
	linux-arch

On Wed, 2006-08-09 at 15:49 +0530, Ananth N Mavinakayanahalli wrote:
> Updated patch ...
> 
> Add the "return_value" macro that just extracts the return value given
> the pt_regs. Useful in situations such as while using function-return
> probes.

return_value definition for s390 see below.

-- 
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.

---

diff -urpN linux-2.6.18-rc3/include/asm-s390/ptrace.h linux-2.6.18-s390/include/asm-s390/ptrace.h
--- linux-2.6.18-rc3/include/asm-s390/ptrace.h	2006-06-29 12:47:32.000000000 +0200
+++ linux-2.6.18-s390/include/asm-s390/ptrace.h	2006-08-09 12:25:23.000000000 +0200
@@ -472,6 +472,7 @@ struct user_regs_struct
 
 #define user_mode(regs) (((regs)->psw.mask & PSW_MASK_PSTATE) != 0)
 #define instruction_pointer(regs) ((regs)->psw.addr & PSW_ADDR_INSN)
+#define return_value(regs)((regs)->gprs[2])
 #define profile_pc(regs) instruction_pointer(regs)
 extern void show_regs(struct pt_regs * regs);
 #endif



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

* Re: [PATCH 2/3] Kprobes: Define retval helper
  2006-08-09  9:45       ` [PATCH 2/3] Kprobes: Define retval helper Christoph Hellwig
  2006-08-09  9:55         ` Andi Kleen
@ 2006-08-09 13:16         ` David Howells
  2006-08-09 15:20           ` Ananth N Mavinakayanahalli
  1 sibling, 1 reply; 6+ messages in thread
From: David Howells @ 2006-08-09 13:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ananth N Mavinakayanahalli, linux-kernel, Prasanna S Panchamukhi,
	Anil S Keshavamurthy, Jim Keniston, linux-arch

Christoph Hellwig <hch@infradead.org> wrote:

> > > Good idea.  You should add parentheses around regs, otherwise the C
> > > preprocessor might bite users.  Also the shouting name is quite ugly.
> > > In fact it should probably go to asm/system.h or similar and not have
> > > a kprobes name - it just extracts the return value from a struct pt_regs
> > > after all.
> > 
> > Done! How does this look? I added it to asm/ptrace.h so it lives along
> > with the instruction_pointer() definition.

I presume we don't care about return values that span multiple registers - for
instance if you return a 64-bit value on i386 it'll wind up in EDX:EAX.

David

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

* Re: [PATCH 2/3] Kprobes: Define retval helper
  2006-08-09 13:16         ` David Howells
@ 2006-08-09 15:20           ` Ananth N Mavinakayanahalli
  0 siblings, 0 replies; 6+ messages in thread
From: Ananth N Mavinakayanahalli @ 2006-08-09 15:20 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, linux-kernel, Prasanna S Panchamukhi,
	Anil S Keshavamurthy, Jim Keniston, linux-arch

On Wed, Aug 09, 2006 at 02:16:04PM +0100, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > > > Good idea.  You should add parentheses around regs, otherwise the C
> > > > preprocessor might bite users.  Also the shouting name is quite ugly.
> > > > In fact it should probably go to asm/system.h or similar and not have
> > > > a kprobes name - it just extracts the return value from a struct pt_regs
> > > > after all.
> > > 
> > > Done! How does this look? I added it to asm/ptrace.h so it lives along
> > > with the instruction_pointer() definition.
> 
> I presume we don't care about return values that span multiple registers - for
> instance if you return a 64-bit value on i386 it'll wind up in EDX:EAX.

Yes. This helper is mostly to address the common case, not the 64-bit
one.

Ananth

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

end of thread, other threads:[~2006-08-09 15:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20060807115537.GA15253@in.ibm.com>
     [not found] ` <20060807120024.GD15253@in.ibm.com>
     [not found]   ` <20060808162559.GB28647@infradead.org>
     [not found]     ` <20060809094311.GA20050@in.ibm.com>
2006-08-09  9:45       ` [PATCH 2/3] Kprobes: Define retval helper Christoph Hellwig
2006-08-09  9:55         ` Andi Kleen
2006-08-09 10:19           ` Ananth N Mavinakayanahalli
2006-08-09 10:28             ` Martin Schwidefsky
2006-08-09 13:16         ` David Howells
2006-08-09 15:20           ` Ananth N Mavinakayanahalli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox