mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-01 00:58:39 +02:00 
			
		
		
		
	tracing: Fix TASK_COMM_LEN in trace event format file
After commit3087c61ed2("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/Y+QaZtz55LIirsUO@google.com/ Link: https://lore.kernel.org/linux-trace-kernel/20230210155921.4610-1-laoar.shao@gmail.com/ Link: https://lore.kernel.org/linux-trace-kernel/20230212151303.12353-1-laoar.shao@gmail.com Cc: stable@vger.kernel.org Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: Kajetan Puchalski <kajetan.puchalski@arm.com> CC: Qais Yousef <qyousef@layalina.io> Fixes:3087c61ed2("tools/testing/selftests/bpf: replace open-coded 16 with TASK_COMM_LEN") Reported-by: John Stultz <jstultz@google.com> Debugged-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Suggested-by: Steven Rostedt <rostedt@goodmis.org> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This commit is contained in:
		
							parent
							
								
									3e46d910d8
								
							
						
					
					
						commit
						b6c7abd1c2
					
				
					 5 changed files with 36 additions and 11 deletions
				
			
		|  | @ -270,6 +270,7 @@ struct trace_event_fields { | ||||||
| 			const int  align; | 			const int  align; | ||||||
| 			const int  is_signed; | 			const int  is_signed; | ||||||
| 			const int  filter_type; | 			const int  filter_type; | ||||||
|  | 			const int  len; | ||||||
| 		}; | 		}; | ||||||
| 		int (*define_fields)(struct trace_event_call *); | 		int (*define_fields)(struct trace_event_call *); | ||||||
| 	}; | 	}; | ||||||
|  |  | ||||||
|  | @ -26,7 +26,8 @@ | ||||||
| #define __array(_type, _item, _len) {					\ | #define __array(_type, _item, _len) {					\ | ||||||
| 	.type = #_type"["__stringify(_len)"]", .name = #_item,		\ | 	.type = #_type"["__stringify(_len)"]", .name = #_item,		\ | ||||||
| 	.size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type),	\ | 	.size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type),	\ | ||||||
| 	.is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER }, | 	.is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER,\ | ||||||
|  | 	.len = _len }, | ||||||
| 
 | 
 | ||||||
| #undef __dynamic_array | #undef __dynamic_array | ||||||
| #define __dynamic_array(_type, _item, _len) {				\ | #define __dynamic_array(_type, _item, _len) {				\ | ||||||
|  |  | ||||||
|  | @ -1282,6 +1282,7 @@ struct ftrace_event_field { | ||||||
| 	int			offset; | 	int			offset; | ||||||
| 	int			size; | 	int			size; | ||||||
| 	int			is_signed; | 	int			is_signed; | ||||||
|  | 	int			len; | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| struct prog_entry; | struct prog_entry; | ||||||
|  |  | ||||||
|  | @ -114,7 +114,7 @@ trace_find_event_field(struct trace_event_call *call, char *name) | ||||||
| 
 | 
 | ||||||
| static int __trace_define_field(struct list_head *head, const char *type, | static int __trace_define_field(struct list_head *head, const char *type, | ||||||
| 				const char *name, int offset, int size, | 				const char *name, int offset, int size, | ||||||
| 				int is_signed, int filter_type) | 				int is_signed, int filter_type, int len) | ||||||
| { | { | ||||||
| 	struct ftrace_event_field *field; | 	struct ftrace_event_field *field; | ||||||
| 
 | 
 | ||||||
|  | @ -133,6 +133,7 @@ static int __trace_define_field(struct list_head *head, const char *type, | ||||||
| 	field->offset = offset; | 	field->offset = offset; | ||||||
| 	field->size = size; | 	field->size = size; | ||||||
| 	field->is_signed = is_signed; | 	field->is_signed = is_signed; | ||||||
|  | 	field->len = len; | ||||||
| 
 | 
 | ||||||
| 	list_add(&field->link, head); | 	list_add(&field->link, head); | ||||||
| 
 | 
 | ||||||
|  | @ -150,14 +151,28 @@ int trace_define_field(struct trace_event_call *call, const char *type, | ||||||
| 
 | 
 | ||||||
| 	head = trace_get_fields(call); | 	head = trace_get_fields(call); | ||||||
| 	return __trace_define_field(head, type, name, offset, size, | 	return __trace_define_field(head, type, name, offset, size, | ||||||
| 				    is_signed, filter_type); | 				    is_signed, filter_type, 0); | ||||||
| } | } | ||||||
| EXPORT_SYMBOL_GPL(trace_define_field); | EXPORT_SYMBOL_GPL(trace_define_field); | ||||||
| 
 | 
 | ||||||
|  | int trace_define_field_ext(struct trace_event_call *call, const char *type, | ||||||
|  | 		       const char *name, int offset, int size, int is_signed, | ||||||
|  | 		       int filter_type, int len) | ||||||
|  | { | ||||||
|  | 	struct list_head *head; | ||||||
|  | 
 | ||||||
|  | 	if (WARN_ON(!call->class)) | ||||||
|  | 		return 0; | ||||||
|  | 
 | ||||||
|  | 	head = trace_get_fields(call); | ||||||
|  | 	return __trace_define_field(head, type, name, offset, size, | ||||||
|  | 				    is_signed, filter_type, len); | ||||||
|  | } | ||||||
|  | 
 | ||||||
| #define __generic_field(type, item, filter_type)			\ | #define __generic_field(type, item, filter_type)			\ | ||||||
| 	ret = __trace_define_field(&ftrace_generic_fields, #type,	\ | 	ret = __trace_define_field(&ftrace_generic_fields, #type,	\ | ||||||
| 				   #item, 0, 0, is_signed_type(type),	\ | 				   #item, 0, 0, is_signed_type(type),	\ | ||||||
| 				   filter_type);			\ | 				   filter_type, 0);			\ | ||||||
| 	if (ret)							\ | 	if (ret)							\ | ||||||
| 		return ret; | 		return ret; | ||||||
| 
 | 
 | ||||||
|  | @ -166,7 +181,7 @@ EXPORT_SYMBOL_GPL(trace_define_field); | ||||||
| 				   "common_" #item,			\ | 				   "common_" #item,			\ | ||||||
| 				   offsetof(typeof(ent), item),		\ | 				   offsetof(typeof(ent), item),		\ | ||||||
| 				   sizeof(ent.item),			\ | 				   sizeof(ent.item),			\ | ||||||
| 				   is_signed_type(type), FILTER_OTHER);	\ | 				   is_signed_type(type), FILTER_OTHER, 0);	\ | ||||||
| 	if (ret)							\ | 	if (ret)							\ | ||||||
| 		return ret; | 		return ret; | ||||||
| 
 | 
 | ||||||
|  | @ -1588,12 +1603,17 @@ static int f_show(struct seq_file *m, void *v) | ||||||
| 		seq_printf(m, "\tfield:%s %s;\toffset:%u;\tsize:%u;\tsigned:%d;\n", | 		seq_printf(m, "\tfield:%s %s;\toffset:%u;\tsize:%u;\tsigned:%d;\n", | ||||||
| 			   field->type, field->name, field->offset, | 			   field->type, field->name, field->offset, | ||||||
| 			   field->size, !!field->is_signed); | 			   field->size, !!field->is_signed); | ||||||
| 	else | 	else if (field->len) | ||||||
| 		seq_printf(m, "\tfield:%.*s %s%s;\toffset:%u;\tsize:%u;\tsigned:%d;\n", | 		seq_printf(m, "\tfield:%.*s %s[%d];\toffset:%u;\tsize:%u;\tsigned:%d;\n", | ||||||
| 			   (int)(array_descriptor - field->type), | 			   (int)(array_descriptor - field->type), | ||||||
| 			   field->type, field->name, | 			   field->type, field->name, | ||||||
| 			   array_descriptor, field->offset, | 			   field->len, field->offset, | ||||||
| 			   field->size, !!field->is_signed); | 			   field->size, !!field->is_signed); | ||||||
|  | 	else | ||||||
|  | 		seq_printf(m, "\tfield:%.*s %s[];\toffset:%u;\tsize:%u;\tsigned:%d;\n", | ||||||
|  | 				(int)(array_descriptor - field->type), | ||||||
|  | 				field->type, field->name, | ||||||
|  | 				field->offset, field->size, !!field->is_signed); | ||||||
| 
 | 
 | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
|  | @ -2379,9 +2399,10 @@ event_define_fields(struct trace_event_call *call) | ||||||
| 			} | 			} | ||||||
| 
 | 
 | ||||||
| 			offset = ALIGN(offset, field->align); | 			offset = ALIGN(offset, field->align); | ||||||
| 			ret = trace_define_field(call, field->type, field->name, | 			ret = trace_define_field_ext(call, field->type, field->name, | ||||||
| 						 offset, field->size, | 						 offset, field->size, | ||||||
| 						 field->is_signed, field->filter_type); | 						 field->is_signed, field->filter_type, | ||||||
|  | 						 field->len); | ||||||
| 			if (WARN_ON_ONCE(ret)) { | 			if (WARN_ON_ONCE(ret)) { | ||||||
| 				pr_err("error code is %d\n", ret); | 				pr_err("error code is %d\n", ret); | ||||||
| 				break; | 				break; | ||||||
|  |  | ||||||
|  | @ -111,7 +111,8 @@ static void __always_unused ____ftrace_check_##name(void)		\ | ||||||
| #define __array(_type, _item, _len) {					\ | #define __array(_type, _item, _len) {					\ | ||||||
| 	.type = #_type"["__stringify(_len)"]", .name = #_item,		\ | 	.type = #_type"["__stringify(_len)"]", .name = #_item,		\ | ||||||
| 	.size = sizeof(_type[_len]), .align = __alignof__(_type),	\ | 	.size = sizeof(_type[_len]), .align = __alignof__(_type),	\ | ||||||
| 	is_signed_type(_type), .filter_type = FILTER_OTHER }, | 	is_signed_type(_type), .filter_type = FILTER_OTHER,			\ | ||||||
|  | 	.len = _len }, | ||||||
| 
 | 
 | ||||||
| #undef __array_desc | #undef __array_desc | ||||||
| #define __array_desc(_type, _container, _item, _len) __array(_type, _item, _len) | #define __array_desc(_type, _container, _item, _len) __array(_type, _item, _len) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Yafang Shao
						Yafang Shao