From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Bhupesh <bhupesh@igalia.com>
Cc: akpm@linux-foundation.org, kernel-dev@igalia.com,
linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-perf-users@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-mm@kvack.org, oliver.sang@intel.com, lkp@intel.com,
laoar.shao@gmail.com, pmladek@suse.com, rostedt@goodmis.org,
mathieu.desnoyers@efficios.com, arnaldo.melo@gmail.com,
alexei.starovoitov@gmail.com, mirq-linux@rere.qmqm.pl,
peterz@infradead.org, willy@infradead.org, david@redhat.com,
viro@zeniv.linux.org.uk, keescook@chromium.org,
ebiederm@xmission.com, brauner@kernel.org, jack@suse.cz,
mingo@redhat.com, juri.lelli@redhat.com, bsegall@google.com,
mgorman@suse.de, vschneid@redhat.com,
linux-trace-kernel@vger.kernel.org, kees@kernel.org,
torvalds@linux-foundation.org
Subject: Re: [PATCH v5 3/3] treewide: Switch from tsk->comm to tsk->comm_str which is 64 bytes long
Date: Wed, 16 Jul 2025 11:10:33 -0700 [thread overview]
Message-ID: <CAEf4BzaGRz6A1wzBa2ZyQWY_4AvUHvLgBF36iCxc9wJJ1ppH0g@mail.gmail.com> (raw)
In-Reply-To: <20250716123916.511889-4-bhupesh@igalia.com>
On Wed, Jul 16, 2025 at 5:40 AM Bhupesh <bhupesh@igalia.com> wrote:
>
> Historically due to the 16-byte length of TASK_COMM_LEN, the
> users of 'tsk->comm' are restricted to use a fixed-size target
> buffer also of TASK_COMM_LEN for 'memcpy()' like use-cases.
>
> To fix the same, Kees suggested in [1] that we can replace tsk->comm,
> with tsk->comm_str, inside 'task_struct':
> union {
> char comm_str[TASK_COMM_EXT_LEN];
> };
>
> where TASK_COMM_EXT_LEN is 64-bytes.
Do we absolutely have to rename task->comm field? I understand as an
intermediate step to not miss any existing users in the kernel
sources, but once that all is done, we can rename that back to comm,
right?
The reason I'm asking is because accessing task->comm is *very common*
with all sorts of BPF programs and scripts. Yes, we have way to deal
with that with BPF CO-RE, but every single use case would need to be
updated now to work both with task->comm name on old kernels and
task->comm_str on new kernels (because BPF programs are written in
more or less kernel version agnostic way, so they have to handle many
kernel releases).
So, unless absolutely necessary, can we please keep the field name the
same? Changing the size of that field is not really a problem for BPF,
so no objections against that.
>
> And then modify 'get_task_comm()' to pass 'tsk->comm_str'
> to the existing users.
>
> This would mean that ABI is maintained while ensuring that:
>
> - Existing users of 'get_task_comm'/ 'set_task_comm' will get 'tsk->comm_str'
> truncated to a maximum of 'TASK_COMM_LEN' (16-bytes) to maintain ABI,
> - New / Modified users of 'get_task_comm'/ 'set_task_comm' will get
> 'tsk->comm_str' supported for a maximum of 'TASK_COMM_EXTLEN' (64-bytes).
>
> Note, that the existing users have not been modified to migrate to
> 'TASK_COMM_EXT_LEN', in case they have hard-coded expectations of
> dealing with only a 'TASK_COMM_LEN' long 'tsk->comm_str'.
>
> [1]. https://lore.kernel.org/all/202505231346.52F291C54@keescook/
>
> Signed-off-by: Bhupesh <bhupesh@igalia.com>
> ---
> arch/arm64/kernel/traps.c | 2 +-
> arch/arm64/kvm/mmu.c | 2 +-
> block/blk-core.c | 2 +-
> block/bsg.c | 2 +-
> drivers/char/random.c | 2 +-
> drivers/hid/hid-core.c | 6 +++---
> drivers/mmc/host/tmio_mmc_core.c | 6 +++---
> drivers/pci/pci-sysfs.c | 2 +-
> drivers/scsi/scsi_ioctl.c | 2 +-
> drivers/tty/serial/serial_core.c | 2 +-
> drivers/tty/tty_io.c | 8 ++++----
> drivers/usb/core/devio.c | 16 ++++++++--------
> drivers/usb/core/message.c | 2 +-
> drivers/vfio/group.c | 2 +-
> drivers/vfio/vfio_iommu_type1.c | 2 +-
> drivers/vfio/vfio_main.c | 2 +-
> drivers/xen/evtchn.c | 2 +-
> drivers/xen/grant-table.c | 2 +-
> fs/binfmt_elf.c | 2 +-
> fs/coredump.c | 4 ++--
> fs/drop_caches.c | 2 +-
> fs/exec.c | 8 ++++----
> fs/ext4/dir.c | 2 +-
> fs/ext4/inode.c | 2 +-
> fs/ext4/namei.c | 2 +-
> fs/ext4/super.c | 12 ++++++------
> fs/hugetlbfs/inode.c | 2 +-
> fs/ioctl.c | 2 +-
> fs/iomap/direct-io.c | 2 +-
> fs/jbd2/transaction.c | 2 +-
> fs/locks.c | 2 +-
> fs/netfs/internal.h | 2 +-
> fs/proc/base.c | 2 +-
> fs/read_write.c | 2 +-
> fs/splice.c | 2 +-
> include/linux/coredump.h | 2 +-
> include/linux/filter.h | 2 +-
> include/linux/ratelimit.h | 2 +-
> include/linux/sched.h | 11 ++++++++---
> init/init_task.c | 2 +-
> ipc/sem.c | 2 +-
> kernel/acct.c | 2 +-
> kernel/audit.c | 4 ++--
> kernel/auditsc.c | 10 +++++-----
> kernel/bpf/helpers.c | 2 +-
> kernel/capability.c | 4 ++--
> kernel/cgroup/cgroup-v1.c | 2 +-
> kernel/cred.c | 4 ++--
> kernel/events/core.c | 2 +-
> kernel/exit.c | 6 +++---
> kernel/fork.c | 9 +++++++--
> kernel/freezer.c | 4 ++--
> kernel/futex/waitwake.c | 2 +-
> kernel/hung_task.c | 10 +++++-----
> kernel/irq/manage.c | 2 +-
> kernel/kthread.c | 2 +-
> kernel/locking/rtmutex.c | 2 +-
> kernel/printk/printk.c | 2 +-
> kernel/sched/core.c | 22 +++++++++++-----------
> kernel/sched/debug.c | 4 ++--
> kernel/signal.c | 6 +++---
> kernel/sys.c | 6 +++---
> kernel/sysctl.c | 2 +-
> kernel/time/itimer.c | 4 ++--
> kernel/time/posix-cpu-timers.c | 2 +-
> kernel/tsacct.c | 2 +-
> kernel/workqueue.c | 6 +++---
> lib/dump_stack.c | 2 +-
> lib/nlattr.c | 6 +++---
> mm/compaction.c | 2 +-
> mm/filemap.c | 4 ++--
> mm/gup.c | 2 +-
> mm/memfd.c | 2 +-
> mm/memory-failure.c | 10 +++++-----
> mm/memory.c | 2 +-
> mm/mmap.c | 4 ++--
> mm/oom_kill.c | 18 +++++++++---------
> mm/page_alloc.c | 4 ++--
> mm/util.c | 2 +-
> net/core/sock.c | 2 +-
> net/dns_resolver/internal.h | 2 +-
> net/ipv4/raw.c | 2 +-
> net/ipv4/tcp.c | 2 +-
> net/socket.c | 2 +-
> security/lsm_audit.c | 4 ++--
> 85 files changed, 171 insertions(+), 161 deletions(-)
>
[...]
next prev parent reply other threads:[~2025-07-16 18:10 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-16 12:39 [PATCH v5 0/3] Add support for long task name Bhupesh
2025-07-16 12:39 ` [PATCH v5 1/3] exec: Remove obsolete comments Bhupesh
2025-07-16 12:39 ` [PATCH v5 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Bhupesh
2025-07-16 12:39 ` [PATCH v5 3/3] treewide: Switch from tsk->comm to tsk->comm_str which is 64 bytes long Bhupesh
2025-07-16 18:10 ` Andrii Nakryiko [this message]
2025-07-16 20:18 ` Bhupesh Sharma
2025-07-16 20:47 ` Andrii Nakryiko
2025-07-17 18:35 ` Linus Torvalds
2025-07-21 7:52 ` Bhupesh Sharma
2025-07-17 4:23 ` kernel test robot
2025-07-24 9:16 ` Askar Safin
2025-07-24 12:38 ` Bhupesh Sharma
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=CAEf4BzaGRz6A1wzBa2ZyQWY_4AvUHvLgBF36iCxc9wJJ1ppH0g@mail.gmail.com \
--to=andrii.nakryiko@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=alexei.starovoitov@gmail.com \
--cc=arnaldo.melo@gmail.com \
--cc=bhupesh@igalia.com \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=bsegall@google.com \
--cc=david@redhat.com \
--cc=ebiederm@xmission.com \
--cc=jack@suse.cz \
--cc=juri.lelli@redhat.com \
--cc=kees@kernel.org \
--cc=keescook@chromium.org \
--cc=kernel-dev@igalia.com \
--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-trace-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=mirq-linux@rere.qmqm.pl \
--cc=oliver.sang@intel.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=vschneid@redhat.com \
--cc=willy@infradead.org \
/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).