From: David Laight <david.laight.linux@gmail.com>
To: "André Almeida" <andrealmeid@igalia.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Steven Rostedt <rostedt@goodmis.org>,
Christian Brauner <brauner@kernel.org>,
Kees Cook <kees@kernel.org>, Shuah Khan <shuah@kernel.org>,
willy@infradead.org, mathieu.desnoyers@efficios.com,
Linus Torvalds <torvalds@linux-foundation.org>,
akpm@linux-foundation.org, Yafang Shao <laoar.shao@gmail.com>,
andrii.nakryiko@gmail.com, arnaldo.melo@gmail.com,
Petr Mladek <pmladek@suse.com>,
linux-kernel@vger.kernel.org, kernel-dev@igalia.com,
linux-mm@kvack.org, linux-api@vger.kernel.org,
linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v8 2/6] treewide: Replace memcpy(..., current->comm) with copy_task_comm()
Date: Fri, 30 Oct 2026 22:34:35 +0000 [thread overview]
Message-ID: <20261030223435.0e802eb7@pumpkin> (raw)
In-Reply-To: <20261030-tonyk-long_name-v8-2-c9b030332254@igalia.com>
On Fri, 30 Oct 2026 18:38:33 -0300
André Almeida <andrealmeid@igalia.com> wrote:
> In order to increase the size of current->comm[] and to avoid breaking any
> existing code, replace memcpy() with copy_task_comm(). This new function
> makes sure that the copy is NUL terminated. This is crucial given that the
> source buffer might be larger than the destination buffer and could
> truncate the NUL character out of it.
I suspect the trace code would be faster it is just copied the (first)
16 characters and worried about whether it was terminated later on.
It does far more 'saves' than trace reads and their performance is
much more important.
David
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> Changes from v4:
> - Remove copy_task_comm() calls for structs other than struct task_struct
> - Add checks for len > TASK_COMM_LEN and len < 0
>
> Changes from v3:
> - Simplify copy_task_comm() to a memcpy + NUL char at the end of buffer
>
> Changes from v2:
> - Bring back custom function.
>
> Changes from v1:
> - New patch, dropped strtostr() from last version
> ---
> include/linux/coredump.h | 2 +-
> include/linux/sched.h | 15 +++++++++++++++
> include/linux/tracepoint.h | 4 ++--
> include/trace/events/block.h | 10 +++++-----
> include/trace/events/coredump.h | 2 +-
> include/trace/events/f2fs.h | 4 ++--
> include/trace/events/oom.h | 2 +-
> include/trace/events/osnoise.h | 2 +-
> include/trace/events/sched.h | 10 +++++-----
> include/trace/events/signal.h | 2 +-
> include/trace/events/task.h | 4 ++--
> .../testing/selftests/bpf/test_kmods/bpf_testmod-events.h | 2 +-
> 12 files changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index 8e857770dd21..c3009edc2595 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -79,7 +79,7 @@ void vfs_coredump(const kernel_siginfo_t *siginfo);
> do { \
> char comm[TASK_COMM_LEN]; \
> /* This will always be NUL terminated. */ \
> - memcpy(comm, current->comm, sizeof(comm)); \
> + copy_task_comm(comm, current, sizeof(comm)); \
> printk_ratelimited(Level "coredump: %d(%*pE): " Format "\n", \
> task_tgid_vnr(current), (int)strlen(comm), comm, ##__VA_ARGS__); \
> } while (0) \
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 0564aabb1393..db529a93525d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2042,6 +2042,21 @@ extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec
> __set_task_comm(tsk, from, false); \
> })
>
> +/*
> + * Copy task name to a buffer. Final result is always a NUL-terminated string.
> + */
> +static __always_inline void copy_task_comm(char *dst, struct task_struct *tsk, size_t len)
> +{
> + const char *_src = tsk->comm;
> + size_t _len = min(len, TASK_COMM_LEN);
> +
> + if (!_len)
> + return;
> +
> + memcpy(dst, _src, _len);
> + dst[_len - 1] = '\0';
> +}
> +
> static __always_inline void scheduler_ipi(void)
> {
> /*
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index e0d838c9ce93..9b0094fa44c6 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -637,10 +637,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> * *
> *
> * TP_fast_assign(
> - * memcpy(__entry->next_comm, next->comm, TASK_COMM_LEN);
> + * copy_task_comm(__entry->next_comm, next, TASK_COMM_LEN);
> * __entry->prev_pid = prev->pid;
> * __entry->prev_prio = prev->prio;
> - * memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN);
> + * copy_task_comm(__entry->prev_comm, prev, TASK_COMM_LEN);
> * __entry->next_pid = next->pid;
> * __entry->next_prio = next->prio;
> * ),
> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
> index 9c97a16850b9..1e3a9ded03bd 100644
> --- a/include/trace/events/block.h
> +++ b/include/trace/events/block.h
> @@ -213,7 +213,7 @@ DECLARE_EVENT_CLASS(block_rq,
>
> blk_fill_rwbs(__entry->rwbs, rq->cmd_flags);
> __get_str(cmd)[0] = '\0';
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("%d,%d %s %u (%s) %llu + %u %s,%u,%u [%s]",
> @@ -410,7 +410,7 @@ DECLARE_EVENT_CLASS(block_bio,
> __entry->sector = bio->bi_iter.bi_sector;
> __entry->nr_sector = bio_sectors(bio);
> blk_fill_rwbs(__entry->rwbs, bio->bi_opf);
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("%d,%d %s %llu + %u [%s]",
> @@ -493,7 +493,7 @@ TRACE_EVENT(block_plug,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("[%s]", __entry->comm)
> @@ -512,7 +512,7 @@ DECLARE_EVENT_CLASS(block_unplug,
>
> TP_fast_assign(
> __entry->nr_rq = depth;
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("[%s] %d", __entry->comm, __entry->nr_rq)
> @@ -563,7 +563,7 @@ TRACE_EVENT(block_split,
> __entry->sector = bio->bi_iter.bi_sector;
> __entry->new_sector = new_sector;
> blk_fill_rwbs(__entry->rwbs, bio->bi_opf);
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("%d,%d %s %llu / %llu [%s]",
> diff --git a/include/trace/events/coredump.h b/include/trace/events/coredump.h
> index c7b9c53fc498..fdd20bc46bb0 100644
> --- a/include/trace/events/coredump.h
> +++ b/include/trace/events/coredump.h
> @@ -32,7 +32,7 @@ TRACE_EVENT(coredump,
>
> TP_fast_assign(
> __entry->sig = sig;
> - memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, current, TASK_COMM_LEN);
> ),
>
> TP_printk("sig=%d comm=%s",
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index df4cd346ae72..e3f2047f9de8 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -2589,7 +2589,7 @@ TRACE_EVENT(f2fs_lock_elapsed_time,
>
> TP_fast_assign(
> __entry->dev = sbi->sb->s_dev;
> - memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, p, TASK_COMM_LEN);
> __entry->pid = p->pid;
> __entry->prio = p->prio;
> __entry->ioprio_class = IOPRIO_PRIO_CLASS(ioprio);
> @@ -2642,7 +2642,7 @@ DECLARE_EVENT_CLASS(f2fs_priority_update,
>
> TP_fast_assign(
> __entry->dev = sbi->sb->s_dev;
> - memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, p, TASK_COMM_LEN);
> __entry->pid = p->pid;
> __entry->lock_name = lock_name;
> __entry->is_write = is_write;
> diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h
> index 9f0a5d1482c4..8bcdc4ffc8d3 100644
> --- a/include/trace/events/oom.h
> +++ b/include/trace/events/oom.h
> @@ -23,7 +23,7 @@ TRACE_EVENT(oom_score_adj_update,
>
> TP_fast_assign(
> __entry->pid = task->pid;
> - memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, task, TASK_COMM_LEN);
> __entry->oom_score_adj = task->signal->oom_score_adj;
> ),
>
> diff --git a/include/trace/events/osnoise.h b/include/trace/events/osnoise.h
> index 3f4273623801..2cf047bb9fb7 100644
> --- a/include/trace/events/osnoise.h
> +++ b/include/trace/events/osnoise.h
> @@ -116,7 +116,7 @@ TRACE_EVENT(thread_noise,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->comm, t->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, t, TASK_COMM_LEN);
> __entry->pid = t->pid;
> __entry->start = start;
> __entry->duration = duration;
> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> index 535860581f15..afb24e9dac91 100644
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -152,7 +152,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, p, TASK_COMM_LEN);
> __entry->pid = p->pid;
> __entry->prio = p->prio; /* XXX SCHED_DEADLINE */
> __entry->target_cpu = task_cpu(p);
> @@ -237,11 +237,11 @@ TRACE_EVENT(sched_switch,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->prev_comm, prev, TASK_COMM_LEN);
> __entry->prev_pid = prev->pid;
> __entry->prev_prio = prev->prio;
> __entry->prev_state = __trace_sched_switch_state(preempt, prev_state, prev);
> - memcpy(__entry->next_comm, next->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->next_comm, next, TASK_COMM_LEN);
> __entry->next_pid = next->pid;
> __entry->next_prio = next->prio;
> /* XXX SCHED_DEADLINE */
> @@ -346,7 +346,7 @@ TRACE_EVENT(sched_process_exit,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, p, TASK_COMM_LEN);
> __entry->pid = p->pid;
> __entry->prio = p->prio; /* XXX SCHED_DEADLINE */
> __entry->group_dead = group_dead;
> @@ -787,7 +787,7 @@ TRACE_EVENT(sched_skip_cpuset_numa,
> ),
>
> TP_fast_assign(
> - memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, tsk, TASK_COMM_LEN);
> __entry->pid = task_pid_nr(tsk);
> __entry->tgid = task_tgid_nr(tsk);
> __entry->ngid = task_numa_group_id(tsk);
> diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
> index 1db7e4b07c01..8fffe6d9bdcc 100644
> --- a/include/trace/events/signal.h
> +++ b/include/trace/events/signal.h
> @@ -67,7 +67,7 @@ TRACE_EVENT(signal_generate,
> TP_fast_assign(
> __entry->sig = sig;
> TP_STORE_SIGINFO(__entry, info);
> - memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, task, TASK_COMM_LEN);
> __entry->pid = task->pid;
> __entry->group = group;
> __entry->result = result;
> diff --git a/include/trace/events/task.h b/include/trace/events/task.h
> index b9a129eb54d9..e3faaeb997ef 100644
> --- a/include/trace/events/task.h
> +++ b/include/trace/events/task.h
> @@ -21,7 +21,7 @@ TRACE_EVENT(task_newtask,
>
> TP_fast_assign(
> __entry->pid = task->pid;
> - memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, task, TASK_COMM_LEN);
> __entry->clone_flags = clone_flags;
> __entry->oom_score_adj = task->signal->oom_score_adj;
> ),
> @@ -46,7 +46,7 @@ TRACE_EVENT(task_rename,
>
> TP_fast_assign(
> __entry->pid = task->pid;
> - memcpy(entry->oldcomm, task->comm, TASK_COMM_LEN);
> + copy_task_comm(entry->oldcomm, task, TASK_COMM_LEN);
> strscpy(entry->newcomm, comm, TASK_COMM_LEN);
> __entry->oom_score_adj = task->signal->oom_score_adj;
> ),
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
> index 45a5e41f3a92..a197622e113c 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
> @@ -20,7 +20,7 @@ TRACE_EVENT(bpf_testmod_test_read,
> ),
> TP_fast_assign(
> __entry->pid = task->pid;
> - memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> + copy_task_comm(__entry->comm, task, TASK_COMM_LEN);
> __entry->off = ctx->off;
> __entry->len = ctx->len;
> ),
>
next prev parent reply other threads:[~2026-10-30 22:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 21:38 [PATCH v8 0/6] sched: Add support for long task name André Almeida
2026-10-30 21:38 ` [PATCH v8 1/6] treewide: Get rid of get_task_comm() André Almeida
2026-10-30 21:38 ` [PATCH v8 2/6] treewide: Replace memcpy(..., current->comm) with copy_task_comm() André Almeida
2026-10-30 22:34 ` David Laight [this message]
2026-10-30 21:38 ` [PATCH v8 3/6] lib/string_kunit: Add test for copy_task_comm() André Almeida
2026-10-30 21:38 ` [PATCH v8 4/6] sched: Extend task command name with TASK_COMM_EXT_LEN André Almeida
2026-10-30 21:38 ` [PATCH v8 5/6] prctl: Add support for long user thread names André Almeida
2026-10-30 21:38 ` [PATCH v8 6/6] selftests: prctl: Add test for long " André Almeida
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=20261030223435.0e802eb7@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andrealmeid@igalia.com \
--cc=andrii.nakryiko@gmail.com \
--cc=arnaldo.melo@gmail.com \
--cc=brauner@kernel.org \
--cc=juri.lelli@redhat.com \
--cc=kees@kernel.org \
--cc=kernel-dev@igalia.com \
--cc=laoar.shao@gmail.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=shuah@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vincent.guittot@linaro.org \
--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