Em Tue, Feb 19, 2019 at 04:36:39PM +0100, Thomas Richter escreveu:
> Commit 489338a717a0 ("perf tests evsel-tp-sched: Fix bitwise operator")
> causes test case 14 "Parse sched tracepoints fields" to fail on s390.
> This test succeeds on x86.
> In fact this test now fails on all architectures with type char
> treated as type unsigned char.
>
> The root cause is the signed-ness of character arrays in the
> tracepoints sched_switch for structure members prev_comm and
> next_comm.
>
> On s390 the output of
> [root@m35lp76 perf]# cat /sys/kernel/debug/tracing/events/\
> sched/sched_switch/format
> name: sched_switch
> ID: 287
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> ...
> field:char prev_comm[16]; offset:8; size:16; signed:0;
> ...
> field:char next_comm[16]; offset:40; size:16; signed:0;
>
> reveals the character arrays prev_comm and next_comm are per
> default unsigned char and have values in the range of 0..255.
>
> On x86 both fields are signed as this output shows:
> [root@f29]# cat /sys/kernel/debug/tracing/events/sched/\
> sched_switch/format
> name: sched_switch
> ID: 287
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> ...
> field:char prev_comm[16]; offset:8; size:16; signed:1;
> ...
> field:char next_comm[16]; offset:40; size:16; signed:1;
>
> and the character arrays prev_comm and next_comm are per
> default signed char and have values in the range of -1..127.
> The implementation of type char is architecture specific.
>
> Since the character arrays in both tracepoints
> sched_switch and sched_wakeup should contain ascii characters,
> simply omit the check for signedness in the test case.
>
> Output before:
> [root@m35lp76 perf]# ./perf test -F 14
> 14: Parse sched tracepoints fields :
> --- start ---
> sched:sched_switch: "prev_comm" signedness(0) is wrong, should be 1
> sched:sched_switch: "next_comm" signedness(0) is wrong, should be 1
> sched:sched_wakeup: "comm" signedness(0) is wrong, should be 1
> ---- end ----
Please indent command output by two chars to avoid things like lines
starting with '---' to break scripts that expect that to be the marker
for the end of the commit log message as below...
> 14: Parse sched tracepoints fields : FAILED!
> [root@m35lp76 perf]#
>
> Output after:
> [root@m35lp76 perf]# ./perf test -Fv 14
> 14: Parse sched tracepoints fields :
> --- start ---
> ---- end ----
> Parse sched tracepoints fields: Ok
> [root@m35lp76 perf]#
>
> Fixes: 489338a717a0 ("perf tests evsel-tp-sched: Fix bitwise operator")
>
> Signed-off-by: Thomas Richter <[email protected]>
> ---
... here ^^^
I'm fixing this up now.
- Arnaldo
> tools/perf/tests/evsel-tp-sched.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/evsel-tp-sched.c
> b/tools/perf/tests/evsel-tp-sched.c
> index 5cbba70bcdd0..ea7acf403727 100644
> --- a/tools/perf/tests/evsel-tp-sched.c
> +++ b/tools/perf/tests/evsel-tp-sched.c
> @@ -43,7 +43,7 @@ int test__perf_evsel__tp_sched_test(struct test *test
> __maybe_unused, int subtes
> return -1;
> }
>
> - if (perf_evsel__test_field(evsel, "prev_comm", 16, true))
> + if (perf_evsel__test_field(evsel, "prev_comm", 16, false))
> ret = -1;
>
> if (perf_evsel__test_field(evsel, "prev_pid", 4, true))
> @@ -55,7 +55,7 @@ int test__perf_evsel__tp_sched_test(struct test *test
> __maybe_unused, int subtes
> if (perf_evsel__test_field(evsel, "prev_state", sizeof(long), true))
> ret = -1;
>
> - if (perf_evsel__test_field(evsel, "next_comm", 16, true))
> + if (perf_evsel__test_field(evsel, "next_comm", 16, false))
> ret = -1;
>
> if (perf_evsel__test_field(evsel, "next_pid", 4, true))
> @@ -73,7 +73,7 @@ int test__perf_evsel__tp_sched_test(struct test *test
> __maybe_unused, int subtes
> return -1;
> }
>
> - if (perf_evsel__test_field(evsel, "comm", 16, true))
> + if (perf_evsel__test_field(evsel, "comm", 16, false))
> ret = -1;
>
> if (perf_evsel__test_field(evsel, "pid", 4, true))
> --
> 2.19.1
--
- Arnaldo
