[RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

Thanks to Andi Kleen gcc 4.6.0 now supports -mfentry with the -pg option
to place a call to __fentry__ at the very beginning of the function
instead of after the fact.

The old way:

00000000000000c4 <atomic_long_add>:
      c4:       55                      push   %rbp
      c5:       48 89 e5                mov    %rsp,%rbp
      c8:       e8 00 00 00 00          callq  cd <atomic_long_add+0x9>
                        c9: R_X86_64_PC32       mcount-0x4
      cd:       f0 48 01 3e             lock add %rdi,(%rsi)
      d1:       c9                      leaveq 
      d2:       c3                      retq   

The new way:

000000000000009e <atomic_long_add>:
      9e:       e8 00 00 00 00          callq  a3 <atomic_long_add+0x5>
                        9f: R_X86_64_PC32       __fentry__-0x4
      a3:       55                      push   %rbp
      a4:       48 89 e5                mov    %rsp,%rbp
      a7:       f0 48 01 3e             lock add %rdi,(%rsi)
      ab:       5d                      pop    %rbp
      ac:       c3                      retq   

Note, with -mfentry, frame pointers is no longer required
by the function tracer. But this patch series still requires
FRAME_POINTER to be set, since I need to figure out a good way to
enable FRAME_POINTER only if gcc doesn't support this. But that can
come later.

With the new __fentry__, we could possible record the parameters
of a function call. This may take some work, and perhaps be
a little like kprobes. But it is doable.

This is still just RFC. I only wrote the code to support x86_64
even though gcc 4.6.0 also supports i386. I figured I would post
this first to get peoples reactions before converting
i386 too. Other archs can soon follow.

-- Steve

The following patches are in:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: rfc/tracing/fentry


Steven Rostedt (4):
      ftrace: Make recordmcount.c handle __fentry__
      ftrace: Add -mfentry to Makefile on function tracer
      ftrace: Do not test frame pointers if -mfentry is used
      ftrace/x86: Add support for -mfentry to x86_64

----
 Makefile                             |    6 +++++-
 arch/x86/Kconfig                     |    1 +
 arch/x86/include/asm/ftrace.h        |    7 ++++++-
 arch/x86/kernel/entry_64.S           |   17 ++++++++++++++++-
 arch/x86/kernel/x8664_ksyms_64.c     |    6 +++++-
 kernel/trace/Kconfig                 |    5 +++++
 kernel/trace/trace_functions_graph.c |    5 ++++-
 scripts/recordmcount.h               |    4 +++-
 8 files changed, 45 insertions(+), 6 deletions(-)

[RFC][PATCH 1/4] ftrace: Make recordmcount.c handle __fentry__

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-Make-recordmcount.c-handle-__fentry__.patch --]
[-- Type: text/plain, Size: 1207 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

With gcc 4.6.0 the -mfentry feature places the function profiling
call at the start of the function. When this is used, the call is
to __fentry__ and not mcount.

Change recordmcount.c to record both callers to __fentry__ and
mcount.

Cc: John Reiser <jreiser@bitwagon.com>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index baf187b..e75ff44 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -276,11 +276,13 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
 				&sym0[Elf_r_sym(relp)];
 			char const *symname = &str0[w(symp->st_name)];
 			char const *mcount = '_' == gpfx ? "_mcount" : "mcount";
+			char const *fentry = "__fentry__";
 
 			if ('.' == symname[0])
 				++symname;  /* ppc64 hack */
 			if (0 == strcmp(mcount, symname) ||
-			    (altmcount && 0 == strcmp(altmcount, symname)))
+			    (altmcount && 0 == strcmp(altmcount, symname)) ||
+			    (0 == strcmp(fentry, symname)))
 				mcountsym = Elf_r_sym(relp);
 		}
 
-- 
1.7.2.3

[RFC][PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer

[Steven Rostedt]

[-- Attachment #1: 0002-ftrace-Add-mfentry-to-Makefile-on-function-tracer.patch --]
[-- Type: text/plain, Size: 1910 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Thanks to Andi Kleen, gcc 4.6.0 now supports -mfentry for x86
(and hopefully soon for other archs). What this does is to have
the function profiler start at the beginning of the function
instead of after the stack is set up. As plain -pg (mcount) is
called after the stack is set up, and in some cases can have issues
with the function graph tracer. It also requires frame pointers to
be enabled.

The -mfentry now calls __fentry__ at the beginning of the function.
This allows for compiling without frame pointers and even has the
ability to access parameters if needed.

If the architecture and the compiler both support -mfentry then
use that instead.

TODO: Allow for FUNCTION_TRACER to be set without FRAME_POINTERS

Cc: Michal Marek <mmarek@suse.cz>
Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile             |    6 +++++-
 kernel/trace/Kconfig |    5 +++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index c9c8c8f..915bf47 100644
--- a/Makefile
+++ b/Makefile
@@ -578,7 +578,11 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly)
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-KBUILD_CFLAGS	+= -pg
+ifdef CONFIG_HAVE_FENTRY
+CC_HAS_FENTRY	:= $(call cc-option, -mfentry -DCC_HAS_FENTRY)
+endif
+KBUILD_CFLAGS	+= -pg $(CC_HAS_FENTRY)
+KBUILD_AFLAGS	+= $(CC_HAS_FENTRY)
 ifdef CONFIG_DYNAMIC_FTRACE
 	ifdef CONFIG_HAVE_C_RECORDMCOUNT
 		BUILD_C_RECORDMCOUNT := y
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 14674dc..7ed3f17 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -49,6 +49,11 @@ config HAVE_SYSCALL_TRACEPOINTS
 	help
 	  See Documentation/trace/ftrace-design.txt
 
+config HAVE_FENTRY
+	bool
+	help
+	  Arch supports the gcc options -pg with -mfentry
+
 config HAVE_C_RECORDMCOUNT
 	bool
 	help
-- 
1.7.2.3

Re: [RFC][PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer

[Steven Rostedt]

On Wed, 2011-02-09 at 15:02 -0500, Steven Rostedt wrote:

>  ifdef CONFIG_FUNCTION_TRACER
> -KBUILD_CFLAGS	+= -pg
> +ifdef CONFIG_HAVE_FENTRY
> +CC_HAS_FENTRY	:= $(call cc-option, -mfentry -DCC_HAS_FENTRY)

I should include a thanks to H. Peter Anvin in the change log for
telling me about the cc-option trick.

-- Steve

> +endif
> +KBUILD_CFLAGS	+= -pg $(CC_HAS_FENTRY)
> +KBUILD_AFLAGS	+= $(CC_HAS_FENTRY)
>  ifdef CONFIG_DYNAMIC_FTRACE
>  	ifdef CONFIG_HAVE_C_RECORDMCOUNT
>  		BUILD_C_RECORDMCOUNT := y

Re: [RFC][PATCH 2/4] ftrace: Add -mfentry to Makefile on function tracer

[H. Peter Anvin]

On 02/09/2011 12:28 PM, Steven Rostedt wrote:
> On Wed, 2011-02-09 at 15:02 -0500, Steven Rostedt wrote:
> 
>>  ifdef CONFIG_FUNCTION_TRACER
>> -KBUILD_CFLAGS	+= -pg
>> +ifdef CONFIG_HAVE_FENTRY
>> +CC_HAS_FENTRY	:= $(call cc-option, -mfentry -DCC_HAS_FENTRY)
> 
> I should include a thanks to H. Peter Anvin in the change log for
> telling me about the cc-option trick.
> 

Actually we should probably calling this -DCC_USING_FENTRY or something
like that, since as coded above it includes the CONFIG_FUNCTION_TRACER
dependency.  This is a good thing -- it makes the conditionals easier --
we just also want to make sure they don't get misunderstood.

	-hpa

[RFC][PATCH 3/4] ftrace: Do not test frame pointers if -mfentry is used

[Steven Rostedt]

[-- Attachment #1: 0003-ftrace-Do-not-test-frame-pointers-if-mfentry-is-used.patch --]
[-- Type: text/plain, Size: 1468 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The function graph has a test to check if the frame pointer is
corrupted, which can happen with various options of gcc with mcount.
But this is not an issue with -mfentry as -mfentry does not need nor use
frame pointers for function graph tracing.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 76b0598..e7bc48b 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -129,7 +129,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 		return;
 	}
 
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_HAS_FENTRY)
 	/*
 	 * The arch may choose to record the frame pointer used
 	 * and check it here to make sure that it is what we expect it
@@ -140,6 +140,9 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 	 *
 	 * Currently, x86_32 with optimize for size (-Os) makes the latest
 	 * gcc do the above.
+	 *
+	 * Note, -mfentry does not use frame pointers, and this test
+	 *  is not needed if CC_HAS_FENTRY is set.
 	 */
 	if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
 		ftrace_graph_stop();
-- 
1.7.2.3

[RFC][PATCH 4/4] ftrace/x86: Add support for -mfentry to x86_64

[Steven Rostedt]

[-- Attachment #1: 0004-ftrace-x86-Add-support-for-mfentry-to-x86_64.patch --]
[-- Type: text/plain, Size: 2913 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

If the kernel is compiled with gcc 4.6.0 which supports -mfentry,
then use that instead of mcount.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig                 |    1 +
 arch/x86/include/asm/ftrace.h    |    7 ++++++-
 arch/x86/kernel/entry_64.S       |   17 ++++++++++++++++-
 arch/x86/kernel/x8664_ksyms_64.c |    6 +++++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d5ed94d..ac1a47e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -32,6 +32,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FENTRY if X86_64
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index db24c22..b1386d8 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -29,11 +29,16 @@
 #endif
 
 #ifdef CONFIG_FUNCTION_TRACER
-#define MCOUNT_ADDR		((long)(mcount))
+#ifdef CC_HAS_FENTRY
+# define MCOUNT_ADDR		((long)(__fentry__))
+#else
+# define MCOUNT_ADDR		((long)(mcount))
+#endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
 #ifndef __ASSEMBLY__
 extern void mcount(void);
+extern void __fentry__(void);
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index aed1ffb..44031ee 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -63,9 +63,16 @@
 	.code64
 #ifdef CONFIG_FUNCTION_TRACER
 #ifdef CONFIG_DYNAMIC_FTRACE
+
+#ifdef CC_HAS_FENTRY
+ENTRY(__fentry__)
+	retq
+END(__fentry__)
+#else
 ENTRY(mcount)
 	retq
 END(mcount)
+#endif
 
 ENTRY(ftrace_caller)
 	cmpl $0, function_trace_stop
@@ -74,7 +81,11 @@ ENTRY(ftrace_caller)
 	MCOUNT_SAVE_FRAME
 
 	movq 0x38(%rsp), %rdi
+#ifdef CC_HAS_FENTRY
+	movq 0x40(%rsp), %rsi
+#else
 	movq 8(%rbp), %rsi
+#endif
 	subq $MCOUNT_INSN_SIZE, %rdi
 
 GLOBAL(ftrace_call)
@@ -133,9 +144,13 @@ ENTRY(ftrace_graph_caller)
 
 	MCOUNT_SAVE_FRAME
 
+#ifdef CC_HAS_FENTRY
+	leaq 0x40(%rsp), %rdi
+#else
 	leaq 8(%rbp), %rdi
-	movq 0x38(%rsp), %rsi
 	movq (%rbp), %rdx
+#endif
+	movq 0x38(%rsp), %rsi
 	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
diff --git a/arch/x86/kernel/x8664_ksyms_64.c b/arch/x86/kernel/x8664_ksyms_64.c
index 1b950d1..7fa3a78 100644
--- a/arch/x86/kernel/x8664_ksyms_64.c
+++ b/arch/x86/kernel/x8664_ksyms_64.c
@@ -13,9 +13,13 @@
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_FUNCTION_TRACER
-/* mcount is defined in assembly */
+/* mcount and __fentry__ are defined in assembly */
+#ifdef CC_HAS_FENTRY
+EXPORT_SYMBOL(__fentry__);
+#else
 EXPORT_SYMBOL(mcount);
 #endif
+#endif
 
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);
-- 
1.7.2.3

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/10 5:02), Steven Rostedt wrote:
> Thanks to Andi Kleen gcc 4.6.0 now supports -mfentry with the -pg option
> to place a call to __fentry__ at the very beginning of the function
> instead of after the fact.
> 
> The old way:
> 
> 00000000000000c4 <atomic_long_add>:
>       c4:       55                      push   %rbp
>       c5:       48 89 e5                mov    %rsp,%rbp
>       c8:       e8 00 00 00 00          callq  cd <atomic_long_add+0x9>
>                         c9: R_X86_64_PC32       mcount-0x4
>       cd:       f0 48 01 3e             lock add %rdi,(%rsi)
>       d1:       c9                      leaveq 
>       d2:       c3                      retq   
> 
> The new way:
> 
> 000000000000009e <atomic_long_add>:
>       9e:       e8 00 00 00 00          callq  a3 <atomic_long_add+0x5>
>                         9f: R_X86_64_PC32       __fentry__-0x4
>       a3:       55                      push   %rbp
>       a4:       48 89 e5                mov    %rsp,%rbp
>       a7:       f0 48 01 3e             lock add %rdi,(%rsi)
>       ab:       5d                      pop    %rbp
>       ac:       c3                      retq   
> 
> Note, with -mfentry, frame pointers is no longer required
> by the function tracer. But this patch series still requires
> FRAME_POINTER to be set, since I need to figure out a good way to
> enable FRAME_POINTER only if gcc doesn't support this. But that can
> come later.
> 
> With the new __fentry__, we could possible record the parameters
> of a function call. This may take some work, and perhaps be
> a little like kprobes. But it is doable.

Hm, very interesting. With this feature and dynamic-ftrace,
it may be possible to use it automatically instead of kprobes
via kprobe-events when user probes function entry.

Thanks,

> This is still just RFC. I only wrote the code to support x86_64
> even though gcc 4.6.0 also supports i386. I figured I would post
> this first to get peoples reactions before converting
> i386 too. Other archs can soon follow.
> 
> -- Steve
> 
> The following patches are in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: rfc/tracing/fentry
> 
> 
> Steven Rostedt (4):
>       ftrace: Make recordmcount.c handle __fentry__
>       ftrace: Add -mfentry to Makefile on function tracer
>       ftrace: Do not test frame pointers if -mfentry is used
>       ftrace/x86: Add support for -mfentry to x86_64
> 
> ----
>  Makefile                             |    6 +++++-
>  arch/x86/Kconfig                     |    1 +
>  arch/x86/include/asm/ftrace.h        |    7 ++++++-
>  arch/x86/kernel/entry_64.S           |   17 ++++++++++++++++-
>  arch/x86/kernel/x8664_ksyms_64.c     |    6 +++++-
>  kernel/trace/Kconfig                 |    5 +++++
>  kernel/trace/trace_functions_graph.c |    5 ++++-
>  scripts/recordmcount.h               |    4 +++-
>  8 files changed, 45 insertions(+), 6 deletions(-)


-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/10 5:02), Steven Rostedt wrote:
> Thanks to Andi Kleen gcc 4.6.0 now supports -mfentry with the -pg option
> to place a call to __fentry__ at the very beginning of the function
> instead of after the fact.
> 
> The old way:
> 
> 00000000000000c4 <atomic_long_add>:
>       c4:       55                      push   %rbp
>       c5:       48 89 e5                mov    %rsp,%rbp
>       c8:       e8 00 00 00 00          callq  cd <atomic_long_add+0x9>
>                         c9: R_X86_64_PC32       mcount-0x4
>       cd:       f0 48 01 3e             lock add %rdi,(%rsi)
>       d1:       c9                      leaveq 
>       d2:       c3                      retq   
> 
> The new way:
> 
> 000000000000009e <atomic_long_add>:
>       9e:       e8 00 00 00 00          callq  a3 <atomic_long_add+0x5>
>                         9f: R_X86_64_PC32       __fentry__-0x4
>       a3:       55                      push   %rbp
>       a4:       48 89 e5                mov    %rsp,%rbp
>       a7:       f0 48 01 3e             lock add %rdi,(%rsi)
>       ab:       5d                      pop    %rbp
>       ac:       c3                      retq   
> 
> Note, with -mfentry, frame pointers is no longer required
> by the function tracer. But this patch series still requires
> FRAME_POINTER to be set, since I need to figure out a good way to
> enable FRAME_POINTER only if gcc doesn't support this. But that can
> come later.
> 
> With the new __fentry__, we could possible record the parameters
> of a function call. This may take some work, and perhaps be
> a little like kprobes. But it is doable.
> 
> This is still just RFC. I only wrote the code to support x86_64
> even though gcc 4.6.0 also supports i386. I figured I would post
> this first to get peoples reactions before converting
> i386 too. Other archs can soon follow.

Oops! with this change, current kprobes might not be able to probe
the entry of functions, because that is always reserved by ftrace!
I think we need to have some new interface for replacing each other
safely...

Thank you,

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Thu, 2011-02-17 at 21:37 +0900, Masami Hiramatsu wrote:

> Oops! with this change, current kprobes might not be able to probe
> the entry of functions, because that is always reserved by ftrace!
> I think we need to have some new interface for replacing each other
> safely...

Good point. I suspect that this wont be ready till .40 anyway. When I
get a chance to work more an this, I'll also include patches where if
-mfentry is activated kprobes will just hook to the mcount caller
instead. Or ftrace itself :)

I'm also working on making the function tracer a bit more flexible. That
is, to let multiple clients control the dynamic trace instead of just
one big global one.

-- Steve

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/17 22:18), Steven Rostedt wrote:
> On Thu, 2011-02-17 at 21:37 +0900, Masami Hiramatsu wrote:
> 
>> Oops! with this change, current kprobes might not be able to probe
>> the entry of functions, because that is always reserved by ftrace!
>> I think we need to have some new interface for replacing each other
>> safely...
> 
> Good point. I suspect that this wont be ready till .40 anyway. When I
> get a chance to work more an this, I'll also include patches where if
> -mfentry is activated kprobes will just hook to the mcount caller
> instead. Or ftrace itself :)

Ah, that's a good idea! :) it could be done without -mfentry too.
But is that possible to modify just one mcount entry? I also worry
about the latency of enabling/disabling one entry.

BTW, without dynamic ftrace (no code modifying), I think we don't
need to reserve mcount code, because no one modifies it.

> 
> I'm also working on making the function tracer a bit more flexible. That
> is, to let multiple clients control the dynamic trace instead of just
> one big global one.

Nice!


-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Fri, 2011-02-18 at 00:34 +0900, Masami Hiramatsu wrote:
> (2011/02/17 22:18), Steven Rostedt wrote:
> > On Thu, 2011-02-17 at 21:37 +0900, Masami Hiramatsu wrote:
> > 
> >> Oops! with this change, current kprobes might not be able to probe
> >> the entry of functions, because that is always reserved by ftrace!
> >> I think we need to have some new interface for replacing each other
> >> safely...
> > 
> > Good point. I suspect that this wont be ready till .40 anyway. When I
> > get a chance to work more an this, I'll also include patches where if
> > -mfentry is activated kprobes will just hook to the mcount caller
> > instead. Or ftrace itself :)
> 
> Ah, that's a good idea! :) it could be done without -mfentry too.
> But is that possible to modify just one mcount entry? I also worry
> about the latency of enabling/disabling one entry.

I would have it go through the ftrace function tracing facility, which
would handle which entry to enable/disable. It still does stopmachine.
Is that an issue to enable/disable kprobes? The "fast" enable/disable
could be done by the called function to just ignore the call.

Also note, if there's other callbacks that are attached to the function
being traced, no stop machine is enabled. The callbacks are just a list
and as long as a function has an associated callback, no code
modification needs to be done to add or remove other callbacks.

> 
> BTW, without dynamic ftrace (no code modifying), I think we don't
> need to reserve mcount code, because no one modifies it.

Correct. And even today, you can remove any kprobe code that checks for
mcount without dynamic ftrace enabled. But I'm not sure if anyone
enables the function tracer without dynamic ftrace, except for debugging
in archs that do not support dynamic ftrace. As the overhead of this is
quite high even when function tracer is disabled.

-- Steve

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/18 0:46), Steven Rostedt wrote:
> On Fri, 2011-02-18 at 00:34 +0900, Masami Hiramatsu wrote:
>> (2011/02/17 22:18), Steven Rostedt wrote:
>>> On Thu, 2011-02-17 at 21:37 +0900, Masami Hiramatsu wrote:
>>>
>>>> Oops! with this change, current kprobes might not be able to probe
>>>> the entry of functions, because that is always reserved by ftrace!
>>>> I think we need to have some new interface for replacing each other
>>>> safely...
>>>
>>> Good point. I suspect that this wont be ready till .40 anyway. When I
>>> get a chance to work more an this, I'll also include patches where if
>>> -mfentry is activated kprobes will just hook to the mcount caller
>>> instead. Or ftrace itself :)
>>
>> Ah, that's a good idea! :) it could be done without -mfentry too.
>> But is that possible to modify just one mcount entry? I also worry
>> about the latency of enabling/disabling one entry.
> 
> I would have it go through the ftrace function tracing facility, which
> would handle which entry to enable/disable. It still does stopmachine.
> Is that an issue to enable/disable kprobes? The "fast" enable/disable
> could be done by the called function to just ignore the call.

I just thought that frequent stop-machine is not so good from the user's
POV. I agree that disabled probe ignoring the call is enough.
Maybe, it could be done with the similar mechanism of jump optimization.

> Also note, if there's other callbacks that are attached to the function
> being traced, no stop machine is enabled. The callbacks are just a list
> and as long as a function has an associated callback, no code
> modification needs to be done to add or remove other callbacks.

Right :)

>> BTW, without dynamic ftrace (no code modifying), I think we don't
>> need to reserve mcount code, because no one modifies it.
> 
> Correct. And even today, you can remove any kprobe code that checks for
> mcount without dynamic ftrace enabled. But I'm not sure if anyone
> enables the function tracer without dynamic ftrace, except for debugging
> in archs that do not support dynamic ftrace. As the overhead of this is
> quite high even when function tracer is disabled.

Indeed. Maybe that is only for that exception case, because it allows
us to put probe even on the mcount call.

Thank you,

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Fri, 2011-02-18 at 01:07 +0900, Masami Hiramatsu wrote:

> I just thought that frequent stop-machine is not so good from the user's
> POV. I agree that disabled probe ignoring the call is enough.
> Maybe, it could be done with the similar mechanism of jump optimization.

I thought jump optimization still calls stop_machine too?

-- Steve

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/18 5:11), Steven Rostedt wrote:
> On Fri, 2011-02-18 at 01:07 +0900, Masami Hiramatsu wrote:
> 
>> I just thought that frequent stop-machine is not so good from the user's
>> POV. I agree that disabled probe ignoring the call is enough.
>> Maybe, it could be done with the similar mechanism of jump optimization.
> 
> I thought jump optimization still calls stop_machine too?

Yes, but now it does batch optimization.
Even if hundreds kprobes are registered separately, jump optimization
has been done in background with a stop_machine per every 256 probes.
(Until optimizing, kprobes can use breakpoints instead)

Thank you,

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Fri, 2011-02-18 at 20:45 +0900, Masami Hiramatsu wrote:
> (2011/02/18 5:11), Steven Rostedt wrote:
> > On Fri, 2011-02-18 at 01:07 +0900, Masami Hiramatsu wrote:
> > 
> >> I just thought that frequent stop-machine is not so good from the user's
> >> POV. I agree that disabled probe ignoring the call is enough.
> >> Maybe, it could be done with the similar mechanism of jump optimization.
> > 
> > I thought jump optimization still calls stop_machine too?
> 
> Yes, but now it does batch optimization.
> Even if hundreds kprobes are registered separately, jump optimization
> has been done in background with a stop_machine per every 256 probes.
> (Until optimizing, kprobes can use breakpoints instead)

But a single optimized kprobe still must use stopmachine.

But it is true that the function tracer does it as one big shot. That
is, it will do all functions in a single stop machine that needs to be
changed. It too is batched, but there is not a limit to that batch.

I would be interested in hearing from users and real use cases that
someone would like to trace functions but stopmachine is too big of a
hammer.

-- Steve

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Mathieu Desnoyers]

[Adding Dominique Toupin, from Ericsson, to CC list]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Fri, 2011-02-18 at 20:45 +0900, Masami Hiramatsu wrote:
> > (2011/02/18 5:11), Steven Rostedt wrote:
> > > On Fri, 2011-02-18 at 01:07 +0900, Masami Hiramatsu wrote:
> > > 
> > >> I just thought that frequent stop-machine is not so good from the user's
> > >> POV. I agree that disabled probe ignoring the call is enough.
> > >> Maybe, it could be done with the similar mechanism of jump optimization.
> > > 
> > > I thought jump optimization still calls stop_machine too?
> > 
> > Yes, but now it does batch optimization.
> > Even if hundreds kprobes are registered separately, jump optimization
> > has been done in background with a stop_machine per every 256 probes.
> > (Until optimizing, kprobes can use breakpoints instead)
> 
> But a single optimized kprobe still must use stopmachine.
> 
> But it is true that the function tracer does it as one big shot. That
> is, it will do all functions in a single stop machine that needs to be
> changed. It too is batched, but there is not a limit to that batch.
> 
> I would be interested in hearing from users and real use cases that
> someone would like to trace functions but stopmachine is too big of a
> hammer.

Hi Steven,

Telecom end users are one of such cases where the latency induced by stop
machine while the system is running is a problem. Dominique Toupin could
certainly tell us more about Ericsson's use-cases.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

RE: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Dominique Toupin]

My understanding is stop_machine will stop all processors for many ms.
Even if most of our systems are not hard real-time they are soft real-time and stopping all cores for a few ms is not allowed.
We can stop a few threads while we are jump patching but all processors is too much for us.

I can send other real use cases if you are interested.


> -----Original Message-----
> From: Mathieu Desnoyers [mailto:mathieu.desnoyers@efficios.com] 
> Sent: 18-Feb-11 10:20
> To: Steven Rostedt; Dominique Toupin
> Cc: Masami Hiramatsu; linux-kernel@vger.kernel.org; Ingo 
> Molnar; Andrew Morton; Thomas Gleixner; Frederic Weisbecker; 
> H. Peter Anvin; Andi Kleen; 2nddept-manager@sdl.hitachi.co.jp
> Subject: Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when 
> supported (this is for x86_64 right now)
> 
> [Adding Dominique Toupin, from Ericsson, to CC list]
> 
> * Steven Rostedt (rostedt@goodmis.org) wrote:
> > On Fri, 2011-02-18 at 20:45 +0900, Masami Hiramatsu wrote:
> > > (2011/02/18 5:11), Steven Rostedt wrote:
> > > > On Fri, 2011-02-18 at 01:07 +0900, Masami Hiramatsu wrote:
> > > > 
> > > >> I just thought that frequent stop-machine is not so 
> good from the 
> > > >> user's POV. I agree that disabled probe ignoring the 
> call is enough.
> > > >> Maybe, it could be done with the similar mechanism of 
> jump optimization.
> > > > 
> > > > I thought jump optimization still calls stop_machine too?
> > > 
> > > Yes, but now it does batch optimization.
> > > Even if hundreds kprobes are registered separately, jump 
> > > optimization has been done in background with a 
> stop_machine per every 256 probes.
> > > (Until optimizing, kprobes can use breakpoints instead)
> > 
> > But a single optimized kprobe still must use stopmachine.
> > 
> > But it is true that the function tracer does it as one big 
> shot. That 
> > is, it will do all functions in a single stop machine that 
> needs to be 
> > changed. It too is batched, but there is not a limit to that batch.
> > 
> > I would be interested in hearing from users and real use cases that 
> > someone would like to trace functions but stopmachine is 
> too big of a 
> > hammer.
> 
> Hi Steven,
> 
> Telecom end users are one of such cases where the latency 
> induced by stop machine while the system is running is a 
> problem. Dominique Toupin could certainly tell us more about 
> Ericsson's use-cases.
> 
> Thanks,
> 
> Mathieu
> 
> --
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant EfficiOS Inc.
> http://www.efficios.com
> 

RE: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Fri, 2011-02-18 at 15:10 -0500, Dominique Toupin wrote:
> My understanding is stop_machine will stop all processors for many ms.

s/ms/us/


> Even if most of our systems are not hard real-time they are soft real-time and stopping all cores for a few ms is not allowed.
> We can stop a few threads while we are jump patching but all processors is too much for us.

I think I could hit a single ms if we enable full function tracing which
disables ~22,000 functions in one shot. But if you enable full function
tracing, the kernel can slow down quite drastically, and that would even
be more problematic than a single ms hic-up. As hackbench showed a %150
slowdown when function tracer was running.

Now the last measurements I took was a few years ago and it was on a 4
CPU box. Perhaps stop_machine() may be a bit more expensive on a 1024
CPU box.

-- Steve

RE: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Dominique Toupin]

If it's 1 us it might be OK for some of our "server" type of system, still the number of cores are growing quite fast and stopping _all_ of them is a bit scary. Some cores are dedicated to a special telecom subsystem which is very sensitive to even very small hic-up, we don't want to stop those cores.

As background info, we can use GDB dynamic tracepoint in those systems because GDB doesn't stop all cores when the tracepoint are inserted with a jump.


> -----Original Message-----
> From: Steven Rostedt [mailto:rostedt@goodmis.org] 
> Sent: 18-Feb-11 15:37
> To: Dominique Toupin
> Cc: Mathieu Desnoyers; Masami Hiramatsu; 
> linux-kernel@vger.kernel.org; Ingo Molnar; Andrew Morton; 
> Thomas Gleixner; Frederic Weisbecker; H. Peter Anvin; Andi 
> Kleen; 2nddept-manager@sdl.hitachi.co.jp
> Subject: RE: [RFC][PATCH 0/4] ftrace: Use -mfentry when 
> supported (this is for x86_64 right now)
> 
> On Fri, 2011-02-18 at 15:10 -0500, Dominique Toupin wrote:
> > My understanding is stop_machine will stop all processors 
> for many ms.
> 
> s/ms/us/
> 
> 
> > Even if most of our systems are not hard real-time they are 
> soft real-time and stopping all cores for a few ms is not allowed.
> > We can stop a few threads while we are jump patching but 
> all processors is too much for us.
> 
> I think I could hit a single ms if we enable full function 
> tracing which disables ~22,000 functions in one shot. But if 
> you enable full function tracing, the kernel can slow down 
> quite drastically, and that would even be more problematic 
> than a single ms hic-up. As hackbench showed a %150 slowdown 
> when function tracer was running.
> 
> Now the last measurements I took was a few years ago and it 
> was on a 4 CPU box. Perhaps stop_machine() may be a bit more 
> expensive on a 1024 CPU box.
> 
> -- Steve
> 
> 
> 

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Andi Kleen]

On Fri, Feb 18, 2011 at 03:10:18PM -0500, Dominique Toupin wrote:
> 
> My understanding is stop_machine will stop all processors for many ms.

I haven't measured it recently, but as long as the callback inside stop
machine is short it definitely shouldn't be "many ms". The latency
is bound by how long each CPU needs to answer to an interrupt, so if
you have some code that disables interrupts for a long time it will take
long -- but then your realtime response will be already bad. 

The interrupts are also done in parallel, so the interrupt latencies
don't add up.

If all the CPUs answer in a reasonable time it's still not a cheap
operation, but nothing that takes "many ms". Most likely it's fine
for most soft real time purposes.

-Andi

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[H. Peter Anvin]

On 02/18/2011 02:39 PM, Andi Kleen wrote:
> On Fri, Feb 18, 2011 at 03:10:18PM -0500, Dominique Toupin wrote:
>>
>> My understanding is stop_machine will stop all processors for many ms.
> 
> I haven't measured it recently, but as long as the callback inside stop
> machine is short it definitely shouldn't be "many ms". The latency
> is bound by how long each CPU needs to answer to an interrupt, so if
> you have some code that disables interrupts for a long time it will take
> long -- but then your realtime response will be already bad. 
> 
> The interrupts are also done in parallel, so the interrupt latencies
> don't add up.
> 
> If all the CPUs answer in a reasonable time it's still not a cheap
> operation, but nothing that takes "many ms". Most likely it's fine
> for most soft real time purposes.
> 

We should also be able to use the breakpoint hack to avoid holding all
the CPUs.  They still need to be interrupted, but that skips the
rendezvous operation.

	-hpa

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Steven Rostedt]

On Fri, 2011-02-18 at 14:45 -0800, H. Peter Anvin wrote:

> We should also be able to use the breakpoint hack to avoid holding all
> the CPUs.  They still need to be interrupted, but that skips the
> rendezvous operation.

As this is about the ftrace code, I'm in the process of analyzing and
updating how the function tracer works. I can look to see if I can
design it so we don't have to always use stop_machine() if a breakpoint
method is in place.

Basically what is needed is to convert a "nop" into a "call" or maybe
the other way around, safely.

Now is it safe to insert a breakpoint (usually a byte I believe), modify
the rest of the instruction and then replace the breakpoint to the new
code? Since the instruction that is being replaced or the instruction
being added is always a nop, the breakpoint handler needs to do nothing
but return to the location after the nop/call.

Is there any synchronization that needs to be done when doing this? Or
can it just be:

	insert_breakpoint();
	update_instruction();
	remove_breakpoint();

Because we need to do this for 22,000 calls in a row.

-- Steve

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[Masami Hiramatsu]

(2011/02/19 8:02), Steven Rostedt wrote:
> On Fri, 2011-02-18 at 14:45 -0800, H. Peter Anvin wrote:
> 
>> We should also be able to use the breakpoint hack to avoid holding all
>> the CPUs.  They still need to be interrupted, but that skips the
>> rendezvous operation.

That's what I've done with text_poke_fixup()
http://lkml.org/lkml/2009/12/18/312

And I think it still not be checked officially from silicon side.

> As this is about the ftrace code, I'm in the process of analyzing and
> updating how the function tracer works. I can look to see if I can
> design it so we don't have to always use stop_machine() if a breakpoint
> method is in place.
> 
> Basically what is needed is to convert a "nop" into a "call" or maybe
> the other way around, safely.
> 
> Now is it safe to insert a breakpoint (usually a byte I believe), modify
> the rest of the instruction and then replace the breakpoint to the new
> code? Since the instruction that is being replaced or the instruction
> being added is always a nop, the breakpoint handler needs to do nothing
> but return to the location after the nop/call.

Yes, at least with text_poke_fixup(), you can call it as below

text_poke_fixup(addr, call_insn, CALL_INSN_SIZE, addr + CALL_INSN_SIZE);

Then, if a processor hits the addr, the breakpoint handler changes its
regs->ip to addr + CALL_INSN_SIZE so that it skips the modifying
instruction.

> Is there any synchronization that needs to be done when doing this? Or
> can it just be:
> 
> 	insert_breakpoint();
> 	update_instruction();
> 	remove_breakpoint();
> 
> Because we need to do this for 22,000 calls in a row.

In the case of text_poke_fixup(), it sends IPI twice for synchronization,
which doesn't stop all cores but current core. Of course, theoretically
this can be reduced by doing it in a batch.

Thank you,

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC][PATCH 0/4] ftrace: Use -mfentry when supported (this is for x86_64 right now)

[hpas]

On 02/18/2011 03:02 PM, Steven Rostedt wrote:
> 
> Is there any synchronization that needs to be done when doing this? Or
> can it just be:
> 
> 	insert_breakpoint();
> 	update_instruction();
> 	remove_breakpoint();
> 
> Because we need to do this for 22,000 calls in a row.
> 

The sequence needs to be:

1. Set up the breakpoint handler so that it can dismiss a breakpoint
interrupt from one of the patching addresses (it can just subtract one
from the return address and IRET).
2. Replace the first byte with a breakpoint instruction.
3. IPI all processors.
4. Write all but the first byte of the new instruction.
5. Write the first byte of the new instruction.
6. IPI all processors.
7. Tear down the breakpoint hander.

Note that "IPI all processors" does not require a rendezvous like
stop_machine(): it just means the issuing processor has to wait until
each processor has been IPI'd (e.g. via smp_call_function()), but those
processors can simply IRET and continue executing.

If the breakpoint handler can be left indefinitely, steps 6-7 can be
omitted (the IRET from the breakpoint handler will provide the necessary
serialization.)

	-hpa