* [PATCH] tracing: remove superfluous sub instructions
@ 2011-01-18 15:28 Jiri Olsa
2011-01-18 15:52 ` Frederic Weisbecker
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2011-01-18 15:28 UTC (permalink / raw)
To: mingo, rostedt, fweisbec; +Cc: linux-kernel, Jiri Olsa
hi,
I think there's no need for substracting MCOUNT_INSN_SIZE from the
IP parameter before calling the function trace (/graph) handler.
Maybe I overlooked something, but all the IP usage I saw ended
up in the kallsyms_lookup function, which does the lookup using the
functions' start/end boundaries to find the correct symbol for pointer.
Thus it seems to me there's no point in substracting the
MCOUNT_INSN_SIZE value from the IP parameter.
I tested for x86_64 and got proper results, I believe it's
the same case for x86_32.
wbr,
jirka
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
arch/x86/kernel/entry_32.S | 3 ---
arch/x86/kernel/entry_64.S | 3 ---
2 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 591e601..92cdb47 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1122,7 +1122,6 @@ ENTRY(ftrace_caller)
pushl %edx
movl 0xc(%esp), %eax
movl 0x4(%ebp), %edx
- subl $MCOUNT_INSN_SIZE, %eax
.globl ftrace_call
ftrace_call:
@@ -1168,7 +1167,6 @@ trace:
pushl %edx
movl 0xc(%esp), %eax
movl 0x4(%ebp), %edx
- subl $MCOUNT_INSN_SIZE, %eax
call *ftrace_trace_function
@@ -1191,7 +1189,6 @@ ENTRY(ftrace_graph_caller)
movl 0xc(%esp), %edx
lea 0x4(%ebp), %eax
movl (%ebp), %ecx
- subl $MCOUNT_INSN_SIZE, %edx
call prepare_ftrace_return
popl %edx
popl %ecx
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index d3b895f..662f8f6 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -75,7 +75,6 @@ ENTRY(ftrace_caller)
movq 0x38(%rsp), %rdi
movq 8(%rbp), %rsi
- subq $MCOUNT_INSN_SIZE, %rdi
GLOBAL(ftrace_call)
call ftrace_stub
@@ -115,7 +114,6 @@ trace:
movq 0x38(%rsp), %rdi
movq 8(%rbp), %rsi
- subq $MCOUNT_INSN_SIZE, %rdi
call *ftrace_trace_function
@@ -136,7 +134,6 @@ ENTRY(ftrace_graph_caller)
leaq 8(%rbp), %rdi
movq 0x38(%rsp), %rsi
movq (%rbp), %rdx
- subq $MCOUNT_INSN_SIZE, %rsi
call prepare_ftrace_return
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: remove superfluous sub instructions
2011-01-18 15:28 [PATCH] tracing: remove superfluous sub instructions Jiri Olsa
@ 2011-01-18 15:52 ` Frederic Weisbecker
2011-01-18 16:22 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Frederic Weisbecker @ 2011-01-18 15:52 UTC (permalink / raw)
To: Jiri Olsa; +Cc: mingo, rostedt, linux-kernel
On Tue, Jan 18, 2011 at 04:28:18PM +0100, Jiri Olsa wrote:
> hi,
>
> I think there's no need for substracting MCOUNT_INSN_SIZE from the
> IP parameter before calling the function trace (/graph) handler.
>
> Maybe I overlooked something, but all the IP usage I saw ended
> up in the kallsyms_lookup function, which does the lookup using the
> functions' start/end boundaries to find the correct symbol for pointer.
>
> Thus it seems to me there's no point in substracting the
> MCOUNT_INSN_SIZE value from the IP parameter.
>
> I tested for x86_64 and got proper results, I believe it's
> the same case for x86_32.
>
> wbr,
> jirka
>
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Well, that sounds right after all. If we are only interested in the
symbol, the instruction that follows "call mcount" is still relevant
as it must belong to the same function.
Steve?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: remove superfluous sub instructions
2011-01-18 15:52 ` Frederic Weisbecker
@ 2011-01-18 16:22 ` Steven Rostedt
2011-01-18 16:41 ` Jiri Olsa
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2011-01-18 16:22 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Jiri Olsa, mingo, linux-kernel
On Tue, 2011-01-18 at 16:52 +0100, Frederic Weisbecker wrote:
> On Tue, Jan 18, 2011 at 04:28:18PM +0100, Jiri Olsa wrote:
> > hi,
> >
> > I think there's no need for substracting MCOUNT_INSN_SIZE from the
> > IP parameter before calling the function trace (/graph) handler.
> >
> > Maybe I overlooked something, but all the IP usage I saw ended
> > up in the kallsyms_lookup function, which does the lookup using the
> > functions' start/end boundaries to find the correct symbol for pointer.
> >
> > Thus it seems to me there's no point in substracting the
> > MCOUNT_INSN_SIZE value from the IP parameter.
> >
> > I tested for x86_64 and got proper results, I believe it's
> > the same case for x86_32.
> >
> > wbr,
> > jirka
> >
> >
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
>
> Well, that sounds right after all. If we are only interested in the
> symbol, the instruction that follows "call mcount" is still relevant
> as it must belong to the same function.
>
> Steve?
NAK, it will break function triggers/probes. The "func:traceon" and
"func:traceoff". They compare the ip to the call location of mcount.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: remove superfluous sub instructions
2011-01-18 16:22 ` Steven Rostedt
@ 2011-01-18 16:41 ` Jiri Olsa
2011-01-18 17:01 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2011-01-18 16:41 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel
On Tue, Jan 18, 2011 at 11:22:42AM -0500, Steven Rostedt wrote:
> On Tue, 2011-01-18 at 16:52 +0100, Frederic Weisbecker wrote:
> > On Tue, Jan 18, 2011 at 04:28:18PM +0100, Jiri Olsa wrote:
> > > hi,
> > >
> > > I think there's no need for substracting MCOUNT_INSN_SIZE from the
> > > IP parameter before calling the function trace (/graph) handler.
> > >
> > > Maybe I overlooked something, but all the IP usage I saw ended
> > > up in the kallsyms_lookup function, which does the lookup using the
> > > functions' start/end boundaries to find the correct symbol for pointer.
> > >
> > > Thus it seems to me there's no point in substracting the
> > > MCOUNT_INSN_SIZE value from the IP parameter.
> > >
> > > I tested for x86_64 and got proper results, I believe it's
> > > the same case for x86_32.
> > >
> > > wbr,
> > > jirka
> > >
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> >
> > Well, that sounds right after all. If we are only interested in the
> > symbol, the instruction that follows "call mcount" is still relevant
> > as it must belong to the same function.
> >
> > Steve?
>
> NAK, it will break function triggers/probes. The "func:traceon" and
> "func:traceoff". They compare the ip to the call location of mcount.
>
> -- Steve
>
>
ops, missed this one..
would it make sense to update the IP inside the function_trace_probe_call
function to save one instruction in the entry code used by all? or it's
not worth it..
jirka
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: remove superfluous sub instructions
2011-01-18 16:41 ` Jiri Olsa
@ 2011-01-18 17:01 ` Steven Rostedt
0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2011-01-18 17:01 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Frederic Weisbecker, mingo, linux-kernel
On Tue, 2011-01-18 at 17:41 +0100, Jiri Olsa wrote:
> ops, missed this one..
>
> would it make sense to update the IP inside the function_trace_probe_call
> function to save one instruction in the entry code used by all? or it's
> not worth it..
That would require function_trace_probe_call() to handle arch
dependencies. I would like to keep arch dependent code in the arch
subsystem when possible.
This change only saves a subtraction to a register. It only gets called
if we are tracing functions. This modification is far into the noise
that is caused by the tracer. I don't think it is worth it as it will
cause head aches with knowing how to handle the ip from the users of the
function tracer.
I do plan on making the function tracer a bit more generic for common
users. I would like to keep the ip at the call site of mcount.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-18 17:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-18 15:28 [PATCH] tracing: remove superfluous sub instructions Jiri Olsa
2011-01-18 15:52 ` Frederic Weisbecker
2011-01-18 16:22 ` Steven Rostedt
2011-01-18 16:41 ` Jiri Olsa
2011-01-18 17:01 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).