Re: [PATCH 2/6] tracing: Replace memcpy() with __get_task_comm()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-06-03 21:19:00
Also in:
bpf, linux-fsdevel, linux-mm, linux-security-module, selinux
On Sun, 2 Jun 2024 10:37:50 +0800 Yafang Shao [off-list ref] wrote:
Using __get_task_comm() to read the task comm ensures that the name is always NUL-terminated, regardless of the source string. This approach also facilitates future extensions to the task comm. Signed-off-by: Yafang Shao <redacted> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> --- include/linux/tracepoint.h | 4 ++-- include/trace/events/block.h | 10 +++++----- include/trace/events/oom.h | 2 +- include/trace/events/osnoise.h | 2 +- include/trace/events/sched.h | 27 ++++++++++++++------------- include/trace/events/signal.h | 2 +- include/trace/events/task.h | 4 ++-- 7 files changed, 26 insertions(+), 25 deletions(-)
[..]
quoted hunk ↗ jump to hunk
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h index 68973f650c26..2a9d7c62c58a 100644 --- a/include/trace/events/sched.h +++ b/include/trace/events/sched.h@@ -9,6 +9,7 @@ #include <linux/sched/numa_balancing.h> #include <linux/tracepoint.h> #include <linux/binfmts.h> +#include <linux/sched.h> /* * Tracepoint for calling kthread_stop, performed to end a kthread:@@ -25,7 +26,7 @@ TRACE_EVENT(sched_kthread_stop, ), TP_fast_assign( - memcpy(__entry->comm, t->comm, TASK_COMM_LEN); + __get_task_comm(__entry->comm, TASK_COMM_LEN, t); __entry->pid = t->pid; ),@@ -152,7 +153,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_template, ), TP_fast_assign( - memcpy(__entry->comm, p->comm, TASK_COMM_LEN); + __get_task_comm(__entry->comm, TASK_COMM_LEN, p); __entry->pid = p->pid; __entry->prio = p->prio; /* XXX SCHED_DEADLINE */ __entry->target_cpu = task_cpu(p);
quoted hunk ↗ jump to hunk
@@ -239,11 +240,11 @@ TRACE_EVENT(sched_switch, ), TP_fast_assign( - memcpy(__entry->next_comm, next->comm, TASK_COMM_LEN); + __get_task_comm(__entry->next_comm, TASK_COMM_LEN, next); __entry->prev_pid = prev->pid; __entry->prev_prio = prev->prio; __entry->prev_state = __trace_sched_switch_state(preempt, prev_state, prev); - memcpy(__entry->prev_comm, prev->comm, TASK_COMM_LEN); + __get_task_comm(__entry->prev_comm, TASK_COMM_LEN, prev); __entry->next_pid = next->pid; __entry->next_prio = next->prio; /* XXX SCHED_DEADLINE */
sched_switch is special so we could probably hold off, but the rest should
be converted to the normal way strings are processed in trace events. That
is, to use __string(), __assign_str() and __get_str() for task->comm. I've
been wanting to do that for a while, but thought that memcpy() was a bit
faster than the need for strlen(). But this now needs to test the length of
comm. This method will also allow comms to be recorded that are larger than
16 bytes (if we extend comm).
TRACE_EVENT(sched_migrate_task,
TP_PROTO(struct task_struct *p, int dest_cpu),
TP_ARGS(p, dest_cpu),
TP_STRUCT__entry(
- __array( char, comm, TASK_COMM_LEN )
+ __string( comm, strlen(comm) )
__field( pid_t, pid )
__field( int, prio )
__field( int, orig_cpu )
__field( int, dest_cpu )
),
TP_fast_assign(
- memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
+ __assign_str(comm);
__entry->pid = p->pid;
__entry->prio = p->prio; /* XXX SCHED_DEADLINE */
__entry->orig_cpu = task_cpu(p);
__entry->dest_cpu = dest_cpu;
),
TP_printk("comm=%s pid=%d prio=%d orig_cpu=%d dest_cpu=%d",
- __entry->comm, __entry->pid, __entry->prio,
+ __get_str(comm), __entry->pid, __entry->prio,
__entry->orig_cpu, __entry->dest_cpu)
);
-- Steve
quoted hunk ↗ jump to hunk
@@ -286,7 +287,7 @@ TRACE_EVENT(sched_migrate_task, ), TP_fast_assign( - memcpy(__entry->comm, p->comm, TASK_COMM_LEN); + __get_task_comm(__entry->comm, TASK_COMM_LEN, p); __entry->pid = p->pid; __entry->prio = p->prio; /* XXX SCHED_DEADLINE */ __entry->orig_cpu = task_cpu(p);