Messages in this thread | | | From | Namhyung Kim <> | Date | Wed, 30 Nov 2022 13:21:29 -0800 | Subject | Re: [PATCH 05/15] perf stat: Remove prefix argument in print_metric_headers() |
| |
On Tue, Nov 29, 2022 at 9:13 PM Ian Rogers <irogers@google.com> wrote: > > On Tue, Nov 29, 2022 at 9:09 PM Ian Rogers <irogers@google.com> wrote: > > > > On Wed, Nov 23, 2022 at 3:23 PM Ian Rogers <irogers@google.com> wrote: > > > > > > On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > It always passes a whitespace to the function, thus we can just add it to the > > > > function body. Furthermore, it's only used in the normal output mode. > > > > > > > > Well, actually CSV used it but it doesn't need to since we don't care about the > > > > indentation or alignment in the CSV output. > > > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > > > > Acked-by: Ian Rogers <irogers@google.com> > > > > > > Thanks, > > > Ian > > > > I suspect this may be responsible for a metric segv that I'm now > > seeing in Arnaldo's tree: > > > > $ gdb --args perf stat -M Backend true > > ... > > Performance counter stats for 'true': > > > > 4,712,355 TOPDOWN.SLOTS # 17.3 % > > tma_core_bound > > > > Program received signal SIGSEGV, Segmentation fault. > > __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:77 > > 77 ../sysdeps/x86_64/multiarch/strlen-evex.S: No such file or directory. > > (gdb) bt > > #0 __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:77 > > #1 0x00007ffff74749a5 in __GI__IO_fputs (str=0x0, fp=0x7ffff75f5680 > > <_IO_2_1_stderr_>) > > at ./libio/iofputs.c:33 > > #2 0x0000555555779f28 in do_new_line_std (config=0x555555e077c0 > > <stat_config>, os=0x7fffffffbf10) > > at util/stat-display.c:356 > > #3 0x000055555577a081 in print_metric_std (config=0x555555e077c0 > > <stat_config>, > > ctx=0x7fffffffbf10, color=0x0, fmt=0x5555558b77b5 "%8.1f", > > unit=0x7fffffffbb10 "% tma_memory_bound", val=13.165355724442199) > > at util/stat-display.c:380 > > #4 0x00005555557768b6 in generic_metric (config=0x555555e077c0 <stat_config>, > > metric_expr=0x55555593d5b7 "((CYCLE_ACTIVITY.STALLS_MEM_ANY + > > EXE_ACTIVITY.BOUND_ON_STORES) / (C > > YCLE_ACTIVITY.STALLS_TOTAL + (EXE_ACTIVITY.1_PORTS_UTIL + tma_retiring > > * EXE_ACTIVITY.2_PORTS_UTIL) > > + EXE_ACTIVITY.BOUND_ON_STORES))"..., metric_events=0x555555f334e0, > > metric_refs=0x555555ec81d0, > > name=0x555555f32e80 "TOPDOWN.SLOTS", metric_name=0x555555f26c80 > > "tma_memory_bound", > > metric_unit=0x55555593d5b1 "100%", runtime=0, map_idx=0, > > out=0x7fffffffbd90, > > st=0x555555e9e620 <rt_stat>) at util/stat-shadow.c:934 > > #5 0x0000555555778cac in perf_stat__print_shadow_stats > > (config=0x555555e077c0 <stat_config>, > > evsel=0x555555f289d0, avg=4712355, map_idx=0, out=0x7fffffffbd90, > > metric_events=0x555555e078e8 <stat_config+296>, st=0x555555e9e620 <rt_stat>) > > at util/stat-shadow.c:1329 > > #6 0x000055555577b6a0 in printout (config=0x555555e077c0 > > <stat_config>, os=0x7fffffffbf10, > > uval=4712355, run=325322, ena=325322, noise=4712355, map_idx=0) at > > util/stat-display.c:741 > > #7 0x000055555577bc74 in print_counter_aggrdata > > (config=0x555555e077c0 <stat_config>, > > counter=0x555555f289d0, s=0, os=0x7fffffffbf10) at util/stat-display.c:838 > > #8 0x000055555577c1d8 in print_counter (config=0x555555e077c0 <stat_config>, > > counter=0x555555f289d0, os=0x7fffffffbf10) at util/stat-display.c:957 > > #9 0x000055555577dba0 in evlist__print_counters (evlist=0x555555ec3610, > > config=0x555555e077c0 <stat_config>, _target=0x555555e01c80 > > <target>, ts=0x0, argc=1, > > argv=0x7fffffffe450) at util/stat-display.c:1413 > > #10 0x00005555555fc821 in print_counters (ts=0x0, argc=1, argv=0x7fffffffe450) > > at builtin-stat.c:1040 > > #11 0x000055555560091a in cmd_stat (argc=1, argv=0x7fffffffe450) at > > builtin-stat.c:2665 > > #12 0x00005555556b1eea in run_builtin (p=0x555555e11f70 > > <commands+336>, argc=4, > > argv=0x7fffffffe450) at perf.c:322 > > #13 0x00005555556b2181 in handle_internal_command (argc=4, > > argv=0x7fffffffe450) at perf.c:376 > > #14 0x00005555556b22d7 in run_argv (argcp=0x7fffffffe27c, > > argv=0x7fffffffe270) at perf.c:420 > > #15 0x00005555556b26ef in main (argc=4, argv=0x7fffffffe450) at perf.c:550 > > (gdb) > > > > Thanks, > > Ian > > More specifically, I think os->prefix needs testing for NULL: > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/stat-display.c?h=perf/core#n356 > so: > fputs(os->prefix, os->fh); > should be: > if (os->prefix) > fputs(os->prefix, os->fh);
Yep, could you please send it as a formal patch?
Thanks, Namhyung
| |