All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/9] perf tools: add support for reading object code
@ 2013-07-30 21:13 Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Hi

Here are some patches that add support for reading object code from vmlinux,
kernel modules and /proc/kcore.

Changes in V2:
	Re-based on Arnaldo's tree's perf/core branch

	perf tools: add test for reading object code
		Use strchr in read_objdump_line()
		Remove unused return value of read_objdump_line()
		Fix double space before "cycles:u"
		Add missing perf_evlist__delete(evlist) in do_test_code_reading()
	perf tools: load kernel maps before using
		Correct spelling of "initialization" in commit message
	perf tools: add support for reading from /proc/kcore
		Do not test kallsyms filename for host buildid because
		/proc/kallsyms is always used if the dso buildid matches
		the host
	perf tools: add kcore to the object code reading test
		Remove redundant "else if (!have_kcore && try_kcore)" clause


Adrian Hunter (9):
      perf tools: add test for reading object code
      perf tools: load kernel maps before using
      perf tools: make it possible to read object code from vmlinux
      perf tools: adjust the vmlinux symtab matches kallsyms test                                                                   
      perf tools: avoid SyS kernel syscall aliases                                                                                  
      perf tools: make it possible to read object code from kernel modules                                                          
      perf tools: add support for reading from /proc/kcore                                                                          
      perf tools: add kcore to the object code reading test                                                                         
      perf tools: allow annotation using /proc/kcore                                                                                
                                                                                                                                    
 tools/perf/Makefile                 |   1 +                                                                                        
 tools/perf/builtin-inject.c         |   2 +-                                                                                       
 tools/perf/builtin-script.c         |   4 +-                                                                                       
 tools/perf/builtin-top.c            |   3 +-                                                                                       
 tools/perf/tests/builtin-test.c     |   4 +                                                                                        
 tools/perf/tests/code-reading.c     | 556 ++++++++++++++++++++++++++++++++++++                                                     
 tools/perf/tests/tests.h            |   1 +                                                                                        
 tools/perf/tests/vmlinux-kallsyms.c |  32 ++-                                                                                      
 tools/perf/util/annotate.c          |  13 +-                                                                                       
 tools/perf/util/build-id.c          |   2 +-                                                                                       
 tools/perf/util/dso.c               |  10 +-                                                                                       
 tools/perf/util/dso.h               |  17 ++                                                                                       
 tools/perf/util/event.c             |  18 +-                                                                                       
 tools/perf/util/machine.c           |  20 +-                                                                                       
 tools/perf/util/map.c               |  67 ++---                                                                                    
 tools/perf/util/map.h               |  13 +                                                                                        
 tools/perf/util/symbol-elf.c        | 166 ++++++++++-                                                                              
 tools/perf/util/symbol-minimal.c    |   7 +                                                                                        
 tools/perf/util/symbol.c            | 273 ++++++++++++++++--                                                                       
 tools/perf/util/symbol.h            |   5 +
 tools/perf/util/thread.h            |   2 +-
 tools/perf/util/unwind.c            |   4 +-
 22 files changed, 1120 insertions(+), 100 deletions(-)
 create mode 100644 tools/perf/tests/code-reading.c


Regards
Adrian

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-31 14:17   ` Arnaldo Carvalho de Melo
  2013-07-31 14:58   ` Arnaldo Carvalho de Melo
  2013-07-30 21:13 ` [PATCH V2 2/9] perf tools: load kernel maps before using Adrian Hunter
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Using the information in mmap events, perf tools can read object
code associated with sampled addresses.  A test is added that
compares bytes read by perf with the same bytes read using
objdump.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Makefile             |   1 +
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/code-reading.c | 501 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 507 insertions(+)
 create mode 100644 tools/perf/tests/code-reading.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index bfd12d0..e0d3d9f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -392,6 +392,7 @@ LIB_OBJS += $(OUTPUT)tests/sw-clock.o
 ifeq ($(ARCH),x86)
 LIB_OBJS += $(OUTPUT)tests/perf-time-to-tsc.o
 endif
+LIB_OBJS += $(OUTPUT)tests/code-reading.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index b7b4049..f5af192 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -100,6 +100,10 @@ static struct test {
 	},
 #endif
 	{
+		.desc = "Test object code reading",
+		.func = test__code_reading,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
new file mode 100644
index 0000000..7c9437d
--- /dev/null
+++ b/tools/perf/tests/code-reading.c
@@ -0,0 +1,501 @@
+#include <sys/types.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <inttypes.h>
+#include <ctype.h>
+#include <string.h>
+
+#include "parse-events.h"
+#include "evlist.h"
+#include "evsel.h"
+#include "thread_map.h"
+#include "cpumap.h"
+#include "machine.h"
+#include "event.h"
+#include "thread.h"
+
+#include "tests.h"
+
+#define BUFSZ	1024
+#define READLEN	128
+
+static unsigned int hex(char c)
+{
+	if (c >= '0' && c <= '9')
+		return c - '0';
+	if (c >= 'a' && c <= 'f')
+		return c - 'a' + 10;
+	return c - 'A' + 10;
+}
+
+static void read_objdump_line(const char *line, size_t line_len, void **buf,
+			      size_t *len)
+{
+	const char *p;
+	size_t i;
+
+	/* Skip to a colon */
+	p = strchr(line, ':');
+	if (!p)
+		return;
+	i = p + 1 - line;
+
+	/* Read bytes */
+	while (*len) {
+		char c1, c2;
+
+		/* Skip spaces */
+		for (; i < line_len; i++) {
+			if (!isspace(line[i]))
+				break;
+		}
+		/* Get 2 hex digits */
+		if (i >= line_len || !isxdigit(line[i]))
+			break;
+		c1 = line[i++];
+		if (i >= line_len || !isxdigit(line[i]))
+			break;
+		c2 = line[i++];
+		/* Followed by a space */
+		if (i < line_len && line[i] && !isspace(line[i]))
+			break;
+		/* Store byte */
+		*(unsigned char *)*buf = (hex(c1) << 4) | hex(c2);
+		*buf += 1;
+		*len -= 1;
+	}
+}
+
+static int read_objdump_output(FILE *f, void **buf, size_t *len)
+{
+	char *line = NULL;
+	size_t line_len;
+	ssize_t ret;
+	int err = 0;
+
+	while (1) {
+		ret = getline(&line, &line_len, f);
+		if (feof(f))
+			break;
+		if (ret < 0) {
+			pr_debug("getline failed\n");
+			err = -1;
+			break;
+		}
+		read_objdump_line(line, ret, buf, len);
+	}
+
+	free(line);
+
+	return err;
+}
+
+static int read_via_objdump(const char *filename, u64 addr, void *buf,
+			    size_t len)
+{
+	char cmd[PATH_MAX * 2];
+	const char *fmt;
+	FILE *f;
+	int ret;
+
+	fmt = "%s -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
+	ret = snprintf(cmd, sizeof(cmd), fmt, "objdump", addr, addr + len,
+		       filename);
+	if (ret <= 0 || (size_t)ret >= sizeof(cmd))
+		return -1;
+
+	pr_debug("Objdump command is: %s\n", cmd);
+
+	f = popen(cmd, "r");
+	if (!f) {
+		pr_debug("popen failed\n");
+		return -1;
+	}
+
+	ret = read_objdump_output(f, &buf, &len);
+	if (len) {
+		pr_debug("objdump read too few bytes\n");
+		if (!ret)
+			ret = len;
+	}
+
+	pclose(f);
+
+	return ret;
+}
+
+static int read_object_code(u64 addr, size_t len, u8 cpumode,
+			    struct thread *thread, struct machine *machine)
+{
+	struct addr_location al;
+	unsigned char buf1[BUFSZ];
+	unsigned char buf2[BUFSZ];
+	size_t ret_len;
+	u64 objdump_addr;
+	int ret;
+
+	pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
+
+	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION, addr,
+			      &al);
+	if (!al.map || !al.map->dso) {
+		pr_debug("thread__find_addr_map failed\n");
+		return -1;
+	}
+
+	pr_debug("File is: %s\n", al.map->dso->long_name);
+
+	if (al.map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
+		pr_debug("Unexpected kernel address - skipping\n");
+		return 0;
+	}
+
+	pr_debug("On file address is: %#"PRIx64"\n", al.addr);
+
+	if (len > BUFSZ)
+		len = BUFSZ;
+
+	/* Do not go off the map */
+	if (addr + len > al.map->end)
+		len = al.map->end - addr;
+
+	/* Read the object code using perf */
+	ret_len = dso__data_read_offset(al.map->dso, machine, al.addr, buf1,
+					len);
+	if (ret_len != len) {
+		pr_debug("dso__data_read_offset failed\n");
+		return -1;
+	}
+
+	/*
+	 * Converting addresses for use by objdump requires more information.
+	 * map__load() does that.  See map__rip_2objdump() for details.
+	 */
+	if (map__load(al.map, NULL))
+		return -1;
+
+	/* Read the object code using objdump */
+	objdump_addr = map__rip_2objdump(al.map, al.addr);
+	ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len);
+	if (ret > 0) {
+		/*
+		 * The kernel maps are inaccurate - assume objdump is right in
+		 * that case.
+		 */
+		if (cpumode == PERF_RECORD_MISC_KERNEL ||
+		    cpumode == PERF_RECORD_MISC_GUEST_KERNEL) {
+			len -= ret;
+			if (len)
+				pr_debug("Reducing len to %zu\n", len);
+			else
+				return -1;
+		}
+	}
+	if (ret < 0) {
+		pr_debug("read_via_objdump failed\n");
+		return -1;
+	}
+
+	/* The results should be identical */
+	if (memcmp(buf1, buf2, len)) {
+		pr_debug("Bytes read differ from those read by objdump\n");
+		return -1;
+	}
+	pr_debug("Bytes read match those read by objdump\n");
+
+	return 0;
+}
+
+static int process_sample_event(struct machine *machine,
+				struct perf_evlist *evlist,
+				union perf_event *event)
+{
+	struct perf_sample sample;
+	struct thread *thread;
+	u8 cpumode;
+
+	if (perf_evlist__parse_sample(evlist, event, &sample)) {
+		pr_debug("perf_evlist__parse_sample failed\n");
+		return -1;
+	}
+
+	thread = machine__findnew_thread(machine, sample.pid);
+	if (!thread) {
+		pr_debug("machine__findnew_thread failed\n");
+		return -1;
+	}
+
+	cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+
+	return read_object_code(sample.ip, READLEN, cpumode, thread, machine);
+}
+
+static int process_event(struct machine *machine, struct perf_evlist *evlist,
+			 union perf_event *event)
+{
+	if (event->header.type == PERF_RECORD_SAMPLE)
+		return process_sample_event(machine, evlist, event);
+
+	if (event->header.type < PERF_RECORD_MAX)
+		return machine__process_event(machine, event);
+
+	return 0;
+}
+
+static int process_events(struct machine *machine, struct perf_evlist *evlist)
+{
+	union perf_event *event;
+	int i, ret;
+
+	for (i = 0; i < evlist->nr_mmaps; i++) {
+		while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) {
+			ret = process_event(machine, evlist, event);
+			if (ret < 0)
+				return ret;
+		}
+	}
+	return 0;
+}
+
+static int comp(const void *a, const void *b)
+{
+	return *(int *)a - *(int *)b;
+}
+
+static void sort_something(void)
+{
+	size_t sz = 40960;
+	int buf[sz], i;
+
+	for (i = 0; i < (int)sz; i++)
+		buf[i] = sz - i - 1;
+
+	qsort(buf, sz, sizeof(int), comp);
+
+	for (i = 0; i < (int)sz; i++) {
+		if (buf[i] != i) {
+			pr_debug("qsort failed\n");
+			break;
+		}
+	}
+}
+
+static void syscall_something(void)
+{
+	int pipefd[2];
+	int i;
+
+	for (i = 0; i < 1000; i++) {
+		if (pipe(pipefd) < 0) {
+			pr_debug("pipe failed\n");
+			break;
+		}
+		close(pipefd[1]);
+		close(pipefd[0]);
+	}
+}
+
+static void fs_something(void)
+{
+	const char *test_file_name = "temp-perf-code-reading-test-file--";
+	FILE *f;
+	int i;
+
+	for (i = 0; i < 100; i++) {
+		f = fopen(test_file_name, "w+");
+		if (f) {
+			fclose(f);
+			unlink(test_file_name);
+		}
+	}
+}
+
+static void do_something(void)
+{
+	fs_something();
+
+	sort_something();
+
+	syscall_something();
+}
+
+enum {
+	TEST_CODE_READING_OK,
+	TEST_CODE_READING_NO_VMLINUX,
+	TEST_CODE_READING_NO_ACCESS,
+};
+
+static int do_test_code_reading(void)
+{
+	struct machines machines;
+	struct machine *machine;
+	struct thread *thread;
+	struct perf_record_opts opts = {
+		.mmap_pages	     = UINT_MAX,
+		.user_freq	     = UINT_MAX,
+		.user_interval	     = ULLONG_MAX,
+		.freq		     = 40000,
+		.target		     = {
+			.uses_mmap   = true,
+		},
+	};
+	struct thread_map *threads = NULL;
+	struct cpu_map *cpus = NULL;
+	struct perf_evlist *evlist = NULL;
+	struct perf_evsel *evsel = NULL;
+	int err = -1, ret;
+	pid_t pid;
+	struct map *map;
+	bool have_vmlinux, excl_kernel = false;
+
+	pid = getpid();
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	ret = machine__create_kernel_maps(machine);
+	if (ret < 0) {
+		pr_debug("machine__create_kernel_maps failed\n");
+		goto out_err;
+	}
+
+	/* Load kernel map */
+	map = machine->vmlinux_maps[MAP__FUNCTION];
+	ret = map__load(map, NULL);
+	if (ret < 0) {
+		pr_debug("map__load failed\n");
+		goto out_err;
+	}
+	have_vmlinux = map->dso->symtab_type == DSO_BINARY_TYPE__VMLINUX;
+	/* No point getting kernel events if there is no vmlinux */
+	if (!have_vmlinux)
+		excl_kernel = true;
+
+	threads = thread_map__new_by_tid(pid);
+	if (!threads) {
+		pr_debug("thread_map__new_by_tid failed\n");
+		goto out_err;
+	}
+
+	ret = perf_event__synthesize_thread_map(NULL, threads,
+						perf_event__process, machine);
+	if (ret < 0) {
+		pr_debug("perf_event__synthesize_thread_map failed\n");
+		goto out_err;
+	}
+
+	thread = machine__findnew_thread(machine, pid);
+	if (!thread) {
+		pr_debug("machine__findnew_thread failed\n");
+		goto out_err;
+	}
+
+	cpus = cpu_map__new(NULL);
+	if (!cpus) {
+		pr_debug("cpu_map__new failed\n");
+		goto out_err;
+	}
+
+	while (1) {
+		const char *str;
+
+		evlist = perf_evlist__new();
+		if (!evlist) {
+			pr_debug("perf_evlist__new failed\n");
+			goto out_err;
+		}
+
+		perf_evlist__set_maps(evlist, cpus, threads);
+
+		if (excl_kernel)
+			str = "cycles:u";
+		else
+			str = "cycles";
+		pr_debug("Parsing event '%s'\n", str);
+		ret = parse_events(evlist, str);
+		if (ret < 0) {
+			pr_debug("parse_events failed\n");
+			goto out_err;
+		}
+
+		perf_evlist__config(evlist, &opts);
+
+		evsel = perf_evlist__first(evlist);
+
+		evsel->attr.comm = 1;
+		evsel->attr.disabled = 1;
+		evsel->attr.enable_on_exec = 0;
+
+		ret = perf_evlist__open(evlist);
+		if (ret < 0) {
+			if (!excl_kernel) {
+				excl_kernel = true;
+				perf_evlist__delete(evlist);
+				continue;
+			}
+			pr_debug("perf_evlist__open failed\n");
+			goto out_err;
+		}
+		break;
+	}
+
+	ret = perf_evlist__mmap(evlist, UINT_MAX, false);
+	if (ret < 0) {
+		pr_debug("perf_evlist__mmap failed\n");
+		goto out_err;
+	}
+
+	perf_evlist__enable(evlist);
+
+	do_something();
+
+	perf_evlist__disable(evlist);
+
+	ret = process_events(machine, evlist);
+	if (ret < 0)
+		goto out_err;
+
+	if (!have_vmlinux)
+		err = TEST_CODE_READING_NO_VMLINUX;
+	else if (excl_kernel)
+		err = TEST_CODE_READING_NO_ACCESS;
+	else
+		err = TEST_CODE_READING_OK;
+out_err:
+	if (evlist) {
+		perf_evlist__disable(evlist);
+		perf_evlist__munmap(evlist);
+		perf_evlist__close(evlist);
+		perf_evlist__delete(evlist);
+	}
+	if (cpus)
+		cpu_map__delete(cpus);
+	if (threads)
+		thread_map__delete(threads);
+	machines__destroy_kernel_maps(&machines);
+	machine__delete_threads(machine);
+	machines__exit(&machines);
+
+	return err;
+}
+
+int test__code_reading(void)
+{
+	int ret;
+
+	ret = do_test_code_reading();
+
+	switch (ret) {
+	case TEST_CODE_READING_OK:
+		return 0;
+	case TEST_CODE_READING_NO_VMLINUX:
+		fprintf(stderr, " (no vmlinux)");
+		return 0;
+	case TEST_CODE_READING_NO_ACCESS:
+		fprintf(stderr, " (no access)");
+		return 0;
+	default:
+		return -1;
+	};
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index d22202a..c748f53 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -36,5 +36,6 @@ int test__bp_signal_overflow(void);
 int test__task_exit(void);
 int test__sw_clock_freq(void);
 int test__perf_time_to_tsc(void);
+int test__code_reading(void);
 
 #endif /* TESTS_H */
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 2/9] perf tools: load kernel maps before using
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-31 14:35   ` Arnaldo Carvalho de Melo
  2013-07-30 21:13 ` [PATCH V2 3/9] perf tools: make it possible to read object code from vmlinux Adrian Hunter
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

In order to use kernel maps to read object code, those
maps must be adjusted to map to the dso file offset.
Because lazy-initialization is used, that is not done
until symbols are loaded.  However the maps are first
used by thread__find_addr_map() before symbols are loaded.
So this patch changes thread__find_addr() to "load" kernel
maps before using them.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-inject.c     |  2 +-
 tools/perf/builtin-script.c     |  4 ++--
 tools/perf/tests/code-reading.c |  2 +-
 tools/perf/util/build-id.c      |  2 +-
 tools/perf/util/event.c         | 18 ++++++++++++++----
 tools/perf/util/thread.h        |  2 +-
 tools/perf/util/unwind.c        |  4 ++--
 7 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 1d8de2e..f012a98 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -206,7 +206,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
 	}
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
-			      event->ip.ip, &al);
+			      event->ip.ip, &al, NULL);
 
 	if (al.map != NULL) {
 		if (!al.map->dso->hit) {
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1cad370..cd616ff 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -341,10 +341,10 @@ static void print_sample_addr(union perf_event *event,
 		return;
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
-			      sample->addr, &al);
+			      sample->addr, &al, NULL);
 	if (!al.map)
 		thread__find_addr_map(thread, machine, cpumode, MAP__VARIABLE,
-				      sample->addr, &al);
+				      sample->addr, &al, NULL);
 
 	al.cpu = sample->cpu;
 	al.sym = NULL;
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 7c9437d..6cd3190 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -138,7 +138,7 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 	pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION, addr,
-			      &al);
+			      &al, NULL);
 	if (!al.map || !al.map->dso) {
 		pr_debug("thread__find_addr_map failed\n");
 		return -1;
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 5295625..3a0f508 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -33,7 +33,7 @@ int build_id__mark_dso_hit(struct perf_tool *tool __maybe_unused,
 	}
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
-			      event->ip.ip, &al);
+			      event->ip.ip, &al, NULL);
 
 	if (al.map != NULL)
 		al.map->dso->hit = 1;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9541270..cc7c0c9 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -592,9 +592,10 @@ int perf_event__process(struct perf_tool *tool __maybe_unused,
 void thread__find_addr_map(struct thread *self,
 			   struct machine *machine, u8 cpumode,
 			   enum map_type type, u64 addr,
-			   struct addr_location *al)
+			   struct addr_location *al, symbol_filter_t filter)
 {
 	struct map_groups *mg = &self->mg;
+	bool load_map = false;
 
 	al->thread = self;
 	al->addr = addr;
@@ -609,11 +610,13 @@ void thread__find_addr_map(struct thread *self,
 	if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
 		al->level = 'k';
 		mg = &machine->kmaps;
+		load_map = true;
 	} else if (cpumode == PERF_RECORD_MISC_USER && perf_host) {
 		al->level = '.';
 	} else if (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest) {
 		al->level = 'g';
 		mg = &machine->kmaps;
+		load_map = true;
 	} else {
 		/*
 		 * 'u' means guest os user space.
@@ -654,8 +657,15 @@ try_again:
 			mg = &machine->kmaps;
 			goto try_again;
 		}
-	} else
+	} else {
+		/*
+		 * Kernel maps might be changed when loading symbols so loading
+		 * must be done prior to using kernel maps.
+		 */
+		if (load_map)
+			map__load(al->map, filter);
 		al->addr = al->map->map_ip(al->map, al->addr);
+	}
 }
 
 void thread__find_addr_location(struct thread *thread, struct machine *machine,
@@ -663,7 +673,7 @@ void thread__find_addr_location(struct thread *thread, struct machine *machine,
 				struct addr_location *al,
 				symbol_filter_t filter)
 {
-	thread__find_addr_map(thread, machine, cpumode, type, addr, al);
+	thread__find_addr_map(thread, machine, cpumode, type, addr, al, filter);
 	if (al->map != NULL)
 		al->sym = map__find_symbol(al->map, al->addr, filter);
 	else
@@ -699,7 +709,7 @@ int perf_event__preprocess_sample(const union perf_event *event,
 		machine__create_kernel_maps(machine);
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
-			      event->ip.ip, al);
+			      event->ip.ip, al, filter);
 	dump_printf(" ...... dso: %s\n",
 		    al->map ? al->map->dso->long_name :
 			al->level == 'H' ? "[hypervisor]" : "<not found>");
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 0fe1f9c..f98d1d9 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -41,7 +41,7 @@ static inline struct map *thread__find_map(struct thread *self,
 
 void thread__find_addr_map(struct thread *thread, struct machine *machine,
 			   u8 cpumode, enum map_type type, u64 addr,
-			   struct addr_location *al);
+			   struct addr_location *al, symbol_filter_t filter);
 
 void thread__find_addr_location(struct thread *thread, struct machine *machine,
 				u8 cpumode, enum map_type type, u64 addr,
diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
index 958723b..5bbd494 100644
--- a/tools/perf/util/unwind.c
+++ b/tools/perf/util/unwind.c
@@ -272,7 +272,7 @@ static struct map *find_map(unw_word_t ip, struct unwind_info *ui)
 	struct addr_location al;
 
 	thread__find_addr_map(ui->thread, ui->machine, PERF_RECORD_MISC_USER,
-			      MAP__FUNCTION, ip, &al);
+			      MAP__FUNCTION, ip, &al, NULL);
 	return al.map;
 }
 
@@ -349,7 +349,7 @@ static int access_dso_mem(struct unwind_info *ui, unw_word_t addr,
 	ssize_t size;
 
 	thread__find_addr_map(ui->thread, ui->machine, PERF_RECORD_MISC_USER,
-			      MAP__FUNCTION, addr, &al);
+			      MAP__FUNCTION, addr, &al, NULL);
 	if (!al.map) {
 		pr_debug("unwind: no map for %lx\n", (unsigned long)addr);
 		return -1;
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 3/9] perf tools: make it possible to read object code from vmlinux
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 2/9] perf tools: load kernel maps before using Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test Adrian Hunter
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

The new "object code reading" test shows that it is not possible
to read object code from vmlinux.  That is because the mappings
do not map to the dso.  This patch fixes that.

A side-effect of changing the kernel map is that the "reloc"
offset must be taken into account.  As a result of that
separate map functions for relocation are no longer needed.

Also fixing up the maps to match the symbols no longer makes
sense and so is not done.

The vmlinux dso data_type is now set to either
DSO_BINARY_TYPE__VMLINUX or DSO_BINARY_TYPE__GUEST_VMLINUX
as approprite, which enables the correct file name to
be determined by dso__binary_type_file().

This patch breaks the "vmlinux symtab matches kallsyms"
test.  That is fixed in a following patch.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/dso.c        |   4 +-
 tools/perf/util/dso.h        |   8 ++++
 tools/perf/util/machine.c    |   4 +-
 tools/perf/util/map.c        |  35 ---------------
 tools/perf/util/symbol-elf.c | 100 +++++++++++++++++++++++++++++++++++++++----
 tools/perf/util/symbol.c     |  22 +++++-----
 6 files changed, 112 insertions(+), 61 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index c4374f0..121583d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -78,6 +78,8 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type,
 			 symbol_conf.symfs, build_id_hex, build_id_hex + 2);
 		break;
 
+	case DSO_BINARY_TYPE__VMLINUX:
+	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
 		snprintf(file, size, "%s%s",
 			 symbol_conf.symfs, dso->long_name);
@@ -95,9 +97,7 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type,
 
 	default:
 	case DSO_BINARY_TYPE__KALLSYMS:
-	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
-	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__JAVA_JIT:
 	case DSO_BINARY_TYPE__NOT_FOUND:
 		ret = -1;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index d51aaf2..02aadaf 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>
 #include <linux/rbtree.h>
+#include <stdbool.h>
 #include "types.h"
 #include "map.h"
 
@@ -146,4 +147,11 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp);
 size_t dso__fprintf_symbols_by_name(struct dso *dso,
 				    enum map_type type, FILE *fp);
 size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp);
+
+static inline bool dso__is_vmlinux(struct dso *dso)
+{
+	return dso->data_type == DSO_BINARY_TYPE__VMLINUX ||
+	       dso->data_type == DSO_BINARY_TYPE__GUEST_VMLINUX;
+}
+
 #endif /* __PERF_DSO */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f9f9d63..dc35dcf 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -628,10 +628,8 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type,
 	struct map *map = machine->vmlinux_maps[type];
 	int ret = dso__load_vmlinux_path(map->dso, map, filter);
 
-	if (ret > 0) {
+	if (ret > 0)
 		dso__set_loaded(map->dso, type);
-		map__reloc_vmlinux(map);
-	}
 
 	return ret;
 }
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 8bcdf9e..5f662a3 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -182,12 +182,6 @@ int map__load(struct map *map, symbol_filter_t filter)
 #endif
 		return -1;
 	}
-	/*
-	 * Only applies to the kernel, as its symtabs aren't relative like the
-	 * module ones.
-	 */
-	if (map->dso->kernel)
-		map__reloc_vmlinux(map);
 
 	return 0;
 }
@@ -513,35 +507,6 @@ int map_groups__clone(struct map_groups *mg,
 	return 0;
 }
 
-static u64 map__reloc_map_ip(struct map *map, u64 ip)
-{
-	return ip + (s64)map->pgoff;
-}
-
-static u64 map__reloc_unmap_ip(struct map *map, u64 ip)
-{
-	return ip - (s64)map->pgoff;
-}
-
-void map__reloc_vmlinux(struct map *map)
-{
-	struct kmap *kmap = map__kmap(map);
-	s64 reloc;
-
-	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->unrelocated_addr)
-		return;
-
-	reloc = (kmap->ref_reloc_sym->unrelocated_addr -
-		 kmap->ref_reloc_sym->addr);
-
-	if (!reloc)
-		return;
-
-	map->map_ip   = map__reloc_map_ip;
-	map->unmap_ip = map__reloc_unmap_ip;
-	map->pgoff    = reloc;
-}
-
 void maps__insert(struct rb_root *maps, struct map *map)
 {
 	struct rb_node **p = &maps->rb_node;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 4b12bf8..ed6f443 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -603,7 +603,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 						     ".gnu.prelink_undo",
 						     NULL) != NULL);
 	} else {
-		ss->adjust_symbols = 0;
+		ss->adjust_symbols = ehdr.e_type == ET_EXEC;
 	}
 
 	ss->name   = strdup(name);
@@ -624,6 +624,37 @@ out_close:
 	return err;
 }
 
+/**
+ * ref_reloc_sym_not_found - has kernel relocation symbol been found.
+ * @kmap: kernel maps and relocation reference symbol
+ *
+ * This function returns %true if we are dealing with the kernel maps and the
+ * relocation reference symbol has not yet been found.  Otherwise %false is
+ * returned.
+ */
+static bool ref_reloc_sym_not_found(struct kmap *kmap)
+{
+	return kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
+	       !kmap->ref_reloc_sym->unrelocated_addr;
+}
+
+/**
+ * ref_reloc - kernel relocation offset.
+ * @kmap: kernel maps and relocation reference symbol
+ *
+ * This function returns the offset of kernel addresses as determined by using
+ * the relocation reference symbol i.e. if the kernel has not been relocated
+ * then the return value is zero.
+ */
+static u64 ref_reloc(struct kmap *kmap)
+{
+	if (kmap && kmap->ref_reloc_sym &&
+	    kmap->ref_reloc_sym->unrelocated_addr)
+		return kmap->ref_reloc_sym->addr -
+		       kmap->ref_reloc_sym->unrelocated_addr;
+	return 0;
+}
+
 int dso__load_sym(struct dso *dso, struct map *map,
 		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
 		  symbol_filter_t filter, int kmodule)
@@ -642,6 +673,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	Elf_Scn *sec, *sec_strndx;
 	Elf *elf;
 	int nr = 0;
+	bool remap_kernel = false, adjust_kernel_syms = false;
 
 	dso->symtab_type = syms_ss->type;
 
@@ -681,7 +713,31 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	dso->adjust_symbols = runtime_ss->adjust_symbols;
+
+	/*
+	 * The kernel relocation symbol is needed in advance in order to adjust
+	 * kernel maps correctly.
+	 */
+	if (ref_reloc_sym_not_found(kmap)) {
+		elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
+			const char *elf_name = elf_sym__name(&sym, symstrs);
+
+			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
+				continue;
+			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
+			break;
+		}
+	}
+
+	dso->adjust_symbols = runtime_ss->adjust_symbols || ref_reloc(kmap);
+	/*
+	 * Initial kernel and module mappings do not map to the dso.  For
+	 * function mappings, flag the fixups.
+	 */
+	if (map->type == MAP__FUNCTION && (dso->kernel || kmodule)) {
+		remap_kernel = true;
+		adjust_kernel_syms = dso->adjust_symbols;
+	}
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
@@ -690,10 +746,6 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		const char *section_name;
 		bool used_opd = false;
 
-		if (kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
-		    strcmp(elf_name, kmap->ref_reloc_sym->name) == 0)
-			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
-
 		if (!is_label && !elf_sym__is_a(&sym, map->type))
 			continue;
 
@@ -745,15 +797,37 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		    (sym.st_value & 1))
 			--sym.st_value;
 
-		if (dso->kernel != DSO_TYPE_USER || kmodule) {
+		if (dso->kernel || kmodule) {
 			char dso_name[PATH_MAX];
 
+			/* Adjust symbol to map to file offset */
+			if (adjust_kernel_syms)
+				sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+
 			if (strcmp(section_name,
 				   (curr_dso->short_name +
 				    dso->short_name_len)) == 0)
 				goto new_symbol;
 
 			if (strcmp(section_name, ".text") == 0) {
+				/*
+				 * The initial kernel mapping is based on
+				 * kallsyms and identity maps.  Overwrite it to
+				 * map to the kernel dso.
+				 */
+				if (remap_kernel && dso->kernel) {
+					remap_kernel = false;
+					map->start = shdr.sh_addr +
+						     ref_reloc(kmap);
+					map->end = map->start + shdr.sh_size;
+					map->pgoff = shdr.sh_offset;
+					map->map_ip = map__map_ip;
+					map->unmap_ip = map__unmap_ip;
+					/* Ensure maps are correctly ordered */
+					map_groups__remove(kmap->kmaps, map);
+					map_groups__insert(kmap->kmaps, map);
+				}
+
 				curr_map = map;
 				curr_dso = dso;
 				goto new_symbol;
@@ -781,8 +855,16 @@ int dso__load_sym(struct dso *dso, struct map *map,
 					dso__delete(curr_dso);
 					goto out_elf_end;
 				}
-				curr_map->map_ip = identity__map_ip;
-				curr_map->unmap_ip = identity__map_ip;
+				if (adjust_kernel_syms) {
+					curr_map->start = shdr.sh_addr +
+							  ref_reloc(kmap);
+					curr_map->end = curr_map->start +
+							shdr.sh_size;
+					curr_map->pgoff = shdr.sh_offset;
+				} else {
+					curr_map->map_ip = identity__map_ip;
+					curr_map->unmap_ip = identity__map_ip;
+				}
 				curr_dso->symtab_type = dso->symtab_type;
 				map_groups__insert(kmap->kmaps, curr_map);
 				dsos__add(&dso->node, curr_dso);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 02718e7..b407c53 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -906,6 +906,10 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
+		if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
+			dso->data_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
+		else
+			dso->data_type = DSO_BINARY_TYPE__VMLINUX;
 		dso__set_long_name(dso, (char *)vmlinux);
 		dso__set_loaded(dso, map->type);
 		pr_debug("Using %s for symbols\n", symfs_vmlinux);
@@ -978,7 +982,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
 			dso__set_long_name(dso,
 					   strdup(symbol_conf.vmlinux_name));
 			dso->lname_alloc = 1;
-			goto out_fixup;
+			return err;
 		}
 		return err;
 	}
@@ -986,7 +990,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
 	if (vmlinux_path != NULL) {
 		err = dso__load_vmlinux_path(dso, map, filter);
 		if (err > 0)
-			goto out_fixup;
+			return err;
 	}
 
 	/* do not try local files if a symfs was given */
@@ -1047,7 +1051,6 @@ do_kallsyms:
 
 	if (err > 0) {
 		dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
-out_fixup:
 		map__fixup_start(map);
 		map__fixup_end(map);
 	}
@@ -1078,7 +1081,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
 		if (symbol_conf.default_guest_vmlinux_name != NULL) {
 			err = dso__load_vmlinux(dso, map,
 				symbol_conf.default_guest_vmlinux_name, filter);
-			goto out_try_fixup;
+			return err;
 		}
 
 		kallsyms_filename = symbol_conf.default_guest_kallsyms;
@@ -1090,15 +1093,10 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
 	}
 
 	err = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
-	if (err > 0)
-		pr_debug("Using %s for symbols\n", kallsyms_filename);
-
-out_try_fixup:
 	if (err > 0) {
-		if (kallsyms_filename != NULL) {
-			machine__mmap_name(machine, path, sizeof(path));
-			dso__set_long_name(dso, strdup(path));
-		}
+		pr_debug("Using %s for symbols\n", kallsyms_filename);
+		machine__mmap_name(machine, path, sizeof(path));
+		dso__set_long_name(dso, strdup(path));
 		map__fixup_start(map);
 		map__fixup_end(map);
 	}
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (2 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 3/9] perf tools: make it possible to read object code from vmlinux Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 5/9] perf tools: avoid SyS kernel syscall aliases Adrian Hunter
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

vmlinux maps now map to the dso and the symbol values
are now file offsets.  For comparison with kallsyms
the virtual memory address is needed which is obtained
by unmapping the symbol value.

The "vmlinux symtab matches kallsyms" is adjusted
accordingly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/vmlinux-kallsyms.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index add1539..e2e1498 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -25,6 +25,7 @@ int test__vmlinux_matches_kallsyms(void)
 	struct machine kallsyms, vmlinux;
 	enum map_type type = MAP__FUNCTION;
 	struct ref_reloc_sym ref_reloc_sym = { .name = "_stext", };
+	u64 mem_start, mem_end;
 
 	/*
 	 * Step 1:
@@ -123,10 +124,14 @@ int test__vmlinux_matches_kallsyms(void)
 		if (sym->start == sym->end)
 			continue;
 
-		first_pair = machine__find_kernel_symbol(&kallsyms, type, sym->start, NULL, NULL);
+		mem_start = vmlinux_map->unmap_ip(vmlinux_map, sym->start);
+		mem_end = vmlinux_map->unmap_ip(vmlinux_map, sym->end);
+
+		first_pair = machine__find_kernel_symbol(&kallsyms, type,
+							 mem_start, NULL, NULL);
 		pair = first_pair;
 
-		if (pair && pair->start == sym->start) {
+		if (pair && pair->start == mem_start) {
 next_pair:
 			if (strcmp(sym->name, pair->name) == 0) {
 				/*
@@ -138,10 +143,11 @@ next_pair:
 				 * off the real size. More than that and we
 				 * _really_ have a problem.
 				 */
-				s64 skew = sym->end - pair->end;
+				s64 skew = mem_end - pair->end;
 				if (llabs(skew) >= page_size)
 					pr_debug("%#" PRIx64 ": diff end addr for %s v: %#" PRIx64 " k: %#" PRIx64 "\n",
-						 sym->start, sym->name, sym->end, pair->end);
+						 mem_start, sym->name, mem_end,
+						 pair->end);
 
 				/*
 				 * Do not count this as a failure, because we
@@ -159,7 +165,7 @@ detour:
 				if (nnd) {
 					struct symbol *next = rb_entry(nnd, struct symbol, rb_node);
 
-					if (next->start == sym->start) {
+					if (next->start == mem_start) {
 						pair = next;
 						goto next_pair;
 					}
@@ -172,10 +178,11 @@ detour:
 				}
 
 				pr_debug("%#" PRIx64 ": diff name v: %s k: %s\n",
-					 sym->start, sym->name, pair->name);
+					 mem_start, sym->name, pair->name);
 			}
 		} else
-			pr_debug("%#" PRIx64 ": %s not on kallsyms\n", sym->start, sym->name);
+			pr_debug("%#" PRIx64 ": %s not on kallsyms\n",
+				 mem_start, sym->name);
 
 		err = -1;
 	}
@@ -208,16 +215,19 @@ detour:
 	for (nd = rb_first(&vmlinux.kmaps.maps[type]); nd; nd = rb_next(nd)) {
 		struct map *pos = rb_entry(nd, struct map, rb_node), *pair;
 
-		pair = map_groups__find(&kallsyms.kmaps, type, pos->start);
+		mem_start = vmlinux_map->unmap_ip(vmlinux_map, pos->start);
+		mem_end = vmlinux_map->unmap_ip(vmlinux_map, pos->end);
+
+		pair = map_groups__find(&kallsyms.kmaps, type, mem_start);
 		if (pair == NULL || pair->priv)
 			continue;
 
-		if (pair->start == pos->start) {
+		if (pair->start == mem_start) {
 			pair->priv = 1;
 			pr_info(" %" PRIx64 "-%" PRIx64 " %" PRIx64 " %s in kallsyms as",
 				pos->start, pos->end, pos->pgoff, pos->dso->name);
-			if (pos->pgoff != pair->pgoff || pos->end != pair->end)
-				pr_info(": \n*%" PRIx64 "-%" PRIx64 " %" PRIx64 "",
+			if (mem_end != pair->end)
+				pr_info(":\n*%" PRIx64 "-%" PRIx64 " %" PRIx64,
 					pair->start, pair->end, pair->pgoff);
 			pr_info(" %s\n", pair->dso->name);
 			pair->priv = 1;
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 5/9] perf tools: avoid SyS kernel syscall aliases
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (3 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 6/9] perf tools: make it possible to read object code from kernel modules Adrian Hunter
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

When removing duplicate symbols, prefer to remove
syscall aliases starting with SyS or compat_SyS.

A side-effect of that is that it results in slightly
improved results for the "vmlinux symtab matches kallsyms"
test.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b407c53..04300dd 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -87,6 +87,7 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
 {
 	s64 a;
 	s64 b;
+	size_t na, nb;
 
 	/* Prefer a symbol with non zero length */
 	a = syma->end - syma->start;
@@ -120,11 +121,21 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
 	else if (a > b)
 		return SYMBOL_B;
 
-	/* If all else fails, choose the symbol with the longest name */
-	if (strlen(syma->name) >= strlen(symb->name))
+	/* Choose the symbol with the longest name */
+	na = strlen(syma->name);
+	nb = strlen(symb->name);
+	if (na > nb)
 		return SYMBOL_A;
-	else
+	else if (na < nb)
+		return SYMBOL_B;
+
+	/* Avoid "SyS" kernel syscall aliases */
+	if (na >= 3 && !strncmp(syma->name, "SyS", 3))
 		return SYMBOL_B;
+	if (na >= 10 && !strncmp(syma->name, "compat_SyS", 10))
+		return SYMBOL_B;
+
+	return SYMBOL_A;
 }
 
 void symbols__fixup_duplicate(struct rb_root *symbols)
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 6/9] perf tools: make it possible to read object code from kernel modules
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (4 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 5/9] perf tools: avoid SyS kernel syscall aliases Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

The new "object code reading" test shows that it is not possible
to read object code from kernel modules.  That is because the mappings
do not map to the dsos.  This patch fixes that.

This involves identifying and flagging relocatable (ELF type ET_REL) files
(e.g. kernel modules) for symbol adjustment and updating map__rip_2objdump()
accordingly.  The kmodule parameter of dso__load_sym() is taken into use
and the module map altered to map to the dso.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/dso.c        |  1 +
 tools/perf/util/dso.h        |  1 +
 tools/perf/util/map.c        | 14 +++++++++-----
 tools/perf/util/symbol-elf.c | 15 +++++++++++++++
 tools/perf/util/symbol.c     | 11 ++++++++---
 5 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 121583d..1955804 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -419,6 +419,7 @@ struct dso *dso__new(const char *name)
 		dso->symtab_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->data_type   = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->loaded = 0;
+		dso->rel = 0;
 		dso->sorted_by_name = 0;
 		dso->has_build_id = 0;
 		dso->kernel = DSO_TYPE_USER;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 02aadaf..735a837 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -85,6 +85,7 @@ struct dso {
 	u8		 lname_alloc:1;
 	u8		 sorted_by_name;
 	u8		 loaded;
+	u8		 rel;
 	u8		 build_id[BUILD_ID_SIZE];
 	const char	 *short_name;
 	char		 *long_name;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 5f662a3..4d599fe 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -248,14 +248,18 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
 
 /*
  * objdump wants/reports absolute IPs for ET_EXEC, and RIPs for ET_DYN.
- * map->dso->adjust_symbols==1 for ET_EXEC-like cases.
+ * map->dso->adjust_symbols==1 for ET_EXEC-like cases except ET_REL which is
+ * relative to section start.
  */
 u64 map__rip_2objdump(struct map *map, u64 rip)
 {
-	u64 addr = map->dso->adjust_symbols ?
-			map->unmap_ip(map, rip) :	/* RIP -> IP */
-			rip;
-	return addr;
+	if (!map->dso->adjust_symbols)
+		return rip;
+
+	if (map->dso->rel)
+		return rip - map->pgoff;
+
+	return map->unmap_ip(map, rip);
 }
 
 void map_groups__init(struct map_groups *mg)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index ed6f443..f00596f 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -599,6 +599,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 	if (dso->kernel == DSO_TYPE_USER) {
 		GElf_Shdr shdr;
 		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
+				ehdr.e_type == ET_REL ||
 				elf_section_by_name(elf, &ehdr, &shdr,
 						     ".gnu.prelink_undo",
 						     NULL) != NULL);
@@ -676,6 +677,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	bool remap_kernel = false, adjust_kernel_syms = false;
 
 	dso->symtab_type = syms_ss->type;
+	dso->rel = syms_ss->ehdr.e_type == ET_REL;
 
 	if (!syms_ss->symtab) {
 		syms_ss->symtab  = syms_ss->dynsym;
@@ -828,11 +830,24 @@ int dso__load_sym(struct dso *dso, struct map *map,
 					map_groups__insert(kmap->kmaps, map);
 				}
 
+				/*
+				 * The initial module mapping is based on
+				 * /proc/modules mapped to offset zero.
+				 * Overwrite it to map to the module dso.
+				 */
+				if (remap_kernel && kmodule) {
+					remap_kernel = false;
+					map->pgoff = shdr.sh_offset;
+				}
+
 				curr_map = map;
 				curr_dso = dso;
 				goto new_symbol;
 			}
 
+			if (!kmap)
+				goto new_symbol;
+
 			snprintf(dso_name, sizeof(dso_name),
 				 "%s%s", dso->short_name, section_name);
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 04300dd..b9056a8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -854,10 +854,15 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 	if (!runtime_ss && syms_ss)
 		runtime_ss = syms_ss;
 
-	if (syms_ss)
-		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, 0);
-	else
+	if (syms_ss) {
+		int km;
+
+		km = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
+		     dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE;
+		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, km);
+	} else {
 		ret = -1;
+	}
 
 	if (ret > 0) {
 		int nr_plt;
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 7/9] perf tools: add support for reading from /proc/kcore
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (5 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 6/9] perf tools: make it possible to read object code from kernel modules Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 8/9] perf tools: add kcore to the object code reading test Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 9/9] perf tools: allow annotation using /proc/kcore Adrian Hunter
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

In the absence of vmlinux, perf tools uses kallsyms
for symbols.  If the user has access, now also map to
/proc/kcore.

The dso data_type is now set to either
DSO_BINARY_TYPE__KCORE or DSO_BINARY_TYPE__GUEST_KCORE
as approprite.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/dso.c            |   5 +
 tools/perf/util/dso.h            |   8 ++
 tools/perf/util/machine.c        |  16 +++
 tools/perf/util/map.c            |  18 ++++
 tools/perf/util/map.h            |  13 +++
 tools/perf/util/symbol-elf.c     |  51 +++++++++
 tools/perf/util/symbol-minimal.c |   7 ++
 tools/perf/util/symbol.c         | 227 ++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/symbol.h         |   5 +
 9 files changed, 347 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 1955804..e3c1ff8 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -95,6 +95,11 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type,
 			 dso->long_name);
 		break;
 
+	case DSO_BINARY_TYPE__KCORE:
+	case DSO_BINARY_TYPE__GUEST_KCORE:
+		snprintf(file, size, "%s", dso->long_name);
+		break;
+
 	default:
 	case DSO_BINARY_TYPE__KALLSYMS:
 	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 735a837..b793053 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -21,6 +21,8 @@ enum dso_binary_type {
 	DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
 	DSO_BINARY_TYPE__GUEST_KMODULE,
 	DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE,
+	DSO_BINARY_TYPE__KCORE,
+	DSO_BINARY_TYPE__GUEST_KCORE,
 	DSO_BINARY_TYPE__NOT_FOUND,
 };
 
@@ -155,4 +157,10 @@ static inline bool dso__is_vmlinux(struct dso *dso)
 	       dso->data_type == DSO_BINARY_TYPE__GUEST_VMLINUX;
 }
 
+static inline bool dso__is_kcore(struct dso *dso)
+{
+	return dso->data_type == DSO_BINARY_TYPE__KCORE ||
+	       dso->data_type == DSO_BINARY_TYPE__GUEST_KCORE;
+}
+
 #endif /* __PERF_DSO */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index dc35dcf..ef3b49c 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -856,6 +856,18 @@ static void machine__set_kernel_mmap_len(struct machine *machine,
 	}
 }
 
+static bool machine__uses_kcore(struct machine *machine)
+{
+	struct dso *dso;
+
+	list_for_each_entry(dso, &machine->kernel_dsos, node) {
+		if (dso__is_kcore(dso))
+			return true;
+	}
+
+	return false;
+}
+
 static int machine__process_kernel_mmap_event(struct machine *machine,
 					      union perf_event *event)
 {
@@ -864,6 +876,10 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 	enum dso_kernel_type kernel_type;
 	bool is_kernel_mmap;
 
+	/* If we have maps from kcore then we do not need or want any others */
+	if (machine__uses_kcore(machine))
+		return 0;
+
 	machine__mmap_name(machine, kmmap_prefix, sizeof(kmmap_prefix));
 	if (machine__is_host(machine))
 		kernel_type = DSO_TYPE_KERNEL;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4d599fe..9e8304c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -555,3 +555,21 @@ struct map *maps__find(struct rb_root *maps, u64 ip)
 
 	return NULL;
 }
+
+struct map *maps__first(struct rb_root *maps)
+{
+	struct rb_node *first = rb_first(maps);
+
+	if (first)
+		return rb_entry(first, struct map, rb_node);
+	return NULL;
+}
+
+struct map *maps__next(struct map *map)
+{
+	struct rb_node *next = rb_next(&map->rb_node);
+
+	if (next)
+		return rb_entry(next, struct map, rb_node);
+	return NULL;
+}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index a887f2c..2cc93cb 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -112,6 +112,8 @@ size_t __map_groups__fprintf_maps(struct map_groups *mg,
 void maps__insert(struct rb_root *maps, struct map *map);
 void maps__remove(struct rb_root *maps, struct map *map);
 struct map *maps__find(struct rb_root *maps, u64 addr);
+struct map *maps__first(struct rb_root *maps);
+struct map *maps__next(struct map *map);
 void map_groups__init(struct map_groups *mg);
 void map_groups__exit(struct map_groups *mg);
 int map_groups__clone(struct map_groups *mg,
@@ -139,6 +141,17 @@ static inline struct map *map_groups__find(struct map_groups *mg,
 	return maps__find(&mg->maps[type], addr);
 }
 
+static inline struct map *map_groups__first(struct map_groups *mg,
+					    enum map_type type)
+{
+	return maps__first(&mg->maps[type]);
+}
+
+static inline struct map *map_groups__next(struct map *map)
+{
+	return maps__next(map);
+}
+
 struct symbol *map_groups__find_symbol(struct map_groups *mg,
 				       enum map_type type, u64 addr,
 				       struct map **mapp,
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index f00596f..4eec803 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -943,6 +943,57 @@ out_elf_end:
 	return err;
 }
 
+static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
+{
+	GElf_Phdr phdr;
+	size_t i, phdrnum;
+	int err;
+	u64 sz;
+
+	if (elf_getphdrnum(elf, &phdrnum))
+		return -1;
+
+	for (i = 0; i < phdrnum; i++) {
+		if (gelf_getphdr(elf, i, &phdr) == NULL)
+			return -1;
+		if (phdr.p_type != PT_LOAD)
+			continue;
+		if (exe) {
+			if (!(phdr.p_flags & PF_X))
+				continue;
+		} else {
+			if (!(phdr.p_flags & PF_R))
+				continue;
+		}
+		sz = min(phdr.p_memsz, phdr.p_filesz);
+		if (!sz)
+			continue;
+		err = mapfn(phdr.p_vaddr, sz, phdr.p_offset, data);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
+		    bool *is_64_bit)
+{
+	int err;
+	Elf *elf;
+
+	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+	if (elf == NULL)
+		return -1;
+
+	if (is_64_bit)
+		*is_64_bit = (gelf_getclass(elf) == ELFCLASS64);
+
+	err = elf_read_maps(elf, exe, mapfn, data);
+
+	elf_end(elf);
+	return err;
+}
+
 void symbol__elf_init(void)
 {
 	elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index a7390cd..3a802c3 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -301,6 +301,13 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 	return 0;
 }
 
+int file__read_maps(int fd __maybe_unused, bool exe __maybe_unused,
+		    mapfn_t mapfn __maybe_unused, void *data __maybe_unused,
+		    bool *is_64_bit __maybe_unused)
+{
+	return -1;
+}
+
 void symbol__elf_init(void)
 {
 }
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b9056a8..77f3b95 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -327,6 +327,16 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
 	return NULL;
 }
 
+static struct symbol *symbols__first(struct rb_root *symbols)
+{
+	struct rb_node *n = rb_first(symbols);
+
+	if (n)
+		return rb_entry(n, struct symbol, rb_node);
+
+	return NULL;
+}
+
 struct symbol_name_rb_node {
 	struct rb_node	rb_node;
 	struct symbol	sym;
@@ -397,6 +407,11 @@ struct symbol *dso__find_symbol(struct dso *dso,
 	return symbols__find(&dso->symbols[type], addr);
 }
 
+struct symbol *dso__first_symbol(struct dso *dso, enum map_type type)
+{
+	return symbols__first(&dso->symbols[type]);
+}
+
 struct symbol *dso__find_symbol_by_name(struct dso *dso, enum map_type type,
 					const char *name)
 {
@@ -533,6 +548,53 @@ static int dso__load_all_kallsyms(struct dso *dso, const char *filename,
 	return kallsyms__parse(filename, &args, map__process_kallsym_symbol);
 }
 
+static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
+					 symbol_filter_t filter)
+{
+	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	struct map *curr_map;
+	struct symbol *pos;
+	int count = 0, moved = 0;
+	struct rb_root *root = &dso->symbols[map->type];
+	struct rb_node *next = rb_first(root);
+
+	while (next) {
+		char *module;
+
+		pos = rb_entry(next, struct symbol, rb_node);
+		next = rb_next(&pos->rb_node);
+
+		module = strchr(pos->name, '\t');
+		if (module)
+			*module = '\0';
+
+		curr_map = map_groups__find(kmaps, map->type, pos->start);
+
+		if (!curr_map || (filter && filter(curr_map, pos))) {
+			rb_erase(&pos->rb_node, root);
+			symbol__delete(pos);
+		} else {
+			pos->start -= curr_map->start - curr_map->pgoff;
+			if (pos->end)
+				pos->end -= curr_map->start - curr_map->pgoff;
+			if (curr_map != map) {
+				rb_erase(&pos->rb_node, root);
+				symbols__insert(
+					&curr_map->dso->symbols[curr_map->type],
+					pos);
+				++moved;
+			} else {
+				++count;
+			}
+		}
+	}
+
+	/* Symbols have been adjusted */
+	dso->adjust_symbols = 1;
+
+	return count + moved;
+}
+
 /*
  * Split the symbols into maps, making sure there are no overlaps, i.e. the
  * kernel range is broken in several maps, named [kernel].N, as we don't have
@@ -674,6 +736,161 @@ bool symbol__restricted_filename(const char *filename,
 	return restricted;
 }
 
+struct kcore_mapfn_data {
+	struct dso *dso;
+	enum map_type type;
+	struct list_head maps;
+};
+
+static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
+{
+	struct kcore_mapfn_data *md = data;
+	struct map *map;
+
+	map = map__new2(start, md->dso, md->type);
+	if (map == NULL)
+		return -ENOMEM;
+
+	map->end = map->start + len;
+	map->pgoff = pgoff;
+
+	list_add(&map->node, &md->maps);
+
+	return 0;
+}
+
+/*
+ * If kallsyms is referenced by name then we look for kcore in the same
+ * directory.
+ */
+static bool kcore_filename_from_kallsyms_filename(char *kcore_filename,
+						  const char *kallsyms_filename)
+{
+	char *name;
+
+	strcpy(kcore_filename, kallsyms_filename);
+	name = strrchr(kcore_filename, '/');
+	if (!name)
+		return false;
+
+	if (!strcmp(name, "/kallsyms")) {
+		strcpy(name, "/kcore");
+		return true;
+	}
+
+	return false;
+}
+
+static int dso__load_kcore(struct dso *dso, struct map *map,
+			   const char *kallsyms_filename)
+{
+	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	struct machine *machine = kmaps->machine;
+	struct kcore_mapfn_data md;
+	struct map *old_map, *new_map, *replacement_map = NULL;
+	bool is_64_bit;
+	int err, fd;
+	char kcore_filename[PATH_MAX];
+	struct symbol *sym;
+
+	/* This function requires that the map is the kernel map */
+	if (map != machine->vmlinux_maps[map->type])
+		return -EINVAL;
+
+	if (!kcore_filename_from_kallsyms_filename(kcore_filename,
+						   kallsyms_filename))
+		return -EINVAL;
+
+	md.dso = dso;
+	md.type = map->type;
+	INIT_LIST_HEAD(&md.maps);
+
+	fd = open(kcore_filename, O_RDONLY);
+	if (fd < 0)
+		return -EINVAL;
+
+	/* Read new maps into temporary lists */
+	err = file__read_maps(fd, md.type == MAP__FUNCTION, kcore_mapfn, &md,
+			      &is_64_bit);
+	if (err)
+		goto out_err;
+
+	if (list_empty(&md.maps)) {
+		err = -EINVAL;
+		goto out_err;
+	}
+
+	/* Remove old maps */
+	old_map = map_groups__first(kmaps, map->type);
+	while (old_map) {
+		struct map *next = map_groups__next(old_map);
+
+		if (old_map != map)
+			map_groups__remove(kmaps, old_map);
+		old_map = next;
+	}
+
+	/* Find the kernel map using the first symbol */
+	sym = dso__first_symbol(dso, map->type);
+	list_for_each_entry(new_map, &md.maps, node) {
+		if (sym && sym->start >= new_map->start &&
+		    sym->start < new_map->end) {
+			replacement_map = new_map;
+			break;
+		}
+	}
+
+	if (!replacement_map)
+		replacement_map = list_entry(md.maps.next, struct map, node);
+
+	/* Add new maps */
+	while (!list_empty(&md.maps)) {
+		new_map = list_entry(md.maps.next, struct map, node);
+		list_del(&new_map->node);
+		if (new_map == replacement_map) {
+			map->start	= new_map->start;
+			map->end	= new_map->end;
+			map->pgoff	= new_map->pgoff;
+			map->map_ip	= new_map->map_ip;
+			map->unmap_ip	= new_map->unmap_ip;
+			map__delete(new_map);
+			/* Ensure maps are correctly ordered */
+			map_groups__remove(kmaps, map);
+			map_groups__insert(kmaps, map);
+		} else {
+			map_groups__insert(kmaps, new_map);
+		}
+	}
+
+	/*
+	 * Set the data type and long name so that kcore can be read via
+	 * dso__data_read_addr().
+	 */
+	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
+		dso->data_type = DSO_BINARY_TYPE__GUEST_KCORE;
+	else
+		dso->data_type = DSO_BINARY_TYPE__KCORE;
+	dso__set_long_name(dso, strdup(kcore_filename));
+
+	close(fd);
+
+	if (map->type == MAP__FUNCTION)
+		pr_debug("Using %s for kernel object code\n", kcore_filename);
+	else
+		pr_debug("Using %s for kernel data\n", kcore_filename);
+
+	return 0;
+
+out_err:
+	while (!list_empty(&md.maps)) {
+		map = list_entry(md.maps.next, struct map, node);
+		list_del(&map->node);
+		map__delete(map);
+	}
+	close(fd);
+	return -EINVAL;
+}
+
 int dso__load_kallsyms(struct dso *dso, const char *filename,
 		       struct map *map, symbol_filter_t filter)
 {
@@ -691,7 +908,10 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
 	else
 		dso->symtab_type = DSO_BINARY_TYPE__KALLSYMS;
 
-	return dso__split_kallsyms(dso, map, filter);
+	if (!dso__load_kcore(dso, map, filename))
+		return dso__split_kallsyms_for_kcore(dso, map, filter);
+	else
+		return dso__split_kallsyms(dso, map, filter);
 }
 
 static int dso__load_perf_map(struct dso *dso, struct map *map,
@@ -1065,7 +1285,7 @@ do_kallsyms:
 		pr_debug("Using %s for symbols\n", kallsyms_filename);
 	free(kallsyms_allocated_filename);
 
-	if (err > 0) {
+	if (err > 0 && !dso__is_kcore(dso)) {
 		dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
 		map__fixup_start(map);
 		map__fixup_end(map);
@@ -1109,8 +1329,9 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
 	}
 
 	err = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
-	if (err > 0) {
+	if (err > 0)
 		pr_debug("Using %s for symbols\n", kallsyms_filename);
+	if (err > 0 && !dso__is_kcore(dso)) {
 		machine__mmap_name(machine, path, sizeof(path));
 		dso__set_long_name(dso, strdup(path));
 		map__fixup_start(map);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 5f720dc..fd5b70e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -215,6 +215,7 @@ struct symbol *dso__find_symbol(struct dso *dso, enum map_type type,
 				u64 addr);
 struct symbol *dso__find_symbol_by_name(struct dso *dso, enum map_type type,
 					const char *name);
+struct symbol *dso__first_symbol(struct dso *dso, enum map_type type);
 
 int filename__read_build_id(const char *filename, void *bf, size_t size);
 int sysfs__read_build_id(const char *filename, void *bf, size_t size);
@@ -247,4 +248,8 @@ void symbols__fixup_duplicate(struct rb_root *symbols);
 void symbols__fixup_end(struct rb_root *symbols);
 void __map_groups__fixup_end(struct map_groups *mg, enum map_type type);
 
+typedef int (*mapfn_t)(u64 start, u64 len, u64 pgoff, void *data);
+int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
+		    bool *is_64_bit);
+
 #endif /* __PERF_SYMBOL */
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 8/9] perf tools: add kcore to the object code reading test
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (6 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  2013-07-30 21:13 ` [PATCH V2 9/9] perf tools: allow annotation using /proc/kcore Adrian Hunter
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Make the "object code reading" test attempt to read from
kcore.

The test uses objdump which struggles with kcore. i.e.
doesn't always work, sometimes takes a long time.
The test has been made to work around those issues.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/code-reading.c | 75 +++++++++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 6cd3190..030cf60 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -107,6 +107,9 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 
 	pr_debug("Objdump command is: %s\n", cmd);
 
+	/* Ignore objdump errors */
+	strcat(cmd, " 2>/dev/null");
+
 	f = popen(cmd, "r");
 	if (!f) {
 		pr_debug("popen failed\n");
@@ -146,7 +149,8 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 
 	pr_debug("File is: %s\n", al.map->dso->long_name);
 
-	if (al.map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
+	if (al.map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
+	    !dso__is_kcore(al.map->dso)) {
 		pr_debug("Unexpected kernel address - skipping\n");
 		return 0;
 	}
@@ -175,6 +179,26 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 	if (map__load(al.map, NULL))
 		return -1;
 
+	/* objdump struggles with kcore - try each map only once */
+	if (dso__is_kcore(al.map->dso)) {
+		static u64 done[1024];
+		static size_t done_cnt;
+		size_t d;
+
+		for (d = 0; d < done_cnt; d++) {
+			if (done[d] == al.map->start) {
+				pr_debug("kcore map tested already");
+				pr_debug(" - skipping\n");
+				return 0;
+			}
+		}
+		if (done_cnt >= ARRAY_SIZE(done)) {
+			pr_debug("Too many kcore maps - skipping\n");
+			return 0;
+		}
+		done[done_cnt++] = al.map->start;
+	}
+
 	/* Read the object code using objdump */
 	objdump_addr = map__rip_2objdump(al.map, al.addr);
 	ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len);
@@ -186,10 +210,19 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 		if (cpumode == PERF_RECORD_MISC_KERNEL ||
 		    cpumode == PERF_RECORD_MISC_GUEST_KERNEL) {
 			len -= ret;
-			if (len)
+			if (len) {
 				pr_debug("Reducing len to %zu\n", len);
-			else
+			} else if (dso__is_kcore(al.map->dso)) {
+				/*
+				 * objdump cannot handle very large segments
+				 * that may be found in kcore.
+				 */
+				pr_debug("objdump failed for kcore");
+				pr_debug(" - skipping\n");
+				return 0;
+			} else {
 				return -1;
+			}
 		}
 	}
 	if (ret < 0) {
@@ -323,10 +356,12 @@ static void do_something(void)
 enum {
 	TEST_CODE_READING_OK,
 	TEST_CODE_READING_NO_VMLINUX,
+	TEST_CODE_READING_NO_KCORE,
 	TEST_CODE_READING_NO_ACCESS,
+	TEST_CODE_READING_NO_KERNEL_OBJ,
 };
 
-static int do_test_code_reading(void)
+static int do_test_code_reading(bool try_kcore)
 {
 	struct machines machines;
 	struct machine *machine;
@@ -347,7 +382,7 @@ static int do_test_code_reading(void)
 	int err = -1, ret;
 	pid_t pid;
 	struct map *map;
-	bool have_vmlinux, excl_kernel = false;
+	bool have_vmlinux, have_kcore, excl_kernel = false;
 
 	pid = getpid();
 
@@ -360,6 +395,10 @@ static int do_test_code_reading(void)
 		goto out_err;
 	}
 
+	/* Force the use of kallsyms instead of vmlinux to try kcore */
+	if (try_kcore)
+		symbol_conf.kallsyms_name = "/proc/kallsyms";
+
 	/* Load kernel map */
 	map = machine->vmlinux_maps[MAP__FUNCTION];
 	ret = map__load(map, NULL);
@@ -367,9 +406,15 @@ static int do_test_code_reading(void)
 		pr_debug("map__load failed\n");
 		goto out_err;
 	}
-	have_vmlinux = map->dso->symtab_type == DSO_BINARY_TYPE__VMLINUX;
-	/* No point getting kernel events if there is no vmlinux */
-	if (!have_vmlinux)
+	have_vmlinux = dso__is_vmlinux(map->dso);
+	have_kcore = dso__is_kcore(map->dso);
+
+	/* 2nd time through we just try kcore */
+	if (try_kcore && !have_kcore)
+		return TEST_CODE_READING_NO_KCORE;
+
+	/* No point getting kernel events if there is no kernel object */
+	if (!have_vmlinux && !have_kcore)
 		excl_kernel = true;
 
 	threads = thread_map__new_by_tid(pid);
@@ -456,7 +501,9 @@ static int do_test_code_reading(void)
 	if (ret < 0)
 		goto out_err;
 
-	if (!have_vmlinux)
+	if (!have_vmlinux && !have_kcore && !try_kcore)
+		err = TEST_CODE_READING_NO_KERNEL_OBJ;
+	else if (!have_vmlinux && !try_kcore)
 		err = TEST_CODE_READING_NO_VMLINUX;
 	else if (excl_kernel)
 		err = TEST_CODE_READING_NO_ACCESS;
@@ -484,7 +531,9 @@ int test__code_reading(void)
 {
 	int ret;
 
-	ret = do_test_code_reading();
+	ret = do_test_code_reading(false);
+	if (!ret)
+		ret = do_test_code_reading(true);
 
 	switch (ret) {
 	case TEST_CODE_READING_OK:
@@ -492,9 +541,15 @@ int test__code_reading(void)
 	case TEST_CODE_READING_NO_VMLINUX:
 		fprintf(stderr, " (no vmlinux)");
 		return 0;
+	case TEST_CODE_READING_NO_KCORE:
+		fprintf(stderr, " (no kcore)");
+		return 0;
 	case TEST_CODE_READING_NO_ACCESS:
 		fprintf(stderr, " (no access)");
 		return 0;
+	case TEST_CODE_READING_NO_KERNEL_OBJ:
+		fprintf(stderr, " (no kernel obj)");
+		return 0;
 	default:
 		return -1;
 	};
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 9/9] perf tools: allow annotation using /proc/kcore
  2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (7 preceding siblings ...)
  2013-07-30 21:13 ` [PATCH V2 8/9] perf tools: add kcore to the object code reading test Adrian Hunter
@ 2013-07-30 21:13 ` Adrian Hunter
  8 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Annotation with /proc/kcore is possible so the logic
is adjusted to allow it.  The main difference is that
/proc/kcore had no symbols so the parsing logic needed
a tweak to read jump offsets.

The other difference is that objdump cannot always
read from kcore.  That seems to be a bug with objdump.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-top.c   |  3 ++-
 tools/perf/util/annotate.c | 13 +++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index bbf4635..3b19a6b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -103,7 +103,8 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
 	/*
 	 * We can't annotate with just /proc/kallsyms
 	 */
-	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
+	if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
+	    !dso__is_kcore(map->dso)) {
 		pr_err("Can't annotate %s: No vmlinux file was found in the "
 		       "path\n", sym->name);
 		sleep(1);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d102716..4ab2f11 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -110,10 +110,10 @@ static int jump__parse(struct ins_operands *ops)
 {
 	const char *s = strchr(ops->raw, '+');
 
-	ops->target.addr = strtoll(ops->raw, NULL, 16);
+	ops->target.addr = strtoull(ops->raw, NULL, 16);
 
 	if (s++ != NULL)
-		ops->target.offset = strtoll(s, NULL, 16);
+		ops->target.offset = strtoull(s, NULL, 16);
 	else
 		ops->target.offset = UINT64_MAX;
 
@@ -821,6 +821,10 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 	if (dl == NULL)
 		return -1;
 
+	if (dl->ops.target.offset == UINT64_MAX)
+		dl->ops.target.offset = dl->ops.target.addr -
+					map__rip_2objdump(map, sym->start);
+
 	disasm__add(&notes->src->source, dl);
 
 	return 0;
@@ -864,7 +868,8 @@ fallback:
 		free_filename = false;
 	}
 
-	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS) {
+	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
+	    !dso__is_kcore(dso)) {
 		char bf[BUILD_ID_SIZE * 2 + 16] = " with build id ";
 		char *build_id_msg = NULL;
 
@@ -898,7 +903,7 @@ fallback:
 	snprintf(command, sizeof(command),
 		 "%s %s%s --start-address=0x%016" PRIx64
 		 " --stop-address=0x%016" PRIx64
-		 " -d %s %s -C %s|grep -v %s|expand",
+		 " -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
 		 objdump_path ? objdump_path : "objdump",
 		 disassembler_style ? "-M " : "",
 		 disassembler_style ? disassembler_style : "",
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
@ 2013-07-31 14:17   ` Arnaldo Carvalho de Melo
  2013-07-31 14:24     ` Peter Zijlstra
  2013-08-03 13:45     ` Adrian Hunter
  2013-07-31 14:58   ` Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-31 14:17 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
> Using the information in mmap events, perf tools can read object
> code associated with sampled addresses.  A test is added that
> compares bytes read by perf with the same bytes read using
> objdump.

So this parses objdump output, and we also already have the annotation
logic that does that too, have you thought about having common routines
for these two cases?

I mean the disasm_line, ins, ins_ops, ins_operands classes, that now
lives in util/annotate.h but could be moved somewhere else,
disconnecting it as much as possible from annotation, because probably
there are more cool things we could do with that... :-)

We could certainly do it incrementally, merging your current patch
series and then working on sharing code on these two use cases, but
perhaps you can do it now?

What do you think?

- Arnaldo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 14:17   ` Arnaldo Carvalho de Melo
@ 2013-07-31 14:24     ` Peter Zijlstra
  2013-08-01  2:32       ` Namhyung Kim
  2013-08-03 13:45     ` Adrian Hunter
  1 sibling, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2013-07-31 14:24 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Adrian Hunter, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian, Ingo Molnar

On Wed, Jul 31, 2013 at 11:17:32AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
> > Using the information in mmap events, perf tools can read object
> > code associated with sampled addresses.  A test is added that
> > compares bytes read by perf with the same bytes read using
> > objdump.
> 
> So this parses objdump output, and we also already have the annotation
> logic that does that too, have you thought about having common routines
> for these two cases?
> 

Or better yet, stop using objdump like this and start using libbfd
directly. The only reason we did horrible things like parsing objdump
output is because nobody knew how the underlying stuff actually worked
and we wanted to have something quick.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 2/9] perf tools: load kernel maps before using
  2013-07-30 21:13 ` [PATCH V2 2/9] perf tools: load kernel maps before using Adrian Hunter
@ 2013-07-31 14:35   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-31 14:35 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Em Wed, Jul 31, 2013 at 12:13:51AM +0300, Adrian Hunter escreveu:
> In order to use kernel maps to read object code, those
> maps must be adjusted to map to the dso file offset.
> Because lazy-initialization is used, that is not done
> until symbols are loaded.  However the maps are first
> used by thread__find_addr_map() before symbols are loaded.
> So this patch changes thread__find_addr() to "load" kernel
> maps before using them.

Argh, yeah, we create maps as we traverse kallsyms, right? I guess this
is there for historical reasons (i.e. it "evolved" like that), but I'm
adding a note to my TODO list to remove that mess, i.e. we should
probably do that as a separate step, i.e. the creation of kernel maps,
detached from actually loading the symbols.

For now I think your patch makes we progress, so I'll apply it.

Thanks,

- Arnaldo
 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/builtin-inject.c     |  2 +-
>  tools/perf/builtin-script.c     |  4 ++--
>  tools/perf/tests/code-reading.c |  2 +-
>  tools/perf/util/build-id.c      |  2 +-
>  tools/perf/util/event.c         | 18 ++++++++++++++----
>  tools/perf/util/thread.h        |  2 +-
>  tools/perf/util/unwind.c        |  4 ++--
>  7 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 1d8de2e..f012a98 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -206,7 +206,7 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
>  	}
>  
>  	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
> -			      event->ip.ip, &al);
> +			      event->ip.ip, &al, NULL);
>  
>  	if (al.map != NULL) {
>  		if (!al.map->dso->hit) {
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 1cad370..cd616ff 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -341,10 +341,10 @@ static void print_sample_addr(union perf_event *event,
>  		return;
>  
>  	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
> -			      sample->addr, &al);
> +			      sample->addr, &al, NULL);
>  	if (!al.map)
>  		thread__find_addr_map(thread, machine, cpumode, MAP__VARIABLE,
> -				      sample->addr, &al);
> +				      sample->addr, &al, NULL);
>  
>  	al.cpu = sample->cpu;
>  	al.sym = NULL;
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 7c9437d..6cd3190 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -138,7 +138,7 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
>  	pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
>  
>  	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION, addr,
> -			      &al);
> +			      &al, NULL);
>  	if (!al.map || !al.map->dso) {
>  		pr_debug("thread__find_addr_map failed\n");
>  		return -1;
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 5295625..3a0f508 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -33,7 +33,7 @@ int build_id__mark_dso_hit(struct perf_tool *tool __maybe_unused,
>  	}
>  
>  	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
> -			      event->ip.ip, &al);
> +			      event->ip.ip, &al, NULL);
>  
>  	if (al.map != NULL)
>  		al.map->dso->hit = 1;
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 9541270..cc7c0c9 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -592,9 +592,10 @@ int perf_event__process(struct perf_tool *tool __maybe_unused,
>  void thread__find_addr_map(struct thread *self,
>  			   struct machine *machine, u8 cpumode,
>  			   enum map_type type, u64 addr,
> -			   struct addr_location *al)
> +			   struct addr_location *al, symbol_filter_t filter)
>  {
>  	struct map_groups *mg = &self->mg;
> +	bool load_map = false;
>  
>  	al->thread = self;
>  	al->addr = addr;
> @@ -609,11 +610,13 @@ void thread__find_addr_map(struct thread *self,
>  	if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
>  		al->level = 'k';
>  		mg = &machine->kmaps;
> +		load_map = true;
>  	} else if (cpumode == PERF_RECORD_MISC_USER && perf_host) {
>  		al->level = '.';
>  	} else if (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest) {
>  		al->level = 'g';
>  		mg = &machine->kmaps;
> +		load_map = true;
>  	} else {
>  		/*
>  		 * 'u' means guest os user space.
> @@ -654,8 +657,15 @@ try_again:
>  			mg = &machine->kmaps;
>  			goto try_again;
>  		}
> -	} else
> +	} else {
> +		/*
> +		 * Kernel maps might be changed when loading symbols so loading
> +		 * must be done prior to using kernel maps.
> +		 */
> +		if (load_map)
> +			map__load(al->map, filter);
>  		al->addr = al->map->map_ip(al->map, al->addr);
> +	}
>  }
>  
>  void thread__find_addr_location(struct thread *thread, struct machine *machine,
> @@ -663,7 +673,7 @@ void thread__find_addr_location(struct thread *thread, struct machine *machine,
>  				struct addr_location *al,
>  				symbol_filter_t filter)
>  {
> -	thread__find_addr_map(thread, machine, cpumode, type, addr, al);
> +	thread__find_addr_map(thread, machine, cpumode, type, addr, al, filter);
>  	if (al->map != NULL)
>  		al->sym = map__find_symbol(al->map, al->addr, filter);
>  	else
> @@ -699,7 +709,7 @@ int perf_event__preprocess_sample(const union perf_event *event,
>  		machine__create_kernel_maps(machine);
>  
>  	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
> -			      event->ip.ip, al);
> +			      event->ip.ip, al, filter);
>  	dump_printf(" ...... dso: %s\n",
>  		    al->map ? al->map->dso->long_name :
>  			al->level == 'H' ? "[hypervisor]" : "<not found>");
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 0fe1f9c..f98d1d9 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -41,7 +41,7 @@ static inline struct map *thread__find_map(struct thread *self,
>  
>  void thread__find_addr_map(struct thread *thread, struct machine *machine,
>  			   u8 cpumode, enum map_type type, u64 addr,
> -			   struct addr_location *al);
> +			   struct addr_location *al, symbol_filter_t filter);
>  
>  void thread__find_addr_location(struct thread *thread, struct machine *machine,
>  				u8 cpumode, enum map_type type, u64 addr,
> diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
> index 958723b..5bbd494 100644
> --- a/tools/perf/util/unwind.c
> +++ b/tools/perf/util/unwind.c
> @@ -272,7 +272,7 @@ static struct map *find_map(unw_word_t ip, struct unwind_info *ui)
>  	struct addr_location al;
>  
>  	thread__find_addr_map(ui->thread, ui->machine, PERF_RECORD_MISC_USER,
> -			      MAP__FUNCTION, ip, &al);
> +			      MAP__FUNCTION, ip, &al, NULL);
>  	return al.map;
>  }
>  
> @@ -349,7 +349,7 @@ static int access_dso_mem(struct unwind_info *ui, unw_word_t addr,
>  	ssize_t size;
>  
>  	thread__find_addr_map(ui->thread, ui->machine, PERF_RECORD_MISC_USER,
> -			      MAP__FUNCTION, addr, &al);
> +			      MAP__FUNCTION, addr, &al, NULL);
>  	if (!al.map) {
>  		pr_debug("unwind: no map for %lx\n", (unsigned long)addr);
>  		return -1;
> -- 
> 1.7.11.7

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
  2013-07-31 14:17   ` Arnaldo Carvalho de Melo
@ 2013-07-31 14:58   ` Arnaldo Carvalho de Melo
  2013-07-31 17:28     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-31 14:58 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
> Using the information in mmap events, perf tools can read object
> code associated with sampled addresses.  A test is added that
> compares bytes read by perf with the same bytes read using
> objdump.

investigating...

[root@zoo ~]# gdb perf
GNU gdb (GDB) Fedora (7.5.1-38.fc18)
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /root/bin/perf...done.
(gdb) run test 21
Starting program: /root/bin/perf test 21
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
21: Test object code reading                               :
Program received signal SIGSEGV, Segmentation fault.
0x000000000045a2d3 in xyarray__entry (xy=0x0, x=0, y=0) at util/xyarray.h:17
17		return &xy->contents[x * xy->row_size + y * xy->entry_size];
Missing separate debuginfos, use: debuginfo-install atk-2.6.0-1.fc18.x86_64 audit-libs-2.3.1-2.fc18.x86_64 bzip2-libs-1.0.6-7.fc18.x86_64 cairo-1.12.14-2.fc18.x86_64 elfutils-libelf-0.155-1.fc18.x86_64 elfutils-libs-0.155-1.fc18.x86_64 expat-2.1.0-4.fc18.x86_64 fontconfig-2.10.2-2.fc18.x86_64 freetype-2.4.10-4.fc18.x86_64 gdk-pixbuf2-2.26.5-1.fc18.x86_64 glib2-2.34.2-2.fc18.x86_64 graphite2-1.1.1-4.fc18.x86_64 gtk2-2.24.19-1.fc18.x86_64 harfbuzz-0.9.12-2.fc18.x86_64 libX11-1.5.99.901-3.20130524gita3bdd2b09.fc18.x86_64 libXau-1.0.6-4.fc18.x86_64 libXcomposite-0.4.3-4.fc18.x86_64 libXcursor-1.1.13-5.20130524git8f677eaea.fc18.x86_64 libXdamage-1.1.3-4.fc18.x86_64 libXext-1.3.1-3.20130524gitdfe6e1f3b.fc18.x86_64 libXfixes-5.0-6.20130524gitc480fe327.fc18.x86_64 libXi-1.6.2.901-1.fc18.x86_64 libXinerama-1.1.2-5.20130524git99c644fc8.fc18.x86_64 libXrandr-1.4.0-4.20130524gitc90f74497.fc18.x86_64 libXrender-0.9.7-5.20130524git786f78fd8.fc18.x86_64 libXxf86vm-1.1.2-5.20130524git4c4123441.fc18.x86_64 libdrm-2.4.45-1.fc18.x86_64 libffi-3.0.10-3.fc18.x86_64 libgcc-4.7.2-8.fc18.x86_64 libicu-49.1.1-8.fc18.x86_64 libpng-1.5.13-1.fc18.x86_64 libselinux-2.1.12-7.3.fc18.x86_64 libstdc++-4.7.2-8.fc18.x86_64 libunwind-1.0.1-5.fc18.x86_64 libwayland-client-1.0.5-1.fc18.x86_64 libwayland-server-1.0.5-1.fc18.x86_64 libxcb-1.9-3.fc18.x86_64 mesa-libEGL-9.2-0.7.20130528.fc18.x86_64 mesa-libGL-9.2-0.7.20130528.fc18.x86_64 mesa-libgbm-9.2-0.7.20130528.fc18.x86_64 mesa-libglapi-9.2-0.7.20130528.fc18.x86_64 nss-softokn-freebl-3.14.3-1.fc18.x86_64 numactl-libs-2.0.7-7.fc18.x86_64 pango-1.32.3-2.fc18.x86_64 pcre-8.31-5.fc18.x86_64 perl-libs-5.16.3-244.fc18.x86_64 pixman-0.28.0-1.fc18.x86_64 python-libs-2.7.3-13.fc18.x86_64 slang-2.2.4-5.fc18.x86_64 systemd-libs-201-2.fc18.7.x86_64 zlib-1.2.7-9.fc18.x86_64
(gdb) bt
#0  0x000000000045a2d3 in xyarray__entry (xy=0x0, x=0, y=0) at util/xyarray.h:17
#1  0x000000000045ade8 in perf_evlist__disable (evlist=0xcd3ad0) at util/evlist.c:238
#2  0x00000000004c1d06 in do_test_code_reading () at tests/code-reading.c:467
#3  0x00000000004c1dca in test__code_reading () at tests/code-reading.c:487
#4  0x0000000000447fb5 in __cmd_test (argc=1, argv=0x7fffffffe4d0, skiplist=0x0) at tests/builtin-test.c:163
#5  0x00000000004482ff in cmd_test (argc=1, argv=0x7fffffffe4d0, prefix=0x0) at tests/builtin-test.c:228
#6  0x000000000041a711 in run_builtin (p=0x7fc140 <commands+480>, argc=2, argv=0x7fffffffe4d0) at perf.c:319
#7  0x000000000041a949 in handle_internal_command (argc=2, argv=0x7fffffffe4d0) at perf.c:376
#8  0x000000000041aa95 in run_argv (argcp=0x7fffffffe3ac, argv=0x7fffffffe3a0) at perf.c:420
#9  0x000000000041ad37 in main (argc=2, argv=0x7fffffffe4d0) at perf.c:521
(gdb)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 14:58   ` Arnaldo Carvalho de Melo
@ 2013-07-31 17:28     ` Arnaldo Carvalho de Melo
  2013-07-31 17:46       ` Arnaldo Carvalho de Melo
  2013-08-03 14:11       ` Adrian Hunter
  0 siblings, 2 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-31 17:28 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

Em Wed, Jul 31, 2013 at 11:58:20AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
> > Using the information in mmap events, perf tools can read object
> > code associated with sampled addresses.  A test is added that
> > compares bytes read by perf with the same bytes read using
> > objdump.
> 
> investigating...
> 
> (gdb) run test 21
> Starting program: /root/bin/perf test 21
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> 21: Test object code reading                               :
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000045a2d3 in xyarray__entry (xy=0x0, x=0, y=0) at util/xyarray.h:17
> 17		return &xy->contents[x * xy->row_size + y * xy->entry_size];


Still investigating, but the attached patch is needed to handle such
failure cases:

[root@zoo ~]# perf test 21
21: Test object code reading                               : FAILED!
[root@zoo ~]# perf test -v 21
21: Test object code reading                               :
--- start ---
Looking at the vmlinux_path (6 entries long)
symsrc__init: cannot get elf header.
Using /lib/modules/3.11.0-rc2+/build/vmlinux for symbols
Parsing event 'cycles'
Parsing event 'cycles:u'
perf_evlist__open failed
---- end ----
Test object code reading: FAILED!
[root@zoo ~]# 

- Arnaldo

[-- Attachment #2: error_path.patch --]
[-- Type: text/plain, Size: 3484 bytes --]

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 7c9437d..9d9b6b5 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -357,7 +357,7 @@ static int do_test_code_reading(void)
 	ret = machine__create_kernel_maps(machine);
 	if (ret < 0) {
 		pr_debug("machine__create_kernel_maps failed\n");
-		goto out_err;
+		goto out_machine_exit;
 	}
 
 	/* Load kernel map */
@@ -365,7 +365,7 @@ static int do_test_code_reading(void)
 	ret = map__load(map, NULL);
 	if (ret < 0) {
 		pr_debug("map__load failed\n");
-		goto out_err;
+		goto out_destroy_kernel_maps;
 	}
 	have_vmlinux = map->dso->symtab_type == DSO_BINARY_TYPE__VMLINUX;
 	/* No point getting kernel events if there is no vmlinux */
@@ -375,26 +375,26 @@ static int do_test_code_reading(void)
 	threads = thread_map__new_by_tid(pid);
 	if (!threads) {
 		pr_debug("thread_map__new_by_tid failed\n");
-		goto out_err;
+		goto out_destroy_kernel_maps;
 	}
 
 	ret = perf_event__synthesize_thread_map(NULL, threads,
 						perf_event__process, machine);
 	if (ret < 0) {
 		pr_debug("perf_event__synthesize_thread_map failed\n");
-		goto out_err;
+		goto out_thread_map_delete;
 	}
 
 	thread = machine__findnew_thread(machine, pid);
 	if (!thread) {
 		pr_debug("machine__findnew_thread failed\n");
-		goto out_err;
+		goto out_thread_map_delete;
 	}
 
 	cpus = cpu_map__new(NULL);
 	if (!cpus) {
 		pr_debug("cpu_map__new failed\n");
-		goto out_err;
+		goto out_thread_map_delete;
 	}
 
 	while (1) {
@@ -403,7 +403,7 @@ static int do_test_code_reading(void)
 		evlist = perf_evlist__new();
 		if (!evlist) {
 			pr_debug("perf_evlist__new failed\n");
-			goto out_err;
+			goto out_cpu_map_delete;
 		}
 
 		perf_evlist__set_maps(evlist, cpus, threads);
@@ -416,7 +416,7 @@ static int do_test_code_reading(void)
 		ret = parse_events(evlist, str);
 		if (ret < 0) {
 			pr_debug("parse_events failed\n");
-			goto out_err;
+			goto out_evlist_delete;
 		}
 
 		perf_evlist__config(evlist, &opts);
@@ -435,7 +435,7 @@ static int do_test_code_reading(void)
 				continue;
 			}
 			pr_debug("perf_evlist__open failed\n");
-			goto out_err;
+			goto out_evlist_delete;
 		}
 		break;
 	}
@@ -443,7 +443,7 @@ static int do_test_code_reading(void)
 	ret = perf_evlist__mmap(evlist, UINT_MAX, false);
 	if (ret < 0) {
 		pr_debug("perf_evlist__mmap failed\n");
-		goto out_err;
+		goto out_evlist_close;
 	}
 
 	perf_evlist__enable(evlist);
@@ -454,7 +454,7 @@ static int do_test_code_reading(void)
 
 	ret = process_events(machine, evlist);
 	if (ret < 0)
-		goto out_err;
+		goto out_evlist_munmap;
 
 	if (!have_vmlinux)
 		err = TEST_CODE_READING_NO_VMLINUX;
@@ -462,19 +462,21 @@ static int do_test_code_reading(void)
 		err = TEST_CODE_READING_NO_ACCESS;
 	else
 		err = TEST_CODE_READING_OK;
-out_err:
-	if (evlist) {
-		perf_evlist__disable(evlist);
-		perf_evlist__munmap(evlist);
-		perf_evlist__close(evlist);
-		perf_evlist__delete(evlist);
-	}
-	if (cpus)
-		cpu_map__delete(cpus);
-	if (threads)
-		thread_map__delete(threads);
+
+out_evlist_munmap:
+	perf_evlist__munmap(evlist);
+out_evlist_close:
+	perf_evlist__close(evlist);
+out_evlist_delete:
+	perf_evlist__delete(evlist);
+out_cpu_map_delete:
+	cpu_map__delete(cpus);
+out_thread_map_delete:
+	thread_map__delete(threads);
+out_destroy_kernel_maps:
 	machines__destroy_kernel_maps(&machines);
 	machine__delete_threads(machine);
+out_machine_exit:
 	machines__exit(&machines);
 
 	return err;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 17:28     ` Arnaldo Carvalho de Melo
@ 2013-07-31 17:46       ` Arnaldo Carvalho de Melo
  2013-08-03 19:37         ` Adrian Hunter
  2013-08-03 14:11       ` Adrian Hunter
  1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-31 17:46 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Em Wed, Jul 31, 2013 at 02:28:33PM -0300, Arnaldo Carvalho de Melo escreveu:
> Still investigating, but the attached patch is needed to handle such
> failure cases:
 
> [root@zoo ~]# perf test 21
> 21: Test object code reading                               : FAILED!
> [root@zoo ~]# perf test -v 21

Lowering the freq to 4kHz gets me to where I think you was at this
point:

[root@zoo ~]# perf test -v 21
21: Test object code reading                               :
--- start ---
Looking at the vmlinux_path (6 entries long)
symsrc__init: cannot get elf header.
Using /lib/modules/3.11.0-rc2+/build/vmlinux for symbols
Parsing event 'cycles'
Reading object code for memory address: 0xffffffff8101ce7d
File is: /lib/modules/3.11.0-rc2+/build/vmlinux
On file address is: 0xffffffff8101ce7d
dso__data_read_offset failed
---- end ----
Test object code reading: FAILED!
[root@zoo ~]# 

I.e. we need the follow on patches to fix this issue, right?

I'll merge my changes with your first patch and continue from there.

- Arnaldo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 14:24     ` Peter Zijlstra
@ 2013-08-01  2:32       ` Namhyung Kim
  0 siblings, 0 replies; 21+ messages in thread
From: Namhyung Kim @ 2013-08-01  2:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, linux-kernel,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith,
	Paul Mackerras, Stephane Eranian, Ingo Molnar

Hi Peter,

On Wed, Jul 31, 2013 at 11:24 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jul 31, 2013 at 11:17:32AM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
>> > Using the information in mmap events, perf tools can read object
>> > code associated with sampled addresses.  A test is added that
>> > compares bytes read by perf with the same bytes read using
>> > objdump.
>>
>> So this parses objdump output, and we also already have the annotation
>> logic that does that too, have you thought about having common routines
>> for these two cases?
>>
>
> Or better yet, stop using objdump like this and start using libbfd
> directly. The only reason we did horrible things like parsing objdump
> output is because nobody knew how the underlying stuff actually worked
> and we wanted to have something quick.

I have similar patch for getting srcline info (using addr2line) based
on Roberto Vitillo's patch.  I'll send the series soon and then look
at the objdump too.


-- 
Thanks,
Namhyung Kim

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 14:17   ` Arnaldo Carvalho de Melo
  2013-07-31 14:24     ` Peter Zijlstra
@ 2013-08-03 13:45     ` Adrian Hunter
  2013-08-05 16:32       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-08-03 13:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

On 31/07/2013 5:17 p.m., Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
>> Using the information in mmap events, perf tools can read object
>> code associated with sampled addresses.  A test is added that
>> compares bytes read by perf with the same bytes read using
>> objdump.
>
> So this parses objdump output, and we also already have the annotation
> logic that does that too, have you thought about having common routines
> for these two cases?

The annotation logic strips out the bytes (--no-show-raw) whereas the 
test extracts only the bytes, so they are not currently compatible.

>
> I mean the disasm_line, ins, ins_ops, ins_operands classes, that now
> lives in util/annotate.h but could be moved somewhere else,
> disconnecting it as much as possible from annotation, because probably
> there are more cool things we could do with that... :-)
>
> We could certainly do it incrementally, merging your current patch
> series and then working on sharing code on these two use cases, but
> perhaps you can do it now?
>
> What do you think?

I expect replacing objdump with library calls will end up being the way 
forward.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 17:28     ` Arnaldo Carvalho de Melo
  2013-07-31 17:46       ` Arnaldo Carvalho de Melo
@ 2013-08-03 14:11       ` Adrian Hunter
  1 sibling, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-08-03 14:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

On 31/07/2013 8:28 p.m., Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 31, 2013 at 11:58:20AM -0300, Arnaldo Carvalho de Melo escreveu:
>> >  Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
>>> >  >  Using the information in mmap events, perf tools can read object
>>> >  >  code associated with sampled addresses.  A test is added that
>>> >  >  compares bytes read by perf with the same bytes read using
>>> >  >  objdump.
>> >
>> >  investigating...
>> >
>> >  (gdb) run test 21
>> >  Starting program: /root/bin/perf test 21
>> >  [Thread debugging using libthread_db enabled]
>> >  Using host libthread_db library "/lib64/libthread_db.so.1".
>> >  21: Test object code reading                               :
>> >  Program received signal SIGSEGV, Segmentation fault.
>> >  0x000000000045a2d3 in xyarray__entry (xy=0x0, x=0, y=0) at util/xyarray.h:17
>> >  17		return&xy->contents[x * xy->row_size + y * xy->entry_size];
>
> Still investigating, but the attached patch is needed to handle such
> failure cases:

I removed the offending perf_evlist__disable() since it was not needed 
anyway.  Otherwise the existing error path was simpler so I kept it in V3.

>
> [root@zoo ~]# perf test 21
> 21: Test object code reading                               : FAILED!
> [root@zoo ~]# perf test -v 21
> 21: Test object code reading                               :
> --- start ---
> Looking at the vmlinux_path (6 entries long)
> symsrc__init: cannot get elf header.
> Using /lib/modules/3.11.0-rc2+/build/vmlinux for symbols
> Parsing event 'cycles'
> Parsing event 'cycles:u'
> perf_evlist__open failed
> ---- end ----
> Test object code reading: FAILED!
> [root@zoo ~]#
>
> - Arnaldo
>
>
> error_path.patch
>
>
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 7c9437d..9d9b6b5 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -357,7 +357,7 @@ static int do_test_code_reading(void)
>   	ret = machine__create_kernel_maps(machine);
>   	if (ret<  0) {
>   		pr_debug("machine__create_kernel_maps failed\n");
> -		goto out_err;
> +		goto out_machine_exit;
>   	}
>
>   	/* Load kernel map */
> @@ -365,7 +365,7 @@ static int do_test_code_reading(void)
>   	ret = map__load(map, NULL);
>   	if (ret<  0) {
>   		pr_debug("map__load failed\n");
> -		goto out_err;
> +		goto out_destroy_kernel_maps;
>   	}
>   	have_vmlinux = map->dso->symtab_type == DSO_BINARY_TYPE__VMLINUX;
>   	/* No point getting kernel events if there is no vmlinux */
> @@ -375,26 +375,26 @@ static int do_test_code_reading(void)
>   	threads = thread_map__new_by_tid(pid);
>   	if (!threads) {
>   		pr_debug("thread_map__new_by_tid failed\n");
> -		goto out_err;
> +		goto out_destroy_kernel_maps;
>   	}
>
>   	ret = perf_event__synthesize_thread_map(NULL, threads,
>   						perf_event__process, machine);
>   	if (ret<  0) {
>   		pr_debug("perf_event__synthesize_thread_map failed\n");
> -		goto out_err;
> +		goto out_thread_map_delete;
>   	}
>
>   	thread = machine__findnew_thread(machine, pid);
>   	if (!thread) {
>   		pr_debug("machine__findnew_thread failed\n");
> -		goto out_err;
> +		goto out_thread_map_delete;
>   	}
>
>   	cpus = cpu_map__new(NULL);
>   	if (!cpus) {
>   		pr_debug("cpu_map__new failed\n");
> -		goto out_err;
> +		goto out_thread_map_delete;
>   	}
>
>   	while (1) {
> @@ -403,7 +403,7 @@ static int do_test_code_reading(void)
>   		evlist = perf_evlist__new();
>   		if (!evlist) {
>   			pr_debug("perf_evlist__new failed\n");
> -			goto out_err;
> +			goto out_cpu_map_delete;
>   		}
>
>   		perf_evlist__set_maps(evlist, cpus, threads);
> @@ -416,7 +416,7 @@ static int do_test_code_reading(void)
>   		ret = parse_events(evlist, str);
>   		if (ret<  0) {
>   			pr_debug("parse_events failed\n");
> -			goto out_err;
> +			goto out_evlist_delete;
>   		}
>
>   		perf_evlist__config(evlist,&opts);
> @@ -435,7 +435,7 @@ static int do_test_code_reading(void)
>   				continue;
>   			}
>   			pr_debug("perf_evlist__open failed\n");
> -			goto out_err;
> +			goto out_evlist_delete;
>   		}
>   		break;
>   	}
> @@ -443,7 +443,7 @@ static int do_test_code_reading(void)
>   	ret = perf_evlist__mmap(evlist, UINT_MAX, false);
>   	if (ret<  0) {
>   		pr_debug("perf_evlist__mmap failed\n");
> -		goto out_err;
> +		goto out_evlist_close;
>   	}
>
>   	perf_evlist__enable(evlist);
> @@ -454,7 +454,7 @@ static int do_test_code_reading(void)
>
>   	ret = process_events(machine, evlist);
>   	if (ret<  0)
> -		goto out_err;
> +		goto out_evlist_munmap;
>
>   	if (!have_vmlinux)
>   		err = TEST_CODE_READING_NO_VMLINUX;
> @@ -462,19 +462,21 @@ static int do_test_code_reading(void)
>   		err = TEST_CODE_READING_NO_ACCESS;
>   	else
>   		err = TEST_CODE_READING_OK;
> -out_err:
> -	if (evlist) {
> -		perf_evlist__disable(evlist);
> -		perf_evlist__munmap(evlist);
> -		perf_evlist__close(evlist);
> -		perf_evlist__delete(evlist);
> -	}
> -	if (cpus)
> -		cpu_map__delete(cpus);
> -	if (threads)
> -		thread_map__delete(threads);
> +
> +out_evlist_munmap:
> +	perf_evlist__munmap(evlist);
> +out_evlist_close:
> +	perf_evlist__close(evlist);
> +out_evlist_delete:
> +	perf_evlist__delete(evlist);
> +out_cpu_map_delete:
> +	cpu_map__delete(cpus);
> +out_thread_map_delete:
> +	thread_map__delete(threads);
> +out_destroy_kernel_maps:
>   	machines__destroy_kernel_maps(&machines);
>   	machine__delete_threads(machine);
> +out_machine_exit:
>   	machines__exit(&machines);
>
>   	return err;
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-07-31 17:46       ` Arnaldo Carvalho de Melo
@ 2013-08-03 19:37         ` Adrian Hunter
  0 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-08-03 19:37 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

On 31/07/2013 8:46 p.m., Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 31, 2013 at 02:28:33PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Still investigating, but the attached patch is needed to handle such
>> failure cases:
>
>> [root@zoo ~]# perf test 21
>> 21: Test object code reading                               : FAILED!
>> [root@zoo ~]# perf test -v 21
>
> Lowering the freq to 4kHz gets me to where I think you was at this
> point:
>
> [root@zoo ~]# perf test -v 21
> 21: Test object code reading                               :
> --- start ---
> Looking at the vmlinux_path (6 entries long)
> symsrc__init: cannot get elf header.
> Using /lib/modules/3.11.0-rc2+/build/vmlinux for symbols
> Parsing event 'cycles'
> Reading object code for memory address: 0xffffffff8101ce7d
> File is: /lib/modules/3.11.0-rc2+/build/vmlinux
> On file address is: 0xffffffff8101ce7d
> dso__data_read_offset failed
> ---- end ----
> Test object code reading: FAILED!
> [root@zoo ~]#
>
> I.e. we need the follow on patches to fix this issue, right?

Yes.  It is using an "identity" map so the memory address and on-file 
address are the same - which doesn't work of course.

>
> I'll merge my changes with your first patch and continue from there.

Please take V3.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 1/9] perf tools: add test for reading object code
  2013-08-03 13:45     ` Adrian Hunter
@ 2013-08-05 16:32       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-08-05 16:32 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa,
	Mike Galbraith, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Ingo Molnar

Em Sat, Aug 03, 2013 at 04:45:03PM +0300, Adrian Hunter escreveu:
> On 31/07/2013 5:17 p.m., Arnaldo Carvalho de Melo wrote:
> >Em Wed, Jul 31, 2013 at 12:13:50AM +0300, Adrian Hunter escreveu:
> >>Using the information in mmap events, perf tools can read object
> >>code associated with sampled addresses.  A test is added that
> >>compares bytes read by perf with the same bytes read using
> >>objdump.

> >So this parses objdump output, and we also already have the annotation
> >logic that does that too, have you thought about having common routines
> >for these two cases?
 
> The annotation logic strips out the bytes (--no-show-raw) whereas
> the test extracts only the bytes, so they are not currently
> compatible.

Yeah, they are not, suggestion was to make it so at some point, as there
are many possible use cases that would use one, the other or both info.
 
> >I mean the disasm_line, ins, ins_ops, ins_operands classes, that now
> >lives in util/annotate.h but could be moved somewhere else,
> >disconnecting it as much as possible from annotation, because probably
> >there are more cool things we could do with that... :-)

> >We could certainly do it incrementally, merging your current patch
> >series and then working on sharing code on these two use cases, but
> >perhaps you can do it now?

> >What do you think?
 
> I expect replacing objdump with library calls will end up being the
> way forward.

Yes, and the recent work from Vitillo/Namhyung shows promise, but then
making it work with existing structs created from parsing the objdump
output shuld be ok.

At some point just dropping the parsing would then be transparent to
users of such structs.

But I should try it myself and see how it goes, to show what I mean :-)

- Arnaldo

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-08-05 16:32 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-30 21:13 [PATCH V2 0/9] perf tools: add support for reading object code Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 1/9] perf tools: add test " Adrian Hunter
2013-07-31 14:17   ` Arnaldo Carvalho de Melo
2013-07-31 14:24     ` Peter Zijlstra
2013-08-01  2:32       ` Namhyung Kim
2013-08-03 13:45     ` Adrian Hunter
2013-08-05 16:32       ` Arnaldo Carvalho de Melo
2013-07-31 14:58   ` Arnaldo Carvalho de Melo
2013-07-31 17:28     ` Arnaldo Carvalho de Melo
2013-07-31 17:46       ` Arnaldo Carvalho de Melo
2013-08-03 19:37         ` Adrian Hunter
2013-08-03 14:11       ` Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 2/9] perf tools: load kernel maps before using Adrian Hunter
2013-07-31 14:35   ` Arnaldo Carvalho de Melo
2013-07-30 21:13 ` [PATCH V2 3/9] perf tools: make it possible to read object code from vmlinux Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 5/9] perf tools: avoid SyS kernel syscall aliases Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 6/9] perf tools: make it possible to read object code from kernel modules Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 8/9] perf tools: add kcore to the object code reading test Adrian Hunter
2013-07-30 21:13 ` [PATCH V2 9/9] perf tools: allow annotation using /proc/kcore Adrian Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.