[PATCH 1/2] perf record: Enable the enable_on_exec flag if record forks the target

[PATCH 1/2] perf record: Enable the enable_on_exec flag if record forks the target

[Arnaldo Carvalho de Melo]

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, target_pid);
+		open_counters(profile_cpu, target_pid, forks);
 	} else {
 		nr_cpus = read_cpu_map();
 		for (i = 0; i < nr_cpus; i++)
-			open_counters(cpumap[i], target_pid);
+			open_counters(cpumap[i], target_pid, forks);
 	}
 
 	if (file_new) {
-- 
1.6.2.5

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

[Arnaldo Carvalho de Melo]

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 annotate with just /proc/kallsyms
+	 */
+	if (map->dso->origin == DSO__ORIG_KERNEL)
+		return -1;
 
 	if (syme->src == NULL) {
 		syme->src = zalloc(sizeof(*source));
 		if (syme->src == NULL)
-			return;
+			return -1;
 		pthread_mutex_init(&syme->src->lock, NULL);
 	}
 
@@ -195,9 +204,6 @@ static void parse_source(struct sym_entry *syme)
 		pthread_mutex_lock(&source->lock);
 		goto out_assign;
 	}
-
-	sym = sym_entry__symbol(syme);
-	map = syme->map;
 	path = map->dso->long_name;
 
 	len = sym->end - sym->start;
@@ -209,7 +215,7 @@ static void parse_source(struct sym_entry *syme)
 
 	file = popen(command, "r");
 	if (!file)
-		return;
+		return -1;
 
 	pthread_mutex_lock(&source->lock);
 	source->lines_tail = &source->lines;
@@ -245,6 +251,7 @@ static void parse_source(struct sym_entry *syme)
 out_assign:
 	sym_filter_entry = syme;
 	pthread_mutex_unlock(&source->lock);
+	return 0;
 }
 
 static void __zero_source_counters(struct sym_entry *syme)
@@ -991,7 +998,17 @@ static void event__process_sample(const event_t *self,
 	if (sym_filter_entry_sched) {
 		sym_filter_entry = sym_filter_entry_sched;
 		sym_filter_entry_sched = NULL;
-		parse_source(sym_filter_entry);
+		if (parse_source(sym_filter_entry) < 0) {
+			struct symbol *sym = sym_entry__symbol(sym_filter_entry);
+
+			pr_err("Can't annotate %s", sym->name);
+			if (sym_filter_entry->map->dso->origin == DSO__ORIG_KERNEL) {
+				pr_err(": No vmlinux file was found in the path:\n");
+				vmlinux_path__fprintf(stderr);
+			} else
+				pr_err(".\n");
+			exit(1);
+		}
 	}
 
 	syme = symbol__priv(al.sym);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 75cd468..292f941 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -18,18 +18,6 @@
 #define NT_GNU_BUILD_ID 3
 #endif
 
-enum dso_origin {
-	DSO__ORIG_KERNEL = 0,
-	DSO__ORIG_JAVA_JIT,
-	DSO__ORIG_BUILD_ID_CACHE,
-	DSO__ORIG_FEDORA,
-	DSO__ORIG_UBUNTU,
-	DSO__ORIG_BUILDID,
-	DSO__ORIG_DSO,
-	DSO__ORIG_KMODULE,
-	DSO__ORIG_NOT_FOUND,
-};
-
 static void dsos__add(struct list_head *head, struct dso *dso);
 static struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
 static int dso__load_kernel_sym(struct dso *self, struct map *map,
@@ -1017,7 +1005,7 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
 				}
 				curr_map->map_ip = identity__map_ip;
 				curr_map->unmap_ip = identity__map_ip;
-				curr_dso->origin = DSO__ORIG_KERNEL;
+				curr_dso->origin = self->origin;
 				map_groups__insert(kmap->kmaps, curr_map);
 				dsos__add(&dsos__kernel, curr_dso);
 				dso__set_loaded(curr_dso, map->type);
@@ -1887,6 +1875,17 @@ out_fail:
 	return -1;
 }
 
+size_t vmlinux_path__fprintf(FILE *fp)
+{
+	int i;
+	size_t printed = 0;
+
+	for (i = 0; i < vmlinux_path__nr_entries; ++i)
+		printed += fprintf(fp, "[%d] %s\n", i, vmlinux_path[i]);
+
+	return printed;
+}
+
 static int setup_list(struct strlist **list, const char *list_str,
 		      const char *list_name)
 {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 280dadd..d983bba 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -149,6 +149,19 @@ size_t dsos__fprintf_buildid(FILE *fp, bool with_hits);
 
 size_t dso__fprintf_buildid(struct dso *self, FILE *fp);
 size_t dso__fprintf(struct dso *self, enum map_type type, FILE *fp);
+
+enum dso_origin {
+	DSO__ORIG_KERNEL = 0,
+	DSO__ORIG_JAVA_JIT,
+	DSO__ORIG_BUILD_ID_CACHE,
+	DSO__ORIG_FEDORA,
+	DSO__ORIG_UBUNTU,
+	DSO__ORIG_BUILDID,
+	DSO__ORIG_DSO,
+	DSO__ORIG_KMODULE,
+	DSO__ORIG_NOT_FOUND,
+};
+
 char dso__symtab_origin(const struct dso *self);
 void dso__set_long_name(struct dso *self, char *name);
 void dso__set_build_id(struct dso *self, void *build_id);
@@ -168,4 +181,6 @@ int kallsyms__parse(const char *filename, void *arg,
 int symbol__init(void);
 bool symbol_type__is_a(char symbol_type, enum map_type map_type);
 
+size_t vmlinux_path__fprintf(FILE *fp);
+
 #endif /* __PERF_SYMBOL */
-- 
1.6.2.5

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

[David Miller]

From: Arnaldo Carvalho de Melo <acme@infradead.org>
Date: Mon, 15 Mar 2010 11:46:58 -0300

> [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]#

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?

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

[Arnaldo Carvalho de Melo]

Em Mon, Mar 15, 2010 at 11:55:26AM -0700, David Miller escreveu:
> From: Arnaldo Carvalho de Melo <acme@infradead.org>
> Date: Mon, 15 Mar 2010 11:46:58 -0300
> > [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]#
> 
> 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?

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

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

[David Miller]

From: Arnaldo Carvalho de Melo <acme@infradead.org>
Date: Mon, 15 Mar 2010 16:21:26 -0300

> Em Mon, Mar 15, 2010 at 11:55:26AM -0700, David Miller escreveu:
>> 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?
> 
> 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.

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.

That's what I was trying to achieve when I sent you the report
:-)

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

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Arnaldo Carvalho de Melo <acme@infradead.org>
> Date: Mon, 15 Mar 2010 16:21:26 -0300
> 
> > Em Mon, Mar 15, 2010 at 11:55:26AM -0700, David Miller escreveu:
> >> 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?
> > 
> > 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.
> 
> 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.
> 
> That's what I was trying to achieve when I sent you the report
> :-)

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

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

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 16 Mar 2010 10:46:11 +0100

> 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. )

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 :-)

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

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Tue, 16 Mar 2010 10:46:11 +0100
> 
> > 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. )
> 
> That's interesting and useful, but wouldn't help my case as the images I'm 
> usually booting are completely stripped.

/me too

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

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

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

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 16 Mar 2010 10:55:23 +0100

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

Absolutely agreed.

[tip:perf/core] perf record: Enable the enable_on_exec flag if record forks the target

[tip-bot for Eric B Munson]

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:
 	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, target_pid);
+		open_counters(profile_cpu, target_pid, forks);
 	} else {
 		nr_cpus = read_cpu_map();
 		for (i = 0; i < nr_cpus; i++)
-			open_counters(cpumap[i], target_pid);
+			open_counters(cpumap[i], target_pid, forks);
 	}
 
 	if (file_new) {