2015-11-04 10:22:20

by Markus Trippelsdorf

[permalink] [raw]
Subject: perf top segfaults on current git

On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
>
> Please pull the latest perf-core-for-linus git tree from:

Since the current merge "perf top" segfaults on my machine:

Program received signal SIGSEGV, Segmentation fault.

__map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
238 return __machine__kernel_map(map->groups->machine, map->type) == map;
(gdb) bt
#0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
#1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
#2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
#3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
#4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
#5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
#6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
#7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
at builtin-top.c:737
#8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
#9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
#10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
#11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
#12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
#13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
#14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
#15 main (argc=2, argv=0x7fffffffe500) at perf.c:608

It only happens occasionally (~every third or fourth run).

--
Markus


2015-11-04 10:49:35

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: perf top segfaults on current git

On 2015.11.04 at 11:15 +0100, Markus Trippelsdorf wrote:
> On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
> >
> > Please pull the latest perf-core-for-linus git tree from:
>
> Since the current merge "perf top" segfaults on my machine:
>
> Program received signal SIGSEGV, Segmentation fault.
>
> __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> 238 return __machine__kernel_map(map->groups->machine, map->type) == map;
> (gdb) bt
> #0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> #1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
> #2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
> filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
> #3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
> #4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
> #5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
> #6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
> sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
> #7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
> at builtin-top.c:737
> #8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
> #9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
> #10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
> #11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
> #12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
> #13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
> #14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
> #15 main (argc=2, argv=0x7fffffffe500) at perf.c:608
>
> It only happens occasionally (~every third or fourth run).

Apparently it started with:

commit cfc5acd4c80b875d2f739d6a93562034aee5563f
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Wed Sep 23 15:15:54 2015 -0300

perf top: Filter symbols based on __map__is_kernel(map)

Instead of using dso->kernel, this is equivalent at the moment,
and helps in reducing the accesses to dso->kernel.

Cc: Adrian Hunter <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index bdaf44f24d5d..38d4d6cac823 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -655,7 +655,7 @@ static int symbol_filter(struct map *map, struct symbol *sym)
{
const char *name = sym->name;

- if (!map->dso->kernel)
+ if (!__map__is_kernel(map))
return 0;
/*
* ppc64 uses function descriptors and appends a '.' to the

--
Markus

2015-11-04 12:27:45

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf top segfaults on current git

On Wed, Nov 04, 2015 at 11:49:30AM +0100, Markus Trippelsdorf wrote:
> On 2015.11.04 at 11:15 +0100, Markus Trippelsdorf wrote:
> > On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
> > >
> > > Please pull the latest perf-core-for-linus git tree from:
> >
> > Since the current merge "perf top" segfaults on my machine:
> >
> > Program received signal SIGSEGV, Segmentation fault.
> >
> > __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > 238 return __machine__kernel_map(map->groups->machine, map->type) == map;
> > (gdb) bt
> > #0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > #1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
> > #2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
> > filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
> > #3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
> > #4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
> > #5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
> > #6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
> > sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
> > #7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
> > at builtin-top.c:737
> > #8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
> > #9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
> > #10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
> > #11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
> > #12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
> > #13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
> > #14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
> > #15 main (argc=2, argv=0x7fffffffe500) at perf.c:608
> >
> > It only happens occasionally (~every third or fourth run).
>
> Apparently it started with:
>
> commit cfc5acd4c80b875d2f739d6a93562034aee5563f
> Author: Arnaldo Carvalho de Melo <[email protected]>
> Date: Wed Sep 23 15:15:54 2015 -0300
>
> perf top: Filter symbols based on __map__is_kernel(map)
>
> Instead of using dso->kernel, this is equivalent at the moment,
> and helps in reducing the accesses to dso->kernel.
>
> Cc: Adrian Hunter <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> Cc: Wang Nan <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index bdaf44f24d5d..38d4d6cac823 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -655,7 +655,7 @@ static int symbol_filter(struct map *map, struct symbol *sym)
> {
> const char *name = sym->name;
>
> - if (!map->dso->kernel)
> + if (!__map__is_kernel(map))
> return 0;
> /*
> * ppc64 uses function descriptors and appends a '.' to the
>

I think it's correct to assume map->groups != NULL,
IMO the change just uncovered another bug

I couldn't reproduce this, but I could find one place where this
assumption is broken.. could you please try attached patch?

thanks,
jirka


---
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4e38c396a897..afc6b56cf749 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -644,6 +644,12 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
return printed;
}

+static void __map_groups__insert(struct map_groups *mg, struct map *map)
+{
+ __maps__insert(&mg->maps[map->type], map);
+ map->groups = mg;
+}
+
static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
{
struct rb_root *root;
@@ -682,7 +688,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

before->end = map->start;
- __maps__insert(maps, before);
+ __map_groups__insert(pos->groups, before);
if (verbose >= 2)
map__fprintf(before, fp);
}
@@ -696,7 +702,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

after->start = map->end;
- __maps__insert(maps, after);
+ __map_groups__insert(pos->groups, after);
if (verbose >= 2)
map__fprintf(after, fp);
}

2015-11-04 12:41:41

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: perf top segfaults on current git

On 2015.11.04 at 13:27 +0100, Jiri Olsa wrote:
> On Wed, Nov 04, 2015 at 11:49:30AM +0100, Markus Trippelsdorf wrote:
> > On 2015.11.04 at 11:15 +0100, Markus Trippelsdorf wrote:
> > > On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
> > > >
> > > > Please pull the latest perf-core-for-linus git tree from:
> > >
> > > Since the current merge "perf top" segfaults on my machine:
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > >
> > > __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > > 238 return __machine__kernel_map(map->groups->machine, map->type) == map;
> > > (gdb) bt
> > > #0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > > #1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
> > > #2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
> > > filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
> > > #3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
> > > #4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
> > > #5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
> > > #6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
> > > sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
> > > #7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
> > > at builtin-top.c:737
> > > #8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
> > > #9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
> > > #10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
> > > #11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
> > > #12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
> > > #13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
> > > #14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
> > > #15 main (argc=2, argv=0x7fffffffe500) at perf.c:608
> > >
> > > It only happens occasionally (~every third or fourth run).
> >
> > Apparently it started with:
> >
> > commit cfc5acd4c80b875d2f739d6a93562034aee5563f
> > Author: Arnaldo Carvalho de Melo <[email protected]>
> > Date: Wed Sep 23 15:15:54 2015 -0300
> >
> > perf top: Filter symbols based on __map__is_kernel(map)
>
> I think it's correct to assume map->groups != NULL,
> IMO the change just uncovered another bug
>
> I couldn't reproduce this, but I could find one place where this
> assumption is broken.. could you please try attached patch?

Your patch fixes the issue.
Thanks.

--
Markus

2015-11-04 13:21:28

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf top segfaults on current git

Em Wed, Nov 04, 2015 at 11:15:33AM +0100, Markus Trippelsdorf escreveu:
> On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
> >
> > Please pull the latest perf-core-for-linus git tree from:
>
> Since the current merge "perf top" segfaults on my machine:
>
> Program received signal SIGSEGV, Segmentation fault.

Thanks for the report, I'll check that.

- Arnaldo

> __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> 238 return __machine__kernel_map(map->groups->machine, map->type) == map;
> (gdb) bt
> #0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> #1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
> #2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
> filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
> #3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
> #4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
> #5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
> #6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
> sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
> #7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
> at builtin-top.c:737
> #8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
> #9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
> #10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
> #11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
> #12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
> #13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
> #14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
> #15 main (argc=2, argv=0x7fffffffe500) at perf.c:608
>
> It only happens occasionally (~every third or fourth run).
>
> --
> Markus

2015-11-04 13:26:59

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf top segfaults on current git

Em Wed, Nov 04, 2015 at 01:27:40PM +0100, Jiri Olsa escreveu:
> On Wed, Nov 04, 2015 at 11:49:30AM +0100, Markus Trippelsdorf wrote:
> > On 2015.11.04 at 11:15 +0100, Markus Trippelsdorf wrote:
> > > On 2015.11.03 at 11:02 +0100, Ingo Molnar wrote:
> > > >
> > > > Please pull the latest perf-core-for-linus git tree from:
> > >
> > > Since the current merge "perf top" segfaults on my machine:
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > >
> > > __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > > 238 return __machine__kernel_map(map->groups->machine, map->type) == map;
> > > (gdb) bt
> > > #0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
> > > #1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry=0x1a88560) at builtin-top.c:658
> > > #2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry=0x1abb7a0, syms_ss=0x7fffffff7680, runtime_ss=runtime_ss@entry=0x7fffffff7680,
> > > filter=filter@entry=0x4393c0 <symbol_filter>, kmodule=0) at util/symbol-elf.c:1082
> > > #3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, filter=filter@entry=0x4393c0 <symbol_filter>) at util/symbol.c:1530
> > > #4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<optimized out>) at util/map.c:289
> > > #5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter>) at util/map.c:333
> > > #6 0x00000000004803b1 in perf_event__preprocess_sample (event=event@entry=0x7ffff6a7a5f0, machine=machine@entry=0x164c7b8, al=al@entry=0x7fffffff8a90,
> > > sample=sample@entry=0x7fffffff8b20) at util/event.c:1045
> > > #7 0x0000000000439837 in perf_event__process_sample (machine=0x164c7b8, sample=0x7fffffff8b20, evsel=0x164c320, event=0x7ffff6a7a5f0, tool=0x7fffffff8ef0)
> > > at builtin-top.c:737
> > > #8 perf_top__mmap_read_idx (top=top@entry=0x7fffffff8ef0, idx=idx@entry=0) at builtin-top.c:870
> > > #9 0x000000000043b9ef in perf_top__mmap_read (top=0x7fffffff8ef0) at builtin-top.c:887
> > > #10 __cmd_top (top=0x7fffffff8ef0) at builtin-top.c:1000
> > > #11 cmd_top (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin-top.c:1329
> > > #12 0x00000000004756b3 in run_builtin (p=p@entry=0x5b0608 <commands+264>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe500) at perf.c:386
> > > #13 0x00000000004203e2 in handle_internal_command (argv=0x7fffffffe500, argc=2) at perf.c:447
> > > #14 run_argv (argv=0x7fffffffe290, argcp=0x7fffffffe29c) at perf.c:491
> > > #15 main (argc=2, argv=0x7fffffffe500) at perf.c:608
> > >
> > > It only happens occasionally (~every third or fourth run).
> >
> > Apparently it started with:
> >
> > commit cfc5acd4c80b875d2f739d6a93562034aee5563f
> > Author: Arnaldo Carvalho de Melo <[email protected]>
> > Date: Wed Sep 23 15:15:54 2015 -0300
> >
> > perf top: Filter symbols based on __map__is_kernel(map)
> >
> > Instead of using dso->kernel, this is equivalent at the moment,
> > and helps in reducing the accesses to dso->kernel.
> >
> > Cc: Adrian Hunter <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: David Ahern <[email protected]>
> > Cc: Frederic Weisbecker <[email protected]>
> > Cc: Jiri Olsa <[email protected]>
> > Cc: Namhyung Kim <[email protected]>
> > Cc: Stephane Eranian <[email protected]>
> > Cc: Wang Nan <[email protected]>
> > Link: http://lkml.kernel.org/n/[email protected]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> >
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index bdaf44f24d5d..38d4d6cac823 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -655,7 +655,7 @@ static int symbol_filter(struct map *map, struct symbol *sym)
> > {
> > const char *name = sym->name;
> >
> > - if (!map->dso->kernel)
> > + if (!__map__is_kernel(map))
> > return 0;
> > /*
> > * ppc64 uses function descriptors and appends a '.' to the
> >
>
> I think it's correct to assume map->groups != NULL,
> IMO the change just uncovered another bug
>
> I couldn't reproduce this, but I could find one place where this
> assumption is broken.. could you please try attached patch?

Looks correct, thanks for the quick fix!

- Arnaldo

> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 4e38c396a897..afc6b56cf749 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -644,6 +644,12 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
> return printed;
> }
>
> +static void __map_groups__insert(struct map_groups *mg, struct map *map)
> +{
> + __maps__insert(&mg->maps[map->type], map);
> + map->groups = mg;
> +}
> +
> static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
> {
> struct rb_root *root;
> @@ -682,7 +688,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
> }
>
> before->end = map->start;
> - __maps__insert(maps, before);
> + __map_groups__insert(pos->groups, before);
> if (verbose >= 2)
> map__fprintf(before, fp);
> }
> @@ -696,7 +702,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
> }
>
> after->start = map->end;
> - __maps__insert(maps, after);
> + __map_groups__insert(pos->groups, after);
> if (verbose >= 2)
> map__fprintf(after, fp);
> }

2015-11-04 14:08:18

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH] perf tools: Insert split maps correctly into origin group

On Wed, Nov 04, 2015 at 01:41:37PM +0100, Markus Trippelsdorf wrote:

SNIP

> > > Apparently it started with:
> > >
> > > commit cfc5acd4c80b875d2f739d6a93562034aee5563f
> > > Author: Arnaldo Carvalho de Melo <[email protected]>
> > > Date: Wed Sep 23 15:15:54 2015 -0300
> > >
> > > perf top: Filter symbols based on __map__is_kernel(map)
> >
> > I think it's correct to assume map->groups != NULL,
> > IMO the change just uncovered another bug
> >
> > I couldn't reproduce this, but I could find one place where this
> > assumption is broken.. could you please try attached patch?
>
> Your patch fixes the issue.
> Thanks.

thanks, sending patch

jirka


---
When new maps are cloned out of split map they are added into
origin map's group, but their groups pointer is not updated.

This could lead to segfault, because map::groups is expected
to be always set as reported by Markus:

__map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
238 return __machine__kernel_map(map->groups->machine, map->type) =
(gdb) bt
#0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
#1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry
#2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry
#3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, fi
#4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<opti
#5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter
...

Adding __map_groups__insert function to add map into groups
together with map::groups pointer update. It takes no lock
as opposed to existing map_groups__insert.

Using __map_groups__insert to add new maps after map split.

Reported-by: Markus Trippelsdorf <[email protected]>
Tested-by: Markus Trippelsdorf <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/map.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4e38c396a897..afc6b56cf749 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -644,6 +644,12 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
return printed;
}

+static void __map_groups__insert(struct map_groups *mg, struct map *map)
+{
+ __maps__insert(&mg->maps[map->type], map);
+ map->groups = mg;
+}
+
static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
{
struct rb_root *root;
@@ -682,7 +688,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

before->end = map->start;
- __maps__insert(maps, before);
+ __map_groups__insert(pos->groups, before);
if (verbose >= 2)
map__fprintf(before, fp);
}
@@ -696,7 +702,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

after->start = map->end;
- __maps__insert(maps, after);
+ __map_groups__insert(pos->groups, after);
if (verbose >= 2)
map__fprintf(after, fp);
}
--
2.4.3

Subject: [tip:perf/urgent] perf tools: Insert split maps correctly into origin group

Commit-ID: cb8382e05817a8104ea0edb63b8e37b8fbecd14c
Gitweb: http://git.kernel.org/tip/cb8382e05817a8104ea0edb63b8e37b8fbecd14c
Author: Jiri Olsa <[email protected]>
AuthorDate: Wed, 4 Nov 2015 15:08:11 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 5 Nov 2015 11:39:38 -0300

perf tools: Insert split maps correctly into origin group

When new maps are cloned out of split map they are added into origin
map's group, but their groups pointer is not updated.

This could lead to a segfault, because map->groups is expected to be
always set as reported by Markus:

__map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
238 return __machine__kernel_map(map->groups->machine, map->type) =
(gdb) bt
#0 __map__is_kernel (map=map@entry=0x1abb7a0) at util/map.c:238
#1 0x00000000004393e4 in symbol_filter (map=map@entry=0x1abb7a0, sym=sym@entry
#2 0x00000000004fcd4d in dso__load_sym (dso=dso@entry=0x166dae0, map=map@entry
#3 0x00000000004a64e0 in dso__load (dso=0x166dae0, map=map@entry=0x1abb7a0, fi
#4 0x00000000004b941f in map__load (filter=0x4393c0 <symbol_filter>, map=<opti
#5 map__find_symbol (map=0x1abb7a0, addr=40188, filter=0x4393c0 <symbol_filter
...

Adding __map_groups__insert function to add map into groups together
with map->groups pointer update. It takes no lock as opposed to existing
map_groups__insert, as maps__fixup_overlappings(), where it is being
called, already has the necessary lock held.

Using __map_groups__insert to add new maps after map split.

Reported-by: Markus Trippelsdorf <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
Tested-by: Markus Trippelsdorf <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Fixes: cfc5acd4c80b ("perf top: Filter symbols based on __map__is_kernel(map)")
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/map.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4e38c39..afc6b56 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -644,6 +644,12 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
return printed;
}

+static void __map_groups__insert(struct map_groups *mg, struct map *map)
+{
+ __maps__insert(&mg->maps[map->type], map);
+ map->groups = mg;
+}
+
static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
{
struct rb_root *root;
@@ -682,7 +688,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

before->end = map->start;
- __maps__insert(maps, before);
+ __map_groups__insert(pos->groups, before);
if (verbose >= 2)
map__fprintf(before, fp);
}
@@ -696,7 +702,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
}

after->start = map->end;
- __maps__insert(maps, after);
+ __map_groups__insert(pos->groups, after);
if (verbose >= 2)
map__fprintf(after, fp);
}