Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements

From: Stephane Eranian
Date: Tue Jan 04 2011 - 09:30:10 EST


Indeed counts is bogus:

./perf stat -i -e cycles date

evt:0x7d4250 counts=0x7d4450 <-- from perf_evsel__alloc_counts()

Tue Jan 4 15:28:36 CET 2011

counter=0x7d4250 counts=(nil) <-- from read_counter_aggr()

Segmentation fault


On Tue, Jan 4, 2011 at 3:19 PM, Arnaldo Carvalho de Melo
<acme@xxxxxxxxxxxxx> wrote:
> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>> Arnaldo,
>> Looks like what's wrong is not ps but count:
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>> 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>> 206 Â Â Â Â Â Â Â Â Â update_stats(&ps->res_stats[i], count[i]);
>> (gdb) print ps
>> $1 = (struct perf_stat *) 0x7d48a0
>> (gdb) print *ps
>> $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>> 0}, {n = 0, mean = 0, M2 = 0}}}
>> (gdb) print count
>> $3 = (u64 *) 0x12
>> (gdb) print *count
>> Cannot access memory at address 0x12
>> (gdb) print count
>> $4 = (u64 *) 0x12
>
> Count is:
>
> u64 *count = counter->counts->aggr.values;
>
> And counter->counts type is:
>
> struct perf_counts_values {
> Â Â Â Âunion {
> Â Â Â Â Â Â Â Âstruct {
> Â Â Â Â Â Â Â Â Â Â Â Âu64 val;
> Â Â Â Â Â Â Â Â Â Â Â Âu64 ena;
> Â Â Â Â Â Â Â Â Â Â Â Âu64 run;
> Â Â Â Â Â Â Â Â};
> Â Â Â Â Â Â Â Âu64 values[3];
> Â Â Â Â};
> };
>
> struct perf_counts {
> Â Â Â Âs8 Â Â Â Â Â Â Â Â Â Â Â Âscaled;
> Â Â Â Âstruct perf_counts_values aggr;
> Â Â Â Âstruct perf_counts_values cpu[];
> };
>
> So for counter->counts->aggr.values to be 0x12 counter->counts must be
> NULL...
>
>>
>> On Tue, Jan 4, 2011 at 3:03 PM, Arnaldo Carvalho de Melo
>> <acme@xxxxxxxxxxxxx> wrote:
>> > Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
>> >> Arnaldo,
>> >>
>> >> The version of perf at tip-x86 commit 9274b36, segfaults for me:
>> >>
>> >> $ gdb perf
>> >> (gdb) r stat date
>> >>
>> >> Program received signal SIGSEGV, Segmentation fault.
>> >> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
>> >> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
>> >> at builtin-stat.c:206
>> >> 206 Â Â Â Â Â Â Â Â Â update_stats(&ps->res_stats[i], count[i]);
>> >> (gdb) bt
>> >> #0 Âcmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
>> >> out>) at builtin-stat.c:206
>> >> #1 Â0x0000000000405c8b in handle_internal_command (argc=2,
>> >> argv=0x7fff6fa2a9f0) at perf.c:286
>> >> #2 Â0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
>> >>
>> >> Most like ps is NULL.
>> >
>> > [acme@felicio linux]$ perf stat date | head -5
>> > Tue Jan Â4 12:03:05 BRST 2011
>> >
>> > ÂPerformance counter stats for 'date':
>> >
>> >       4,525 cache-misses       #   Â7.640 M/sec
>> >      343,171 cache-references     #  Â579.405 M/sec
>> >      Â14,853 branch-misses      Â#   Â8.214 %
>> >      180,833 branches         #  Â305.316 M/sec
>> >      897,837 instructions       #   Â0.000 IPC  Â(scaled from 67.67%)
>> > Â Â <not counted> cycles
>> >        201 page-faults       Â#   Â0.339 M/sec
>> >         1 CPU-migrations      #   Â0.002 M/sec
>> >         1 context-switches     #   Â0.002 M/sec
>> >     Â0.592282 task-clock-msecs     #   Â0.583 CPUs
>> >
>> > Â Â Â Â0.001015291 Âseconds time elapsed
>> >
>> > [acme@felicio linux]$
>> >
>> > Re-reading the code now, thanks!
>> >
>> > - Arnaldo
>> >
>> >> On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@xxxxxxx> wrote:
>> >> >
>> >> > * Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxx> wrote:
>> >> >
>> >> >> Hi Ingo, Peter et al,
>> >> >>
>> >> >> Â Â Â Trying to simplify using the API for tools and more specifically for
>> >> >> 'perf test' regression tests, please take a look, perhaps starting from the last
>> >> >> changeset, that implements a regression test using the resulting library API.
>> >> >>
>> >> >> Â Â Â It also reduces the 'perf' tool footprint by not using hard coded array
>> >> >> sizes, more need to be done, but should be a good start, one changeset shows a
>> >> >> good reduction in BSS use.
>> >> >>
>> >> >> Â Â Â Suggestions for naming most welcome, I thought about using "event__",
>> >> >> but that is taken, "perf_event__", but thought it would clash with "event_t",
>> >> >> so used the jargon used for the '-e' parameters: "Event Selector", but don't
>> >> >> like it that much, what do you think?
>> >> >>
>> >> >> Â Â Â Writing the first regression test I think there are more ways to simplify,
>> >> >> on top of these, like not requiring a thread_map, i.e. passing NULL for that
>> >> >> parameter would mean: self-monitor, etc.
>> >> >>
>> >> >> Â Â Â Â If you are pleased as-is, please pull from:
>> >> >>
>> >> >> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
>> >> >>
>> >> >> Regards,
>> >> >>
>> >> >> - Arnaldo
>> >> >>
>> >> >> Arnaldo Carvalho de Melo (11):
>> >> >> Â perf tools: Introduce event selectors
>> >> >> Â perf evsel: Adopt MATCH_EVENT macro from 'stat'
>> >> >> Â perf util: Move do_read from session to util
>> >> >> Â perf evsel: Delete the event selectors at exit
>> >> >> Â perf evsel: Steal the counter reading routines from stat
>> >> >> Â perf evsel: Introduce per cpu and per thread open helpers
>> >> >> Â perf tools: Refactor cpumap to hold nr and the map
>> >> >> Â perf tools: Refactor all_tids to hold nr and the map
>> >> >> Â perf evsel: Use {cpu,thread}_map to shorten list of parameters
>> >> >> Â perf evsel: Auto allocate resources needed for some methods
>> >> >> Â perf test: Add test for counting open syscalls
>> >> >>
>> >> >> Âtools/perf/Makefile        Â|  Â4 +
>> >> >> Âtools/perf/builtin-record.c    Â| Â152 +++++++--------
>> >> >> Âtools/perf/builtin-stat.c     Â| Â368 +++++++++++++++---------------------
>> >> >> Âtools/perf/builtin-test.c     Â|  83 ++++++++
>> >> >> Âtools/perf/builtin-top.c      | Â221 ++++++++++++----------
>> >> >> Âtools/perf/perf.c         Â|  Â2 +
>> >> >> Âtools/perf/util/cpumap.c      | Â123 +++++++++---
>> >> >> Âtools/perf/util/cpumap.h      |  10 +-
>> >> >> Âtools/perf/util/evsel.c      Â| Â186 ++++++++++++++++++
>> >> >> Âtools/perf/util/evsel.h      Â| Â115 +++++++++++
>> >> >> Âtools/perf/util/header.c      |  15 +-
>> >> >> Âtools/perf/util/header.h      |  Â3 +-
>> >> >> Âtools/perf/util/parse-events.c   |  58 ++++--
>> >> >> Âtools/perf/util/parse-events.h   |  18 ++-
>> >> >> Âtools/perf/util/session.c     Â|  22 +--
>> >> >> Âtools/perf/util/session.h     Â|  Â1 -
>> >> >> Âtools/perf/util/thread.c      |  43 +++--
>> >> >> Âtools/perf/util/thread.h      |  15 ++-
>> >> >> Âtools/perf/util/trace-event-info.c | Â 30 ++--
>> >> >> Âtools/perf/util/trace-event.h   Â|  Â5 +-
>> >> >> Âtools/perf/util/util.c       |  17 ++
>> >> >> Âtools/perf/util/util.h       |  Â1 +
>> >> >> Âtools/perf/util/xyarray.c     Â|  20 ++
>> >> >> Âtools/perf/util/xyarray.h     Â|  20 ++
>> >> >> Â24 files changed, 1013 insertions(+), 519 deletions(-)
>> >> >> Âcreate mode 100644 tools/perf/util/evsel.c
>> >> >> Âcreate mode 100644 tools/perf/util/evsel.h
>> >> >> Âcreate mode 100644 tools/perf/util/xyarray.c
>> >> >> Âcreate mode 100644 tools/perf/util/xyarray.h
>> >> >
>> >> > Pulled, thanks a lot Arnaldo!
>> >> >
>> >> > Â Â Â ÂIngo
>> >> >
>> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/