linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* perf top segfaults on current git
@ 2015-11-04 10:15 Markus Trippelsdorf
  2015-11-04 10:49 ` Markus Trippelsdorf
  2015-11-04 13:21 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 8+ messages in thread
From: Markus Trippelsdorf @ 2015-11-04 10:15 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Thomas Gleixner, Jiri Olsa, Andrew Morton

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

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

* Re: perf top segfaults on current git
  2015-11-04 10:15 perf top segfaults on current git Markus Trippelsdorf
@ 2015-11-04 10:49 ` Markus Trippelsdorf
  2015-11-04 12:27   ` Jiri Olsa
  2015-11-04 13:21 ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 8+ messages in thread
From: Markus Trippelsdorf @ 2015-11-04 10:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Thomas Gleixner, Jiri Olsa, Andrew Morton

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 <acme@redhat.com>
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 <adrian.hunter@intel.com>
    Cc: Borislav Petkov <bp@suse.de>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Stephane Eranian <eranian@google.com>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: http://lkml.kernel.org/n/tip-1pc2v63iphtifovw3bv0bo1v@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

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

* Re: perf top segfaults on current git
  2015-11-04 10:49 ` Markus Trippelsdorf
@ 2015-11-04 12:27   ` Jiri Olsa
  2015-11-04 12:41     ` Markus Trippelsdorf
  2015-11-04 13:26     ` perf top segfaults on current git Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 8+ messages in thread
From: Jiri Olsa @ 2015-11-04 12:27 UTC (permalink / raw)
  To: Markus Trippelsdorf
  Cc: Ingo Molnar, linux-kernel, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Thomas Gleixner, Andrew Morton

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 <acme@redhat.com>
> 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 <adrian.hunter@intel.com>
>     Cc: Borislav Petkov <bp@suse.de>
>     Cc: David Ahern <dsahern@gmail.com>
>     Cc: Frederic Weisbecker <fweisbec@gmail.com>
>     Cc: Jiri Olsa <jolsa@redhat.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: Stephane Eranian <eranian@google.com>
>     Cc: Wang Nan <wangnan0@huawei.com>
>     Link: http://lkml.kernel.org/n/tip-1pc2v63iphtifovw3bv0bo1v@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> 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);
 		}

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

* Re: perf top segfaults on current git
  2015-11-04 12:27   ` Jiri Olsa
@ 2015-11-04 12:41     ` Markus Trippelsdorf
  2015-11-04 14:08       ` [PATCH] perf tools: Insert split maps correctly into origin group Jiri Olsa
  2015-11-04 13:26     ` perf top segfaults on current git Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 8+ messages in thread
From: Markus Trippelsdorf @ 2015-11-04 12:41 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Ingo Molnar, linux-kernel, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Thomas Gleixner, Andrew Morton

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 <acme@redhat.com>
> > 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

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

* Re: perf top segfaults on current git
  2015-11-04 10:15 perf top segfaults on current git Markus Trippelsdorf
  2015-11-04 10:49 ` Markus Trippelsdorf
@ 2015-11-04 13:21 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-04 13:21 UTC (permalink / raw)
  To: Markus Trippelsdorf
  Cc: Ingo Molnar, linux-kernel, Peter Zijlstra, Thomas Gleixner,
	Jiri Olsa, Andrew Morton

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

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

* Re: perf top segfaults on current git
  2015-11-04 12:27   ` Jiri Olsa
  2015-11-04 12:41     ` Markus Trippelsdorf
@ 2015-11-04 13:26     ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-04 13:26 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Markus Trippelsdorf, Ingo Molnar, linux-kernel, Peter Zijlstra,
	Thomas Gleixner, Andrew Morton

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 <acme@redhat.com>
> > 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 <adrian.hunter@intel.com>
> >     Cc: Borislav Petkov <bp@suse.de>
> >     Cc: David Ahern <dsahern@gmail.com>
> >     Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >     Cc: Jiri Olsa <jolsa@redhat.com>
> >     Cc: Namhyung Kim <namhyung@kernel.org>
> >     Cc: Stephane Eranian <eranian@google.com>
> >     Cc: Wang Nan <wangnan0@huawei.com>
> >     Link: http://lkml.kernel.org/n/tip-1pc2v63iphtifovw3bv0bo1v@git.kernel.org
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > 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);
>  		}

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

* [PATCH] perf tools: Insert split maps correctly into origin group
  2015-11-04 12:41     ` Markus Trippelsdorf
@ 2015-11-04 14:08       ` Jiri Olsa
  2015-11-08  7:29         ` [tip:perf/urgent] " tip-bot for Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2015-11-04 14:08 UTC (permalink / raw)
  To: Markus Trippelsdorf
  Cc: Ingo Molnar, linux-kernel, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Thomas Gleixner, Andrew Morton

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 <acme@redhat.com>
> > > 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 <markus@trippelsdorf.de>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Link: http://lkml.kernel.org/n/tip-b66q3nbv0m456f2evlhtuw3u@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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


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

* [tip:perf/urgent] perf tools: Insert split maps correctly into origin group
  2015-11-04 14:08       ` [PATCH] perf tools: Insert split maps correctly into origin group Jiri Olsa
@ 2015-11-08  7:29         ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-11-08  7:29 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, a.p.zijlstra, markus, tglx, mingo, linux-kernel, hpa,
	jolsa, akpm, acme

Commit-ID:  cb8382e05817a8104ea0edb63b8e37b8fbecd14c
Gitweb:     http://git.kernel.org/tip/cb8382e05817a8104ea0edb63b8e37b8fbecd14c
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 4 Nov 2015 15:08:11 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <markus@trippelsdorf.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20151104140811.GA32664@krava.brq.redhat.com
Fixes: cfc5acd4c80b ("perf top: Filter symbols based on __map__is_kernel(map)")
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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);
 		}

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

end of thread, other threads:[~2015-11-08  7:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-04 10:15 perf top segfaults on current git Markus Trippelsdorf
2015-11-04 10:49 ` Markus Trippelsdorf
2015-11-04 12:27   ` Jiri Olsa
2015-11-04 12:41     ` Markus Trippelsdorf
2015-11-04 14:08       ` [PATCH] perf tools: Insert split maps correctly into origin group Jiri Olsa
2015-11-08  7:29         ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2015-11-04 13:26     ` perf top segfaults on current git Arnaldo Carvalho de Melo
2015-11-04 13:21 ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).