LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH -mm 0/5] Phase 2 of task comm cleanups
@ 2021-12-04  9:52 Yafang Shao
  2021-12-04  9:52 ` [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16 Yafang Shao
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

This is the followup work of task comm cleanups[1].

In this phase, the hard-coded 16 is replaced by either TASK_COMM_LEN or
TASK_COMM_LEN_16, to make it grepable. The difference between this two 
marcos is: 
- TASK_COMM_LEN
  The size should be same with the TASK_COMM_LEN defined in linux/sched.h.
  For the src file which can't include linux/sched.h, a macro with the 
  the same name is defined in this file specifically. 
- TASK_COMM_LEN_16
  The size must be a fixed-size 16. It may be exposed to userspace so we
  can't change it. 

In order to include vmlinux.h in bpf progs under sample/bpf or
tools/testing/selftests/bpf, some structs are renamed and some included
headers are removed.

1. https://lore.kernel.org/lkml/20211120112738.45980-1-laoar.shao@gmail.com/

Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>

Yafang Shao (5):
  elfcore: replace old hard-code 16 with TASK_COMM_LEN_16
  cn_proc: replaced old hard-coded 16 with TASK_COMM_LEN_16
  samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN
  tools/perf: replace hard-coded 16 with TASK_COMM_LEN
  bpf/progs: replace hard-coded 16 with TASK_COMM_LEN

 include/linux/elfcore-compat.h                        |  8 ++------
 include/linux/elfcore.h                               |  9 ++-------
 include/linux/sched.h                                 |  5 +++++
 include/uapi/linux/cn_proc.h                          |  4 +++-
 samples/bpf/tracex2_kern.c                            |  3 ++-
 samples/bpf/tracex2_user.c                            |  3 ++-
 tools/perf/tests/evsel-tp-sched.c                     |  8 +++++---
 tools/testing/selftests/bpf/prog_tests/ringbuf.c      |  9 +++++----
 .../testing/selftests/bpf/prog_tests/ringbuf_multi.c  |  8 +++++---
 .../selftests/bpf/prog_tests/sk_storage_tracing.c     |  3 ++-
 .../testing/selftests/bpf/prog_tests/test_overhead.c  |  3 ++-
 .../selftests/bpf/prog_tests/trampoline_count.c       |  3 ++-
 .../selftests/bpf/progs/test_core_reloc_kernel.c      | 11 +++++------
 tools/testing/selftests/bpf/progs/test_ringbuf.c      |  8 ++++----
 .../testing/selftests/bpf/progs/test_ringbuf_multi.c  |  8 ++++----
 .../selftests/bpf/progs/test_sk_storage_tracing.c     |  4 ++--
 16 files changed, 52 insertions(+), 45 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16
  2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
@ 2021-12-04  9:52 ` Yafang Shao
  2021-12-08 18:39   ` Steven Rostedt
  2021-12-04  9:52 ` [PATCH -mm 2/5] cn_proc: replaced old hard-coded " Yafang Shao
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

A new macro TASK_COMM_LEN_16 is introduced for the old hard-coded 16 to
make it more grepable. As explained above this marco, the difference
between TASK_COMM_LEN and TASK_COMM_LEN_16 is that TASK_COMM_LEN_16 must
be a fixed size 16 and can't be changed.

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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 include/linux/elfcore-compat.h | 8 ++------
 include/linux/elfcore.h        | 9 ++-------
 include/linux/sched.h          | 5 +++++
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/elfcore-compat.h b/include/linux/elfcore-compat.h
index 54feb64e9b5d..69fa1a728964 100644
--- a/include/linux/elfcore-compat.h
+++ b/include/linux/elfcore-compat.h
@@ -5,6 +5,7 @@
 #include <linux/elf.h>
 #include <linux/elfcore.h>
 #include <linux/compat.h>
+#include <linux/sched.h>
 
 /*
  * Make sure these layouts match the linux/elfcore.h native definitions.
@@ -43,12 +44,7 @@ struct compat_elf_prpsinfo
 	__compat_uid_t			pr_uid;
 	__compat_gid_t			pr_gid;
 	compat_pid_t			pr_pid, pr_ppid, pr_pgrp, pr_sid;
-	/*
-	 * The hard-coded 16 is derived from TASK_COMM_LEN, but it can't be
-	 * changed as it is exposed to userspace. We'd better make it hard-coded
-	 * here.
-	 */
-	char				pr_fname[16];
+	char				pr_fname[TASK_COMM_LEN_16];
 	char				pr_psargs[ELF_PRARGSZ];
 };
 
diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
index 746e081879a5..d3bb4bd3c985 100644
--- a/include/linux/elfcore.h
+++ b/include/linux/elfcore.h
@@ -65,13 +65,8 @@ struct elf_prpsinfo
 	__kernel_gid_t	pr_gid;
 	pid_t	pr_pid, pr_ppid, pr_pgrp, pr_sid;
 	/* Lots missing */
-	/*
-	 * The hard-coded 16 is derived from TASK_COMM_LEN, but it can't be
-	 * changed as it is exposed to userspace. We'd better make it hard-coded
-	 * here.
-	 */
-	char	pr_fname[16];	/* filename of executable */
-	char	pr_psargs[ELF_PRARGSZ];	/* initial part of arg list */
+	char	pr_fname[TASK_COMM_LEN_16];	/* filename of executable */
+	char	pr_psargs[ELF_PRARGSZ];		/* initial part of arg list */
 };
 
 static inline void elf_core_copy_regs(elf_gregset_t *elfregs, struct pt_regs *regs)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index c79bd7ee6029..8d963a50a2a8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -279,6 +279,11 @@ struct task_group;
  * BPF programs.
  */
 enum {
+	/*
+	 * For the old hard-coded 16, which is exposed to userspace and can't
+	 * be changed.
+	 */
+	TASK_COMM_LEN_16 = 16,
 	TASK_COMM_LEN = 16,
 };
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH -mm 2/5] cn_proc: replaced old hard-coded 16 with TASK_COMM_LEN_16
  2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
  2021-12-04  9:52 ` [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16 Yafang Shao
@ 2021-12-04  9:52 ` Yafang Shao
  2021-12-08 18:40   ` Steven Rostedt
  2021-12-04  9:52 ` [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN Yafang Shao
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

This TASK_COMM_LEN_16 has the same meaning with the macro defined in
linux/sched.h, but we can't include linux/sched.h in a UAPI header, so
we should specifically define it in the cn_proc.h.

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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 include/uapi/linux/cn_proc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
index db210625cee8..6dcccaed383f 100644
--- a/include/uapi/linux/cn_proc.h
+++ b/include/uapi/linux/cn_proc.h
@@ -21,6 +21,8 @@
 
 #include <linux/types.h>
 
+#define TASK_COMM_LEN_16 16
+
 /*
  * Userspace sends this enum to register with the kernel that it is listening
  * for events on the connector.
@@ -110,7 +112,7 @@ struct proc_event {
 		struct comm_proc_event {
 			__kernel_pid_t process_pid;
 			__kernel_pid_t process_tgid;
-			char           comm[16];
+			char           comm[TASK_COMM_LEN_16];
 		} comm;
 
 		struct coredump_proc_event {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
  2021-12-04  9:52 ` [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16 Yafang Shao
  2021-12-04  9:52 ` [PATCH -mm 2/5] cn_proc: replaced old hard-coded " Yafang Shao
@ 2021-12-04  9:52 ` Yafang Shao
  2021-12-04 16:45   ` Alexei Starovoitov
  2021-12-04  9:52 ` [PATCH -mm 4/5] tools/perf: " Yafang Shao
  2021-12-04  9:52 ` [PATCH -mm 5/5] bpf/progs: " Yafang Shao
  4 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

The comm used in tracex2 should have the same size with the comm in
task_struct, we'd better define the size of it as TASK_COMM_LEN to make it
more grepable.

linux/sched.h can be included in tracex2 kernel code, but it can't be
included in tracex2 userspace code. So a new marco TASK_COMM_LEN is
defined in tracex2 userspace code.

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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 samples/bpf/tracex2_kern.c | 3 ++-
 samples/bpf/tracex2_user.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 5bc696bac27d..51dbaf765cd5 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -7,6 +7,7 @@
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/version.h>
+#include <linux/sched.h>
 #include <uapi/linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
@@ -65,7 +66,7 @@ static unsigned int log2l(unsigned long v)
 }
 
 struct hist_key {
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 	u64 pid_tgid;
 	u64 uid_gid;
 	u64 index;
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
index 1626d51dfffd..b728a946d83d 100644
--- a/samples/bpf/tracex2_user.c
+++ b/samples/bpf/tracex2_user.c
@@ -12,6 +12,7 @@
 
 #define MAX_INDEX	64
 #define MAX_STARS	38
+#define TASK_COMM_LEN	16
 
 /* my_map, my_hist_map */
 static int map_fd[2];
@@ -28,7 +29,7 @@ static void stars(char *str, long val, long max, int width)
 }
 
 struct task {
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 	__u64 pid_tgid;
 	__u64 uid_gid;
 };
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH -mm 4/5] tools/perf: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
                   ` (2 preceding siblings ...)
  2021-12-04  9:52 ` [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN Yafang Shao
@ 2021-12-04  9:52 ` Yafang Shao
  2021-12-08 18:43   ` Steven Rostedt
  2021-12-04  9:52 ` [PATCH -mm 5/5] bpf/progs: " Yafang Shao
  4 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

evsel-tp-sched will verify the task comm len in sched:sched_switch
and sched:sched_wakeup tracepoints. The len must be same with TASK_COMM_LEN
defined in linux/sched.h. In order to make it grepable, we'd better replace
the hard-coded 16 with TASK_COMM_LEN.

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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 tools/perf/tests/evsel-tp-sched.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/evsel-tp-sched.c b/tools/perf/tests/evsel-tp-sched.c
index cf4da3d748c2..83e0ce2e676f 100644
--- a/tools/perf/tests/evsel-tp-sched.c
+++ b/tools/perf/tests/evsel-tp-sched.c
@@ -5,6 +5,8 @@
 #include "tests.h"
 #include "debug.h"
 
+#define TASK_COMM_LEN 16
+
 static int evsel__test_field(struct evsel *evsel, const char *name, int size, bool should_be_signed)
 {
 	struct tep_format_field *field = evsel__field(evsel, name);
@@ -43,7 +45,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
 		return -1;
 	}
 
-	if (evsel__test_field(evsel, "prev_comm", 16, false))
+	if (evsel__test_field(evsel, "prev_comm", TASK_COMM_LEN, false))
 		ret = -1;
 
 	if (evsel__test_field(evsel, "prev_pid", 4, true))
@@ -55,7 +57,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
 	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(evsel, "next_comm", TASK_COMM_LEN, false))
 		ret = -1;
 
 	if (evsel__test_field(evsel, "next_pid", 4, true))
@@ -73,7 +75,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
 		return -1;
 	}
 
-	if (evsel__test_field(evsel, "comm", 16, false))
+	if (evsel__test_field(evsel, "comm", TASK_COMM_LEN, false))
 		ret = -1;
 
 	if (evsel__test_field(evsel, "pid", 4, true))
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH -mm 5/5] bpf/progs: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
                   ` (3 preceding siblings ...)
  2021-12-04  9:52 ` [PATCH -mm 4/5] tools/perf: " Yafang Shao
@ 2021-12-04  9:52 ` Yafang Shao
  2021-12-04 16:44   ` Alexei Starovoitov
  4 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-04  9:52 UTC (permalink / raw)
  To: akpm, rostedt, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko
  Cc: linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Yafang Shao, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

The comm used in these bpf progs should have the same size with the comm
field in struct task_struct defined in linux/sched.h. We'd better define
the size as TASK_COMM_LEN to make it more grepable.

The bpf progs kernel code can inlcude vmlinux.h generated by BTF CO-RE
to use TASK_COMM_LEN, while the userspace code can't include it so
TASK_COMM_LEN is specifically defined in the userspace code.

In order to fix redefinitions caused by the new included vmlinux.h, some
headers are removed and some structs are renamed.

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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 tools/testing/selftests/bpf/prog_tests/ringbuf.c      |  9 +++++----
 .../testing/selftests/bpf/prog_tests/ringbuf_multi.c  |  8 +++++---
 .../selftests/bpf/prog_tests/sk_storage_tracing.c     |  3 ++-
 .../testing/selftests/bpf/prog_tests/test_overhead.c  |  3 ++-
 .../selftests/bpf/prog_tests/trampoline_count.c       |  3 ++-
 .../selftests/bpf/progs/test_core_reloc_kernel.c      | 11 +++++------
 tools/testing/selftests/bpf/progs/test_ringbuf.c      |  8 ++++----
 .../testing/selftests/bpf/progs/test_ringbuf_multi.c  |  8 ++++----
 .../selftests/bpf/progs/test_sk_storage_tracing.c     |  4 ++--
 9 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf.c b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
index 9a80fe8a6427..39e43245db0a 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
@@ -15,14 +15,15 @@
 #include "test_ringbuf.lskel.h"
 
 #define EDONE 7777
+#define TASK_COMM_LEN 16
 
 static int duration = 0;
 
-struct sample {
+struct sample_ringbuf {
 	int pid;
 	int seq;
 	long value;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 static int sample_cnt;
@@ -39,7 +40,7 @@ static int atomic_xchg(int *cnt, int val)
 
 static int process_sample(void *ctx, void *data, size_t len)
 {
-	struct sample *s = data;
+	struct sample_ringbuf *s = data;
 
 	atomic_inc(&sample_cnt);
 
@@ -83,7 +84,7 @@ static void *poll_thread(void *input)
 
 void test_ringbuf(void)
 {
-	const size_t rec_sz = BPF_RINGBUF_HDR_SZ + sizeof(struct sample);
+	const size_t rec_sz = BPF_RINGBUF_HDR_SZ + sizeof(struct sample_ringbuf);
 	pthread_t thread;
 	long bg_ret = -1;
 	int err, cnt, rb_fd;
diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
index e945195b24c9..cb592555513f 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
@@ -4,19 +4,21 @@
 #include <sys/epoll.h>
 #include "test_ringbuf_multi.skel.h"
 
+#define TASK_COMM_LEN 16
+
 static int duration = 0;
 
-struct sample {
+struct sample_ringbuf {
 	int pid;
 	int seq;
 	long value;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 static int process_sample(void *ctx, void *data, size_t len)
 {
 	int ring = (unsigned long)ctx;
-	struct sample *s = data;
+	struct sample_ringbuf *s = data;
 
 	switch (s->seq) {
 	case 0:
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c b/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
index 547ae53cde74..dbbdbf4400d7 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_storage_tracing.c
@@ -11,11 +11,12 @@
 
 #define LO_ADDR6 "::1"
 #define TEST_COMM "test_progs"
+#define TASK_COMM_LEN 16
 
 struct sk_stg {
 	__u32 pid;
 	__u32 last_notclose_state;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 static struct test_sk_storage_tracing *skel;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_overhead.c b/tools/testing/selftests/bpf/prog_tests/test_overhead.c
index 123c68c1917d..7fe60ec12fc4 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_overhead.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_overhead.c
@@ -6,6 +6,7 @@
 #include <test_progs.h>
 
 #define MAX_CNT 100000
+#define TASK_COMM_LEN 16
 
 static __u64 time_get_ns(void)
 {
@@ -67,7 +68,7 @@ void test_test_overhead(void)
 	struct bpf_object *obj;
 	struct bpf_link *link;
 	int err, duration = 0;
-	char comm[16] = {};
+	char comm[TASK_COMM_LEN] = {};
 
 	if (CHECK_FAIL(prctl(PR_GET_NAME, comm, 0L, 0L, 0L)))
 		return;
diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
index fc146671b20a..da83f7408aa8 100644
--- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
+++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
@@ -5,6 +5,7 @@
 #include <test_progs.h>
 
 #define MAX_TRAMP_PROGS 38
+#define TASK_COMM_LEN 16
 
 struct inst {
 	struct bpf_object *obj;
@@ -51,7 +52,7 @@ void serial_test_trampoline_count(void)
 	int err, i = 0, duration = 0;
 	struct bpf_object *obj;
 	struct bpf_link *link;
-	char comm[16] = {};
+	char comm[TASK_COMM_LEN] = {};
 
 	/* attach 'allowed' trampoline programs */
 	for (i = 0; i < MAX_TRAMP_PROGS; i++) {
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index 145028b52ad8..7b1bb73c3501 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2019 Facebook
 
-#include <linux/bpf.h>
-#include <stdint.h>
+#include <vmlinux.h>
 #include <stdbool.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_core_read.h>
@@ -23,11 +22,11 @@ struct core_reloc_kernel_output {
 	int comm_len;
 };
 
-struct task_struct {
+struct task_struct_reloc {
 	int pid;
 	int tgid;
-	char comm[16];
-	struct task_struct *group_leader;
+	char comm[TASK_COMM_LEN];
+	struct task_struct_reloc *group_leader;
 };
 
 #define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
@@ -35,7 +34,7 @@ struct task_struct {
 SEC("raw_tracepoint/sys_enter")
 int test_core_kernel(void *ctx)
 {
-	struct task_struct *task = (void *)bpf_get_current_task();
+	struct task_struct_reloc *task = (void *)bpf_get_current_task();
 	struct core_reloc_kernel_output *out = (void *)&data.out;
 	uint64_t pid_tgid = bpf_get_current_pid_tgid();
 	uint32_t real_tgid = (uint32_t)pid_tgid;
diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf.c b/tools/testing/selftests/bpf/progs/test_ringbuf.c
index eaa7d9dba0be..45b28965f3a5 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf.c
@@ -1,16 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2020 Facebook
 
-#include <linux/bpf.h>
+#include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
 
 char _license[] SEC("license") = "GPL";
 
-struct sample {
+struct sample_ringbuf {
 	int pid;
 	int seq;
 	long value;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 struct {
@@ -39,7 +39,7 @@ SEC("fentry/__x64_sys_getpgid")
 int test_ringbuf(void *ctx)
 {
 	int cur_pid = bpf_get_current_pid_tgid() >> 32;
-	struct sample *sample;
+	struct sample_ringbuf *sample;
 	int zero = 0;
 
 	if (cur_pid != pid)
diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
index 197b86546dca..7c9db148f168 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
@@ -1,16 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2020 Facebook
 
-#include <linux/bpf.h>
+#include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
 
 char _license[] SEC("license") = "GPL";
 
-struct sample {
+struct sample_ringbuf {
 	int pid;
 	int seq;
 	long value;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 struct ringbuf_map {
@@ -55,7 +55,7 @@ SEC("tp/syscalls/sys_enter_getpgid")
 int test_ringbuf(void *ctx)
 {
 	int cur_pid = bpf_get_current_pid_tgid() >> 32;
-	struct sample *sample;
+	struct sample_ringbuf *sample;
 	void *rb;
 	int zero = 0;
 
diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
index 6dc1f28fc4b6..cc41a09e3130 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
@@ -9,7 +9,7 @@
 struct sk_stg {
 	__u32 pid;
 	__u32 last_notclose_state;
-	char comm[16];
+	char comm[TASK_COMM_LEN];
 };
 
 struct {
@@ -27,7 +27,7 @@ struct {
 	__type(value, int);
 } del_sk_stg_map SEC(".maps");
 
-char task_comm[16] = "";
+char task_comm[TASK_COMM_LEN] = "";
 
 SEC("tp_btf/inet_sock_set_state")
 int BPF_PROG(trace_inet_sock_set_state, struct sock *sk, int oldstate,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 5/5] bpf/progs: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 ` [PATCH -mm 5/5] bpf/progs: " Yafang Shao
@ 2021-12-04 16:44   ` Alexei Starovoitov
  2021-12-05  2:44     ` Yafang Shao
  0 siblings, 1 reply; 15+ messages in thread
From: Alexei Starovoitov @ 2021-12-04 16:44 UTC (permalink / raw)
  To: Yafang Shao
  Cc: Andrew Morton, Steven Rostedt, Kees Cook, Petr Mladek, david,
	Arnaldo Carvalho de Melo, Andrii Nakryiko, linux-mm, bpf,
	linux-perf-users, Linux-Fsdevel, LKML, Mathieu Desnoyers,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat, Dec 4, 2021 at 1:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
>  static int process_sample(void *ctx, void *data, size_t len)
>  {
> -       struct sample *s = data;
> +       struct sample_ringbuf *s = data;

This is becoming pointless churn.
Nack.

> index 145028b52ad8..7b1bb73c3501 100644
> --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> @@ -1,8 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2019 Facebook
>
> -#include <linux/bpf.h>
> -#include <stdint.h>
> +#include <vmlinux.h>
>  #include <stdbool.h>
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_core_read.h>
> @@ -23,11 +22,11 @@ struct core_reloc_kernel_output {
>         int comm_len;
>  };
>
> -struct task_struct {
> +struct task_struct_reloc {

Churn that is not even compile tested.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 ` [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN Yafang Shao
@ 2021-12-04 16:45   ` Alexei Starovoitov
  0 siblings, 0 replies; 15+ messages in thread
From: Alexei Starovoitov @ 2021-12-04 16:45 UTC (permalink / raw)
  To: Yafang Shao
  Cc: Andrew Morton, Steven Rostedt, Kees Cook, Petr Mladek, david,
	Arnaldo Carvalho de Melo, Andrii Nakryiko, linux-mm, bpf,
	linux-perf-users, Linux-Fsdevel, LKML, Mathieu Desnoyers,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat, Dec 4, 2021 at 1:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
> index 1626d51dfffd..b728a946d83d 100644
> --- a/samples/bpf/tracex2_user.c
> +++ b/samples/bpf/tracex2_user.c
> @@ -12,6 +12,7 @@
>
>  #define MAX_INDEX      64
>  #define MAX_STARS      38
> +#define TASK_COMM_LEN  16
>
>  /* my_map, my_hist_map */
>  static int map_fd[2];
> @@ -28,7 +29,7 @@ static void stars(char *str, long val, long max, int width)
>  }
>
>  struct task {
> -       char comm[16];
> +       char comm[TASK_COMM_LEN];

Also Nack.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 5/5] bpf/progs: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04 16:44   ` Alexei Starovoitov
@ 2021-12-05  2:44     ` Yafang Shao
  2021-12-05  3:13       ` Alexei Starovoitov
  0 siblings, 1 reply; 15+ messages in thread
From: Yafang Shao @ 2021-12-05  2:44 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrew Morton, Steven Rostedt, Kees Cook, Petr Mladek,
	David Hildenbrand, Arnaldo Carvalho de Melo, Andrii Nakryiko,
	linux-mm, bpf, linux-perf-use.,
	Linux-Fsdevel, LKML, Mathieu Desnoyers, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sun, Dec 5, 2021 at 12:44 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Dec 4, 2021 at 1:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> >  static int process_sample(void *ctx, void *data, size_t len)
> >  {
> > -       struct sample *s = data;
> > +       struct sample_ringbuf *s = data;
>
> This is becoming pointless churn.
> Nack.
>
> > index 145028b52ad8..7b1bb73c3501 100644
> > --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > @@ -1,8 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  // Copyright (c) 2019 Facebook
> >
> > -#include <linux/bpf.h>
> > -#include <stdint.h>
> > +#include <vmlinux.h>
> >  #include <stdbool.h>
> >  #include <bpf/bpf_helpers.h>
> >  #include <bpf/bpf_core_read.h>
> > @@ -23,11 +22,11 @@ struct core_reloc_kernel_output {
> >         int comm_len;
> >  };
> >
> > -struct task_struct {
> > +struct task_struct_reloc {
>
> Churn that is not even compile tested.

It is strange that I have successfully compiled it....
Below is the compile log,

$ cat make.log | grep test_core_reloc_kernel
  CLNG-BPF [test_maps] test_core_reloc_kernel.o
  GEN-SKEL [test_progs] test_core_reloc_kernel.skel.h
  CLNG-BPF [test_maps] test_core_reloc_kernel.o
  GEN-SKEL [test_progs-no_alu32] test_core_reloc_kernel.skel.h

Also there's no error in the compile log.

-- 
Thanks
Yafang

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 5/5] bpf/progs: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-05  2:44     ` Yafang Shao
@ 2021-12-05  3:13       ` Alexei Starovoitov
  2021-12-05  7:25         ` Yafang Shao
  0 siblings, 1 reply; 15+ messages in thread
From: Alexei Starovoitov @ 2021-12-05  3:13 UTC (permalink / raw)
  To: Yafang Shao
  Cc: Andrew Morton, Steven Rostedt, Kees Cook, Petr Mladek,
	David Hildenbrand, Arnaldo Carvalho de Melo, Andrii Nakryiko,
	linux-mm, bpf, linux-perf-use.,
	Linux-Fsdevel, LKML, Mathieu Desnoyers, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat, Dec 4, 2021 at 6:45 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Sun, Dec 5, 2021 at 12:44 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sat, Dec 4, 2021 at 1:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > >  static int process_sample(void *ctx, void *data, size_t len)
> > >  {
> > > -       struct sample *s = data;
> > > +       struct sample_ringbuf *s = data;
> >
> > This is becoming pointless churn.
> > Nack.
> >
> > > index 145028b52ad8..7b1bb73c3501 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > > @@ -1,8 +1,7 @@
> > >  // SPDX-License-Identifier: GPL-2.0
> > >  // Copyright (c) 2019 Facebook
> > >
> > > -#include <linux/bpf.h>
> > > -#include <stdint.h>
> > > +#include <vmlinux.h>
> > >  #include <stdbool.h>
> > >  #include <bpf/bpf_helpers.h>
> > >  #include <bpf/bpf_core_read.h>
> > > @@ -23,11 +22,11 @@ struct core_reloc_kernel_output {
> > >         int comm_len;
> > >  };
> > >
> > > -struct task_struct {
> > > +struct task_struct_reloc {
> >
> > Churn that is not even compile tested.
>
> It is strange that I have successfully compiled it....
> Below is the compile log,
>
> $ cat make.log | grep test_core_reloc_kernel
>   CLNG-BPF [test_maps] test_core_reloc_kernel.o
>   GEN-SKEL [test_progs] test_core_reloc_kernel.skel.h
>   CLNG-BPF [test_maps] test_core_reloc_kernel.o
>   GEN-SKEL [test_progs-no_alu32] test_core_reloc_kernel.skel.h
>
> Also there's no error in the compile log.

and ran the tests too?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 5/5] bpf/progs: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-05  3:13       ` Alexei Starovoitov
@ 2021-12-05  7:25         ` Yafang Shao
  0 siblings, 0 replies; 15+ messages in thread
From: Yafang Shao @ 2021-12-05  7:25 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrew Morton, Steven Rostedt, Kees Cook, Petr Mladek,
	David Hildenbrand, Arnaldo Carvalho de Melo, Andrii Nakryiko,
	linux-mm, bpf, linux-perf-use.,
	Linux-Fsdevel, LKML, Mathieu Desnoyers, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sun, Dec 5, 2021 at 11:13 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Dec 4, 2021 at 6:45 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Sun, Dec 5, 2021 at 12:44 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Sat, Dec 4, 2021 at 1:53 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > >
> > > >  static int process_sample(void *ctx, void *data, size_t len)
> > > >  {
> > > > -       struct sample *s = data;
> > > > +       struct sample_ringbuf *s = data;
> > >
> > > This is becoming pointless churn.
> > > Nack.
> > >
> > > > index 145028b52ad8..7b1bb73c3501 100644
> > > > --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > > > +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > > > @@ -1,8 +1,7 @@
> > > >  // SPDX-License-Identifier: GPL-2.0
> > > >  // Copyright (c) 2019 Facebook
> > > >
> > > > -#include <linux/bpf.h>
> > > > -#include <stdint.h>
> > > > +#include <vmlinux.h>
> > > >  #include <stdbool.h>
> > > >  #include <bpf/bpf_helpers.h>
> > > >  #include <bpf/bpf_core_read.h>
> > > > @@ -23,11 +22,11 @@ struct core_reloc_kernel_output {
> > > >         int comm_len;
> > > >  };
> > > >
> > > > -struct task_struct {
> > > > +struct task_struct_reloc {
> > >
> > > Churn that is not even compile tested.
> >
> > It is strange that I have successfully compiled it....
> > Below is the compile log,
> >
> > $ cat make.log | grep test_core_reloc_kernel
> >   CLNG-BPF [test_maps] test_core_reloc_kernel.o
> >   GEN-SKEL [test_progs] test_core_reloc_kernel.skel.h
> >   CLNG-BPF [test_maps] test_core_reloc_kernel.o
> >   GEN-SKEL [test_progs-no_alu32] test_core_reloc_kernel.skel.h
> >
> > Also there's no error in the compile log.
>
> and ran the tests too?

My bad. I thought it was just a name change, which will work well if
it can be compiled successfully.
But it seems the task_struct in this file is a dummy struct, which
will be relocated to the real task_struct defined in the kernel.
We can't include vmlinux.h in this file, as it is for the relocation test.

-- 
Thanks
Yafang

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16
  2021-12-04  9:52 ` [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16 Yafang Shao
@ 2021-12-08 18:39   ` Steven Rostedt
  0 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2021-12-08 18:39 UTC (permalink / raw)
  To: Yafang Shao
  Cc: akpm, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko,
	linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Mathieu Desnoyers, Alexei Starovoitov, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat,  4 Dec 2021 09:52:52 +0000
Yafang Shao <laoar.shao@gmail.com> wrote:

> A new macro TASK_COMM_LEN_16 is introduced for the old hard-coded 16 to
> make it more grepable. As explained above this marco, the difference
> between TASK_COMM_LEN and TASK_COMM_LEN_16 is that TASK_COMM_LEN_16 must
> be a fixed size 16 and can't be changed.

You could add:

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 2/5] cn_proc: replaced old hard-coded 16 with TASK_COMM_LEN_16
  2021-12-04  9:52 ` [PATCH -mm 2/5] cn_proc: replaced old hard-coded " Yafang Shao
@ 2021-12-08 18:40   ` Steven Rostedt
  0 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2021-12-08 18:40 UTC (permalink / raw)
  To: Yafang Shao
  Cc: akpm, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko,
	linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Mathieu Desnoyers, Alexei Starovoitov, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat,  4 Dec 2021 09:52:53 +0000
Yafang Shao <laoar.shao@gmail.com> wrote:

> This TASK_COMM_LEN_16 has the same meaning with the macro defined in
> linux/sched.h, but we can't include linux/sched.h in a UAPI header, so
> we should specifically define it in the cn_proc.h.
> 

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> 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: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Petr Mladek <pmladek@suse.com>
> ---
>  include/uapi/linux/cn_proc.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
> index db210625cee8..6dcccaed383f 100644
> --- a/include/uapi/linux/cn_proc.h
> +++ b/include/uapi/linux/cn_proc.h
> @@ -21,6 +21,8 @@
>  
>  #include <linux/types.h>
>  
> +#define TASK_COMM_LEN_16 16
> +
>  /*
>   * Userspace sends this enum to register with the kernel that it is listening
>   * for events on the connector.
> @@ -110,7 +112,7 @@ struct proc_event {
>  		struct comm_proc_event {
>  			__kernel_pid_t process_pid;
>  			__kernel_pid_t process_tgid;
> -			char           comm[16];
> +			char           comm[TASK_COMM_LEN_16];
>  		} comm;
>  
>  		struct coredump_proc_event {


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 4/5] tools/perf: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-04  9:52 ` [PATCH -mm 4/5] tools/perf: " Yafang Shao
@ 2021-12-08 18:43   ` Steven Rostedt
  2021-12-09  2:42     ` Yafang Shao
  0 siblings, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2021-12-08 18:43 UTC (permalink / raw)
  To: Yafang Shao
  Cc: akpm, keescook, pmladek, david, arnaldo.melo, andrii.nakryiko,
	linux-mm, bpf, linux-perf-users, linux-fsdevel, linux-kernel,
	Mathieu Desnoyers, Alexei Starovoitov, Michal Miroslaw,
	Peter Zijlstra, Matthew Wilcox, Al Viro

On Sat,  4 Dec 2021 09:52:55 +0000
Yafang Shao <laoar.shao@gmail.com> wrote:

> @@ -43,7 +45,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
>  		return -1;
>  	}
>  
> -	if (evsel__test_field(evsel, "prev_comm", 16, false))
> +	if (evsel__test_field(evsel, "prev_comm", TASK_COMM_LEN, false))
>  		ret = -1;
>  
>  	if (evsel__test_field(evsel, "prev_pid", 4, true))
> @@ -55,7 +57,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
>  	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(evsel, "next_comm", TASK_COMM_LEN, false))
>  		ret = -1;
>  
>  	if (evsel__test_field(evsel, "next_pid", 4, true))
> @@ -73,7 +75,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
>  		return -1;
>  	}
>  
> -	if (evsel__test_field(evsel, "comm", 16, false))
> +	if (evsel__test_field(evsel, "comm", TASK_COMM_LEN, false))

Shouldn't all these be TASK_COMM_LEN_16?

-- Steve

>  		ret = -1;
>  
>  	if (evsel__test_field(evsel, "pid", 4, true))


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -mm 4/5] tools/perf: replace hard-coded 16 with TASK_COMM_LEN
  2021-12-08 18:43   ` Steven Rostedt
@ 2021-12-09  2:42     ` Yafang Shao
  0 siblings, 0 replies; 15+ messages in thread
From: Yafang Shao @ 2021-12-09  2:42 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrew Morton, Kees Cook, Petr Mladek, David Hildenbrand,
	Arnaldo Carvalho de Melo, Andrii Nakryiko, Linux MM, bpf,
	linux-perf-use.,
	Linux-Fsdevel, LKML, Mathieu Desnoyers, Alexei Starovoitov,
	Michal Miroslaw, Peter Zijlstra, Matthew Wilcox, Al Viro

On Thu, Dec 9, 2021 at 2:43 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sat,  4 Dec 2021 09:52:55 +0000
> Yafang Shao <laoar.shao@gmail.com> wrote:
>
> > @@ -43,7 +45,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
> >               return -1;
> >       }
> >
> > -     if (evsel__test_field(evsel, "prev_comm", 16, false))
> > +     if (evsel__test_field(evsel, "prev_comm", TASK_COMM_LEN, false))
> >               ret = -1;
> >
> >       if (evsel__test_field(evsel, "prev_pid", 4, true))
> > @@ -55,7 +57,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
> >       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(evsel, "next_comm", TASK_COMM_LEN, false))
> >               ret = -1;
> >
> >       if (evsel__test_field(evsel, "next_pid", 4, true))
> > @@ -73,7 +75,7 @@ static int test__perf_evsel__tp_sched_test(struct test_suite *test __maybe_unuse
> >               return -1;
> >       }
> >
> > -     if (evsel__test_field(evsel, "comm", 16, false))
> > +     if (evsel__test_field(evsel, "comm", TASK_COMM_LEN, false))
>
> Shouldn't all these be TASK_COMM_LEN_16?
>

The value here must be the same with TASK_COMM_LEN, so I use TASK_COMM_LEN here.
But we may also change the code as
https://lore.kernel.org/lkml/20211101060419.4682-9-laoar.shao@gmail.com/
if TASK_COMM_LEN is changed, so TASK_COMM_LEN_16 is also okay here.
I will change it to TASK_COMM_LEN_16 in the next version.

>
> >               ret = -1;
> >
> >       if (evsel__test_field(evsel, "pid", 4, true))
>


-- 
Thanks
Yafang

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-12-09  2:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-04  9:52 [PATCH -mm 0/5] Phase 2 of task comm cleanups Yafang Shao
2021-12-04  9:52 ` [PATCH -mm 1/5] elfcore: replace old hard-code 16 with TASK_COMM_LEN_16 Yafang Shao
2021-12-08 18:39   ` Steven Rostedt
2021-12-04  9:52 ` [PATCH -mm 2/5] cn_proc: replaced old hard-coded " Yafang Shao
2021-12-08 18:40   ` Steven Rostedt
2021-12-04  9:52 ` [PATCH -mm 3/5] samples/bpf/tracex2: replace hard-coded 16 with TASK_COMM_LEN Yafang Shao
2021-12-04 16:45   ` Alexei Starovoitov
2021-12-04  9:52 ` [PATCH -mm 4/5] tools/perf: " Yafang Shao
2021-12-08 18:43   ` Steven Rostedt
2021-12-09  2:42     ` Yafang Shao
2021-12-04  9:52 ` [PATCH -mm 5/5] bpf/progs: " Yafang Shao
2021-12-04 16:44   ` Alexei Starovoitov
2021-12-05  2:44     ` Yafang Shao
2021-12-05  3:13       ` Alexei Starovoitov
2021-12-05  7:25         ` Yafang Shao

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).