All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] x86: entry_64 - introduce FTRACE_ frame macro
@ 2008-12-12 18:41 Cyrill Gorcunov
  2008-12-12 20:19 ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 18:41 UTC (permalink / raw)
  To: Ingo Molnar, H. Peter Anvin, Thomas Gleixner
  Cc: LKML, Steven Rostedt, Alexander van Heukelum

Impact: clean up

Itroduce FTRACE_SAVE/RESTORE_FRAME which allow us to
save a number of lines on source level.

Also fix a comment in ftrace.h.

---
Actually I'm not that sure about bringing new macros there
since we already have a lot so feel free to just drop it.

Maybe it would be good to comment args passed
to ftrace_call on ABI level :)

Ah, the patch is on top of -tip e3bdf48

 arch/x86/include/asm/ftrace.h |    2 -
 arch/x86/kernel/entry_64.S    |   70 +++++++++++++++---------------------------
 2 files changed, 26 insertions(+), 46 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
+++ linux-2.6.git/arch/x86/include/asm/ftrace.h
@@ -46,7 +46,7 @@ struct ftrace_ret_stack {
 /*
  * Primary handler of a function return.
  * It relays on ftrace_return_to_handler.
- * Defined in entry32.S
+ * Defined in entry_32/64.S
  */
 extern void return_to_handler(void);
 
Index: linux-2.6.git/arch/x86/kernel/entry_64.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/entry_64.S
+++ linux-2.6.git/arch/x86/kernel/entry_64.S
@@ -66,10 +66,7 @@ ENTRY(mcount)
 	retq
 END(mcount)
 
-ENTRY(ftrace_caller)
-	cmpl $0, function_trace_stop
-	jne  ftrace_stub
-
+	.macro FTRACE_SAVE_FRAME
 	/* taken from glibc */
 	subq $0x38, %rsp
 	movq %rax, (%rsp)
@@ -79,15 +76,9 @@ ENTRY(ftrace_caller)
 	movq %rdi, 32(%rsp)
 	movq %r8, 40(%rsp)
 	movq %r9, 48(%rsp)
+	.endm
 
-	movq 0x38(%rsp), %rdi
-	movq 8(%rbp), %rsi
-	subq $MCOUNT_INSN_SIZE, %rdi
-
-.globl ftrace_call
-ftrace_call:
-	call ftrace_stub
-
+	.macro FTRACE_RESTORE_FRAME
 	movq 48(%rsp), %r9
 	movq 40(%rsp), %r8
 	movq 32(%rsp), %rdi
@@ -96,6 +87,23 @@ ftrace_call:
 	movq 8(%rsp), %rcx
 	movq (%rsp), %rax
 	addq $0x38, %rsp
+	.endm
+
+ENTRY(ftrace_caller)
+	cmpl $0, function_trace_stop
+	jne  ftrace_stub
+
+	FTRACE_SAVE_FRAME
+
+	movq 0x38(%rsp), %rdi
+	movq 8(%rbp), %rsi
+	subq $MCOUNT_INSN_SIZE, %rdi
+
+.globl ftrace_call
+ftrace_call:
+	call ftrace_stub
+
+	FTRACE_RESTORE_FRAME
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
@@ -129,15 +137,7 @@ ftrace_stub:
 	retq
 
 trace:
-	/* taken from glibc */
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
@@ -145,14 +145,7 @@ trace:
 
 	call   *ftrace_trace_function
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
 
 	jmp ftrace_stub
 END(mcount)
@@ -164,14 +157,7 @@ ENTRY(ftrace_graph_caller)
 	cmpl $0, function_trace_stop
 	jne ftrace_stub
 
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	leaq 8(%rbp), %rdi
 	movq 0x38(%rsp), %rsi
@@ -179,14 +165,8 @@ ENTRY(ftrace_graph_caller)
 
 	call	prepare_ftrace_return
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
+
 	retq
 END(ftrace_graph_caller)
 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 18:41 [RFC] x86: entry_64 - introduce FTRACE_ frame macro Cyrill Gorcunov
@ 2008-12-12 20:19 ` Steven Rostedt
  2008-12-12 20:47   ` Cyrill Gorcunov
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2008-12-12 20:19 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, LKML,
	Alexander van Heukelum, Steven Rostedt, fweisbec


On Fri, 2008-12-12 at 21:41 +0300, Cyrill Gorcunov wrote:
> Impact: clean up
> 
> Itroduce FTRACE_SAVE/RESTORE_FRAME which allow us to
> save a number of lines on source level.
> 
> Also fix a comment in ftrace.h.
> 
> ---
> Actually I'm not that sure about bringing new macros there
> since we already have a lot so feel free to just drop it.
> 
> Maybe it would be good to comment args passed
> to ftrace_call on ABI level :)

Actually that is pretty much the same as mcount call. But we may change
it in the future. We probably should document (if it is not already) the
ftrace call graph ABI.

> 
> Ah, the patch is on top of -tip e3bdf48

If you have tested this with and without dynamic ftrace, for both the
function tracer and the function graph tracer, then you have my...

Acked-by: Steven Rostedt <srostedt@redhat.com>

-- Steve

> 
>  arch/x86/include/asm/ftrace.h |    2 -
>  arch/x86/kernel/entry_64.S    |   70 +++++++++++++++---------------------------
>  2 files changed, 26 insertions(+), 46 deletions(-)
> 
> Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
> +++ linux-2.6.git/arch/x86/include/asm/ftrace.h
> @@ -46,7 +46,7 @@ struct ftrace_ret_stack {
>  /*
>   * Primary handler of a function return.
>   * It relays on ftrace_return_to_handler.
> - * Defined in entry32.S
> + * Defined in entry_32/64.S
>   */
>  extern void return_to_handler(void);
>  
> Index: linux-2.6.git/arch/x86/kernel/entry_64.S
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/entry_64.S
> +++ linux-2.6.git/arch/x86/kernel/entry_64.S
> @@ -66,10 +66,7 @@ ENTRY(mcount)
>  	retq
>  END(mcount)
>  
> -ENTRY(ftrace_caller)
> -	cmpl $0, function_trace_stop
> -	jne  ftrace_stub
> -
> +	.macro FTRACE_SAVE_FRAME
>  	/* taken from glibc */
>  	subq $0x38, %rsp
>  	movq %rax, (%rsp)
> @@ -79,15 +76,9 @@ ENTRY(ftrace_caller)
>  	movq %rdi, 32(%rsp)
>  	movq %r8, 40(%rsp)
>  	movq %r9, 48(%rsp)
> +	.endm
>  
> -	movq 0x38(%rsp), %rdi
> -	movq 8(%rbp), %rsi
> -	subq $MCOUNT_INSN_SIZE, %rdi
> -
> -.globl ftrace_call
> -ftrace_call:
> -	call ftrace_stub
> -
> +	.macro FTRACE_RESTORE_FRAME
>  	movq 48(%rsp), %r9
>  	movq 40(%rsp), %r8
>  	movq 32(%rsp), %rdi
> @@ -96,6 +87,23 @@ ftrace_call:
>  	movq 8(%rsp), %rcx
>  	movq (%rsp), %rax
>  	addq $0x38, %rsp
> +	.endm
> +
> +ENTRY(ftrace_caller)
> +	cmpl $0, function_trace_stop
> +	jne  ftrace_stub
> +
> +	FTRACE_SAVE_FRAME
> +
> +	movq 0x38(%rsp), %rdi
> +	movq 8(%rbp), %rsi
> +	subq $MCOUNT_INSN_SIZE, %rdi
> +
> +.globl ftrace_call
> +ftrace_call:
> +	call ftrace_stub
> +
> +	FTRACE_RESTORE_FRAME
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  .globl ftrace_graph_call
> @@ -129,15 +137,7 @@ ftrace_stub:
>  	retq
>  
>  trace:
> -	/* taken from glibc */
> -	subq $0x38, %rsp
> -	movq %rax, (%rsp)
> -	movq %rcx, 8(%rsp)
> -	movq %rdx, 16(%rsp)
> -	movq %rsi, 24(%rsp)
> -	movq %rdi, 32(%rsp)
> -	movq %r8, 40(%rsp)
> -	movq %r9, 48(%rsp)
> +	FTRACE_SAVE_FRAME
>  
>  	movq 0x38(%rsp), %rdi
>  	movq 8(%rbp), %rsi
> @@ -145,14 +145,7 @@ trace:
>  
>  	call   *ftrace_trace_function
>  
> -	movq 48(%rsp), %r9
> -	movq 40(%rsp), %r8
> -	movq 32(%rsp), %rdi
> -	movq 24(%rsp), %rsi
> -	movq 16(%rsp), %rdx
> -	movq 8(%rsp), %rcx
> -	movq (%rsp), %rax
> -	addq $0x38, %rsp
> +	FTRACE_RESTORE_FRAME
>  
>  	jmp ftrace_stub
>  END(mcount)
> @@ -164,14 +157,7 @@ ENTRY(ftrace_graph_caller)
>  	cmpl $0, function_trace_stop
>  	jne ftrace_stub
>  
> -	subq $0x38, %rsp
> -	movq %rax, (%rsp)
> -	movq %rcx, 8(%rsp)
> -	movq %rdx, 16(%rsp)
> -	movq %rsi, 24(%rsp)
> -	movq %rdi, 32(%rsp)
> -	movq %r8, 40(%rsp)
> -	movq %r9, 48(%rsp)
> +	FTRACE_SAVE_FRAME
>  
>  	leaq 8(%rbp), %rdi
>  	movq 0x38(%rsp), %rsi
> @@ -179,14 +165,8 @@ ENTRY(ftrace_graph_caller)
>  
>  	call	prepare_ftrace_return
>  
> -	movq 48(%rsp), %r9
> -	movq 40(%rsp), %r8
> -	movq 32(%rsp), %rdi
> -	movq 24(%rsp), %rsi
> -	movq 16(%rsp), %rdx
> -	movq 8(%rsp), %rcx
> -	movq (%rsp), %rax
> -	addq $0x38, %rsp
> +	FTRACE_RESTORE_FRAME
> +
>  	retq
>  END(ftrace_graph_caller)
>  


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 20:19 ` Steven Rostedt
@ 2008-12-12 20:47   ` Cyrill Gorcunov
  2008-12-12 20:52     ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 20:47 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, LKML,
	Alexander van Heukelum, Steven Rostedt, fweisbec

[Steven Rostedt - Fri, Dec 12, 2008 at 03:19:27PM -0500]
| 
... 
| > 
| > Ah, the patch is on top of -tip e3bdf48
| 
| If you have tested this with and without dynamic ftrace, for both the
| function tracer and the function graph tracer, then you have my...
| 
| Acked-by: Steven Rostedt <srostedt@redhat.com>
| 
| -- Steve
...

Thanks Steven! Actually just catched a bug :) I placed
these macros under CONFIG_DYNAMIC_FTRACE but have them
used even if it was undefined -- will fix shortly.

Well... I think the macros should be placed in differen
location (maybe in header) to not overload the source.

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 20:47   ` Cyrill Gorcunov
@ 2008-12-12 20:52     ` Steven Rostedt
  2008-12-12 20:57       ` Cyrill Gorcunov
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2008-12-12 20:52 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec


On Fri, 12 Dec 2008, Cyrill Gorcunov wrote:

> [Steven Rostedt - Fri, Dec 12, 2008 at 03:19:27PM -0500]
> | 
> ... 
> | > 
> | > Ah, the patch is on top of -tip e3bdf48
> | 
> | If you have tested this with and without dynamic ftrace, for both the
> | function tracer and the function graph tracer, then you have my...
> | 
> | Acked-by: Steven Rostedt <srostedt@redhat.com>
> | 
> | -- Steve
> ...
> 
> Thanks Steven! Actually just catched a bug :) I placed
> these macros under CONFIG_DYNAMIC_FTRACE but have them
> used even if it was undefined -- will fix shortly.
> 
> Well... I think the macros should be placed in differen
> location (maybe in header) to not overload the source.

You can put them in arch/x86/include/asm/ftrace.h That's assembly safe.

-- Steve


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 20:52     ` Steven Rostedt
@ 2008-12-12 20:57       ` Cyrill Gorcunov
  2008-12-12 20:59         ` Cyrill Gorcunov
  2008-12-12 21:22         ` Steven Rostedt
  0 siblings, 2 replies; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 20:57 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec

[Steven Rostedt - Fri, Dec 12, 2008 at 03:52:56PM -0500]
| 
... 
| You can put them in arch/x86/include/asm/ftrace.h That's assembly safe.
| 
| -- Steve
| 

Yep, while you were typing the message I was testing the change :)
Do you like it?

		- Cyrill -

---
 arch/x86/include/asm/ftrace.h |   29 ++++++++++++++++++++-
 arch/x86/kernel/entry_64.S    |   57 +++++-------------------------------------
 2 files changed, 35 insertions(+), 51 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
+++ linux-2.6.git/arch/x86/include/asm/ftrace.h
@@ -1,6 +1,33 @@
 #ifndef _ASM_X86_FTRACE_H
 #define _ASM_X86_FTRACE_H
 
+#ifdef __ASSEMBLY__
+
+	.macro FTRACE_SAVE_FRAME
+	/* taken from glibc */
+	subq $0x38, %rsp
+	movq %rax, (%rsp)
+	movq %rcx, 8(%rsp)
+	movq %rdx, 16(%rsp)
+	movq %rsi, 24(%rsp)
+	movq %rdi, 32(%rsp)
+	movq %r8, 40(%rsp)
+	movq %r9, 48(%rsp)
+	.endm
+
+	.macro FTRACE_RESTORE_FRAME
+	movq 48(%rsp), %r9
+	movq 40(%rsp), %r8
+	movq 32(%rsp), %rdi
+	movq 24(%rsp), %rsi
+	movq 16(%rsp), %rdx
+	movq 8(%rsp), %rcx
+	movq (%rsp), %rax
+	addq $0x38, %rsp
+	.endm
+
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((long)(mcount))
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
@@ -46,7 +73,7 @@ struct ftrace_ret_stack {
 /*
  * Primary handler of a function return.
  * It relays on ftrace_return_to_handler.
- * Defined in entry32.S
+ * Defined in entry_32/64.S
  */
 extern void return_to_handler(void);
 
Index: linux-2.6.git/arch/x86/kernel/entry_64.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/entry_64.S
+++ linux-2.6.git/arch/x86/kernel/entry_64.S
@@ -70,15 +70,7 @@ ENTRY(ftrace_caller)
 	cmpl $0, function_trace_stop
 	jne  ftrace_stub
 
-	/* taken from glibc */
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
@@ -88,14 +80,7 @@ ENTRY(ftrace_caller)
 ftrace_call:
 	call ftrace_stub
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
@@ -129,15 +114,7 @@ ftrace_stub:
 	retq
 
 trace:
-	/* taken from glibc */
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
@@ -145,14 +122,7 @@ trace:
 
 	call   *ftrace_trace_function
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
 
 	jmp ftrace_stub
 END(mcount)
@@ -164,14 +134,7 @@ ENTRY(ftrace_graph_caller)
 	cmpl $0, function_trace_stop
 	jne ftrace_stub
 
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	leaq 8(%rbp), %rdi
 	movq 0x38(%rsp), %rsi
@@ -179,14 +142,8 @@ ENTRY(ftrace_graph_caller)
 
 	call	prepare_ftrace_return
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
+
 	retq
 END(ftrace_graph_caller)
 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 20:57       ` Cyrill Gorcunov
@ 2008-12-12 20:59         ` Cyrill Gorcunov
  2008-12-12 21:22         ` Steven Rostedt
  1 sibling, 0 replies; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 20:59 UTC (permalink / raw)
  To: Steven Rostedt, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum, fweisbec

[Cyrill Gorcunov - Fri, Dec 12, 2008 at 11:57:29PM +0300]
| [Steven Rostedt - Fri, Dec 12, 2008 at 03:52:56PM -0500]
| | 
| ... 
| | You can put them in arch/x86/include/asm/ftrace.h That's assembly safe.
| | 
| | -- Steve
| | 
| 
| Yep, while you were typing the message I was testing the change :)
| Do you like it?
| 
| 		- Cyrill -

By "testing" I mean "to compare objdump output".

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 20:57       ` Cyrill Gorcunov
  2008-12-12 20:59         ` Cyrill Gorcunov
@ 2008-12-12 21:22         ` Steven Rostedt
  2008-12-12 21:27           ` Cyrill Gorcunov
  2008-12-12 21:35           ` Cyrill Gorcunov
  1 sibling, 2 replies; 17+ messages in thread
From: Steven Rostedt @ 2008-12-12 21:22 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec


On Fri, 12 Dec 2008, Cyrill Gorcunov wrote:

> [Steven Rostedt - Fri, Dec 12, 2008 at 03:52:56PM -0500]
> | 
> ... 
> | You can put them in arch/x86/include/asm/ftrace.h That's assembly safe.
> | 
> | -- Steve
> | 
> 
> Yep, while you were typing the message I was testing the change :)
> Do you like it?
> 
> 		- Cyrill -
> 
> ---
>  arch/x86/include/asm/ftrace.h |   29 ++++++++++++++++++++-
>  arch/x86/kernel/entry_64.S    |   57 +++++-------------------------------------
>  2 files changed, 35 insertions(+), 51 deletions(-)
> 
> Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
> +++ linux-2.6.git/arch/x86/include/asm/ftrace.h
> @@ -1,6 +1,33 @@
>  #ifndef _ASM_X86_FTRACE_H
>  #define _ASM_X86_FTRACE_H
>  
> +#ifdef __ASSEMBLY__

Why add assembly condition? We only want this if CONFIG_FUNCTION_TRACER 
is enabled, right?  We could combine it with the current #ifndef inside 
the CONFIG_FUNCTION_TRACER. But I would make do:

#ifdef __ASSEMBLY__
<your stuff>
#else
< C stuff >
#endif

-- Steve


> +
> +	.macro FTRACE_SAVE_FRAME
> +	/* taken from glibc */
> +	subq $0x38, %rsp
> +	movq %rax, (%rsp)
> +	movq %rcx, 8(%rsp)
> +	movq %rdx, 16(%rsp)
> +	movq %rsi, 24(%rsp)
> +	movq %rdi, 32(%rsp)
> +	movq %r8, 40(%rsp)
> +	movq %r9, 48(%rsp)
> +	.endm
> +
> +	.macro FTRACE_RESTORE_FRAME
> +	movq 48(%rsp), %r9
> +	movq 40(%rsp), %r8
> +	movq 32(%rsp), %rdi
> +	movq 24(%rsp), %rsi
> +	movq 16(%rsp), %rdx
> +	movq 8(%rsp), %rcx
> +	movq (%rsp), %rax
> +	addq $0x38, %rsp
> +	.endm
> +
> +#endif
> +
>  #ifdef CONFIG_FUNCTION_TRACER
>  #define MCOUNT_ADDR		((long)(mcount))
>  #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
> @@ -46,7 +73,7 @@ struct ftrace_ret_stack {
>  /*
>   * Primary handler of a function return.
>   * It relays on ftrace_return_to_handler.
> - * Defined in entry32.S
> + * Defined in entry_32/64.S
>   */
>  extern void return_to_handler(void);
>  
> Index: linux-2.6.git/arch/x86/kernel/entry_64.S
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/entry_64.S
> +++ linux-2.6.git/arch/x86/kernel/entry_64.S
> @@ -70,15 +70,7 @@ ENTRY(ftrace_caller)
>  	cmpl $0, function_trace_stop
>  	jne  ftrace_stub
>  
> -	/* taken from glibc */
> -	subq $0x38, %rsp
> -	movq %rax, (%rsp)
> -	movq %rcx, 8(%rsp)
> -	movq %rdx, 16(%rsp)
> -	movq %rsi, 24(%rsp)
> -	movq %rdi, 32(%rsp)
> -	movq %r8, 40(%rsp)
> -	movq %r9, 48(%rsp)
> +	FTRACE_SAVE_FRAME
>  
>  	movq 0x38(%rsp), %rdi
>  	movq 8(%rbp), %rsi
> @@ -88,14 +80,7 @@ ENTRY(ftrace_caller)
>  ftrace_call:
>  	call ftrace_stub
>  
> -	movq 48(%rsp), %r9
> -	movq 40(%rsp), %r8
> -	movq 32(%rsp), %rdi
> -	movq 24(%rsp), %rsi
> -	movq 16(%rsp), %rdx
> -	movq 8(%rsp), %rcx
> -	movq (%rsp), %rax
> -	addq $0x38, %rsp
> +	FTRACE_RESTORE_FRAME
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  .globl ftrace_graph_call
> @@ -129,15 +114,7 @@ ftrace_stub:
>  	retq
>  
>  trace:
> -	/* taken from glibc */
> -	subq $0x38, %rsp
> -	movq %rax, (%rsp)
> -	movq %rcx, 8(%rsp)
> -	movq %rdx, 16(%rsp)
> -	movq %rsi, 24(%rsp)
> -	movq %rdi, 32(%rsp)
> -	movq %r8, 40(%rsp)
> -	movq %r9, 48(%rsp)
> +	FTRACE_SAVE_FRAME
>  
>  	movq 0x38(%rsp), %rdi
>  	movq 8(%rbp), %rsi
> @@ -145,14 +122,7 @@ trace:
>  
>  	call   *ftrace_trace_function
>  
> -	movq 48(%rsp), %r9
> -	movq 40(%rsp), %r8
> -	movq 32(%rsp), %rdi
> -	movq 24(%rsp), %rsi
> -	movq 16(%rsp), %rdx
> -	movq 8(%rsp), %rcx
> -	movq (%rsp), %rax
> -	addq $0x38, %rsp
> +	FTRACE_RESTORE_FRAME
>  
>  	jmp ftrace_stub
>  END(mcount)
> @@ -164,14 +134,7 @@ ENTRY(ftrace_graph_caller)
>  	cmpl $0, function_trace_stop
>  	jne ftrace_stub
>  
> -	subq $0x38, %rsp
> -	movq %rax, (%rsp)
> -	movq %rcx, 8(%rsp)
> -	movq %rdx, 16(%rsp)
> -	movq %rsi, 24(%rsp)
> -	movq %rdi, 32(%rsp)
> -	movq %r8, 40(%rsp)
> -	movq %r9, 48(%rsp)
> +	FTRACE_SAVE_FRAME
>  
>  	leaq 8(%rbp), %rdi
>  	movq 0x38(%rsp), %rsi
> @@ -179,14 +142,8 @@ ENTRY(ftrace_graph_caller)
>  
>  	call	prepare_ftrace_return
>  
> -	movq 48(%rsp), %r9
> -	movq 40(%rsp), %r8
> -	movq 32(%rsp), %rdi
> -	movq 24(%rsp), %rsi
> -	movq 16(%rsp), %rdx
> -	movq 8(%rsp), %rcx
> -	movq (%rsp), %rax
> -	addq $0x38, %rsp
> +	FTRACE_RESTORE_FRAME
> +
>  	retq
>  END(ftrace_graph_caller)
>  
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 21:22         ` Steven Rostedt
@ 2008-12-12 21:27           ` Cyrill Gorcunov
  2008-12-12 21:35           ` Cyrill Gorcunov
  1 sibling, 0 replies; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 21:27 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec

[Steven Rostedt - Fri, Dec 12, 2008 at 04:22:49PM -0500]
| 
| On Fri, 12 Dec 2008, Cyrill Gorcunov wrote:
| 
| > [Steven Rostedt - Fri, Dec 12, 2008 at 03:52:56PM -0500]
| > | 
| > ... 
| > | You can put them in arch/x86/include/asm/ftrace.h That's assembly safe.
| > | 
| > | -- Steve
| > | 
| > 
| > Yep, while you were typing the message I was testing the change :)
| > Do you like it?
| > 
| > 		- Cyrill -
| > 
| > ---
| >  arch/x86/include/asm/ftrace.h |   29 ++++++++++++++++++++-
| >  arch/x86/kernel/entry_64.S    |   57 +++++-------------------------------------
| >  2 files changed, 35 insertions(+), 51 deletions(-)
| > 
| > Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
| > ===================================================================
| > --- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
| > +++ linux-2.6.git/arch/x86/include/asm/ftrace.h
| > @@ -1,6 +1,33 @@
| >  #ifndef _ASM_X86_FTRACE_H
| >  #define _ASM_X86_FTRACE_H
| >  
| > +#ifdef __ASSEMBLY__
| 
| Why add assembly condition? We only want this if CONFIG_FUNCTION_TRACER 
| is enabled, right?  We could combine it with the current #ifndef inside 
| the CONFIG_FUNCTION_TRACER. But I would make do:
| 
| #ifdef __ASSEMBLY__
| <your stuff>
| #else
| < C stuff >
| #endif
| 
| -- Steve
...

OK, will back shortly.

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 21:22         ` Steven Rostedt
  2008-12-12 21:27           ` Cyrill Gorcunov
@ 2008-12-12 21:35           ` Cyrill Gorcunov
  2008-12-12 21:41             ` Cyrill Gorcunov
  2008-12-12 21:56             ` Steven Rostedt
  1 sibling, 2 replies; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 21:35 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec

[Steven Rostedt - Fri, Dec 12, 2008 at 04:22:49PM -0500]
...
| > Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
| > ===================================================================
| > --- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
| > +++ linux-2.6.git/arch/x86/include/asm/ftrace.h
| > @@ -1,6 +1,33 @@
| >  #ifndef _ASM_X86_FTRACE_H
| >  #define _ASM_X86_FTRACE_H
| >  
| > +#ifdef __ASSEMBLY__
| 
| Why add assembly condition? We only want this if CONFIG_FUNCTION_TRACER 
| is enabled, right?  We could combine it with the current #ifndef inside 
| the CONFIG_FUNCTION_TRACER. But I would make do:
| 
| #ifdef __ASSEMBLY__
| <your stuff>
| #else
| < C stuff >
| #endif
| 
| -- Steve

Steve, here is how it could look like:
(I liked first proposal more :)

---
 arch/x86/include/asm/ftrace.h |   36 ++++++++++++++++++++++----
 arch/x86/kernel/entry_64.S    |   57 +++++-------------------------------------
 2 files changed, 37 insertions(+), 56 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
+++ linux-2.6.git/arch/x86/include/asm/ftrace.h
@@ -1,11 +1,37 @@
 #ifndef _ASM_X86_FTRACE_H
 #define _ASM_X86_FTRACE_H
 
+#ifdef __ASSEMBLY__
+
+	.macro FTRACE_SAVE_FRAME
+	/* taken from glibc */
+	subq $0x38, %rsp
+	movq %rax, (%rsp)
+	movq %rcx, 8(%rsp)
+	movq %rdx, 16(%rsp)
+	movq %rsi, 24(%rsp)
+	movq %rdi, 32(%rsp)
+	movq %r8, 40(%rsp)
+	movq %r9, 48(%rsp)
+	.endm
+
+	.macro FTRACE_RESTORE_FRAME
+	movq 48(%rsp), %r9
+	movq 40(%rsp), %r8
+	movq 32(%rsp), %rdi
+	movq 24(%rsp), %rsi
+	movq 16(%rsp), %rdx
+	movq 8(%rsp), %rcx
+	movq (%rsp), %rax
+	addq $0x38, %rsp
+	.endm
+
+#else /* !__ASSEMBLY__ */
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((long)(mcount))
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
-#ifndef __ASSEMBLY__
 extern void mcount(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
@@ -25,13 +51,10 @@ struct dyn_arch_ftrace {
 };
 
 #endif /*  CONFIG_DYNAMIC_FTRACE */
-#endif /* __ASSEMBLY__ */
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 
-#ifndef __ASSEMBLY__
-
 /*
  * Stack of return addresses for functions
  * of a thread.
@@ -46,11 +69,12 @@ struct ftrace_ret_stack {
 /*
  * Primary handler of a function return.
  * It relays on ftrace_return_to_handler.
- * Defined in entry32.S
+ * Defined in entry_32/64.S
  */
 extern void return_to_handler(void);
 
-#endif /* __ASSEMBLY__ */
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
+#endif /* !__ASSEMBLY__ */
+
 #endif /* _ASM_X86_FTRACE_H */
Index: linux-2.6.git/arch/x86/kernel/entry_64.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/entry_64.S
+++ linux-2.6.git/arch/x86/kernel/entry_64.S
@@ -70,15 +70,7 @@ ENTRY(ftrace_caller)
 	cmpl $0, function_trace_stop
 	jne  ftrace_stub
 
-	/* taken from glibc */
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
@@ -88,14 +80,7 @@ ENTRY(ftrace_caller)
 ftrace_call:
 	call ftrace_stub
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
@@ -129,15 +114,7 @@ ftrace_stub:
 	retq
 
 trace:
-	/* taken from glibc */
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
@@ -145,14 +122,7 @@ trace:
 
 	call   *ftrace_trace_function
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
 
 	jmp ftrace_stub
 END(mcount)
@@ -164,14 +134,7 @@ ENTRY(ftrace_graph_caller)
 	cmpl $0, function_trace_stop
 	jne ftrace_stub
 
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
+	FTRACE_SAVE_FRAME
 
 	leaq 8(%rbp), %rdi
 	movq 0x38(%rsp), %rsi
@@ -179,14 +142,8 @@ ENTRY(ftrace_graph_caller)
 
 	call	prepare_ftrace_return
 
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
+	FTRACE_RESTORE_FRAME
+
 	retq
 END(ftrace_graph_caller)
 
		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 21:35           ` Cyrill Gorcunov
@ 2008-12-12 21:41             ` Cyrill Gorcunov
  2008-12-12 21:56             ` Steven Rostedt
  1 sibling, 0 replies; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-12 21:41 UTC (permalink / raw)
  To: Steven Rostedt, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum, fweisbec

[Cyrill Gorcunov - Sat, Dec 13, 2008 at 12:35:38AM +0300]
| 
...
| | Why add assembly condition? We only want this if CONFIG_FUNCTION_TRACER 
| | is enabled, right?  We could combine it with the current #ifndef inside 
| | the CONFIG_FUNCTION_TRACER. But I would make do:
| | 
| | #ifdef __ASSEMBLY__
| | <your stuff>
| | #else
| | < C stuff >
| | #endif
| | 
| | -- Steve
| 
| Steve, here is how it could look like:
| (I liked first proposal more :)

If you meant to put them under CONFIG_FUNCTION_TRACER only it would
lead to more hard to parse by human representation I believe :)
That is why I put __ASSEMBLY__ in that way (I mean only two sections
__ASSEMBLY__ and #else and all original stuff here) Btw there was an
error in ftrace.h. Iirc gas know nothing about (long) so MCOUNT_ADDR
should be under #ifndef actually. But that is not that serious since
we dont use it now in *.S (don't we?).

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 21:35           ` Cyrill Gorcunov
  2008-12-12 21:41             ` Cyrill Gorcunov
@ 2008-12-12 21:56             ` Steven Rostedt
  2008-12-13  8:20               ` Cyrill Gorcunov
  1 sibling, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2008-12-12 21:56 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec


On Sat, 13 Dec 2008, Cyrill Gorcunov wrote:

> [Steven Rostedt - Fri, Dec 12, 2008 at 04:22:49PM -0500]
> ...
> | > Index: linux-2.6.git/arch/x86/include/asm/ftrace.h
> | > ===================================================================
> | > --- linux-2.6.git.orig/arch/x86/include/asm/ftrace.h
> | > +++ linux-2.6.git/arch/x86/include/asm/ftrace.h
> | > @@ -1,6 +1,33 @@
> | >  #ifndef _ASM_X86_FTRACE_H
> | >  #define _ASM_X86_FTRACE_H
> | >  
> | > +#ifdef __ASSEMBLY__
> | 
> | Why add assembly condition? We only want this if CONFIG_FUNCTION_TRACER 
> | is enabled, right?  We could combine it with the current #ifndef inside 
> | the CONFIG_FUNCTION_TRACER. But I would make do:
> | 
> | #ifdef __ASSEMBLY__
> | <your stuff>
> | #else
> | < C stuff >
> | #endif
> | 
> | -- Steve
> 
> Steve, here is how it could look like:
> (I liked first proposal more :)

I see the confusion. In ftrace.h the function graph tracer config is not 
inside CONFIG_FUNCTION_TRACER, even though it is dependent on it (or 
was). OK, just do your first proposal then.

-- Steve


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-12 21:56             ` Steven Rostedt
@ 2008-12-13  8:20               ` Cyrill Gorcunov
  2008-12-13 14:46                 ` Frédéric Weisbecker
  0 siblings, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-13  8:20 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Steven Rostedt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	LKML, Alexander van Heukelum, fweisbec

[Steven Rostedt - Fri, Dec 12, 2008 at 04:56:47PM -0500]
...
| > Steve, here is how it could look like:
| > (I liked first proposal more :)
| 
| I see the confusion. In ftrace.h the function graph tracer config is not 
| inside CONFIG_FUNCTION_TRACER, even though it is dependent on it (or 
| was). OK, just do your first proposal then.
| 
| -- Steve
| 

ok, I sent it yesterday as

	http://lkml.org/lkml/2008/12/12/301
	[PATCH] x86: entry_64 - introduce FTRACE_ frame macro v2

Ingo, will you pick it up?

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-13  8:20               ` Cyrill Gorcunov
@ 2008-12-13 14:46                 ` Frédéric Weisbecker
  2008-12-13 14:59                   ` Steven Rostedt
  2008-12-13 15:16                   ` Cyrill Gorcunov
  0 siblings, 2 replies; 17+ messages in thread
From: Frédéric Weisbecker @ 2008-12-13 14:46 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Steven Rostedt, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum

2008/12/13 Cyrill Gorcunov <gorcunov@gmail.com>:
> [Steven Rostedt - Fri, Dec 12, 2008 at 04:56:47PM -0500]
> ...
> | > Steve, here is how it could look like:
> | > (I liked first proposal more :)
> |
> | I see the confusion. In ftrace.h the function graph tracer config is not
> | inside CONFIG_FUNCTION_TRACER, even though it is dependent on it (or
> | was). OK, just do your first proposal then.
> |
> | -- Steve
> |
>
> ok, I sent it yesterday as
>
>        http://lkml.org/lkml/2008/12/12/301
>        [PATCH] x86: entry_64 - introduce FTRACE_ frame macro v2
>
> Ingo, will you pick it up?
>
>                - Cyrill -
>

Hi,

Looks good and clarify the asm bits.

I wonder if the function_graph_tracer should continue to necessarily
depend on the function tracer.
It was more simplier to do so but perhaps that could be avoided with
small modifications.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-13 14:46                 ` Frédéric Weisbecker
@ 2008-12-13 14:59                   ` Steven Rostedt
  2008-12-13 15:28                     ` Frédéric Weisbecker
  2008-12-13 15:16                   ` Cyrill Gorcunov
  1 sibling, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2008-12-13 14:59 UTC (permalink / raw)
  To: Frédéric Weisbecker
  Cc: Cyrill Gorcunov, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum


On Sat, 13 Dec 2008, Fr?d?ric Weisbecker wrote:
> 2008/12/13 Cyrill Gorcunov <gorcunov@gmail.com>:
> Hi,
> 
> Looks good and clarify the asm bits.
> 
> I wonder if the function_graph_tracer should continue to necessarily
> depend on the function tracer.
> It was more simplier to do so but perhaps that could be avoided with
> small modifications.

It could be, but remember, the dynamic ftrace gives the ability to specify 
which function can be chosen.

But on the other hand, I guess it would be fine to have it separate. There 
is no reason that it can not be. I would still keep the dynamic part 
dependent on function tracer. At least for now.

-- Steve


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-13 14:46                 ` Frédéric Weisbecker
  2008-12-13 14:59                   ` Steven Rostedt
@ 2008-12-13 15:16                   ` Cyrill Gorcunov
  2008-12-13 15:26                     ` Frédéric Weisbecker
  1 sibling, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov @ 2008-12-13 15:16 UTC (permalink / raw)
  To: Frédéric Weisbecker
  Cc: Steven Rostedt, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum

[Frédéric Weisbecker - Sat, Dec 13, 2008 at 03:46:03PM +0100]
| 2008/12/13 Cyrill Gorcunov <gorcunov@gmail.com>:
| > [Steven Rostedt - Fri, Dec 12, 2008 at 04:56:47PM -0500]
| > ...
| > | > Steve, here is how it could look like:
| > | > (I liked first proposal more :)
| > |
| > | I see the confusion. In ftrace.h the function graph tracer config is not
| > | inside CONFIG_FUNCTION_TRACER, even though it is dependent on it (or
| > | was). OK, just do your first proposal then.
| > |
| > | -- Steve
| > |
| >
| > ok, I sent it yesterday as
| >
| >        http://lkml.org/lkml/2008/12/12/301
| >        [PATCH] x86: entry_64 - introduce FTRACE_ frame macro v2
| >
| > Ingo, will you pick it up?
| >
| >                - Cyrill -
| >
| 
| Hi,
| 
| Looks good and clarify the asm bits.
| 
| I wonder if the function_graph_tracer should continue to necessarily
| depend on the function tracer.
| It was more simplier to do so but perhaps that could be avoided with
| small modifications.
| 

Hi Frédéric,

I didn't play with tracer internals since was more concerned about
asm part of code. If you find that we better should introduce
additional deps here or change a "deps graph" itself - I think it
will be easy to place a new patch on top of this (or replace my
patch) :)

The only thing I would change in mine patch is -- subq/addq $0x38, %rsp
since the following code uses decimal system but not a big deal anyway.

		- Cyrill -

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-13 15:16                   ` Cyrill Gorcunov
@ 2008-12-13 15:26                     ` Frédéric Weisbecker
  0 siblings, 0 replies; 17+ messages in thread
From: Frédéric Weisbecker @ 2008-12-13 15:26 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Steven Rostedt, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum

2008/12/13 Cyrill Gorcunov <gorcunov@gmail.com>:
> Hi Frédéric,
>
> I didn't play with tracer internals since was more concerned about
> asm part of code. If you find that we better should introduce
> additional deps here or change a "deps graph" itself - I think it
> will be easy to place a new patch on top of this (or replace my
> patch) :)
>
> The only thing I would change in mine patch is -- subq/addq $0x38, %rsp
> since the following code uses decimal system but not a big deal anyway.
>
>                - Cyrill -


Yes, don't worry, these comments weren't relied on your patch :-)
If some work has to be done on deps, it will be on top of your patch
if Ingo applies it.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC] x86: entry_64 - introduce FTRACE_ frame macro
  2008-12-13 14:59                   ` Steven Rostedt
@ 2008-12-13 15:28                     ` Frédéric Weisbecker
  0 siblings, 0 replies; 17+ messages in thread
From: Frédéric Weisbecker @ 2008-12-13 15:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Cyrill Gorcunov, Steven Rostedt, Ingo Molnar, H. Peter Anvin,
	Thomas Gleixner, LKML, Alexander van Heukelum

2008/12/13 Steven Rostedt <rostedt@goodmis.org>:
> It could be, but remember, the dynamic ftrace gives the ability to specify
> which function can be chosen.
>
> But on the other hand, I guess it would be fine to have it separate. There
> is no reason that it can not be. I would still keep the dynamic part
> dependent on function tracer. At least for now.
>
> -- Steve

I agree. I don't think it will require much modifications.
I think I will change it for 2.6.30....

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-12-13 15:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-12 18:41 [RFC] x86: entry_64 - introduce FTRACE_ frame macro Cyrill Gorcunov
2008-12-12 20:19 ` Steven Rostedt
2008-12-12 20:47   ` Cyrill Gorcunov
2008-12-12 20:52     ` Steven Rostedt
2008-12-12 20:57       ` Cyrill Gorcunov
2008-12-12 20:59         ` Cyrill Gorcunov
2008-12-12 21:22         ` Steven Rostedt
2008-12-12 21:27           ` Cyrill Gorcunov
2008-12-12 21:35           ` Cyrill Gorcunov
2008-12-12 21:41             ` Cyrill Gorcunov
2008-12-12 21:56             ` Steven Rostedt
2008-12-13  8:20               ` Cyrill Gorcunov
2008-12-13 14:46                 ` Frédéric Weisbecker
2008-12-13 14:59                   ` Steven Rostedt
2008-12-13 15:28                     ` Frédéric Weisbecker
2008-12-13 15:16                   ` Cyrill Gorcunov
2008-12-13 15:26                     ` Frédéric Weisbecker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.