Skip to contentSkip to conversation

perf updates

GIT PULL New Frederic Weisbecker 3 participants 8 messages Apr 4, 2010 – Apr 5, 2010

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/core

Thanks,
Frederic
---

Frederic Weisbecker (2):
perf: Drop the frame reliablity check
perf: Fetch hot regs from the template caller

Arnd Bergmann (1):
perf_event: Make perf fd non seekable

Hitoshi Mitake (1):
Swap inclusion order of util.h and string.h in util/string.c

 arch/x86/kernel/cpu/perf_event.c |    3 +--
 include/trace/ftrace.h           |   23 ++++++++++++-----------
 kernel/perf_event.c              |    1 +
 tools/perf/util/string.c         |    2 +-
 4 files changed, 15 insertions(+), 14 deletions(-)
#1 lore raw

PATCH 1/4 perf: Drop the frame reliablity check

It is useless now that we have a pure stack frame walker, as given addr are always reliable.

Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
---
 arch/x86/kernel/cpu/perf_event.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/perf_event.c +1 -2
index 65e9c5e..353a174 100644
@@ -1602,8 +1602,7 @@ static void backtrace_address(void *data, unsigned long addr, int reliable)
 {
 	struct perf_callchain_entry *entry = data;
 
-	if (reliable)
-		callchain_store(entry, addr);
+	callchain_store(entry, addr);
 }
 
 static const struct stacktrace_ops backtrace_ops = {
Signature · 1 line
1.6.2.3
#2 lore raw

PATCH 2/4 perf: Fetch hot regs from the template caller

In reply to Frederic Weisbecker

Trace events can be defined from a template using DECLARE_EVENT_CLASS/DEFINE_EVENT or directly with TRACE_EVENT.

In both cases we have a template tracepoint handler, used to record the trace, to which we pass our ftrace event instance.

In the function level, if the class is named "foo" and the event is named "blah", we have the following chain of calls:

perf_trace_blah() -> perf_trace_templ_foo()

In the case we have several events sharing the class "blah", we'll have multiple users of perf_trace_templ_foo(), and it won't be inlined by the compiler. This is usually what happens with the DECLARE_EVENT_CLASS/DEFINE_EVENT based definition.

But if perf_trace_blah() is the only caller of perf_trace_templ_foo() there are fair chances that it will be inlined.

The problem is that we fetch the regs from perf_trace_templ_foo() after we rewinded the frame pointer to the second caller, we want to reach the caller of perf_trace_blah() to get the right source of the event. And we do this by always assuming that perf_trace_templ_foo() is not inlined. But as shown above this is not always true. And if it is inlined we miss the first caller, losing the most important level of precision.

We get:

	    61.31%       ls  [kernel.kallsyms]  [k] do_softirq
                         |
                         --- do_softirq
                             irq_exit
                             do_IRQ
                             common_interrupt
                            |
                            |--25.00%-- tty_buffer_request_room

Instead of:

	    61.31%       ls  [kernel.kallsyms]  [k] __do_softirq
                         |
                         --- __do_softirq
                             do_softirq
                             irq_exit
                             do_IRQ
                             common_interrupt
                            |
                            |--25.00%-- tty_buffer_request_room

To fix this, we fetch the regs from perf_trace_blah() rather than perf_trace_templ_foo() so that we don't have to deal with inlining surprises.

That also bring us the advantage of having the true source of the event even if we don't have frame pointers.

Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
---
 include/trace/ftrace.h |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)
include/trace/ftrace.h +12 -11
index ea6f9d4..882c648 100644
@@ -758,13 +758,12 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 static notrace void							\
 perf_trace_templ_##call(struct ftrace_event_call *event_call,		\
-			    proto)					\
+			struct pt_regs *__regs, proto)			\
 {									\
 	struct ftrace_data_offsets_##call __maybe_unused __data_offsets;\
 	struct ftrace_raw_##call *entry;				\
 	u64 __addr = 0, __count = 1;					\
 	unsigned long irq_flags;					\
-	struct pt_regs *__regs;						\
 	int __entry_size;						\
 	int __data_size;						\
 	int rctx;							\
@@ -785,20 +784,22 @@ perf_trace_templ_##call(struct ftrace_event_call *event_call,		\
 									\
 	{ assign; }							\
 									\
-	__regs = &__get_cpu_var(perf_trace_regs);			\
-	perf_fetch_caller_regs(__regs, 2);				\
-									\
 	perf_trace_buf_submit(entry, __entry_size, rctx, __addr,	\
 			       __count, irq_flags, __regs);		\
 }
 
 #undef DEFINE_EVENT
-#define DEFINE_EVENT(template, call, proto, args)		\
-static notrace void perf_trace_##call(proto)			\
-{								\
-	struct ftrace_event_call *event_call = &event_##call;	\
-								\
-	perf_trace_templ_##template(event_call, args);		\
+#define DEFINE_EVENT(template, call, proto, args)			\
+static notrace void perf_trace_##call(proto)				\
+{									\
+	struct ftrace_event_call *event_call = &event_##call;		\
+	struct pt_regs *__regs = &get_cpu_var(perf_trace_regs);		\
+									\
+	perf_fetch_caller_regs(__regs, 1);				\
+									\
+	perf_trace_templ_##template(event_call, __regs, args);		\
+									\
+	put_cpu_var(perf_trace_regs);					\
 }
 
 #undef DEFINE_EVENT_PRINT
Signature · 1 line
1.6.2.3
#3 lore raw

PATCH 3/4 perf_event: Make perf fd non seekable

In reply to Frederic Weisbecker

From: Arnd Bergmann <[email protected]>

Perf_event does not need seeking, so prevent it in order to get rid of default_llseek, which uses the BKL.

Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
[drop the nonseekable_open, not needed for anon inodes]
---
 kernel/perf_event.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
kernel/perf_event.c +1 -0
index 63fbce1..4aa50ff 100644
@@ -2645,6 +2645,7 @@ static int perf_fasync(int fd, struct file *filp, int on)
 }
 
 static const struct file_operations perf_fops = {
+	.llseek			= no_llseek,
 	.release		= perf_release,
 	.read			= perf_read,
 	.poll			= perf_poll,
Signature · 1 line
1.6.2.3
#4 lore raw

PATCH 4/4 Swap inclusion order of util.h and string.h in util/string.c

In reply to Frederic Weisbecker

From: Hitoshi Mitake <[email protected]>

Currently util/string.c includes headers in this order: string.h, util.h But this causes a build error because __USE_GNU definition is needed for strndup() definition:

% make -j
touch .perf.dev.null
CC util/string.o
cc1: warnings being treated as errors
util/string.c: In function ‘argv_split’:
util/string.c:171: error: implicit declaration of function ‘strndup’ util/string.c:171: error: incompatible implicit declaration of built-in function ‘strndup’

So this patch swaps the headers inclusion order. util.h defines _GNU_SOURCE, and /usr/include/features.h defines __USE_GNU as 1 if _GNU_SOURCE is defined.

Cc Peter Zijlstra <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>

LKML-Reference: <[email protected]>

---
 tools/perf/util/string.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
tools/perf/util/string.c +1 -1
index d438924..0409fc7 100644
@@ -1,5 +1,5 @@
-#include "string.h"
 #include "util.h"
+#include "string.h"
 
 #define K 1024LL
 /*
Signature · 1 line
1.6.2.3
#5 lore raw
In reply to Frederic Weisbecker
Quoted 15 lines · Frederic Weisbecker

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/core

Thanks,
Frederic
---

Frederic Weisbecker (2):
perf: Drop the frame reliablity check
perf: Fetch hot regs from the template caller

Quoted 5 lines

Arnd Bergmann (1):
perf_event: Make perf fd non seekable

Hitoshi Mitake (1):
Swap inclusion order of util.h and string.h in util/string.c

I just did a last minute rebase, just to prepend the title of this one with "perf: "

Quoted 7 lines

 arch/x86/kernel/cpu/perf_event.c |    3 +--
 include/trace/ftrace.h           |   23 ++++++++++++-----------
 kernel/perf_event.c              |    1 +
 tools/perf/util/string.c         |    2 +-
 4 files changed, 15 insertions(+), 14 deletions(-)
#6 lore raw
Quoted 19 lines · Frederic Weisbecker
Quoted 18 lines · Frederic Weisbecker

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/core

Thanks,
Frederic
---

Frederic Weisbecker (2):
perf: Drop the frame reliablity check
perf: Fetch hot regs from the template caller

Arnd Bergmann (1):
perf_event: Make perf fd non seekable

Quoted 15 lines
Quoted 2 lines

Hitoshi Mitake (1):
Swap inclusion order of util.h and string.h in util/string.c

I just did a last minute rebase, just to prepend the title of this one with "perf: "

Quoted 7 lines

 arch/x86/kernel/cpu/perf_event.c |    3 +--
 include/trace/ftrace.h           |   23 ++++++++++++-----------
 kernel/perf_event.c              |    1 +
 tools/perf/util/string.c         |    2 +-
 4 files changed, 15 insertions(+), 14 deletions(-)

Pulled, thanks a lot Frederic!

Ingo

#7 lore raw
In reply to Frederic Weisbecker
Quoted 19 lines · Frederic Weisbecker
Quoted 18 lines · Frederic Weisbecker

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/core

Thanks,
Frederic
---

Frederic Weisbecker (2):
perf: Drop the frame reliablity check
perf: Fetch hot regs from the template caller

Arnd Bergmann (1):
perf_event: Make perf fd non seekable

Quoted 6 lines
Quoted 2 lines

Hitoshi Mitake (1):
Swap inclusion order of util.h and string.h in util/string.c

I just did a last minute rebase, just to prepend the title of this one with "perf: "

Ah, sorry, my naming of title was not good...

Thanks,
Hitoshi