Skip to contentSkip to conversation

perf tools: add support for reading object code

PATCH Superseded Adrian Hunter 5 participants 21 messages Jul 25, 2013 – Aug 3, 2013 perf events

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Adrian Hunter

author reviewer
#0 lore raw

cover perf tools: add support for reading object code

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

author reviewer
#1 lore raw

PATCH 1/9 perf tools: add test for reading object code

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 <[email protected]>
---
 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
tools/perf/Makefile +1 -0
index cc81445..4f25ec9 100644
@@ -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
tools/perf/tests/builtin-test.c +4 -0
index 5ee3933..67df59a 100644
@@ -98,6 +98,10 @@ static struct test {
 		.func = test__sample_parsing,
 	},
 	{
+		.desc = "Test object code reading",
+		.func = test__code_reading,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/code-reading.c new file +502 -0

502 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index 90e3056..fda12266 100644
@@ -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 */
Signature · 1 line
1.7.11.7
maintainer
#2 lore raw

Re: [PATCH 1/9] perf tools: add test for reading object code

Hi Adrian,

Just a few nitpicks below..

Quoted 5 lines · Adrian Hunter

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]

tools/perf/tests/code-reading.c line 43

Quoted 13 lines
tools/perf/tests/code-reading.c new file +13 -0
+
+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() ?

Quoted 17 lines
tools/perf/tests/code-reading.c new file +17 -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++];

tools/perf/tests/code-reading.c line 70

Quoted 10 lines
tools/perf/tests/code-reading.c new file +10 -0
+		/* 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..

Quoted 4 lines
tools/perf/tests/code-reading.c new file +4 -0
+}
+
+static int read_objdump_output(FILE *f, void **buf, size_t *len)
+{

tools/perf/tests/code-reading.c line 84

Quoted 10 lines
tools/perf/tests/code-reading.c new file +10 -0
+	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.

Quoted 11 lines
tools/perf/tests/code-reading.c new file +11 -0
+			pr_debug("getline failed\n");
+			err = -1;
+			break;
+		}
+	}
+
+	free(line);
+
+	return err;
+}
+

[SNIP]

tools/perf/tests/code-reading.c line 340

Quoted 11 lines
tools/perf/tests/code-reading.c new file +11 -0
+
+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)?

Quoted 64 lines
tools/perf/tests/code-reading.c new file +64 -0
+		.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;
+

tools/perf/tests/code-reading.c line 414

Quoted 10 lines
tools/perf/tests/code-reading.c new file +10 -0
+		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.

Quoted 12 lines
tools/perf/tests/code-reading.c new file +12 -0
+		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);

tools/perf/tests/code-reading.c line 436

Quoted 10 lines
tools/perf/tests/code-reading.c new file +10 -0
+
+		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

Quoted 77 lines
tools/perf/tests/code-reading.c new file +66 -0
+			}
+			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;
+	};
+}
tools/perf/tests/tests.h +1 -0
index 90e3056..fda12266 100644
@@ -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 */
author reviewer
#3 lore raw

Re: [PATCH 1/9] perf tools: add test for reading object code

Quoted 12 lines · Namhyung Kim

Hi Adrian,

Just a few nitpicks below..

Quoted 5 lines · Adrian Hunter

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]

Quoted 15 lines

tools/perf/tests/code-reading.c line 43

Quoted 13 lines
tools/perf/tests/code-reading.c new file +13 -0
+
+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

Quoted 22 lines
Quoted 22 lines
tools/perf/tests/code-reading.c new file +21 -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 */
Quoted 8 lines

tools/perf/tests/code-reading.c line 70

Quoted 6 lines
tools/perf/tests/code-reading.c new file +6 -0
+		*(unsigned char *)*buf = (hex(c1)<<  4) | hex(c2);
+		*buf += 1;
+		*len -= 1;
+	}
+
+	return 0;

It seems this function always returns 0..

Fixed in V2

Quoted 18 lines
Quoted 6 lines
tools/perf/tests/code-reading.c new file +4 -0
+}
+
+static int read_objdump_output(FILE *f, void **buf, size_t *len)
+{

tools/perf/tests/code-reading.c

Quoted 10 lines
tools/perf/tests/code-reading.c new file +10 -0
+	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

Quoted 14 lines
Quoted 13 lines
tools/perf/tests/code-reading.c new file +11 -0
+			pr_debug("getline failed\n");
+			err = -1;
+			break;
+		}
+	}
+
+	free(line);
+
+	return err;
+}
+

[SNIP]

Quoted 13 lines

tools/perf/tests/code-reading.c line 340

Quoted 11 lines
tools/perf/tests/code-reading.c new file +11 -0
+
+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.

Quoted 70 lines
Quoted 70 lines
tools/perf/tests/code-reading.c new file +68 -0
+		.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;
Quoted 8 lines

tools/perf/tests/code-reading.c line 414

Quoted 6 lines
tools/perf/tests/code-reading.c new file +6 -0
+		}
+
+		perf_evlist__set_maps(evlist, cpus, threads);
+
+		if (excl_kernel)
+			str =  "cycles:u";

A double whitespace.

Fixed in V2

Quoted 18 lines
Quoted 18 lines
tools/perf/tests/code-reading.c new file +16 -0
+		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;
Quoted 8 lines

tools/perf/tests/code-reading.c line 436

Quoted 6 lines
tools/perf/tests/code-reading.c new file +6 -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

Quoted 82 lines

Thanks,
Namhyung

Quoted 77 lines
tools/perf/tests/code-reading.c new file +66 -0
+			}
+			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;
+	};
+}
tools/perf/tests/tests.h +1 -0
index 90e3056..fda12266 100644
@@ -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 */
#4 lore raw

Re: [PATCH 1/9] perf tools: add test for reading object code

Em Wed, Jul 31, 2013 at 12:11:13AM +0300, Adrian Hunter escreveu:

Quoted 15 lines · Namhyung Kim
Quoted 5 lines
Quoted 5 lines
tools/perf/tests/code-reading.c new file +5 -0
+
+static int do_test_code_reading(void)
+{
+	struct machines machines;
+	struct machine *machine;
Quoted 8 lines

tools/perf/tests/code-reading.c line 340

Quoted 6 lines
tools/perf/tests/code-reading.c new file +6 -0
+	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)?

Quoted 1 line

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

author reviewer
#5 lore raw

Re: [PATCH 1/9] perf tools: add test for reading object code

Quoted 29 lines · Arnaldo Carvalho de Melo

Em Wed, Jul 31, 2013 at 12:11:13AM +0300, Adrian Hunter escreveu:

Quoted 15 lines · Namhyung Kim
Quoted 5 lines
Quoted 5 lines
tools/perf/tests/code-reading.c new file +5 -0
+
+static int do_test_code_reading(void)
+{
+	struct machines machines;
+	struct machine *machine;
Quoted 8 lines

tools/perf/tests/code-reading.c line 340

Quoted 6 lines
tools/perf/tests/code-reading.c new file +6 -0
+	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)?

Quoted 1 line

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
Quoted 7 lines

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

author reviewer
#6 lore raw

PATCH 2/9 perf tools: load kernel maps before using

In reply to Adrian Hunter

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 <[email protected]>
---
 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(-)
tools/perf/builtin-inject.c +1 -1
index eb33da6..178660f 100644
@@ -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) {
tools/perf/builtin-script.c +2 -2
index 05e1db2..54456a9 100644
@@ -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;
tools/perf/tests/code-reading.c +1 -1
index dba66d5..087e6cf 100644
@@ -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;
tools/perf/util/build-id.c +1 -1
index fb58409..c6862ce 100644
@@ -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;
tools/perf/util/event.c +14 -4
index 6861d19..c4148f3 100644
@@ -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>");
tools/perf/util/thread.h +1 -1
index fc464bc..69ee2b9 100644
@@ -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,
tools/perf/util/unwind.c +2 -2
index 958723b..5bbd494 100644
@@ -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;
Signature · 1 line
1.7.11.7
#7 lore raw

Re: [PATCH 2/9] perf tools: load kernel maps before using

Quoted 3 lines · Adrian Hunter

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

Quoted 4 lines

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

author reviewer
#8 lore raw

PATCH 3/9 perf tools: make it possible to read object code from vmlinux

In reply to Adrian Hunter

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 <[email protected]>
---
 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(-)
tools/perf/util/dso.c +2 -2
index c4374f0..121583d 100644
@@ -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;
tools/perf/util/dso.h +8 -0
index d51aaf2..02aadaf 100644
@@ -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 */
tools/perf/util/machine.c +1 -3
index 16b84ac..1be0382 100644
@@ -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;
 }
tools/perf/util/map.c +0 -35
index 8bcdf9e..5f662a3 100644
@@ -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;
tools/perf/util/symbol-elf.c +91 -9

150 lines not on this page · raw message

tools/perf/util/symbol.c +10 -12
index d5528e1..aaeaaaa 100644
@@ -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);
 	}
Signature · 1 line
1.7.11.7
author reviewer
#9 lore raw

PATCH 4/9 perf tools: adjust the vmlinux symtab matches kallsyms test

In reply to Adrian Hunter

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 <[email protected]>
---
 tools/perf/tests/vmlinux-kallsyms.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)
tools/perf/tests/vmlinux-kallsyms.c +21 -11
index add1539..e2e1498 100644
@@ -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;
Signature · 1 line
1.7.11.7
author reviewer
#10 lore raw

PATCH 5/9 perf tools: avoid SyS kernel syscall aliases

In reply to Adrian Hunter

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 <[email protected]>
---
 tools/perf/util/symbol.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
tools/perf/util/symbol.c +14 -3
index aaeaaaa..1b97088 100644
@@ -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)
Signature · 1 line
1.7.11.7
author reviewer
#11 lore raw

PATCH 6/9 perf tools: make it possible to read object code from kernel modules

In reply to Adrian Hunter

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 <[email protected]>
---
 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(-)
tools/perf/util/dso.c +1 -0
index 121583d..1955804 100644
@@ -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;
tools/perf/util/dso.h +1 -0
index 02aadaf..735a837 100644
@@ -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;
tools/perf/util/map.c +9 -5
index 5f662a3..4d599fe 100644
@@ -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)
tools/perf/util/symbol-elf.c +15 -0
index ed6f443..f00596f 100644
@@ -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);
tools/perf/util/symbol.c +8 -3
index 1b97088..7080ef0 100644
@@ -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;
Signature · 1 line
1.7.11.7
author reviewer
#12 lore raw

PATCH 7/9 perf tools: add support for reading from /proc/kcore

In reply to Adrian Hunter

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 <[email protected]>
---
 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(-)
tools/perf/util/dso.c +5 -0
index 1955804..e3c1ff8 100644
@@ -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:
tools/perf/util/dso.h +8 -0
index 735a837..b793053 100644
@@ -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 */
tools/perf/util/machine.c +16 -0
index 1be0382..db3fea9 100644
@@ -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;
tools/perf/util/map.c +18 -0
index 4d599fe..9e8304c 100644
@@ -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;
+}
tools/perf/util/map.h +13 -0
index a887f2c..2cc93cb 100644
@@ -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,
tools/perf/util/symbol-elf.c +51 -0

57 lines not on this page · raw message

tools/perf/util/symbol-minimal.c +7 -0
index a7390cd..3a802c3 100644
@@ -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)
 {
 }
tools/perf/util/symbol.c +245 -3

291 lines not on this page · raw message

tools/perf/util/symbol.h +5 -0
index 5f720dc..fd5b70e 100644
@@ -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 */
Signature · 1 line
1.7.11.7
maintainer
#13 lore raw

Re: [PATCH 7/9] perf tools: add support for reading from /proc/kcore

Hi,

Quoted 7 lines · Adrian Hunter

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]

Quoted 27 lines
tools/perf/util/symbol.c +27 -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);

tools/perf/util/symbol.c bool symbol__restricted_filename(const char *filename, · line 797

Quoted 10 lines
tools/perf/util/symbol.c +10 -0
+	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

Quoted 6 lines
tools/perf/util/symbol.c +6 -0
+		strcpy(kcore_filename, "/proc/kcore");
+		return true;
+	}
+
+	return false;
+}
author reviewer
#14 lore raw

Re: [PATCH 7/9] perf tools: add support for reading from /proc/kcore

Quoted 44 lines · Namhyung Kim

Hi,

Quoted 7 lines · Adrian Hunter

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]

Quoted 32 lines
tools/perf/util/symbol.c +32 -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")) {
Quoted 8 lines

tools/perf/util/symbol.c bool symbol__restricted_filename(const char *filename, · line 797

Quoted 5 lines
tools/perf/util/symbol.c +5 -0
+		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.

Quoted 11 lines

Thanks,
Namhyung

Quoted 6 lines
tools/perf/util/symbol.c +6 -0
+		strcpy(kcore_filename, "/proc/kcore");
+		return true;
+	}
+
+	return false;
+}
author reviewer
#15 lore raw

PATCH 8/9 perf tools: add kcore to the object code reading test

In reply to Adrian Hunter

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 <[email protected]>
---
 tools/perf/tests/code-reading.c | 77 +++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 10 deletions(-)
tools/perf/tests/code-reading.c +67 -10
index 087e6cf..5fffc6f 100644
@@ -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;
 	};
Signature · 1 line
1.7.11.7
maintainer
#16 lore raw

Re: [PATCH 8/9] perf tools: add kcore to the object code reading test

Quoted 7 lines · Adrian Hunter

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]

tools/perf/tests/code-reading.c static int read_object_code(u64 addr, size_t len, u8 cpumode, · line 155

Quoted 3 lines
tools/perf/tests/code-reading.c +2 -1
-	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?

Quoted 3 lines
tools/perf/tests/code-reading.c +0 -0
 		pr_debug("Unexpected kernel address - skipping\n");
 		return 0;
 	}

[SNIP]

Quoted 11 lines
tools/perf/tests/code-reading.c +7 -0
+
+	/* 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);

tools/perf/tests/code-reading.c static int do_test_code_reading(void) · line 510

Quoted 10 lines
tools/perf/tests/code-reading.c +5 -1
@@ -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.

tools/perf/tests/code-reading.c int test__code_reading(void) · line 538

Quoted 10 lines
tools/perf/tests/code-reading.c +2 -1
 	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

Quoted 20 lines
tools/perf/tests/code-reading.c +7 -0
+		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;
 	};
author reviewer
#17 lore raw

Re: [PATCH 8/9] perf tools: add kcore to the object code reading test

Quoted 8 lines · Namhyung Kim
Quoted 7 lines · Adrian Hunter

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.

Quoted 8 lines

[SNIP]

tools/perf/tests/code-reading.c static int read_object_code(u64 addr, size_t len, u8 cpumode, · line 155

Quoted 3 lines
tools/perf/tests/code-reading.c +2 -1
-	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

Quoted 21 lines
Quoted 5 lines

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

[SNIP]

Quoted 14 lines
tools/perf/tests/code-reading.c +7 -0
+
+	/* 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;
Quoted 10 lines

tools/perf/tests/code-reading.c static int do_test_code_reading(void) · line 510

Quoted 7 lines
tools/perf/tests/code-reading.c +5 -1
-	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

Quoted 9 lines
Quoted 9 lines
tools/perf/tests/code-reading.c +0 -0
  	else if (excl_kernel)
  		err = TEST_CODE_READING_NO_ACCESS;
  	else

@@ -485,7 +534,9 @@ int test__code_reading(void)
  {
  	int ret;
Quoted 7 lines

tools/perf/tests/code-reading.c int test__code_reading(void) · line 538

Quoted 3 lines
tools/perf/tests/code-reading.c +2 -1
-	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.

Quoted 26 lines

?

Thanks,
Namhyung

Quoted 20 lines
tools/perf/tests/code-reading.c +7 -0
+		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;
  	};
author reviewer
#18 lore raw

PATCH 9/9 perf tools: allow annotation using /proc/kcore

In reply to Adrian Hunter

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 <[email protected]>
---
 tools/perf/builtin-top.c   |  3 ++-
 tools/perf/util/annotate.c | 13 +++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)
tools/perf/builtin-top.c +2 -1
index 0ecec5f..4585c08 100644
@@ -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);
tools/perf/util/annotate.c +9 -4
index d102716..4ab2f11 100644
@@ -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 : "",
Signature · 1 line
1.7.11.7
#19 lore raw
In reply to Adrian Hunter
Quoted 4 lines · Adrian Hunter

Hi

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

Nice feature!

Thanks,

Ingo

#20 lore raw
In reply to Adrian Hunter
Quoted 4 lines · Adrian Hunter

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