18 May, 2010

2 commits

  • OPT_SET_INT was renamed to OPT_SET_UINT since the only use in these
    tools is to set something that has an enum type, that is builtin
    compatible with unsigned int.

    Several string constifications were done to make OPT_STRING require a
    const char * type.

    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Paul Mackerras
    Cc: Peter Zijlstra
    Cc: Stephane Eranian
    Cc: Tom Zanussi
    LKML-Reference:
    Signed-off-by: Arnaldo Carvalho de Melo

    Arnaldo Carvalho de Melo
     
  • To avoid problems like the one fixed by Stephane Eranian in 3de29ca, now
    we'll got this instead:

    bench/sched-messaging.c:259: error: negative width in bit-field ‘’
    bench/sched-messaging.c:261: error: negative width in bit-field ‘’

    Which is rather cryptic, but is how BUILD_BUG_ON_ZERO works, so kernel
    hackers should be already used to this.

    With it in place found some problems, fixed by changing the affected
    variables to sensible types or changed some OPT_INTEGER to OPT_UINTEGER.

    Next csets will go thru converting each of the remaining OPT_ so that
    review can be made easier by grouping changes per type per patch.

    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Paul Mackerras
    Cc: Peter Zijlstra
    Cc: Stephane Eranian
    Cc: Tom Zanussi
    LKML-Reference:
    Signed-off-by: Arnaldo Carvalho de Melo

    Arnaldo Carvalho de Melo
     

15 May, 2010

1 commit

  • The events_stats.total field is too generic, rename it to .total_period,
    and also add a comment explaining that it is the sum of all the .period
    fields in samples, that is needed because we use auto-freq to avoid
    sampling artifacts.

    Ditto for events_stats.lost, that is the sum of all lost_event.lost
    fields, i.e. the number of events the kernel dropped.

    Looking at the users, builtin-sched.c can make use of these fields and
    stop doing it again.

    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Paul Mackerras
    Cc: Peter Zijlstra
    Cc: Tom Zanussi
    LKML-Reference:
    Signed-off-by: Arnaldo Carvalho de Melo

    Arnaldo Carvalho de Melo
     

03 May, 2010

1 commit

  • Currently, perf 'live mode' writes build-ids at the end of the
    session, which isn't actually useful for processing live mode events.

    What would be better would be to have the build-ids sent before any of
    the samples that reference them, which can be done by processing the
    event stream and retrieving the build-ids on the first hit. Doing
    that in perf-record itself, however, is off-limits.

    This patch introduces perf-inject, which does the same job while
    leaving perf-record untouched. Normal mode perf still records the
    build-ids at the end of the session as it should, but for live mode,
    perf-inject can be injected in between the record and report steps
    e.g.:

    perf record -o - ./hackbench 10 | perf inject -v -b | perf report -v -i -

    perf-inject reads a perf-record event stream and repipes it to stdout.
    At any point the processing code can inject other events into the
    event stream - in this case build-ids (-b option) are read and
    injected as needed into the event stream.

    Build-ids are just the first user of perf-inject - potentially
    anything that needs userspace processing to augment the trace stream
    with additional information could make use of this facility.

    Cc: Ingo Molnar
    Cc: Steven Rostedt
    Cc: Frédéric Weisbecker
    LKML-Reference:
    Signed-off-by: Tom Zanussi
    Signed-off-by: Arnaldo Carvalho de Melo

    Tom Zanussi
     

24 Apr, 2010

1 commit

  • Use the new generic sample events reordering from perf sched,
    this drops the need of multiplexing the buffers on record time,
    improving the scalability of perf sched.

    Signed-off-by: Frederic Weisbecker
    Cc: Peter Zijlstra
    Cc: Arnaldo Carvalho de Melo
    Cc: Paul Mackerras
    Cc: Hitoshi Mitake
    Cc: Ingo Molnar
    Cc: Masami Hiramatsu
    Cc: Tom Zanussi

    Frederic Weisbecker
     

14 Apr, 2010

1 commit

  • Parsing an option from the command line with OPT_BOOLEAN on a
    bool data type would not work on a big-endian machine due to the
    manner in which the boolean was being cast into an int and
    incremented. For example, running 'perf probe --list' on a
    PowerPC machine would fail to properly set the list_events bool
    and would therefore print out the usage information and
    terminate.

    This patch makes OPT_BOOLEAN work as expected with a bool
    datatype. For cases where the original OPT_BOOLEAN was
    intentionally being used to increment an int each time it was
    passed in on the command line, this patch introduces OPT_INCR
    with the old behaviour of OPT_BOOLEAN (the verbose variable is
    currently the only such example of this).

    I have reviewed every use of OPT_BOOLEAN to verify that a true
    C99 bool was passed. Where integers were used, I verified that
    they were only being used for boolean logic and changed them to
    bools to ensure that they would not be mistakenly used as ints.
    The major exception was the verbose variable which now uses
    OPT_INCR instead of OPT_BOOLEAN.

    Signed-off-by: Ian Munsie
    Acked-by: David S. Miller
    Cc: # NOTE: wont apply to .3[34].x cleanly, please backport
    Cc: Git development list
    Cc: Ian Munsie
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    Cc: Arnaldo Carvalho de Melo
    Cc: KOSAKI Motohiro
    Cc: Hitoshi Mitake
    Cc: Rusty Russell
    Cc: Frederic Weisbecker
    Cc: Eric B Munson
    Cc: Valdis.Kletnieks@vt.edu
    Cc: WANG Cong
    Cc: Thiago Farina
    Cc: Masami Hiramatsu
    Cc: Xiao Guangrong
    Cc: Jaswinder Singh Rajput
    Cc: Arjan van de Ven
    Cc: OGAWA Hirofumi
    Cc: Mike Galbraith
    Cc: Tom Zanussi
    Cc: Anton Blanchard
    Cc: John Kacur
    Cc: Li Zefan
    Cc: Steven Rostedt
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Ian Munsie
     

08 Apr, 2010

1 commit

  • Using 'pahole --packable' I found some structs that could be reorganized
    to eliminate alignment holes, in some cases getting them to be cacheline
    multiples.

    [acme@doppio linux-2.6-tip]$ codiff perf.old ~/bin/perf
    builtin-annotate.c:
    struct perf_session | -8
    struct perf_header | -8
    2 structs changed

    builtin-diff.c:
    struct sample_data | -8
    1 struct changed
    diff__process_sample_event | -8
    1 function changed, 8 bytes removed, diff: -8

    builtin-sched.c:
    struct sched_atom | -8
    1 struct changed

    builtin-timechart.c:
    struct per_pid | -8
    1 struct changed
    cmd_timechart | -16
    1 function changed, 16 bytes removed, diff: -16

    builtin-probe.c:
    struct perf_probe_point | -8
    struct perf_probe_event | -8
    2 structs changed
    opt_add_probe_event | -3
    1 function changed, 3 bytes removed, diff: -3

    util/probe-finder.c:
    struct probe_finder | -8
    1 struct changed
    find_kprobe_trace_events | -16
    1 function changed, 16 bytes removed, diff: -16

    /home/acme/bin/perf:
    4 functions changed, 43 bytes removed, diff: -43
    [acme@doppio linux-2.6-tip]$

    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Arnaldo Carvalho de Melo

    Arnaldo Carvalho de Melo
     

16 Jan, 2010

1 commit

  • Since they can come from another architecture with bigger
    pointers, i.e. processing a 64-bit perf.data on a 32-bit arch.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

28 Dec, 2009

3 commits


16 Dec, 2009

1 commit


15 Dec, 2009

1 commit


14 Dec, 2009

7 commits


12 Dec, 2009

1 commit

  • That does all the initialization boilerplate, opening the file,
    reading the header, checking if it is valid, etc.

    And that will as well have the threads list, kmap (now) global
    variable, etc, so that we can handle two (or more) perf.data files
    describing sessions to compare.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

10 Dec, 2009

1 commit

  • When we have a maximum latency reported for a task, we need a
    convenient way to find the matching location to the raw traces
    or to perf sched map that shows where the task has been
    eventually scheduled in. This gives a pointer to retrieve the
    events that occured during this max latency.

    Signed-off-by: Frederic Weisbecker
    Reviewed-by: Xiao Guangrong
    Cc: Peter Zijlstra
    Cc: Arnaldo Carvalho de Melo
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Frederic Weisbecker
     

09 Dec, 2009

1 commit

  • In current code, task's execute time is got by reading
    '/proc//sched' file, it's wrong if the task is created
    by pthread_create(), because every thread task has same pid.

    This way also has two demerits:

    1: 'perf sched replay' can't work if the kernel is not
    compiled with the 'CONFIG_SCHED_DEBUG' option

    2: perf tool should depend on proc file system

    So, this patch uses PERF_COUNT_SW_TASK_CLOCK to get task's
    execution time instead of reading /proc file.

    Changelog v2 -> v3:
    use PERF_COUNT_SW_TASK_CLOCK instead of rusage() as Ingo's
    suggestion

    Reported-by: Torok Edwin
    Signed-off-by: Xiao Guangrong
    Cc: Xiao Guangrong
    Cc: Peter Zijlstra
    Cc: Frederic Weisbecker
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Xiao Guangrong
     

07 Dec, 2009

4 commits

  • raw->size is not used, this patch just cleans it up.

    Signed-off-by: Xiao Guangrong
    Cc: Frederic Weisbecker
    Cc: Paul Mackerras
    Cc: OGAWA Hirofumi
    Cc: Peter Zijlstra
    Cc: Li Zefan
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Xiao Guangrong
     
  • We use 'data.raw_data' parameter to call process_raw_event(),
    but data.raw_data buffer not include data size. it can make perf
    tool crash.

    This bug was introduced by commit 180f95e29a ("perf: Make common
    SAMPLE_EVENT parser").

    Signed-off-by: Xiao Guangrong
    Cc: Pekka Enberg
    Cc: Eduard - Gabriel Munteanu
    Cc: Frederic Weisbecker
    Cc: Paul Mackerras
    Cc: OGAWA Hirofumi
    Cc: Peter Zijlstra
    Cc: Li Zefan
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Xiao Guangrong
     
  • If we use 'perf sched trace', it will call symbol__init() again,
    and can lead to a perf tool crash:

    [root@localhost perf]# ./perf sched trace
    *** glibc detected *** ./perf: free(): invalid next size (normal): 0x094c1898 ***
    ======= Backtrace: =========
    /lib/libc.so.6[0xb7602404]
    /lib/libc.so.6(cfree+0x96)[0xb76043b6]
    ./perf[0x80730fe]
    ./perf[0x8074c97]
    ./perf[0x805eb59]
    ./perf[0x80536fd]
    ./perf[0x804b618]
    ./perf[0x804bdc3]
    /lib/libc.so.6(__libc_start_main+0xe5)[0xb75a9735]
    ./perf[0x804af81]
    ======= Memory map: ========
    08048000-08158000 r-xp 00000000 fe:00 556831 /home/eric/....
    08158000-08168000 rw-p 0010f000 fe:00 556831 /home/eric/...
    08168000-085fe000 rw-p 00000000 00:00 0
    094ab000-094cc000 rw-p 00000000 00:00 0 [heap]

    Signed-off-by: Xiao Guangrong
    LKML-Reference:
    Cc: Peter Zijlstra
    Cc: Mike Galbraith
    Cc: Paul Mackerras
    Cc: Arnaldo Carvalho de Melo
    Cc: Frederic Weisbecker
    Signed-off-by: Ingo Molnar

    Xiao Guangrong
     
  • Currently, sample event data is parsed for each commands, and it
    is assuming that the data is not including other data. (E.g.
    timechart, trace, etc. can't parse the event if it has
    PERF_SAMPLE_CALLCHAIN)

    So, even if we record the superset data for multiple commands at
    a time, commands can't parse. etc.

    To fix it, this makes common sample event parser, and use it to
    parse sample event correctly. (PERF_SAMPLE_READ is unsupported
    for now though, it seems to be not using.)

    Signed-off-by: OGAWA Hirofumi
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    Cc: Arnaldo Carvalho de Melo
    Cc: Frederic Weisbecker
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    OGAWA Hirofumi
     

28 Nov, 2009

1 commit

  • While implementing event__preprocess_sample, that will do all of
    the symbol lookup in one convenient function, I noticed that
    util/process_event.[ch] were not being used at all, then started
    looking if there were other functions that could be shared
    and...

    All those functions really don't need to receive offset + head,
    the only thing they did was common to all of them, so do it at
    one place instead.

    Stats about number of each type of event processed now is done
    in a central place.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: John Kacur
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

24 Nov, 2009

3 commits

  • This way we type less characters and it looks more like the
    kzalloc kernel counterpart.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     
  • And also express its configuration toggles via a struct.

    Now all one has to do is to call symbol__init(NULL) if the
    defaults are OK, or pass a struct symbol_conf pointer with the
    desired configuration.

    If a tool uses kernel_maps__find_symbol() to look at the kernel
    and modules mappings for a symbol but didn't call symbol__init()
    first, that will generate a one time warning too, alerting the
    subcommand developer that symbol__init() must be called.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     
  • Now that we can check the buildid to see if it really matches,
    this can be done safely:

    vmlinux
    /boot/vmlinux
    /boot/vmlinux-
    /lib/modules//build/vmlinux
    /usr/lib/debug/lib/modules/%s/vmlinux

    More can be added - if you know about distros that put the
    vmlinux somewhere else please let us know.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frédéric Weisbecker
    Cc: Mike Galbraith
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

02 Nov, 2009

1 commit

  • Before we were storing this in the DSO, but in fact this is a
    property of the 'symbol' class, not something that will vary
    among DSOs, so move it to a global variable and initialize it
    using the existing symbol__init routine.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frederic Weisbecker
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    Cc: Mike Galbraith
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

23 Oct, 2009

1 commit

  • We were using eprintf in some places, that looks at a global
    'verbose' level, and at other places passing a 'v' parameter to
    specify the verbosity level, unify it by introducing
    pr_{err,warning,debug,etc}, just like in the kernel.

    Signed-off-by: Arnaldo Carvalho de Melo
    Cc: Frederic Weisbecker
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    Cc: Mike Galbraith
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Arnaldo Carvalho de Melo
     

19 Oct, 2009

1 commit


17 Oct, 2009

1 commit

  • In each case, if the NULL test on thread is needed, then the
    dereference should be after the NULL test.

    A simplified version of the semantic match that detects this
    problem is as follows (http://coccinelle.lip6.fr/):

    //
    @match exists@
    expression x, E;
    identifier fld;
    @@

    * x->fld
    ... when != \(x = E\|&x\)
    * x == NULL
    //

    Signed-off-by: Julia Lawall
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Julia Lawall
     

15 Oct, 2009

1 commit


13 Oct, 2009

1 commit


12 Oct, 2009

2 commits

  • To refresh, trying to sched record only one CPU results in bogus
    latencies as below.

    I fixed^Wmade it stop doing the bad thing today, by
    following task migration events properly.

    Before:

    marge:/root/tmp # taskset -c 1 perf sched record -C 0 -- sleep 10
    marge:/root/tmp # perf sched lat
    -----------------------------------------------------------------------------------------
    Task | Runtime ms | Switches | Average delay ms | Maximum delay ms |
    -----------------------------------------------------------------------------------------
    Xorg:4943 | 1.290 ms | 1 | avg: 1670.132 ms | max: 1670.132 ms |
    hald-addon-stor:3569 | 0.091 ms | 3 | avg: 658.609 ms | max: 1975.797 ms |
    hald-addon-stor:3573 | 0.209 ms | 4 | avg: 499.138 ms | max: 1990.565 ms |
    audispd:4270 | 0.012 ms | 1 | avg: 0.015 ms | max: 0.015 ms |
    ....

    marge:/root/tmp # perf sched trace|grep 'Xorg:4943'
    swapper-0 [000] 401.184013288: sched_stat_runtime: task: Xorg:4943 runtime: 1233188 [ns], vruntime: 19105169779 [ns]
    rt2870TimerQHan-4947 [000] 402.854140127: sched_stat_wait: task: Xorg:4943 wait: 580073 [ns]
    rt2870TimerQHan-4947 [000] 402.854141770: sched_migrate_task: task Xorg:4943 [140] from: 1 to: 0
    rt2870TimerQHan-4947 [000] 402.854143854: sched_stat_wait: task: Xorg:4943 wait: 0 [ns]
    rt2870TimerQHan-4947 [000] 402.854145397: sched_switch: task rt2870TimerQHan:4947 [140] (D) ==> Xorg:4943 [140]
    Xorg-4943 [000] 402.854193133: sched_stat_runtime: task: Xorg:4943 runtime: 56546 [ns], vruntime: 11766332500 [ns]
    Xorg-4943 [000] 402.854196842: sched_switch: task Xorg:4943 [140] (S) ==> swapper:0 [140]

    After:

    marge:/root/tmp # taskset -c 1 perf sched record -C 0 -- sleep 10
    marge:/root/tmp # perf sched lat
    -----------------------------------------------------------------------------------------
    Task | Runtime ms | Switches | Average delay ms | Maximum delay ms |
    -----------------------------------------------------------------------------------------
    amarokapp:11150 | 271.297 ms | 878 | avg: 0.130 ms | max: 1.057 ms |
    konsole:5965 | 1.370 ms | 12 | avg: 0.092 ms | max: 0.855 ms |
    Xorg:4943 | 179.980 ms | 1109 | avg: 0.087 ms | max: 1.206 ms |
    hald-addon-stor:3574 | 0.212 ms | 9 | avg: 0.040 ms | max: 0.169 ms |
    hald-addon-stor:3570 | 0.223 ms | 9 | avg: 0.037 ms | max: 0.223 ms |
    klauncher:5864 | 0.550 ms | 8 | avg: 0.032 ms | max: 0.048 ms |

    The 'Maximum delay ms' results are now sane.

    Signed-off-by: Mike Galbraith
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Mike Galbraith
     
  • The following perf build warnings/errors in function
    argument types:

    builtin-sched.c:1894: warning: passing argument 1 of 'sort_dimension__add' discards qualifiers from pointer target type
    util/trace-event-parse.c:685: warning: passing argument 2 of 'read_expected' discards qualifiers from pointer target type
    util/trace-event-parse.c:741: warning: passing argument 4 of 'test_type_token' discards qualifiers from pointer target type
    util/trace-event-parse.c:706: warning: passing argument 2 of 'read_expected_item' discards qualifiers from pointer target type

    ... trigger because older GCC is not able to prove that
    sort_dimension__add() does not change the string.

    Some goes for test_type_token().

    Fix this by improving type consistency.

    Signed-off-by: Randy Dunlap
    Acked-by: Frederic Weisbecker
    Acked-by: Peter Zijlstra
    Cc: Paul Mackerras
    LKML-Reference:
    [ Also remove ugly type cast now unnecessary. ]
    Signed-off-by: Ingo Molnar

    Randy Dunlap
     

09 Oct, 2009

1 commit

  • This reverts commit 9a92b479b2f088ee2d3194243f4c8e59b1b8c9c2 ("perf
    tools: Improve thread comm resolution in perf sched") and fixes the
    real bug.

    The bug was elsewhere:

    We are failing to resolve thread names in perf sched because the
    table of threads we are building, on top of comm events, has a per
    process granularity. But perf sched, unlike the other perf tools,
    needs a per thread granularity as we are profiling every tasks
    individually.

    So fix it by building our threads table using the tid instead of
    the pid as the thread identifier.

    v2: Revert the previous fix - it is not really needed

    Signed-off-by: Frederic Weisbecker
    Cc: Peter Zijlstra
    Cc: Arnaldo Carvalho de Melo
    Cc: Mike Galbraith
    Cc: Paul Mackerras
    LKML-Reference:
    Signed-off-by: Ingo Molnar

    Frederic Weisbecker