[PATCH 1/2] tracing: Embed 'char comm[16]' in a structure

[David Laight <david.laight.linux@gmail.com>]

Embedding the array in a stucture makes the size explicit and lets
structure copies be used.

Limit the size to 16 charatacters even if task_struct.comm is
made larger (there are plans to increase it).

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 kernel/trace/blktrace.c              | 28 +++++++++----------
 kernel/trace/trace.c                 |  3 +-
 kernel/trace/trace.h                 |  9 ++++--
 kernel/trace/trace_events_filter.c   |  2 +-
 kernel/trace/trace_events_hist.c     | 26 +++++++----------
 kernel/trace/trace_functions_graph.c | 10 +++----
 kernel/trace/trace_output.c          | 24 ++++++++--------
 kernel/trace/trace_sched_switch.c    | 42 ++++++++++++++++------------
 8 files changed, 75 insertions(+), 69 deletions(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 8cd2520b4c99..68ffc95548b7 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1590,20 +1590,20 @@ static void blk_log_dump_pdu(struct trace_seq *s,
 
 static void blk_log_generic(struct trace_seq *s, const struct trace_entry *ent, bool has_cg)
 {
-	char cmd[TASK_COMM_LEN];
+	struct trace_comm cmd;
 
-	trace_find_cmdline(ent->pid, cmd);
+	trace_find_cmdline(ent->pid, &cmd);
 
 	if (t_action(ent) & BLK_TC_ACT(BLK_TC_PC)) {
 		trace_seq_printf(s, "%u ", t_bytes(ent));
 		blk_log_dump_pdu(s, ent, has_cg);
-		trace_seq_printf(s, "[%s]\n", cmd);
+		trace_seq_printf(s, "[%s]\n", cmd.comm);
 	} else {
 		if (t_sec(ent))
 			trace_seq_printf(s, "%llu + %u [%s]\n",
-						t_sector(ent), t_sec(ent), cmd);
+						t_sector(ent), t_sec(ent), cmd.comm);
 		else
-			trace_seq_printf(s, "[%s]\n", cmd);
+			trace_seq_printf(s, "[%s]\n", cmd.comm);
 	}
 }
 
@@ -1637,30 +1637,30 @@ static void blk_log_remap(struct trace_seq *s, const struct trace_entry *ent, bo
 
 static void blk_log_plug(struct trace_seq *s, const struct trace_entry *ent, bool has_cg)
 {
-	char cmd[TASK_COMM_LEN];
+	struct trace_comm cmd;
 
-	trace_find_cmdline(ent->pid, cmd);
+	trace_find_cmdline(ent->pid, &cmd);
 
-	trace_seq_printf(s, "[%s]\n", cmd);
+	trace_seq_printf(s, "[%s]\n", cmd.comm);
 }
 
 static void blk_log_unplug(struct trace_seq *s, const struct trace_entry *ent, bool has_cg)
 {
-	char cmd[TASK_COMM_LEN];
+	struct trace_comm cmd;
 
-	trace_find_cmdline(ent->pid, cmd);
+	trace_find_cmdline(ent->pid, &cmd);
 
-	trace_seq_printf(s, "[%s] %llu\n", cmd, get_pdu_int(ent, has_cg));
+	trace_seq_printf(s, "[%s] %llu\n", cmd.comm, get_pdu_int(ent, has_cg));
 }
 
 static void blk_log_split(struct trace_seq *s, const struct trace_entry *ent, bool has_cg)
 {
-	char cmd[TASK_COMM_LEN];
+	struct trace_comm cmd;
 
-	trace_find_cmdline(ent->pid, cmd);
+	trace_find_cmdline(ent->pid, &cmd);
 
 	trace_seq_printf(s, "%llu / %llu [%s]\n", t_sector(ent),
-			 get_pdu_int(ent, has_cg), cmd);
+			 get_pdu_int(ent, has_cg), cmd.comm);
 }
 
 static void blk_log_msg(struct trace_seq *s, const struct trace_entry *ent,
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6eb4d3097a4d..7de658b8ee0d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -52,6 +52,7 @@
 #include <linux/sort.h>
 #include <linux/io.h> /* vmap_page_range() */
 #include <linux/fs_context.h>
+#include <linux/trace_printk.h>
 
 #include <asm/setup.h> /* COMMAND_LINE_SIZE */
 
@@ -2972,7 +2973,7 @@ print_trace_header(struct seq_file *m, struct trace_iterator *iter)
 	seq_puts(m, "#    -----------------\n");
 	seq_printf(m, "#    | task: %.16s-%d "
 		   "(uid:%d nice:%ld policy:%ld rt_prio:%ld)\n",
-		   data->comm, data->pid,
+		   data->comm.comm, data->pid,
 		   from_kuid_munged(seq_user_ns(m), data->uid), data->nice,
 		   data->policy, data->rt_priority);
 	seq_puts(m, "#    -----------------\n");
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..afd59d79e1fe 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -183,6 +183,11 @@ struct fexit_trace_entry_head {
 
 struct trace_array;
 
+/* task_struct->comm[] may be truncated to save memory/width */
+struct trace_comm {
+	char comm[16];
+};
+
 /*
  * The CPU trace array - it consists of thousands of trace entries
  * plus some other descriptor data: (for example which task started
@@ -203,7 +208,7 @@ struct trace_array_cpu {
 	u64			preempt_timestamp;
 	pid_t			pid;
 	kuid_t			uid;
-	char			comm[TASK_COMM_LEN];
+	struct trace_comm	comm;
 
 #ifdef CONFIG_FUNCTION_TRACER
 	int			ftrace_ignore_pid;
@@ -906,7 +911,7 @@ void trace_last_func_repeats(struct trace_array *tr,
 
 extern u64 ftrace_now(int cpu);
 
-extern void trace_find_cmdline(int pid, char comm[]);
+extern void trace_find_cmdline(int pid, struct trace_comm *comm);
 extern int trace_find_tgid(int pid);
 extern void trace_event_follow_fork(struct trace_array *tr, bool enable);
 
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 609325f57942..749887aff315 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -994,7 +994,7 @@ static int filter_pred_comm(struct filter_pred *pred, void *event)
 	int cmp;
 
 	cmp = pred->regex->match(current->comm, pred->regex,
-				TASK_COMM_LEN);
+				sizeof(current->comm));
 	return cmp ^ pred->not;
 }
 
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 0dbbf6cca9bc..1b51491b2a41 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -680,7 +680,7 @@ struct track_data {
 };
 
 struct hist_elt_data {
-	char *comm;
+	struct trace_comm *comm;
 	u64 *var_ref_vals;
 	char **field_var_str;
 	int n_field_var_str;
@@ -756,7 +756,7 @@ static struct track_data *track_data_alloc(unsigned int key_len,
 
 	data->elt.private_data = elt_data;
 
-	elt_data->comm = kzalloc(TASK_COMM_LEN, GFP_KERNEL);
+	elt_data->comm = kzalloc_obj(*elt_data->comm);
 	if (!elt_data->comm) {
 		track_data_free(data);
 		return ERR_PTR(-ENOMEM);
@@ -1608,19 +1608,19 @@ parse_hist_trigger_attrs(struct trace_array *tr, char *trigger_str)
 	return ERR_PTR(ret);
 }
 
-static inline void save_comm(char *comm, struct task_struct *task)
+static inline void save_comm(struct trace_comm *comm, struct task_struct *task)
 {
 	if (!task->pid) {
-		strcpy(comm, "<idle>");
+		strcpy(comm->comm, "<idle>");
 		return;
 	}
 
 	if (WARN_ON_ONCE(task->pid < 0)) {
-		strcpy(comm, "<XXX>");
+		strcpy(comm->comm, "<XXX>");
 		return;
 	}
 
-	strscpy(comm, task->comm, TASK_COMM_LEN);
+	strscpy(comm->comm, task->comm);
 }
 
 static void hist_elt_data_free(struct hist_elt_data *elt_data)
@@ -1646,7 +1646,6 @@ static void hist_trigger_elt_data_free(struct tracing_map_elt *elt)
 static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt)
 {
 	struct hist_trigger_data *hist_data = elt->map->private_data;
-	unsigned int size = TASK_COMM_LEN;
 	struct hist_elt_data *elt_data;
 	struct hist_field *hist_field;
 	unsigned int i, n_str;
@@ -1659,7 +1658,7 @@ static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt)
 		hist_field = hist_data->fields[i];
 
 		if (hist_field->flags & HIST_FIELD_FL_EXECNAME) {
-			elt_data->comm = kzalloc(size, GFP_KERNEL);
+			elt_data->comm = kzalloc_obj(*elt_data->comm);
 			if (!elt_data->comm) {
 				kfree(elt_data);
 				return -ENOMEM;
@@ -1677,8 +1676,6 @@ static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt)
 
 	BUILD_BUG_ON(STR_VAR_LEN_MAX & (sizeof(u64) - 1));
 
-	size = STR_VAR_LEN_MAX;
-
 	elt_data->field_var_str = kcalloc(n_str, sizeof(char *), GFP_KERNEL);
 	if (!elt_data->field_var_str) {
 		hist_elt_data_free(elt_data);
@@ -1687,7 +1684,7 @@ static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt)
 	elt_data->n_field_var_str = n_str;
 
 	for (i = 0; i < n_str; i++) {
-		elt_data->field_var_str[i] = kzalloc(size, GFP_KERNEL);
+		elt_data->field_var_str[i] = kzalloc(STR_VAR_LEN_MAX, GFP_KERNEL);
 		if (!elt_data->field_var_str[i]) {
 			hist_elt_data_free(elt_data);
 			return -ENOMEM;
@@ -3449,7 +3446,7 @@ static bool cond_snapshot_update(struct trace_array *tr, void *cond_data)
 	elt_data = context->elt->private_data;
 	track_elt_data = track_data->elt.private_data;
 	if (elt_data->comm)
-		strscpy(track_elt_data->comm, elt_data->comm, TASK_COMM_LEN);
+		track_elt_data->comm = elt_data->comm;
 
 	track_data->updated = true;
 
@@ -5505,16 +5502,13 @@ static void hist_trigger_print_key(struct seq_file *m,
 				   uval, (void *)(uintptr_t)uval);
 		} else if (key_field->flags & HIST_FIELD_FL_EXECNAME) {
 			struct hist_elt_data *elt_data = elt->private_data;
-			char *comm;
 
 			if (WARN_ON_ONCE(!elt_data))
 				return;
 
-			comm = elt_data->comm;
-
 			uval = *(u64 *)(key + key_field->offset);
 			seq_printf(m, "%s: %-16s[%10llu]", field_name,
-				   comm, uval);
+				   elt_data->comm->comm, uval);
 		} else if (key_field->flags & HIST_FIELD_FL_SYSCALL) {
 			const char *syscall_name;
 
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 0d2d3a2ea7dd..e46c5aa0d4e4 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -561,19 +561,19 @@ static void print_graph_cpu(struct trace_seq *s, int cpu)
 
 static void print_graph_proc(struct trace_seq *s, pid_t pid)
 {
-	char comm[TASK_COMM_LEN];
+	struct trace_comm comm;
 	/* sign + log10(MAX_INT) + '\0' */
 	char pid_str[12];
 	int spaces = 0;
 	int len;
 	int i;
 
-	trace_find_cmdline(pid, comm);
-	comm[7] = '\0';
+	trace_find_cmdline(pid, &comm);
+	comm.comm[7] = '\0';
 	sprintf(pid_str, "%d", pid);
 
 	/* 1 stands for the "-" character */
-	len = strlen(comm) + strlen(pid_str) + 1;
+	len = strlen(comm.comm) + strlen(pid_str) + 1;
 
 	if (len < TRACE_GRAPH_PROCINFO_LENGTH)
 		spaces = TRACE_GRAPH_PROCINFO_LENGTH - len;
@@ -582,7 +582,7 @@ static void print_graph_proc(struct trace_seq *s, pid_t pid)
 	for (i = 0; i < spaces / 2; i++)
 		trace_seq_putc(s, ' ');
 
-	trace_seq_printf(s, "%s-%s", comm, pid_str);
+	trace_seq_printf(s, "%s-%s", comm.comm, pid_str);
 
 	/* Last spaces to align center */
 	for (i = 0; i < spaces - (spaces / 2); i++)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index a5ad76175d10..58405291b44f 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -554,12 +554,12 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry)
 static int
 lat_print_generic(struct trace_seq *s, struct trace_entry *entry, int cpu)
 {
-	char comm[TASK_COMM_LEN];
+	struct trace_comm comm;
 
-	trace_find_cmdline(entry->pid, comm);
+	trace_find_cmdline(entry->pid, &comm);
 
 	trace_seq_printf(s, "%8.8s-%-7d %3d",
-			 comm, entry->pid, cpu);
+			 comm.comm, entry->pid, cpu);
 
 	return trace_print_lat_fmt(s, entry);
 }
@@ -658,11 +658,11 @@ int trace_print_context(struct trace_iterator *iter)
 	struct trace_array *tr = iter->tr;
 	struct trace_seq *s = &iter->seq;
 	struct trace_entry *entry = iter->ent;
-	char comm[TASK_COMM_LEN];
+	struct trace_comm comm;
 
-	trace_find_cmdline(entry->pid, comm);
+	trace_find_cmdline(entry->pid, &comm);
 
-	trace_seq_printf(s, "%16s-%-7d ", comm, entry->pid);
+	trace_seq_printf(s, "%16s-%-7d ", comm.comm, entry->pid);
 
 	if (tr->trace_flags & TRACE_ITER(RECORD_TGID)) {
 		unsigned int tgid = trace_find_tgid(entry->pid);
@@ -700,13 +700,13 @@ int trace_print_lat_context(struct trace_iterator *iter)
 	entry = iter->ent;
 
 	if (verbose) {
-		char comm[TASK_COMM_LEN];
+		struct trace_comm comm;
 
-		trace_find_cmdline(entry->pid, comm);
+		trace_find_cmdline(entry->pid, &comm);
 
 		trace_seq_printf(
 			s, "%16s %7d %3d %d %08x %08lx ",
-			comm, entry->pid, iter->cpu, entry->flags,
+			comm.comm, entry->pid, iter->cpu, entry->flags,
 			entry->preempt_count & 0xf, iter->idx);
 	} else {
 		lat_print_generic(s, entry, iter->cpu);
@@ -1276,7 +1276,7 @@ static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
 					     char *delim)
 {
 	struct ctx_switch_entry *field;
-	char comm[TASK_COMM_LEN];
+	struct trace_comm comm;
 	int S, T;
 
 
@@ -1284,7 +1284,7 @@ static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
 
 	T = task_index_to_char(field->next_state);
 	S = task_index_to_char(field->prev_state);
-	trace_find_cmdline(field->next_pid, comm);
+	trace_find_cmdline(field->next_pid, &comm);
 	trace_seq_printf(&iter->seq,
 			 " %7d:%3d:%c %s [%03d] %7d:%3d:%c %s\n",
 			 field->prev_pid,
@@ -1293,7 +1293,7 @@ static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
 			 field->next_cpu,
 			 field->next_pid,
 			 field->next_prio,
-			 T, comm);
+			 T, comm.comm);
 
 	return trace_handle_return(&iter->seq);
 }
diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
index e9f0ff962660..972883643097 100644
--- a/kernel/trace/trace_sched_switch.c
+++ b/kernel/trace/trace_sched_switch.c
@@ -172,27 +172,33 @@ struct saved_cmdlines_buffer {
 	unsigned *map_cmdline_to_pid;
 	unsigned cmdline_num;
 	int cmdline_idx;
-	char saved_cmdlines[];
+	struct trace_comm saved_cmdlines[];
 };
 static struct saved_cmdlines_buffer *savedcmd;
 
 /* Holds the size of a cmdline and pid element */
 #define SAVED_CMDLINE_MAP_ELEMENT_SIZE(s)			\
-	(TASK_COMM_LEN + sizeof((s)->map_cmdline_to_pid[0]))
+	(sizeof(struct trace_comm) + sizeof((s)->map_cmdline_to_pid[0]))
 
-static inline char *get_saved_cmdlines(int idx)
+static inline struct trace_comm *get_saved_cmdlines(int idx)
 {
-	return &savedcmd->saved_cmdlines[idx * TASK_COMM_LEN];
+	return &savedcmd->saved_cmdlines[idx];
 }
 
-static inline void set_cmdline(int idx, const char *cmdline)
+static inline void set_cmdline(int idx, const struct task_struct *tsk)
 {
-	strscpy(get_saved_cmdlines(idx), cmdline, TASK_COMM_LEN);
+	struct trace_comm *comm = get_saved_cmdlines(idx);
+
+	BUILD_BUG_ON(sizeof(comm->comm) > sizeof(tsk->comm));
+
+	memcpy(comm->comm, tsk->comm, sizeof comm->comm);
+	if (sizeof(comm->comm) != sizeof(tsk->comm))
+		comm->comm[ARRAY_SIZE(comm->comm) - 1] = 0;
 }
 
 static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)
 {
-	int order = get_order(sizeof(*s) + s->cmdline_num * TASK_COMM_LEN);
+	int order = get_order(sizeof(*s) + s->cmdline_num * sizeof(struct trace_comm));
 
 	kmemleak_free(s);
 	free_pages((unsigned long)s, order);
@@ -222,7 +228,7 @@ static struct saved_cmdlines_buffer *allocate_cmdlines_buffer(unsigned int val)
 	s->cmdline_num = val;
 
 	/* Place map_cmdline_to_pid array right after saved_cmdlines */
-	s->map_cmdline_to_pid = (unsigned *)&s->saved_cmdlines[val * TASK_COMM_LEN];
+	s->map_cmdline_to_pid = (unsigned *)&s->saved_cmdlines[val];
 
 	memset(&s->map_pid_to_cmdline, NO_CMDLINE_MAP,
 	       sizeof(s->map_pid_to_cmdline));
@@ -273,25 +279,25 @@ int trace_save_cmdline(struct task_struct *tsk)
 	}
 
 	savedcmd->map_cmdline_to_pid[idx] = tsk->pid;
-	set_cmdline(idx, tsk->comm);
+	set_cmdline(idx, tsk);
 
 	arch_spin_unlock(&trace_cmdline_lock);
 
 	return 1;
 }
 
-static void __trace_find_cmdline(int pid, char comm[])
+static void __trace_find_cmdline(int pid, struct trace_comm *comm)
 {
 	unsigned map;
 	int tpid;
 
 	if (!pid) {
-		strcpy(comm, "<idle>");
+		strcpy(comm->comm, "<idle>");
 		return;
 	}
 
 	if (WARN_ON_ONCE(pid < 0)) {
-		strcpy(comm, "<XXX>");
+		strcpy(comm->comm, "<XXX>");
 		return;
 	}
 
@@ -300,14 +306,14 @@ static void __trace_find_cmdline(int pid, char comm[])
 	if (map != NO_CMDLINE_MAP) {
 		tpid = savedcmd->map_cmdline_to_pid[map];
 		if (tpid == pid) {
-			strscpy(comm, get_saved_cmdlines(map), TASK_COMM_LEN);
+			*comm = *get_saved_cmdlines(map);
 			return;
 		}
 	}
-	strcpy(comm, "<...>");
+	strcpy(comm->comm, "<...>");
 }
 
-void trace_find_cmdline(int pid, char comm[])
+void trace_find_cmdline(int pid, struct trace_comm *comm)
 {
 	preempt_disable();
 	arch_spin_lock(&trace_cmdline_lock);
@@ -561,11 +567,11 @@ static void saved_cmdlines_stop(struct seq_file *m, void *v)
 
 static int saved_cmdlines_show(struct seq_file *m, void *v)
 {
-	char buf[TASK_COMM_LEN];
+	struct trace_comm buf;
 	unsigned int *pid = v;
 
-	__trace_find_cmdline(*pid, buf);
-	seq_printf(m, "%d %s\n", *pid, buf);
+	__trace_find_cmdline(*pid, &buf);
+	seq_printf(m, "%d %s\n", *pid, buf.comm);
 	return 0;
 }
 
-- 
2.39.5