Skip to content

Commit b6c7abd

Browse files
laoarrostedt
authored andcommitted
tracing: Fix TASK_COMM_LEN in trace event format file
After commit 3087c61 ("tools/testing/selftests/bpf: replace open-coded 16 with TASK_COMM_LEN"), the content of the format file under /sys/kernel/tracing/events/task/task_newtask was changed from field:char comm[16]; offset:12; size:16; signed:0; to field:char comm[TASK_COMM_LEN]; offset:12; size:16; signed:0; John reported that this change breaks older versions of perfetto. Then Mathieu pointed out that this behavioral change was caused by the use of __stringify(_len), which happens to work on macros, but not on enum labels. And he also gave the suggestion on how to fix it: :One possible solution to make this more robust would be to extend :struct trace_event_fields with one more field that indicates the length :of an array as an actual integer, without storing it in its stringified :form in the type, and do the formatting in f_show where it belongs. The result as follows after this change, $ cat /sys/kernel/tracing/events/task/task_newtask/format field:char comm[16]; offset:12; size:16; signed:0; Link: https://lore.kernel.org/lkml/[email protected]/ Link: https://lore.kernel.org/linux-trace-kernel/[email protected]/ Link: https://lore.kernel.org/linux-trace-kernel/[email protected] Cc: [email protected] Cc: Alexei Starovoitov <[email protected]> Cc: Kajetan Puchalski <[email protected]> CC: Qais Yousef <[email protected]> Fixes: 3087c61 ("tools/testing/selftests/bpf: replace open-coded 16 with TASK_COMM_LEN") Reported-by: John Stultz <[email protected]> Debugged-by: Mathieu Desnoyers <[email protected]> Suggested-by: Mathieu Desnoyers <[email protected]> Suggested-by: Steven Rostedt <[email protected]> Signed-off-by: Yafang Shao <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 3e46d91 commit b6c7abd

File tree

5 files changed

+36
-11
lines changed

5 files changed

+36
-11
lines changed

include/linux/trace_events.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ struct trace_event_fields {
270270
const int align;
271271
const int is_signed;
272272
const int filter_type;
273+
const int len;
273274
};
274275
int (*define_fields)(struct trace_event_call *);
275276
};

include/trace/stages/stage4_event_fields.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
#define __array(_type, _item, _len) { \
2727
.type = #_type"["__stringify(_len)"]", .name = #_item, \
2828
.size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type), \
29-
.is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER },
29+
.is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER,\
30+
.len = _len },
3031

3132
#undef __dynamic_array
3233
#define __dynamic_array(_type, _item, _len) { \

kernel/trace/trace.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,7 @@ struct ftrace_event_field {
12821282
int offset;
12831283
int size;
12841284
int is_signed;
1285+
int len;
12851286
};
12861287

12871288
struct prog_entry;

kernel/trace/trace_events.c

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ trace_find_event_field(struct trace_event_call *call, char *name)
114114

115115
static int __trace_define_field(struct list_head *head, const char *type,
116116
const char *name, int offset, int size,
117-
int is_signed, int filter_type)
117+
int is_signed, int filter_type, int len)
118118
{
119119
struct ftrace_event_field *field;
120120

@@ -133,6 +133,7 @@ static int __trace_define_field(struct list_head *head, const char *type,
133133
field->offset = offset;
134134
field->size = size;
135135
field->is_signed = is_signed;
136+
field->len = len;
136137

137138
list_add(&field->link, head);
138139

@@ -150,14 +151,28 @@ int trace_define_field(struct trace_event_call *call, const char *type,
150151

151152
head = trace_get_fields(call);
152153
return __trace_define_field(head, type, name, offset, size,
153-
is_signed, filter_type);
154+
is_signed, filter_type, 0);
154155
}
155156
EXPORT_SYMBOL_GPL(trace_define_field);
156157

158+
int trace_define_field_ext(struct trace_event_call *call, const char *type,
159+
const char *name, int offset, int size, int is_signed,
160+
int filter_type, int len)
161+
{
162+
struct list_head *head;
163+
164+
if (WARN_ON(!call->class))
165+
return 0;
166+
167+
head = trace_get_fields(call);
168+
return __trace_define_field(head, type, name, offset, size,
169+
is_signed, filter_type, len);
170+
}
171+
157172
#define __generic_field(type, item, filter_type) \
158173
ret = __trace_define_field(&ftrace_generic_fields, #type, \
159174
#item, 0, 0, is_signed_type(type), \
160-
filter_type); \
175+
filter_type, 0); \
161176
if (ret) \
162177
return ret;
163178

@@ -166,7 +181,7 @@ EXPORT_SYMBOL_GPL(trace_define_field);
166181
"common_" #item, \
167182
offsetof(typeof(ent), item), \
168183
sizeof(ent.item), \
169-
is_signed_type(type), FILTER_OTHER); \
184+
is_signed_type(type), FILTER_OTHER, 0); \
170185
if (ret) \
171186
return ret;
172187

@@ -1588,12 +1603,17 @@ static int f_show(struct seq_file *m, void *v)
15881603
seq_printf(m, "\tfield:%s %s;\toffset:%u;\tsize:%u;\tsigned:%d;\n",
15891604
field->type, field->name, field->offset,
15901605
field->size, !!field->is_signed);
1591-
else
1592-
seq_printf(m, "\tfield:%.*s %s%s;\toffset:%u;\tsize:%u;\tsigned:%d;\n",
1606+
else if (field->len)
1607+
seq_printf(m, "\tfield:%.*s %s[%d];\toffset:%u;\tsize:%u;\tsigned:%d;\n",
15931608
(int)(array_descriptor - field->type),
15941609
field->type, field->name,
1595-
array_descriptor, field->offset,
1610+
field->len, field->offset,
15961611
field->size, !!field->is_signed);
1612+
else
1613+
seq_printf(m, "\tfield:%.*s %s[];\toffset:%u;\tsize:%u;\tsigned:%d;\n",
1614+
(int)(array_descriptor - field->type),
1615+
field->type, field->name,
1616+
field->offset, field->size, !!field->is_signed);
15971617

15981618
return 0;
15991619
}
@@ -2379,9 +2399,10 @@ event_define_fields(struct trace_event_call *call)
23792399
}
23802400

23812401
offset = ALIGN(offset, field->align);
2382-
ret = trace_define_field(call, field->type, field->name,
2402+
ret = trace_define_field_ext(call, field->type, field->name,
23832403
offset, field->size,
2384-
field->is_signed, field->filter_type);
2404+
field->is_signed, field->filter_type,
2405+
field->len);
23852406
if (WARN_ON_ONCE(ret)) {
23862407
pr_err("error code is %d\n", ret);
23872408
break;

kernel/trace/trace_export.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ static void __always_unused ____ftrace_check_##name(void) \
111111
#define __array(_type, _item, _len) { \
112112
.type = #_type"["__stringify(_len)"]", .name = #_item, \
113113
.size = sizeof(_type[_len]), .align = __alignof__(_type), \
114-
is_signed_type(_type), .filter_type = FILTER_OTHER },
114+
is_signed_type(_type), .filter_type = FILTER_OTHER, \
115+
.len = _len },
115116

116117
#undef __array_desc
117118
#define __array_desc(_type, _container, _item, _len) __array(_type, _item, _len)

0 commit comments

Comments
 (0)