Re: [PATCHv3 00/56] perf stat: Add scripting support

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Oct 08, 2015 at 06:37:43PM +0000, Liang, Kan wrote:
> 
> Hi Jirka
> 
> I'm doing some test with the perf/stat_script tree.
> Here are the issues I found so far.

hi,
thanks a lot for testing!

> 
> 1 test case failed to build:
> 
>   CC       tests/stat.o
> tests/stat.c: In function âtest__synthesize_statâ:
> tests/stat.c:82:3: error: missing initializer for field âenaâ of
> âstruct <anonymous>â [-Werror=missing-field-initializers]
>    .ena = 200,
>    ^
> In file included from tests/stat.c:5:0:
> /home/lk/group_read/test/perf/tools/perf/util/counts.h:10:8:
> note: âenaâ declared here
>     u64 ena;
>         ^
> tests/stat.c:83:3: error: missing initializer for field ârunâ of
> âstruct <anonymous>â [-Werror=missing-field-initializers]
>    .run = 300,
>    ^
> In file included from tests/stat.c:5:0:
> /home/lk/group_read/test/perf/tools/perf/util/counts.h:11:8:
> note: ârunâ declared here
>     u64 run;
>         ^
> cc1: all warnings being treated as errors
> mv: cannot stat âtests/.stat.o.tmpâ: No such file or directory
> make[3]: *** [tests/stat.o] Error 1
> make[2]: *** [tests] Error 2
> make[1]: *** [perf-in.o] Error 2
> make: *** [all] Error 2

saw this on another server.. have fix already

> 
> 
> 2. it still doesn't work well with uncore event.
> 
> Here is an example:
> sudo ./perf stat -e uncore_imc_1/cas_count_read/ -a --interval-print 100
> --per-socket record -- sleep 1
> #           time socket cpus             counts unit events
>      0.100231142 S0        1               0.04 MiB  uncore_imc_1/cas_count_read/
>      0.100231142 S1        1               0.08 MiB  uncore_imc_1/cas_count_read/
>      0.200583556 S0        1               0.02 MiB  uncore_imc_1/cas_count_read/
>      0.200583556 S1        1               0.02 MiB  uncore_imc_1/cas_count_read/
> 
> sudo ./perf stat report --per-socket
> #           time socket cpus             counts unit events
>      0.100231142 S0       36               0.12 MiB  uncore_imc_1/cas_count_read/
>      0.100231142 S1       28      <not counted> 
> MiB  uncore_imc_1/cas_count_read/
>      0.200583556 S0       36               0.04 MiB  uncore_imc_1/cas_count_read/
>      0.200583556 S1       28      <not counted> 
> MiB  uncore_imc_1/cas_count_read/
> 
> The results are not consistent. 
> 
> I checked the perf report -D result. It looks the wrong CPU id is stored.
> In my system, the uncore event should be monitored in CPU 0 and CPU 19
> But it shows CPU0 and CPU1.
> 
> 0x520 [0x30]: PERF_RECORD_STAT
> ... id 22385, cpu 0, thread 0
> 
> 0x550 [0x30]: PERF_RECORD_STAT
> ... id 22386, cpu 1, thread 0

it's the pmu's cpumask I did not take into account
I think we'll have to send cpumask as attr_update

> 
> 
> 3. I think you have already handled new record type for perf stat record.
> But the "unhandled!" message still show in the perf report -D.      
> 
> 0x598 [0x30]: PERF_RECORD_STAT
> ... id 22385, cpu 0, thread 0
> ... value 1752, enabled 501473097, running 501473097
> : unhandled!      <-------

yep, I was wondering about that as well.. but it's not 'handled' in perf report,
it just displays the raw output of the event.. but I think we could remove that
not to confuse

and perhaps we could also add -D for perf stat report

> 
> 
> 4. Core dump when applying --per-core -a for stat record.
> 
> sudo ./perf stat -e 'cycles' --per-core -a record sleep 1
> 
> ** Error in `./perf': corrupted double-linked list: 0x00000000036eb560 **
> ======= Backtrace: =========
> /lib64/libc.so.6[0x3002e7bbe7]
> /lib64/libc.so.6[0x3002e7d2b5]
> ./perf(perf_env__exit+0xfd)[0x484d1d]
> ./perf[0x47c45f]
> ./perf(main+0x610)[0x421ac0]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x3002e21b45]
> ./perf[0x421bd9]

could not reproduce this one.. any chance you could compile with
DEBUG=1 and re-run in gdb for more details? like which of the
frees got crazy.. ?

> 5.  perf stat record may not monitor on specific CPU/whole-system.
> However, perf stat report can be set to aggregate per core/socket.
> If so, there is problem to show the data.
> I think we need some warning here.
> 
> For example,
> 
> sudo ./perf stat -e 'cycles' record sleep 1
> 
>  Performance counter stats for 'sleep 1':
> 
>            858,928      cycles
> 
>        1.001120735 seconds time elapsed
> 
> sudo ./perf stat report -A
> 
>  Performance counter stats for '/home/lk/group_read/test/perf/
> tools/perf/perf stat -e cycles record sleep 1':
> 
> CPU-1              858,928      cycles
> 
>        1.001120735 seconds time elapsed

yep, we need to disable those options for process data 

thanks,
jirka