Home » Mailing list archives » linux-kernel » 2010 » March » 16

Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing

Previous thread: [resend][GIT] kbuild by Michal Marek on Monday, March 15, 2010 - 7:05 am. (1 message)

Next thread: kfifo has temporarily invalid in pointer? by Robert P. J. Day on Monday, March 15, 2010 - 7:58 am. (3 messages)
[view original message]
From: Arnaldo Carvalho de Melo
Subject: [PATCH 1/2] perf record: Enable the enable_on_exec flag if record forks the target
Date: Monday, March 15, 2010 - 7:46 am

From: Eric B Munson <ebmunson@us.ibm.com>

When forking its target, perf record can capture data from before the target
application is started.  Perf stat uses the enable_on_exec flag in the event
attributes to keep from displaying events from before the target program
starts, this patch adds the same functionality to perf record when it is will
fork the target process.

Signed-off-by: Eric B Munson <ebmunson@us.ibm.com>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bed175d..962cdbf 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -225,7 +225,7 @@ static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int n
 	return h_attr;
 }
 
-static void create_counter(int counter, int cpu, pid_t pid)
+static void create_counter(int counter, int cpu, pid_t pid, bool forks)
 {
 	char *filter = filters[counter];
 	struct perf_event_attr *attr = attrs + counter;
@@ -277,6 +277,9 @@ static void create_counter(int counter, int cpu, pid_t pid)
 	attr->inherit		= inherit;
 	attr->disabled		= 1;
 
+	if (forks)
+		attr->enable_on_exec = 1;
+
 try_again:
 	fd[nr_cpu][counter] = sys_perf_event_open(attr, pid, cpu, group_fd, 0);
 
@@ -381,13 +384,13 @@ try_again:
 	ioctl(fd[nr_cpu][counter], PERF_EVENT_IOC_ENABLE);
 }
 
-static void open_counters(int cpu, pid_t pid)
+static void open_counters(int cpu, pid_t pid, bool forks)
 {
 	int counter;
 
 	group_fd = -1;
 	for (counter = 0; counter < nr_counters; counter++)
-		create_counter(counter, cpu, pid);
+		create_counter(counter, cpu, pid, forks);
 
 	nr_cpu++;
 }
@@ -547,11 +550,11 @@ static int __cmd_record(int argc, const char **argv)
 
 
 	if ((!system_wide && !inherit) || profile_cpu != -1) {
-		open_counters(profile_cpu, ...
[ message continues ]
[view original message]
From: Arnaldo Carvalho de Melo
Subject: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Monday, March 15, 2010 - 7:46 am

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Before this patch this message would very briefly appear on the screen and then
the screen would get updates only on the top, for number of interrupts received,
etc, but no annotation would be performed:

[root@doppio linux-2.6-tip]# perf top -s n_tty_write > /tmp/bla
objdump: '[kernel.kallsyms]': No such file

Now this is what the user gets:

[root@doppio linux-2.6-tip]# perf top -s n_tty_write
Can't annotate n_tty_write: No vmlinux file was found in the path:
[0] vmlinux
[1] /boot/vmlinux
[2] /boot/vmlinux-2.6.33-rc5
[3] /lib/modules/2.6.33-rc5/build/vmlinux
[4] /usr/lib/debug/lib/modules/2.6.33-rc5/vmlinux
[root@doppio linux-2.6-tip]#

This bug was introduced when we added automatic search for vmlinux, before that
time the user had to specify a vmlinux file.

Reported-by: David S. Miller <davem@davemloft.net>
Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |   33 +++++++++++++++++++++++++--------
 tools/perf/util/symbol.c |   25 ++++++++++++-------------
 tools/perf/util/symbol.h |   15 +++++++++++++++
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 57e232f..c968bd3 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -169,7 +169,7 @@ static void sig_winch_handler(int sig __used)
 	update_print_entries(&winsize);
 }
 
-static void parse_source(struct sym_entry *syme)
+static int parse_source(struct sym_entry *syme)
 {
 	struct symbol *sym;
 	struct sym_entry_source *source;
@@ -180,12 +180,21 @@ static void parse_source(struct sym_entry *syme)
 	u64 len;
 
 	if (!syme)
-		return;
+		return -1;
+
+	sym = sym_entry__symbol(syme);
+	map = syme->map;
+
+	/*
+	 * We can't ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Monday, March 15, 2010 - 11:55 am

From: Arnaldo Carvalho de Melo <acme@infradead.org>

Arnaldo, if perf top can use the kallsyms to do it's normal task, why
can't it use that for symbol annotations too?  Isn't there enough
information available?

--


[ message continues ]
[view original message]
From: Arnaldo Carvalho de Melo
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Monday, March 15, 2010 - 12:21 pm

Annotation is done by objdump -dS, that requires an executable file to
do ASM annotation, and one with DWARF info for source code annotation.
What we have with /proc/kallsyms is just the symtab.

- Arnaldo
--


[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Monday, March 15, 2010 - 12:26 pm

From: Arnaldo Carvalho de Melo <acme@infradead.org>

Ok, I really think we should link with libopcodes or similar so we can
handle the simple assembler annotation case without requiring the
kernel image being available.

--


[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Tuesday, March 16, 2010 - 2:46 am

i'd really like that to happen - i.e. if we had the kernel (and modules) image 
exposed as a (almost-)standard ELF object via /sys or so.

We already have some aspects of that, via /sys/kernel/notes, but it should be 
done for real. That would also make build-id support less of a hack.

( I'd not include CFI debuginfo in there though - that would be way too large.
  More compressed debuginfo could be included perhaps. )

	Ingo
--


[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Tuesday, March 16, 2010 - 2:51 am

From: Ingo Molnar <mingo@elte.hu>

That's interesting and useful, but wouldn't help my case as the images
I'm usually booting are completely stripped.

That's why I end up with "[kernel.kallsyms]" for kernel profiling :-)
--


[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Tuesday, March 16, 2010 - 2:55 am

The kernel image itself is still present in RAM, obviously. So are the symbols 
(packed for kallsyms).

So exposing a virtual ELF image via /sys (or /proc/kernel.so) wouldnt take up 
much more RAM and would be rather useful, right?

	Ingo
--


[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 2/2] perf top: Properly notify the user that vmlinux is missing
Date: Tuesday, March 16, 2010 - 2:58 am

From: Ingo Molnar <mingo@elte.hu>

Absolutely agreed.
--


[ message continues ]
[view original message]
From: tip-bot for Eric B Munson
Subject: [tip:perf/core] perf record: Enable the enable_on_exec flag if record forks the target
Date: Monday, March 15, 2010 - 8:15 am

Commit-ID:  bedbfdea31daf3880745001d56450c683959ee7e
Gitweb:     http://git.kernel.org/tip/bedbfdea31daf3880745001d56450c683959ee7e
Author:     Eric B Munson <ebmunson@us.ibm.com>
AuthorDate: Mon, 15 Mar 2010 11:46:57 -0300
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 15 Mar 2010 16:08:22 +0100

perf record: Enable the enable_on_exec flag if record forks the target

When forking its target, perf record can capture data from
before the target application is started.  Perf stat uses the
enable_on_exec flag in the event attributes to keep from
displaying events from before the target program starts, this
patch adds the same functionality to perf record when it is will
fork the target process.

Signed-off-by: Eric B Munson <ebmunson@us.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1268664418-28328-1-git-send-email-acme@infradead.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-record.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bed175d..962cdbf 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -225,7 +225,7 @@ static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int n
 	return h_attr;
 }
 
-static void create_counter(int counter, int cpu, pid_t pid)
+static void create_counter(int counter, int cpu, pid_t pid, bool forks)
 {
 	char *filter = filters[counter];
 	struct perf_event_attr *attr = attrs + counter;
@@ -277,6 +277,9 @@ static void create_counter(int counter, int cpu, pid_t pid)
 	attr->inherit		= inherit;
 	attr->disabled		= 1;
 
+	if (forks)
+		attr->enable_on_exec = 1;
+
 try_again:
 	fd[nr_cpu][counter] = sys_perf_event_open(attr, pid, cpu, group_fd, 0);
 
@@ -381,13 +384,13 @@ try_again:
 ...
[ message continues ]
Previous thread: [resend][GIT] kbuild by Michal Marek on Monday, March 15, 2010 - 7:05 am. (1 message)

Next thread: kfifo has temporarily invalid in pointer? by Robert P. J. Day on Monday, March 15, 2010 - 7:58 am. (3 messages)