2021-10-21 03:46:49

by Yafang Shao

[permalink] [raw]
Subject: [PATCH v5 03/15] sched.h: introduce TASK_COMM_LEN_16

There're many hard-coded 16 used to store task comm in the kernel, that
makes it error prone if we want to change the value of TASK_COMM_LEN. A
new marco TASK_COMM_LEN_16 is introduced to replace these old ones, then
we can easily grep them.

Signed-off-by: Yafang Shao <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Petr Mladek <[email protected]>
---
include/linux/sched.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index c1a927ddec64..62d5b30d310c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -274,6 +274,8 @@ struct task_group;

#define get_current_state() READ_ONCE(current->__state)

+/* To replace the old hard-coded 16 */
+#define TASK_COMM_LEN_16 16
/* Task command name length: */
#define TASK_COMM_LEN 16

--
2.17.1


2021-10-21 21:57:55

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v5 03/15] sched.h: introduce TASK_COMM_LEN_16

On Wed, Oct 20, 2021 at 8:45 PM Yafang Shao <[email protected]> wrote:
>
> There're many hard-coded 16 used to store task comm in the kernel, that
> makes it error prone if we want to change the value of TASK_COMM_LEN. A
> new marco TASK_COMM_LEN_16 is introduced to replace these old ones, then
> we can easily grep them.
>
> Signed-off-by: Yafang Shao <[email protected]>
> Cc: Mathieu Desnoyers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Petr Mladek <[email protected]>
> ---
> include/linux/sched.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index c1a927ddec64..62d5b30d310c 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -274,6 +274,8 @@ struct task_group;
>
> #define get_current_state() READ_ONCE(current->__state)
>
> +/* To replace the old hard-coded 16 */
> +#define TASK_COMM_LEN_16 16
> /* Task command name length: */
> #define TASK_COMM_LEN 16

Can we please convert these two constants into enum? That will allow
BPF applications to deal with such kernel change more easily because
these constants will now be available as part of kernel BTF.

Something like this should be completely equivalent for all the kernel uses:

enum {
TASK_COMM_LEN = 16,
TASK_COMM_LEN_16 = 16,
};

When later TASK_COMM_LEN is defined as = 24, BPF applications will be
able to deal with that by querying BTF through BPF CO-RE.

>
> --
> 2.17.1
>

2021-10-22 06:26:28

by Yafang Shao

[permalink] [raw]
Subject: Re: [PATCH v5 03/15] sched.h: introduce TASK_COMM_LEN_16

On Fri, Oct 22, 2021 at 5:55 AM Andrii Nakryiko
<[email protected]> wrote:
>
> On Wed, Oct 20, 2021 at 8:45 PM Yafang Shao <[email protected]> wrote:
> >
> > There're many hard-coded 16 used to store task comm in the kernel, that
> > makes it error prone if we want to change the value of TASK_COMM_LEN. A
> > new marco TASK_COMM_LEN_16 is introduced to replace these old ones, then
> > we can easily grep them.
> >
> > Signed-off-by: Yafang Shao <[email protected]>
> > Cc: Mathieu Desnoyers <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Steven Rostedt <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Petr Mladek <[email protected]>
> > ---
> > include/linux/sched.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index c1a927ddec64..62d5b30d310c 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -274,6 +274,8 @@ struct task_group;
> >
> > #define get_current_state() READ_ONCE(current->__state)
> >
> > +/* To replace the old hard-coded 16 */
> > +#define TASK_COMM_LEN_16 16
> > /* Task command name length: */
> > #define TASK_COMM_LEN 16
>
> Can we please convert these two constants into enum? That will allow
> BPF applications to deal with such kernel change more easily because
> these constants will now be available as part of kernel BTF.
>
> Something like this should be completely equivalent for all the kernel uses:
>
> enum {
> TASK_COMM_LEN = 16,
> TASK_COMM_LEN_16 = 16,
> };
>
> When later TASK_COMM_LEN is defined as = 24, BPF applications will be
> able to deal with that by querying BTF through BPF CO-RE.
>

Sure. I will convert it to enum.


--
Thanks
Yafang