On Mon, Apr 21, 2014 at 07:43:50PM +0400, Alexander Yarygin wrote:
SNIP
>
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1346,6 +1346,12 @@ static struct evlist_test test__events[] = {
> .name = "{cycles,cache-misses,branch-misses}:D",
> .check = test__pinned_group,
> },
> +#if defined(__s390x__)
> + [42] = {
> + .name = "kvm-s390:kvm_s390_create_vm",
> + .check = test__checkevent_tracepoint,
> + },
> +#endif /* and what will be the next number: 42 or 43? */
> };
>
> static struct evlist_test test__events_pmu[] = {
>
>
> Because it breaks explicit numbering of test__events[].
> I can suggest to move numeration into evlist_test, i.e.
>
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1174,25 +1174,30 @@ static int test__all_tracepoints(struct perf_evlist *evlist)
> struct evlist_test {
> const char *name;
> __u32 type;
> + int index;
> int (*check)(struct perf_evlist *evlist);
> };
>
> static struct evlist_test test__events[] = {
> - [0] = {
> + {
> .name = "syscalls:sys_enter_open",
> .check = test__checkevent_tracepoint,
> + .index = 0;
> },
> ...
>
> or just to remove it?
right, that numbering is there to ease the search for test,
and is printed for -v option
the 'index' field should be fine, and please print it
out for '-v' option
>
> How do you think?
>
>
> And a bit of offtopic :)
> Apparently, s390 doesn't have syscalls:*, so some of the tests
> don't work properly (or maybe I missed something? I set CONFIG_FTRACE_SYSCALLS
> to 'y' in my config: still no syscalls:*).
>
> What do you think about this idea:
>
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1177,13 +1177,21 @@ struct evlist_test {
> int (*check)(struct perf_evlist *evlist);
> };
>
> +#if !defined(__s390x__)
> +#define TP_SYS_NAME "syscalls"
> +#define TP_EVENT_NAME "sys_enter_open"
> +#else
> +#define TP_SYS_NAME "sched"
> +#define TP_EVENT_NAME "sched_wakeup"
> +#endif
> +
> static struct evlist_test test__events[] = {
> [0] = {
> - .name = "syscalls:sys_enter_open",
> + .name = TP_SYS_NAME ":" TP_EVENT_NAME,
> .check = test__checkevent_tracepoint,
> },
>
> ... and so on?
that looks fine.. also we could use just generic tracepoints
like the 'sched' ones
>
> Also, test_pmu() looks at /sys/bus/event_source/devices/cpu/
> but instead of "cpu/" on s390 there are "cpum_sf/" and "cpum_cf/",
> so pmu tests don't work either..
we could #ifdef the pmu name for s390 and other archs
or make it more fancy and detect it by using pmu.c code,
please check perf_pmu__scan,perf_pmu__find functions
thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
