From: Kees Cook <keescook@chromium.org>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: akpm@linux-foundation.org, rostedt@goodmis.org,
mathieu.desnoyers@efficios.com, arnaldo.melo@gmail.com,
pmladek@suse.com, peterz@infradead.org, viro@zeniv.linux.org.uk,
valentin.schneider@arm.com, qiang.zhang@windriver.com,
robdclark@chromium.org, christian@brauner.io,
dietmar.eggemann@arm.com, mingo@redhat.com,
juri.lelli@redhat.com, vincent.guittot@linaro.org,
davem@davemloft.net, kuba@kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, kafai@fb.com,
songliubraving@fb.com, yhs@fb.com, john.fastabend@gmail.com,
kpsingh@kernel.org, dennis.dalessandro@cornelisnetworks.com,
mike.marciniszyn@cornelisnetworks.com, dledford@redhat.com,
jgg@ziepe.ca, linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
bpf@vger.kernel.org, linux-perf-users@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, oliver.sang@intel.com,
lkp@intel.com, Andrii Nakryiko <andrii.nakryiko@gmail.com>
Subject: Re: [PATCH v6 09/12] tools/perf/test: make perf test adopt to task comm size change
Date: Mon, 25 Oct 2021 14:26:24 -0700 [thread overview]
Message-ID: <202110251426.2921B7BDD7@keescook> (raw)
In-Reply-To: <20211025083315.4752-10-laoar.shao@gmail.com>
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
>
--
Kees Cook
next prev parent reply other threads:[~2021-10-25 21:26 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 8:33 [PATCH v6 00/12] extend task comm from 16 to 24 Yafang Shao
2021-10-25 8:33 ` [PATCH v6 01/12] fs/exec: make __set_task_comm always set a nul ternimated string Yafang Shao
2021-10-25 21:07 ` Kees Cook
2021-10-26 1:48 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 02/12] fs/exec: make __get_task_comm always get a nul terminated string Yafang Shao
2021-10-25 21:08 ` Kees Cook
2021-10-26 1:49 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 03/12] drivers/connector: make connector comm always nul ternimated Yafang Shao
2021-10-25 21:14 ` Kees Cook
2021-10-26 1:50 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 04/12] drivers/infiniband: make setup_ctxt always get a nul terminated task comm Yafang Shao
2021-10-25 18:20 ` Dennis Dalessandro
2021-10-25 21:16 ` Kees Cook
2021-10-26 1:52 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 05/12] elfcore: make prpsinfo " Yafang Shao
2021-10-25 21:18 ` Kees Cook
2021-10-26 1:56 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 06/12] samples/bpf/test_overhead_kprobe_kern: make it adopt to task comm size change Yafang Shao
2021-10-25 21:20 ` Kees Cook
2021-10-25 8:33 ` [PATCH v6 07/12] samples/bpf/offwaketime_kern: make sched_switch tracepoint args adopt to " Yafang Shao
2021-10-25 21:21 ` Kees Cook
2021-10-26 1:56 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 08/12] tools/bpf/bpftool/skeleton: make it adopt to task " Yafang Shao
2021-10-25 21:24 ` Kees Cook
2021-10-26 2:18 ` Yafang Shao
2021-10-26 13:12 ` Steven Rostedt
2021-10-26 13:55 ` Yafang Shao
2021-10-26 14:02 ` Yafang Shao
2021-10-26 16:09 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 09/12] tools/perf/test: make perf test " Yafang Shao
2021-10-25 21:26 ` Kees Cook [this message]
2021-10-25 8:33 ` [PATCH v6 10/12] tools/testing/selftests/bpf: make it " Yafang Shao
2021-10-25 21:29 ` Kees Cook
2021-10-26 2:21 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 11/12] sched.h: extend task comm from 16 to 24 Yafang Shao
2021-10-25 21:30 ` Kees Cook
2021-10-26 2:22 ` Yafang Shao
2021-10-25 8:33 ` [PATCH v6 12/12] kernel/kthread: show a warning if kthread's comm is truncated Yafang Shao
2021-10-25 21:35 ` Kees Cook
2021-10-26 2:23 ` Yafang Shao
2021-10-27 20:10 ` Petr Mladek
2021-10-28 1:42 ` Yafang Shao
2021-10-29 7:44 ` Yafang Shao
2021-10-25 18:10 ` [PATCH v6 00/12] extend task comm from 16 to 24 Alexei Starovoitov
2021-10-25 21:05 ` Steven Rostedt
2021-10-25 21:06 ` Kees Cook
2021-10-26 10:35 ` Petr Mladek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202110251426.2921B7BDD7@keescook \
--to=keescook@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=arnaldo.melo@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=christian@brauner.io \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dennis.dalessandro@cornelisnetworks.com \
--cc=dietmar.eggemann@arm.com \
--cc=dledford@redhat.com \
--cc=jgg@ziepe.ca \
--cc=john.fastabend@gmail.com \
--cc=juri.lelli@redhat.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=laoar.shao@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=lkp@intel.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mike.marciniszyn@cornelisnetworks.com \
--cc=mingo@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=oliver.sang@intel.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=qiang.zhang@windriver.com \
--cc=robdclark@chromium.org \
--cc=rostedt@goodmis.org \
--cc=songliubraving@fb.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.guittot@linaro.org \
--cc=viro@zeniv.linux.org.uk \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).