All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	David Ahern <dsahern@gmail.com>, Jiri Olsa <jolsa@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Wang Nan <wangnan0@huawei.com>
Subject: [PATCH 08/10] perf symbols: Accept symbols starting at address 0
Date: Wed,  3 May 2017 10:58:24 -0300	[thread overview]
Message-ID: <20170503135826.28675-9-acme@kernel.org> (raw)
In-Reply-To: <20170503135826.28675-1-acme@kernel.org>

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

That is the case of _text on s390, and we have some functions that return an
address, using address zero to report problems, oops.

This would lead the symbol loading routines to not use "_text" as the reference
relocation symbol, or the first symbol for the kernel, but use instead
"_stext", that is at the same address on x86_64 and others, but not on s390:

  [acme@localhost perf-4.11.0-rc6]$ head -15 /proc/kallsyms
  0000000000000000 T _text
  0000000000000418 t iplstart
  0000000000000800 T start
  000000000000080a t .base
  000000000000082e t .sk8x8
  0000000000000834 t .gotr
  0000000000000842 t .cmd
  0000000000000846 t .parm
  000000000000084a t .lowcase
  0000000000010000 T startup
  0000000000010010 T startup_kdump
  0000000000010214 t startup_kdump_relocated
  0000000000011000 T startup_continue
  00000000000112a0 T _ehead
  0000000000100000 T _stext
  [acme@localhost perf-4.11.0-rc6]$

Which in turn would make 'perf test vmlinux' to fail because it wouldn't find
the symbols before "_stext" in kallsyms.

Fix it by using the return value only for errors and storing the
address, when the symbol is successfully found, in a provided pointer
arg.

Before this patch:

After:

  [acme@localhost perf-4.11.0-rc6]$ tools/perf/perf test -v 1
   1: vmlinux symtab matches kallsyms            :
  --- start ---
  test child forked, pid 40693
  Looking at the vmlinux_path (8 entries long)
  Using /usr/lib/debug/lib/modules/3.10.0-654.el7.s390x/vmlinux for symbols
  ERR : 0: _text not on kallsyms
  ERR : 0x418: iplstart not on kallsyms
  ERR : 0x800: start not on kallsyms
  ERR : 0x80a: .base not on kallsyms
  ERR : 0x82e: .sk8x8 not on kallsyms
  ERR : 0x834: .gotr not on kallsyms
  ERR : 0x842: .cmd not on kallsyms
  ERR : 0x846: .parm not on kallsyms
  ERR : 0x84a: .lowcase not on kallsyms
  ERR : 0x10000: startup not on kallsyms
  ERR : 0x10010: startup_kdump not on kallsyms
  ERR : 0x10214: startup_kdump_relocated not on kallsyms
  ERR : 0x11000: startup_continue not on kallsyms
  ERR : 0x112a0: _ehead not on kallsyms
  <SNIP warnings>
  test child finished with -1
  ---- end ----
  vmlinux symtab matches kallsyms: FAILED!
  [acme@localhost perf-4.11.0-rc6]$

After:

  [acme@localhost perf-4.11.0-rc6]$ tools/perf/perf test -v 1
   1: vmlinux symtab matches kallsyms            :
  --- start ---
  test child forked, pid 47160
  <SNIP warnings>
  test child finished with 0
  ---- end ----
  vmlinux symtab matches kallsyms: Ok
  [acme@localhost perf-4.11.0-rc6]$

Reported-by: Michael Petlan <mpetlan@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-9x9bwgd3btwdk1u51xie93fz@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-buildid-cache.c | 13 ++++++++-----
 tools/perf/util/event.c            |  9 +++++----
 tools/perf/util/event.h            |  4 ++--
 tools/perf/util/machine.c          | 28 +++++++++++++++++-----------
 tools/perf/util/symbol.c           | 11 +++++------
 5 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 64b44e81c771..9eba7f1add1f 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -49,19 +49,22 @@ static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
 	char to[PATH_MAX];
 	const char *name;
 	u64 addr1 = 0, addr2 = 0;
-	int i;
+	int i, err = -1;
 
 	scnprintf(from, sizeof(from), "%s/kallsyms", from_dir);
 	scnprintf(to, sizeof(to), "%s/kallsyms", to_dir);
 
 	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
-		addr1 = kallsyms__get_function_start(from, name);
-		if (addr1)
+		err = kallsyms__get_function_start(from, name, &addr1);
+		if (!err)
 			break;
 	}
 
-	if (name)
-		addr2 = kallsyms__get_function_start(to, name);
+	if (err)
+		return false;
+
+	if (kallsyms__get_function_start(to, name, &addr2))
+		return false;
 
 	return addr1 == addr2;
 }
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 437194fe0434..dc5c3bb69d73 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -768,15 +768,16 @@ static int find_symbol_cb(void *arg, const char *name, char type,
 	return 1;
 }
 
-u64 kallsyms__get_function_start(const char *kallsyms_filename,
-				 const char *symbol_name)
+int kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name, u64 *addr)
 {
 	struct process_symbol_args args = { .name = symbol_name, };
 
 	if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
-		return 0;
+		return -1;
 
-	return args.start;
+	*addr = args.start;
+	return 0;
 }
 
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index cfbe32bd7413..27ac047490c3 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -675,8 +675,8 @@ size_t perf_event__fprintf_cpu_map(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_namespaces(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf(union perf_event *event, FILE *fp);
 
-u64 kallsyms__get_function_start(const char *kallsyms_filename,
-				 const char *symbol_name);
+int kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name, u64 *addr);
 
 void *cpu_map_data__alloc(struct cpu_map *map, size_t *size, u16 *type, int *max);
 void  cpu_map_data__synthesize(struct cpu_map_data *data, struct cpu_map *map,
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7a47f52ccfcc..d97e014c3df3 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -796,11 +796,11 @@ const char *ref_reloc_sym_names[] = {"_text", "_stext", NULL};
  * Returns the name of the start symbol in *symbol_name. Pass in NULL as
  * symbol_name if it's not that important.
  */
-static u64 machine__get_running_kernel_start(struct machine *machine,
-					     const char **symbol_name)
+static int machine__get_running_kernel_start(struct machine *machine,
+					     const char **symbol_name, u64 *start)
 {
 	char filename[PATH_MAX];
-	int i;
+	int i, err = -1;
 	const char *name;
 	u64 addr = 0;
 
@@ -810,21 +810,28 @@ static u64 machine__get_running_kernel_start(struct machine *machine,
 		return 0;
 
 	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
-		addr = kallsyms__get_function_start(filename, name);
-		if (addr)
+		err = kallsyms__get_function_start(filename, name, &addr);
+		if (!err)
 			break;
 	}
 
+	if (err)
+		return -1;
+
 	if (symbol_name)
 		*symbol_name = name;
 
-	return addr;
+	*start = addr;
+	return 0;
 }
 
 int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
 {
 	int type;
-	u64 start = machine__get_running_kernel_start(machine, NULL);
+	u64 start = 0;
+
+	if (machine__get_running_kernel_start(machine, NULL, &start))
+		return -1;
 
 	/* In case of renewal the kernel map, destroy previous one */
 	machine__destroy_kernel_maps(machine);
@@ -1185,8 +1192,8 @@ static int machine__create_modules(struct machine *machine)
 int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
-	const char *name;
-	u64 addr;
+	const char *name = NULL;
+	u64 addr = 0;
 	int ret;
 
 	if (kernel == NULL)
@@ -1211,8 +1218,7 @@ int machine__create_kernel_maps(struct machine *machine)
 	 */
 	map_groups__fixup_end(&machine->kmaps);
 
-	addr = machine__get_running_kernel_start(machine, &name);
-	if (!addr) {
+	if (machine__get_running_kernel_start(machine, &name, &addr)) {
 	} else if (maps__set_kallsyms_ref_reloc_sym(machine->vmlinux_maps, name, addr)) {
 		machine__destroy_kernel_maps(machine);
 		return -1;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2cb7665e9973..b349e8eda0e2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -466,7 +466,7 @@ void dso__insert_symbol(struct dso *dso, enum map_type type, struct symbol *sym)
 struct symbol *dso__find_symbol(struct dso *dso,
 				enum map_type type, u64 addr)
 {
-	if (dso->last_find_result[type].addr != addr) {
+	if (dso->last_find_result[type].addr != addr || dso->last_find_result[type].symbol == NULL) {
 		dso->last_find_result[type].addr   = addr;
 		dso->last_find_result[type].symbol = symbols__find(&dso->symbols[type], addr);
 	}
@@ -1075,8 +1075,9 @@ static int validate_kcore_addresses(const char *kallsyms_filename,
 	if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) {
 		u64 start;
 
-		start = kallsyms__get_function_start(kallsyms_filename,
-						     kmap->ref_reloc_sym->name);
+		if (kallsyms__get_function_start(kallsyms_filename,
+						 kmap->ref_reloc_sym->name, &start))
+			return -ENOENT;
 		if (start != kmap->ref_reloc_sym->addr)
 			return -EINVAL;
 	}
@@ -1248,9 +1249,7 @@ static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
 	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
 		return 0;
 
-	addr = kallsyms__get_function_start(filename,
-					    kmap->ref_reloc_sym->name);
-	if (!addr)
+	if (kallsyms__get_function_start(filename, kmap->ref_reloc_sym->name, &addr))
 		return -1;
 
 	*delta = addr - kmap->ref_reloc_sym->addr;
-- 
2.9.3

  parent reply	other threads:[~2017-05-03 13:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-03 13:58 [GIT PULL 00/10] perf/core improvements and fixes Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 01/10] perf buildid: Move prototypes from util.h to build-id.h Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 02/10] perf tools: Move event prototypes from util.h to event.h Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 03/10] perf memswap: Split the byteswap memory range wrappers from util.[ch] Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 04/10] perf tools: Move HAS_BOOL define to where perl headers are used Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 05/10] perf ui gtk: Move gtk .so name to the only place where it is used Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 06/10] perf units: Move parse_tag_value() to units.[ch] Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 07/10] tools lib string: Adopt prefixcmp() from perf and subcmd Arnaldo Carvalho de Melo
2017-05-03 13:58 ` Arnaldo Carvalho de Melo [this message]
2017-05-03 13:58 ` [PATCH 09/10] perf symbols: Allow user probes on versioned symbols Arnaldo Carvalho de Melo
2017-05-03 13:58 ` [PATCH 10/10] perf config: Refactor a duplicated code for obtaining config file name Arnaldo Carvalho de Melo
2017-05-03 17:30 ` [GIT PULL 00/10] perf/core improvements and fixes Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170503135826.28675-9-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=dsahern@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.