All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] perf tools: add support for reading object code
@ 2013-07-25 14:01 Adrian Hunter
  2013-07-25 14:01 ` [PATCH 1/9] perf tools: add test " Adrian Hunter
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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.


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     | 559 ++++++++++++++++++++++++++++++++++++
 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            | 294 +++++++++++++++++--
 tools/perf/util/symbol.h            |   5 +
 tools/perf/util/thread.h            |   2 +-
 tools/perf/util/unwind.c            |   4 +-
 22 files changed, 1144 insertions(+), 100 deletions(-)
 create mode 100644 tools/perf/tests/code-reading.c


Regards
Adrian

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

* [PATCH 1/9] perf tools: add test for reading object code
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-29  6:28   ` Namhyung Kim
  2013-07-25 14:01 ` [PATCH 2/9] perf tools: load kernel maps before using Adrian Hunter
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 | 502 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 508 insertions(+)
 create mode 100644 tools/perf/tests/code-reading.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index cc81445..4f25ec9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -391,6 +391,7 @@ LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o
 LIB_OBJS += $(OUTPUT)tests/task-exit.o
 LIB_OBJS += $(OUTPUT)tests/sw-clock.o
 LIB_OBJS += $(OUTPUT)tests/sample-parsing.o
+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 5ee3933..67df59a 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -98,6 +98,10 @@ static struct test {
 		.func = test__sample_parsing,
 	},
 	{
+		.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..dba66d5
--- /dev/null
+++ b/tools/perf/tests/code-reading.c
@@ -0,0 +1,502 @@
+#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 int read_objdump_line(char *line, size_t line_len, void **buf,
+			     size_t *len)
+{
+	size_t i;
+
+	/* Skip to a colon */
+	for (i = 0; i < line_len; i++) {
+		if (line[i] == ':')
+			break;
+	}
+	if (line[i++] != ':')
+		return 0;
+
+	/* 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;
+	}
+
+	return 0;
+}
+
+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 || read_objdump_line(line, ret, buf, len)) {
+			pr_debug("getline failed\n");
+			err = -1;
+			break;
+		}
+	}
+
+	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, 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, 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;
+				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 90e3056..fda12266 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__sample_parsing(void);
+int test__code_reading(void);
 
 #endif /* TESTS_H */
-- 
1.7.11.7


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

* [PATCH 2/9] perf tools: load kernel maps before using
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
  2013-07-25 14:01 ` [PATCH 1/9] perf tools: add test " Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 20:10   ` Ingo Molnar
  2013-07-25 14:01 ` [PATCH 3/9] perf tools: make it possible to read object code from vmlinux Adrian Hunter
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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-initialzation 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 eb33da6..178660f 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,
-			      sample->ip, &al);
+			      sample->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 05e1db2..54456a9 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 dba66d5..087e6cf 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -140,7 +140,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 fb58409..c6862ce 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -34,7 +34,7 @@ int build_id__mark_dso_hit(struct perf_tool *tool __maybe_unused,
 	}
 
 	thread__find_addr_map(thread, machine, cpumode, MAP__FUNCTION,
-			      sample->ip, &al);
+			      sample->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 6861d19..c4148f3 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
@@ -700,7 +710,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,
-			      sample->ip, al);
+			      sample->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 fc464bc..69ee2b9 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -42,7 +42,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 3/9] perf tools: make it possible to read object code from vmlinux
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
  2013-07-25 14:01 ` [PATCH 1/9] perf tools: add test " Adrian Hunter
  2013-07-25 14:01 ` [PATCH 2/9] perf tools: load kernel maps before using Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 14:01 ` [PATCH 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test Adrian Hunter
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 16b84ac..1be0382 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -638,10 +638,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 d5528e1..aaeaaaa 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -903,6 +903,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);
@@ -975,7 +979,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;
 	}
@@ -983,7 +987,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 */
@@ -1044,7 +1048,6 @@ do_kallsyms:
 
 	if (err > 0) {
 		dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
-out_fixup:
 		map__fixup_start(map);
 		map__fixup_end(map);
 	}
@@ -1075,7 +1078,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;
@@ -1087,15 +1090,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 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (2 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 3/9] perf tools: make it possible to read object code from vmlinux Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 14:01 ` [PATCH 5/9] perf tools: avoid SyS kernel syscall aliases Adrian Hunter
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 5/9] perf tools: avoid SyS kernel syscall aliases
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (3 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 4/9] perf tools: adjust the vmlinux symtab matches kallsyms test Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 14:01 ` [PATCH 6/9] perf tools: make it possible to read object code from kernel modules Adrian Hunter
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 aaeaaaa..1b97088 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 6/9] perf tools: make it possible to read object code from kernel modules
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (4 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 5/9] perf tools: avoid SyS kernel syscall aliases Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 14:01 ` [PATCH 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 1b97088..7080ef0 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 7/9] perf tools: add support for reading from /proc/kcore
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (5 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 6/9] perf tools: make it possible to read object code from kernel modules Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-30  4:37   ` Namhyung Kim
  2013-07-25 14:01 ` [PATCH 8/9] perf tools: add kcore to the object code reading test Adrian Hunter
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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         | 248 ++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/symbol.h         |   5 +
 9 files changed, 368 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 1be0382..db3fea9 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -866,6 +866,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)
 {
@@ -874,6 +886,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 7080ef0..e76e20b 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,182 @@ 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;
+}
+
+static bool is_host_buildid_str(const char *str)
+{
+	u8 host_build_id[BUILD_ID_SIZE];
+	char host_build_id_str[BUILD_ID_SIZE * 2 + 1];
+
+	if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
+				 sizeof(host_build_id)))
+		return false;
+
+	build_id__sprintf(host_build_id, sizeof(host_build_id),
+			  host_build_id_str);
+
+	return !strcmp(str, host_build_id_str);
+}
+
+/*
+ * If kallsyms is referenced by name then we look for kcore in the same
+ * directory.  Otherwise we use /proc/kcore but only if the buildid matches the
+ * host.
+ */
+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;
+	}
+
+	if (is_host_buildid_str(name)) {
+		strcpy(kcore_filename, "/proc/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 +929,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,
@@ -1062,7 +1303,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);
@@ -1106,8 +1347,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 8/9] perf tools: add kcore to the object code reading test
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (6 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-30  5:24   ` Namhyung Kim
  2013-07-25 14:01 ` [PATCH 9/9] perf tools: allow annotation using /proc/kcore Adrian Hunter
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 | 77 +++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 087e6cf..5fffc6f 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -109,6 +109,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");
@@ -148,7 +151,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;
 	}
@@ -177,6 +181,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);
@@ -188,10 +212,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) {
@@ -325,10 +358,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;
@@ -349,7 +384,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();
 
@@ -362,6 +397,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);
@@ -369,9 +408,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);
@@ -457,8 +502,12 @@ 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 (!have_kcore && try_kcore)
+		err = TEST_CODE_READING_NO_KCORE;
 	else if (excl_kernel)
 		err = TEST_CODE_READING_NO_ACCESS;
 	else
@@ -485,7 +534,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:
@@ -493,9 +544,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 9/9] perf tools: allow annotation using /proc/kcore
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (7 preceding siblings ...)
  2013-07-25 14:01 ` [PATCH 8/9] perf tools: add kcore to the object code reading test Adrian Hunter
@ 2013-07-25 14:01 ` Adrian Hunter
  2013-07-25 20:22 ` [PATCH 0/9] perf tools: add support for reading object code Ingo Molnar
  2013-07-27 12:27 ` Jiri Olsa
  10 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-25 14:01 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 0ecec5f..4585c08 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 2/9] perf tools: load kernel maps before using
  2013-07-25 14:01 ` [PATCH 2/9] perf tools: load kernel maps before using Adrian Hunter
@ 2013-07-25 20:10   ` Ingo Molnar
  0 siblings, 0 replies; 21+ messages in thread
From: Ingo Molnar @ 2013-07-25 20:10 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian


* Adrian Hunter <adrian.hunter@intel.com> wrote:

> In order to use kernel maps to read object code, those
> maps must be adjusted to map to the dso file offset.
> Because lazy-initialzation is used, that is not done

s/initialization

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

Thanks,

	Ingo

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

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


* Adrian Hunter <adrian.hunter@intel.com> wrote:

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

Nice feature!

Thanks,

	Ingo

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

* Re: [PATCH 0/9] perf tools: add support for reading object code
  2013-07-25 14:01 [PATCH 0/9] perf tools: add support for reading object code Adrian Hunter
                   ` (9 preceding siblings ...)
  2013-07-25 20:22 ` [PATCH 0/9] perf tools: add support for reading object code Ingo Molnar
@ 2013-07-27 12:27 ` Jiri Olsa
  10 siblings, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2013-07-27 12:27 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, linux-kernel, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian, Ingo Molnar

On Thu, Jul 25, 2013 at 05:01:21PM +0300, Adrian Hunter wrote:
> Hi
> 
> Here are some patches that add support for reading object code from vmlinux,
> kernel modules and /proc/kcore.

hum.. looks like it's based on your previous fixies
plus some from David? I couldn't get it cleanly applied
on latest Arnaldo's perf/core.

Please make it clear or better provide a git tree/branch ;-)

thanks,
jirka

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

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

Hi Adrian,

Just a few nitpicks below..


On Thu, 25 Jul 2013 17:01:22 +0300, Adrian Hunter wrote:
> 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.
>
[SNIP]
> +
> +static int read_objdump_line(char *line, size_t line_len, void **buf,
> +			     size_t *len)
> +{
> +	size_t i;
> +
> +	/* Skip to a colon */
> +	for (i = 0; i < line_len; i++) {
> +		if (line[i] == ':')
> +			break;
> +	}
> +	if (line[i++] != ':')
> +		return 0;

strchr() ?

> +
> +	/* 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;
> +	}
> +
> +	return 0;

It seems this function always returns 0..


> +}
> +
> +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 || read_objdump_line(line, ret, buf, len)) {

so no need to check the return value.


> +			pr_debug("getline failed\n");
> +			err = -1;
> +			break;
> +		}
> +	}
> +
> +	free(line);
> +
> +	return err;
> +}
> +
[SNIP]
> +
> +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,

Is it intended to use the freq of 40000 instead of 4000 (default)?


> +		.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, 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";

A double whitespace.


> +		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;
> +				continue;

It seems the evlist is leaked.  perf_evlist__delete(evlist) is needed.

Thanks,
Namhyung


> +			}
> +			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 90e3056..fda12266 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__sample_parsing(void);
> +int test__code_reading(void);
>  
>  #endif /* TESTS_H */

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

* Re: [PATCH 7/9] perf tools: add support for reading from /proc/kcore
  2013-07-25 14:01 ` [PATCH 7/9] perf tools: add support for reading from /proc/kcore Adrian Hunter
@ 2013-07-30  4:37   ` Namhyung Kim
  2013-07-30 21:12     ` Adrian Hunter
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2013-07-30  4:37 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, Ingo Molnar

Hi,

On Thu, 25 Jul 2013 17:01:28 +0300, Adrian Hunter wrote:
> 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.

[SNIP]
> +
> +static bool is_host_buildid_str(const char *str)
> +{
> +	u8 host_build_id[BUILD_ID_SIZE];
> +	char host_build_id_str[BUILD_ID_SIZE * 2 + 1];
> +
> +	if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
> +				 sizeof(host_build_id)))
> +		return false;
> +
> +	build_id__sprintf(host_build_id, sizeof(host_build_id),
> +			  host_build_id_str);
> +
> +	return !strcmp(str, host_build_id_str);
> +}
> +
> +/*
> + * If kallsyms is referenced by name then we look for kcore in the same
> + * directory.  Otherwise we use /proc/kcore but only if the buildid matches the
> + * host.
> + */
> +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;
> +	}
> +
> +	if (is_host_buildid_str(name)) {

IIUC the name should start with '/' but build-id is not.  So doesn't it
always fail?

Thanks,
Namhyung


> +		strcpy(kcore_filename, "/proc/kcore");
> +		return true;
> +	}
> +
> +	return false;
> +}

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

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

On Thu, 25 Jul 2013 17:01:29 +0300, Adrian Hunter wrote:
> 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.
>
[SNIP]

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

I was confused.  So this means that the symbols came from the kallsyms
but actual binary data came from the kcore, right?


>  		pr_debug("Unexpected kernel address - skipping\n");
>  		return 0;
>  	}

[SNIP]
> +
> +	/* 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);
> @@ -457,8 +502,12 @@ 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 (!have_kcore && try_kcore)
> +		err = TEST_CODE_READING_NO_KCORE;

It seems that the above line is not reachable since we already bailed
out the second test if we don't have kcore.


>  	else if (excl_kernel)
>  		err = TEST_CODE_READING_NO_ACCESS;
>  	else
> @@ -485,7 +534,9 @@ int test__code_reading(void)
>  {
>  	int ret;
>  
> -	ret = do_test_code_reading();
> +	ret = do_test_code_reading(false);
> +	if (!ret)

Shouldn't it be

	if (ret)
?

Thanks,
Namhyung


> +		ret = do_test_code_reading(true);
>  
>  	switch (ret) {
>  	case TEST_CODE_READING_OK:
> @@ -493,9 +544,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;
>  	};

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

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

On 29/07/2013 9:28 a.m., Namhyung Kim wrote:
> Hi Adrian,
>
> Just a few nitpicks below..
>
>
> On Thu, 25 Jul 2013 17:01:22 +0300, Adrian Hunter wrote:
>> 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.
>>
> [SNIP]
>> +
>> +static int read_objdump_line(char *line, size_t line_len, void **buf,
>> +			     size_t *len)
>> +{
>> +	size_t i;
>> +
>> +	/* Skip to a colon */
>> +	for (i = 0; i<  line_len; i++) {
>> +		if (line[i] == ':')
>> +			break;
>> +	}
>> +	if (line[i++] != ':')
>> +		return 0;
>
> strchr() ?

Fixed in V2

>
>> +
>> +	/* 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;
>> +	}
>> +
>> +	return 0;
>
> It seems this function always returns 0..

Fixed in V2

>
>
>> +}
>> +
>> +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 || read_objdump_line(line, ret, buf, len)) {
>
> so no need to check the return value.

Fixed in V2

>
>
>> +			pr_debug("getline failed\n");
>> +			err = -1;
>> +			break;
>> +		}
>> +	}
>> +
>> +	free(line);
>> +
>> +	return err;
>> +}
>> +
> [SNIP]
>> +
>> +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,
>
> Is it intended to use the freq of 40000 instead of 4000 (default)?

Yes.  The "workload" is small so a higher sampling rate is preferable.

>
>
>> +		.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, 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";
>
> A double whitespace.

Fixed in V2

>
>
>> +		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;
>> +				continue;
>
> It seems the evlist is leaked.  perf_evlist__delete(evlist) is needed.

Fixed in V2

>
> Thanks,
> Namhyung
>
>
>> +			}
>> +			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 90e3056..fda12266 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__sample_parsing(void);
>> +int test__code_reading(void);
>>
>>   #endif /* TESTS_H */


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

* Re: [PATCH 7/9] perf tools: add support for reading from /proc/kcore
  2013-07-30  4:37   ` Namhyung Kim
@ 2013-07-30 21:12     ` Adrian Hunter
  0 siblings, 0 replies; 21+ messages in thread
From: Adrian Hunter @ 2013-07-30 21:12 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, Ingo Molnar

On 30/07/2013 7:37 a.m., Namhyung Kim wrote:
> Hi,
>
> On Thu, 25 Jul 2013 17:01:28 +0300, Adrian Hunter wrote:
>> 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.
>
> [SNIP]
>> +
>> +static bool is_host_buildid_str(const char *str)
>> +{
>> +	u8 host_build_id[BUILD_ID_SIZE];
>> +	char host_build_id_str[BUILD_ID_SIZE * 2 + 1];
>> +
>> +	if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
>> +				 sizeof(host_build_id)))
>> +		return false;
>> +
>> +	build_id__sprintf(host_build_id, sizeof(host_build_id),
>> +			  host_build_id_str);
>> +
>> +	return !strcmp(str, host_build_id_str);
>> +}
>> +
>> +/*
>> + * If kallsyms is referenced by name then we look for kcore in the same
>> + * directory.  Otherwise we use /proc/kcore but only if the buildid matches the
>> + * host.
>> + */
>> +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;
>> +	}
>> +
>> +	if (is_host_buildid_str(name)) {
>
> IIUC the name should start with '/' but build-id is not.  So doesn't it
> always fail?

In fact I realized the whole thing is not necessary. Fixed in V2.

>
> Thanks,
> Namhyung
>
>
>> +		strcpy(kcore_filename, "/proc/kcore");
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}


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

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

On 30/07/2013 8:24 a.m., Namhyung Kim wrote:
> On Thu, 25 Jul 2013 17:01:29 +0300, Adrian Hunter wrote:
>> 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.
>>
> [SNIP]
>
>> -	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)) {
>
> I was confused.  So this means that the symbols came from the kallsyms
> but actual binary data came from the kcore, right?

Yes

>
>
>>   		pr_debug("Unexpected kernel address - skipping\n");
>>   		return 0;
>>   	}
>
> [SNIP]
>> +
>> +	/* 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);
>> @@ -457,8 +502,12 @@ 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 (!have_kcore&&  try_kcore)
>> +		err = TEST_CODE_READING_NO_KCORE;
>
> It seems that the above line is not reachable since we already bailed
> out the second test if we don't have kcore.

Fixed in V2

>
>
>>   	else if (excl_kernel)
>>   		err = TEST_CODE_READING_NO_ACCESS;
>>   	else
>> @@ -485,7 +534,9 @@ int test__code_reading(void)
>>   {
>>   	int ret;
>>
>> -	ret = do_test_code_reading();
>> +	ret = do_test_code_reading(false);
>> +	if (!ret)
>
> Shouldn't it be
>
> 	if (ret)

No.  Zero is returned if vmlinux is found and the kernel is not 
excluded.  In all other cases, there is no point trying kcore.
Note if vmlinux is not found, then kcore will be tried the first
time around.

> ?
>
> Thanks,
> Namhyung
>
>
>> +		ret = do_test_code_reading(true);
>>
>>   	switch (ret) {
>>   	case TEST_CODE_READING_OK:
>> @@ -493,9 +544,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;
>>   	};


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

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

Em Wed, Jul 31, 2013 at 12:11:13AM +0300, Adrian Hunter escreveu:
> On 29/07/2013 9:28 a.m., Namhyung Kim wrote:
> >>+
> >>+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,
> >
> >Is it intended to use the freq of 40000 instead of 4000 (default)?
> 
> Yes.  The "workload" is small so a higher sampling rate is preferable.

But preferably one that is less than:

[root@zoo ~]# cat /proc/sys/kernel/perf_event_max_sample_rate
25000
[root@zoo ~]#

That is the reason why this test is failing on this Ivy Bridge notebook:

[root@zoo ~]# dmesg | grep perf_event_max_sample_rate
[ 4068.969761] perf samples too long (2552 > 2500), lowering kernel.perf_event_max_sample_rate to 50000
[ 4069.842176] perf samples too long (5039 > 5000), lowering kernel.perf_event_max_sample_rate to 25000
[root@zoo ~]# 

Look at kernel/events/core.c.

So keeping it at 4k may not be a bad idea, or at least have code that reads
/proc/sys/kernel/perf_event_max_sample_rate if using the highest freq is
desired.

- Arnaldo

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

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

On 31/07/2013 8:43 p.m., Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 31, 2013 at 12:11:13AM +0300, Adrian Hunter escreveu:
>> On 29/07/2013 9:28 a.m., Namhyung Kim wrote:
>>>> +
>>>> +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,
>>>
>>> Is it intended to use the freq of 40000 instead of 4000 (default)?
>>
>> Yes.  The "workload" is small so a higher sampling rate is preferable.
>
> But preferably one that is less than:
>
> [root@zoo ~]# cat /proc/sys/kernel/perf_event_max_sample_rate
> 25000
> [root@zoo ~]#
>
> That is the reason why this test is failing on this Ivy Bridge notebook:
>
> [root@zoo ~]# dmesg | grep perf_event_max_sample_rate
> [ 4068.969761] perf samples too long (2552>  2500), lowering kernel.perf_event_max_sample_rate to 50000
> [ 4069.842176] perf samples too long (5039>  5000), lowering kernel.perf_event_max_sample_rate to 25000
> [root@zoo ~]#
>
> Look at kernel/events/core.c.
>
> So keeping it at 4k may not be a bad idea, or at least have code that reads
> /proc/sys/kernel/perf_event_max_sample_rate if using the highest freq is
> desired.

I dropped it to 4k in V3.


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

end of thread, other threads:[~2013-08-03 13:47 UTC | newest]

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

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.