| Message ID | 20211025083315.4752-10-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | extend task comm from 16 to 24 | expand |
On Mon, Oct 25, 2021 at 08:33:12AM +0000, Yafang Shao wrote: > kernel test robot reported a perf-test failure after I extended task comm > size from 16 to 24. The failure as follows, > > 2021-10-13 18:00:46 sudo /usr/src/perf_selftests-x86_64-rhel-8.3-317419b91ef4eff4e2f046088201e4dc4065caa0/tools/perf/perf test 15 > 15: Parse sched tracepoints fields : FAILED! > > The reason is perf-test requires a fixed-size task comm. If we extend > task comm size to 24, it will not equil with the required size 16 in perf > test. > > After some analyzation, I found perf itself can adopt to the size > change, for example, below is the output of perf-sched after I extend > comm size to 24 - > > task 614 ( kthreadd: 84), nr_events: 1 > task 615 ( systemd: 843), nr_events: 1 > task 616 ( networkd-dispat: 1026), nr_events: 1 > task 617 ( systemd: 846), nr_events: 1 > > $ cat /proc/843/comm > networkd-dispatcher > > The task comm can be displayed correctly as expected. > > Replace old hard-coded 16 with the new one can fix the warning, but we'd > better make the test accept both old and new sizes, then it can be > backward compatibility. > > After this patch, the perf-test succeeds no matter task comm is 16 or > 24 - > > 15: Parse sched tracepoints fields : Ok > > This patch is a preparation for the followup patch. > > Reported-by: kernel test robot <oliver.sang@intel.com> > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> > Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Kees Cook <keescook@chromium.org> > Cc: Petr Mladek <pmladek@suse.com> I'll let the perf folks comment on this one. :) -Kees > --- > tools/include/linux/sched.h | 11 +++++++++++ > tools/perf/tests/evsel-tp-sched.c | 26 ++++++++++++++++++++------ > 2 files changed, 31 insertions(+), 6 deletions(-) > create mode 100644 tools/include/linux/sched.h > > diff --git a/tools/include/linux/sched.h b/tools/include/linux/sched.h > new file mode 100644 > index 000000000000..0d575afd7f43 > --- /dev/null > +++ b/tools/include/linux/sched.h > @@ -0,0 +1,11 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _TOOLS_LINUX_SCHED_H > +#define _TOOLS_LINUX_SCHED_H > + > +/* Keep both length for backward compatibility */ > +enum { > + TASK_COMM_LEN_16 = 16, > + TASK_COMM_LEN = 24, > +}; > + > +#endif /* _TOOLS_LINUX_SCHED_H */ > diff --git a/tools/perf/tests/evsel-tp-sched.c b/tools/perf/tests/evsel-tp-sched.c > index f9e34bd26cf3..029f2a8c8e51 100644 > --- a/tools/perf/tests/evsel-tp-sched.c > +++ b/tools/perf/tests/evsel-tp-sched.c > @@ -1,11 +1,13 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/err.h> > +#include <linux/sched.h> > #include <traceevent/event-parse.h> > #include "evsel.h" > #include "tests.h" > #include "debug.h" > > -static int evsel__test_field(struct evsel *evsel, const char *name, int size, bool should_be_signed) > +static int evsel__test_field_alt(struct evsel *evsel, const char *name, > + int size, int alternate_size, bool should_be_signed) > { > struct tep_format_field *field = evsel__field(evsel, name); > int is_signed; > @@ -23,15 +25,24 @@ static int evsel__test_field(struct evsel *evsel, const char *name, int size, bo > ret = -1; > } > > - if (field->size != size) { > - pr_debug("%s: \"%s\" size (%d) should be %d!\n", > + if (field->size != size && field->size != alternate_size) { > + pr_debug("%s: \"%s\" size (%d) should be %d", > evsel->name, name, field->size, size); > + if (alternate_size > 0) > + pr_debug(" or %d", alternate_size); > + pr_debug("!\n"); > ret = -1; > } > > return ret; > } > > +static int evsel__test_field(struct evsel *evsel, const char *name, > + int size, bool should_be_signed) > +{ > + return evsel__test_field_alt(evsel, name, size, -1, should_be_signed); > +} > + > int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtest __maybe_unused) > { > struct evsel *evsel = evsel__newtp("sched", "sched_switch"); > @@ -42,7 +53,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes > return -1; > } > > - if (evsel__test_field(evsel, "prev_comm", 16, false)) > + if (evsel__test_field_alt(evsel, "prev_comm", TASK_COMM_LEN_16, > + TASK_COMM_LEN, false)) > ret = -1; > > if (evsel__test_field(evsel, "prev_pid", 4, true)) > @@ -54,7 +66,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes > if (evsel__test_field(evsel, "prev_state", sizeof(long), true)) > ret = -1; > > - if (evsel__test_field(evsel, "next_comm", 16, false)) > + if (evsel__test_field_alt(evsel, "next_comm", TASK_COMM_LEN_16, > + TASK_COMM_LEN, false)) > ret = -1; > > if (evsel__test_field(evsel, "next_pid", 4, true)) > @@ -72,7 +85,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes > return -1; > } > > - if (evsel__test_field(evsel, "comm", 16, false)) > + if (evsel__test_field_alt(evsel, "comm", TASK_COMM_LEN_16, > + TASK_COMM_LEN, false)) > ret = -1; > > if (evsel__test_field(evsel, "pid", 4, true)) > -- > 2.17.1 >
diff --git a/tools/include/linux/sched.h b/tools/include/linux/sched.h new file mode 100644 index 000000000000..0d575afd7f43 --- /dev/null +++ b/tools/include/linux/sched.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_LINUX_SCHED_H +#define _TOOLS_LINUX_SCHED_H + +/* Keep both length for backward compatibility */ +enum { + TASK_COMM_LEN_16 = 16, + TASK_COMM_LEN = 24, +}; + +#endif /* _TOOLS_LINUX_SCHED_H */ diff --git a/tools/perf/tests/evsel-tp-sched.c b/tools/perf/tests/evsel-tp-sched.c index f9e34bd26cf3..029f2a8c8e51 100644 --- a/tools/perf/tests/evsel-tp-sched.c +++ b/tools/perf/tests/evsel-tp-sched.c @@ -1,11 +1,13 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/err.h> +#include <linux/sched.h> #include <traceevent/event-parse.h> #include "evsel.h" #include "tests.h" #include "debug.h" -static int evsel__test_field(struct evsel *evsel, const char *name, int size, bool should_be_signed) +static int evsel__test_field_alt(struct evsel *evsel, const char *name, + int size, int alternate_size, bool should_be_signed) { struct tep_format_field *field = evsel__field(evsel, name); int is_signed; @@ -23,15 +25,24 @@ static int evsel__test_field(struct evsel *evsel, const char *name, int size, bo ret = -1; } - if (field->size != size) { - pr_debug("%s: \"%s\" size (%d) should be %d!\n", + if (field->size != size && field->size != alternate_size) { + pr_debug("%s: \"%s\" size (%d) should be %d", evsel->name, name, field->size, size); + if (alternate_size > 0) + pr_debug(" or %d", alternate_size); + pr_debug("!\n"); ret = -1; } return ret; } +static int evsel__test_field(struct evsel *evsel, const char *name, + int size, bool should_be_signed) +{ + return evsel__test_field_alt(evsel, name, size, -1, should_be_signed); +} + int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtest __maybe_unused) { struct evsel *evsel = evsel__newtp("sched", "sched_switch"); @@ -42,7 +53,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes return -1; } - if (evsel__test_field(evsel, "prev_comm", 16, false)) + if (evsel__test_field_alt(evsel, "prev_comm", TASK_COMM_LEN_16, + TASK_COMM_LEN, false)) ret = -1; if (evsel__test_field(evsel, "prev_pid", 4, true)) @@ -54,7 +66,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes if (evsel__test_field(evsel, "prev_state", sizeof(long), true)) ret = -1; - if (evsel__test_field(evsel, "next_comm", 16, false)) + if (evsel__test_field_alt(evsel, "next_comm", TASK_COMM_LEN_16, + TASK_COMM_LEN, false)) ret = -1; if (evsel__test_field(evsel, "next_pid", 4, true)) @@ -72,7 +85,8 @@ int test__perf_evsel__tp_sched_test(struct test *test __maybe_unused, int subtes return -1; } - if (evsel__test_field(evsel, "comm", 16, false)) + if (evsel__test_field_alt(evsel, "comm", TASK_COMM_LEN_16, + TASK_COMM_LEN, false)) ret = -1; if (evsel__test_field(evsel, "pid", 4, true))
kernel test robot reported a perf-test failure after I extended task comm size from 16 to 24. The failure as follows, 2021-10-13 18:00:46 sudo /usr/src/perf_selftests-x86_64-rhel-8.3-317419b91ef4eff4e2f046088201e4dc4065caa0/tools/perf/perf test 15 15: Parse sched tracepoints fields : FAILED! The reason is perf-test requires a fixed-size task comm. If we extend task comm size to 24, it will not equil with the required size 16 in perf test. After some analyzation, I found perf itself can adopt to the size change, for example, below is the output of perf-sched after I extend comm size to 24 - task 614 ( kthreadd: 84), nr_events: 1 task 615 ( systemd: 843), nr_events: 1 task 616 ( networkd-dispat: 1026), nr_events: 1 task 617 ( systemd: 846), nr_events: 1 $ cat /proc/843/comm networkd-dispatcher The task comm can be displayed correctly as expected. Replace old hard-coded 16 with the new one can fix the warning, but we'd better make the test accept both old and new sizes, then it can be backward compatibility. After this patch, the perf-test succeeds no matter task comm is 16 or 24 - 15: Parse sched tracepoints fields : Ok This patch is a preparation for the followup patch. Reported-by: kernel test robot <oliver.sang@intel.com> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Kees Cook <keescook@chromium.org> Cc: Petr Mladek <pmladek@suse.com> --- tools/include/linux/sched.h | 11 +++++++++++ tools/perf/tests/evsel-tp-sched.c | 26 ++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 tools/include/linux/sched.h