Hi Ingo,
Please consider pulling from:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
Regards,
- Arnaldo
Arnaldo Carvalho de Melo (2):
perf symbols: Improve kallsyms symbol end addr calculation
perf test: Look forward for symbol aliases
Franck Bui-Huu (1):
perf probe: Fix wrong warning in __show_one_line() if read(1) errors
happen
tools/perf/builtin-test.c | 23 ++++++++++++++---
tools/perf/util/event.c | 3 +-
tools/perf/util/probe-event.c | 2 +-
tools/perf/util/symbol.c | 56 ++++++++++++++++++++++++++++++----------
tools/perf/util/symbol.h | 2 +-
5 files changed, 65 insertions(+), 21 deletions(-)
From: Arnaldo Carvalho de Melo <[email protected]>
Not just before, fixing these false positives:
[acme@mica linux]$ perf test -v 1
1: vmlinux symtab matches kallsyms:
--- start ---
Looking at the vmlinux_path (6 entries long)
Using //lib/modules/2.6.37-rc5-00180-ge06b6bf/build/vmlinux for symbols
0xffffffff81058dc0: diff name v: sys_vm86old k: sys_ni_syscall
0xffffffff81058dc0: diff name v: sys_vm86 k: sys_ni_syscall
0xffffffff81058dc0: diff name v: sys_subpage_prot k: sys_ni_syscall
0xffffffff810b5f7c: diff name v: probe_kernel_write k: __probe_kernel_write
0xffffffff810b5fe5: diff name v: probe_kernel_read k: __probe_kernel_read
0xffffffff811bc380: diff name v: __memset k: memset
0xffffffff81384a98: diff name v: __sched_text_start k: sleep_on_common
0xffffffff81386750: diff name v: __sched_text_end k: _raw_spin_trylock
0xffffffff8138cee8: diff name v: __irqentry_text_start k: do_IRQ
0xffffffff8138f079: diff name v: __start_notes k: _etext
0xffffffff8138f079: diff name v: __stop_notes k: _etext
---- end ----
vmlinux symtab matches kallsyms: FAILED!
[acme@mica linux]$
Some are weak functions, others are just markers, etc. They get in the rb tree
with the same addr, so we need to look around to find the symbol with the same
name.
We were looking just at the previous entries with the same addr, look forward
too.
Cc: Frederic Weisbecker <[email protected]>
Cc: Han Pingtian <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-test.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 035b9fa..e0c3f47 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -119,10 +119,16 @@ static int test__vmlinux_matches_kallsyms(void)
* end addresses too.
*/
for (nd = rb_first(&vmlinux_map->dso->symbols[type]); nd; nd = rb_next(nd)) {
- struct symbol *pair;
+ struct symbol *pair, *first_pair;
+ bool backwards = true;
sym = rb_entry(nd, struct symbol, rb_node);
- pair = machine__find_kernel_symbol(&kallsyms, type, sym->start, NULL, NULL);
+
+ if (sym->start == sym->end)
+ continue;
+
+ first_pair = machine__find_kernel_symbol(&kallsyms, type, sym->start, NULL, NULL);
+ pair = first_pair;
if (pair && pair->start == sym->start) {
next_pair:
@@ -143,8 +149,10 @@ next_pair:
pr_debug("%#Lx: diff end addr for %s v: %#Lx k: %#Lx\n",
sym->start, sym->name, sym->end, pair->end);
} else {
- struct rb_node *nnd = rb_prev(&pair->rb_node);
-
+ struct rb_node *nnd;
+detour:
+ nnd = backwards ? rb_prev(&pair->rb_node) :
+ rb_next(&pair->rb_node);
if (nnd) {
struct symbol *next = rb_entry(nnd, struct symbol, rb_node);
@@ -153,6 +161,13 @@ next_pair:
goto next_pair;
}
}
+
+ if (backwards) {
+ backwards = false;
+ pair = first_pair;
+ goto detour;
+ }
+
pr_debug("%#Lx: diff name v: %s k: %s\n",
sym->start, sym->name, pair->name);
}
--
1.6.2.5
From: Franck Bui-Huu <[email protected]>
This was introduced by commit fde52dbd7f71934aba4e150f3d1d51e826a08850.
Cc: H. Peter Anvin <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Franck Bui-Huu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 099336e..4bde988 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -309,7 +309,7 @@ static int __show_one_line(FILE *fp, int l, bool skip, bool show_num)
return 1;
error:
if (ferror(fp)) {
- pr_warning("Source file is shorter than expected.\n");
+ pr_warning("File read error: %s\n", strerror(errno));
return -1;
}
return 0;
--
1.6.2.5
From: Arnaldo Carvalho de Melo <[email protected]>
For kallsyms we don't have the symbol address end, so we do an extra pass and
set the symbol end addr as being the start of the next minus one.
But this was being done just after we filtered the symbols of a
particular type (functions, variables), so the symbol end was sometimes
after what it really is.
Fixing up symbol end also was falling apart when we have symbol aliases,
then the end address of all but the last alias was being set to be
before its start.
Fix it up by checking for symbol aliases and making the kallsyms__parse
routine use the next symbol, whatever its type, as the limit for the
previous symbol, passing that end address to the callback.
This was detected by the 'perf test' synthetic paranoid regression
tests, fix it up so that even that case doesn't mislead us.
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/event.c | 3 +-
tools/perf/util/symbol.c | 56 ++++++++++++++++++++++++++++++++++-----------
tools/perf/util/symbol.h | 2 +-
3 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 183aedd..2302ec0 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -332,7 +332,8 @@ struct process_symbol_args {
u64 start;
};
-static int find_symbol_cb(void *arg, const char *name, char type, u64 start)
+static int find_symbol_cb(void *arg, const char *name, char type,
+ u64 start, u64 end __used)
{
struct process_symbol_args *args = arg;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 561db63..2ea1a2e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -22,6 +22,10 @@
#include <limits.h>
#include <sys/utsname.h>
+#ifndef KSYM_NAME_LEN
+#define KSYM_NAME_LEN 128
+#endif
+
#ifndef NT_GNU_BUILD_ID
#define NT_GNU_BUILD_ID 3
#endif
@@ -93,7 +97,7 @@ static void symbols__fixup_end(struct rb_root *self)
prev = curr;
curr = rb_entry(nd, struct symbol, rb_node);
- if (prev->end == prev->start)
+ if (prev->end == prev->start && prev->end != curr->start)
prev->end = curr->start - 1;
}
@@ -426,16 +430,25 @@ size_t dso__fprintf(struct dso *self, enum map_type type, FILE *fp)
int kallsyms__parse(const char *filename, void *arg,
int (*process_symbol)(void *arg, const char *name,
- char type, u64 start))
+ char type, u64 start, u64 end))
{
char *line = NULL;
size_t n;
- int err = 0;
+ int err = -1;
+ u64 prev_start = 0;
+ char prev_symbol_type = 0;
+ char *prev_symbol_name;
FILE *file = fopen(filename, "r");
if (file == NULL)
goto out_failure;
+ prev_symbol_name = malloc(KSYM_NAME_LEN);
+ if (prev_symbol_name == NULL)
+ goto out_close;
+
+ err = 0;
+
while (!feof(file)) {
u64 start;
int line_len, len;
@@ -455,14 +468,33 @@ int kallsyms__parse(const char *filename, void *arg,
continue;
symbol_type = toupper(line[len]);
- symbol_name = line + len + 2;
+ len += 2;
+ symbol_name = line + len;
+ len = line_len - len;
- err = process_symbol(arg, symbol_name, symbol_type, start);
- if (err)
+ if (len >= KSYM_NAME_LEN) {
+ err = -1;
break;
+ }
+
+ if (prev_symbol_type) {
+ u64 end = start;
+ if (end != prev_start)
+ --end;
+ err = process_symbol(arg, prev_symbol_name,
+ prev_symbol_type, prev_start, end);
+ if (err)
+ break;
+ }
+
+ memcpy(prev_symbol_name, symbol_name, len + 1);
+ prev_symbol_type = symbol_type;
+ prev_start = start;
}
+ free(prev_symbol_name);
free(line);
+out_close:
fclose(file);
return err;
@@ -484,7 +516,7 @@ static u8 kallsyms2elf_type(char type)
}
static int map__process_kallsym_symbol(void *arg, const char *name,
- char type, u64 start)
+ char type, u64 start, u64 end)
{
struct symbol *sym;
struct process_kallsyms_args *a = arg;
@@ -493,11 +525,8 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
if (!symbol_type__is_a(type, a->map->type))
return 0;
- /*
- * Will fix up the end later, when we have all symbols sorted.
- */
- sym = symbol__new(start, 0, kallsyms2elf_type(type), name);
-
+ sym = symbol__new(start, end - start + 1,
+ kallsyms2elf_type(type), name);
if (sym == NULL)
return -ENOMEM;
/*
@@ -650,7 +679,6 @@ int dso__load_kallsyms(struct dso *self, const char *filename,
if (dso__load_all_kallsyms(self, filename, map) < 0)
return -1;
- symbols__fixup_end(&self->symbols[map->type]);
if (self->kernel == DSO_TYPE_GUEST_KERNEL)
self->origin = DSO__ORIG_GUEST_KERNEL;
else
@@ -2162,7 +2190,7 @@ struct process_args {
};
static int symbol__in_kernel(void *arg, const char *name,
- char type __used, u64 start)
+ char type __used, u64 start, u64 end __used)
{
struct process_args *args = arg;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bcd2f98..7b8c27b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -215,7 +215,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits);
int build_id__sprintf(const u8 *self, int len, char *bf);
int kallsyms__parse(const char *filename, void *arg,
int (*process_symbol)(void *arg, const char *name,
- char type, u64 start));
+ char type, u64 start, u64 end));
void machine__destroy_kernel_maps(struct machine *self);
int __machine__create_kernel_maps(struct machine *self, struct dso *kernel);
--
1.6.2.5
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Hi Ingo,
>
> Please consider pulling from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
>
> Regards,
>
> - Arnaldo
>
> Arnaldo Carvalho de Melo (2):
> perf symbols: Improve kallsyms symbol end addr calculation
> perf test: Look forward for symbol aliases
>
> Franck Bui-Huu (1):
> perf probe: Fix wrong warning in __show_one_line() if read(1) errors
> happen
>
> tools/perf/builtin-test.c | 23 ++++++++++++++---
> tools/perf/util/event.c | 3 +-
> tools/perf/util/probe-event.c | 2 +-
> tools/perf/util/symbol.c | 56 ++++++++++++++++++++++++++++++----------
> tools/perf/util/symbol.h | 2 +-
> 5 files changed, 65 insertions(+), 21 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo