Re: [PATCH v2] tracing: Fix ftrace event field alignments

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Wed, 4 Feb 2026 11:36:28 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The fields of ftrace specific events (events used to save ftrace internal
> events like function traces and trace_printk) are generated similarly to
> how normal trace event fields are generated. That is, the fields are added
> to a trace_events_fields array that saves the name, offset, size,
> alignment and signness of the field. It is used to produce the output in
> the format file in tracefs so that tooling knows how to parse the binary
> data of the trace events.
> 
> The issue is that some of the ftrace event structures are packed. The
> function graph exit event structures are one of them. The 64 bit calltime
> and rettime fields end up 4 byte aligned, but the algorithm to show to
> userspace shows them as 8 byte aligned.
> 
> The macros that create the ftrace events has one for embedded structure
> fields. There's two macros for theses fields:
> 
>   __field_desc() and __field_packed()
> 
> The difference of the latter macro is that it treats the field as packed.
> 
> Rename that field to __field_desc_packed() and create replace the
> __field_packed() to be a normal field that is packed and have the calltime
> and rettime use those.
> 
> This showed up on 32bit architectures for function graph time fields. It
> had:
> 
>  ~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
> [..]
>         field:unsigned long func;       offset:8;       size:4; signed:0;
>         field:unsigned int depth;       offset:12;      size:4; signed:0;
>         field:unsigned int overrun;     offset:16;      size:4; signed:0;
>         field:unsigned long long calltime;      offset:24;      size:8; signed:0;
>         field:unsigned long long rettime;       offset:32;      size:8; signed:0;
> 
> Notice that overrun is at offset 16 with size 4, where in the structure
> calltime is at offset 20 (16 + 4), but it shows the offset at 24. That's
> because it used the alignment of unsigned long long when used as a
> declaration and not as a member of a structure where it would be aligned
> by word size (in this case 4).
> 
> By using the proper structure alignment, the format has it at the correct
> offset:
> 
>  ~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
> [..]
>         field:unsigned long func;       offset:8;       size:4; signed:0;
>         field:unsigned int depth;       offset:12;      size:4; signed:0;
>         field:unsigned int overrun;     offset:16;      size:4; signed:0;
>         field:unsigned long long calltime;      offset:20;      size:8; signed:0;
>         field:unsigned long long rettime;       offset:28;      size:8; signed:0;
> 
> Cc: stable@vger.kernel.org
> Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()")
> Reported-by: "jempty.liang" <imntjempty@163.com>
> Closes: https://lore.kernel.org/all/20260130015740.212343-1-imntjempty@163.com/
> Closes: https://lore.kernel.org/all/20260202123342.2544795-1-imntjempty@163.com/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> ---
> Changes since v1: https://patch.msgid.link/20260202113024.61d5c1fd@gandalf.local.home
> 
> - Instead of using an alignment for structures, create a new macro
>   called __field_desc_packed() to use for packed structure fields
>   and have __field_packed() be used for normal packed fields.
> 
>  kernel/trace/trace.h         |  7 +++++--
>  kernel/trace/trace_entries.h | 32 ++++++++++++++++----------------
>  kernel/trace/trace_export.c  | 21 +++++++++++++++------
>  3 files changed, 36 insertions(+), 24 deletions(-)
> 
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b6d42fe06115..c11edec5d8f5 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -68,14 +68,17 @@ enum trace_type {
>  #undef __field_fn
>  #define __field_fn(type, item)		type	item;
>  
> +#undef __field_packed
> +#define __field_packed(type, item)	type	item;
> +
>  #undef __field_struct
>  #define __field_struct(type, item)	__field(type, item)
>  
>  #undef __field_desc
>  #define __field_desc(type, container, item)
>  
> -#undef __field_packed
> -#define __field_packed(type, container, item)
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item)
>  
>  #undef __array
>  #define __array(type, item, size)	type	item[size];
> diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
> index f6a8d29c0d76..54417468fdeb 100644
> --- a/kernel/trace/trace_entries.h
> +++ b/kernel/trace/trace_entries.h
> @@ -79,8 +79,8 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry,
>  
>  	F_STRUCT(
>  		__field_struct(	struct ftrace_graph_ent,	graph_ent	)
> -		__field_packed(	unsigned long,	graph_ent,	func		)
> -		__field_packed(	unsigned long,	graph_ent,	depth		)
> +		__field_desc_packed(unsigned long,	graph_ent,	func	)
> +		__field_desc_packed(unsigned long,	graph_ent,	depth	)
>  		__dynamic_array(unsigned long,	args				)
>  	),
>  
> @@ -96,9 +96,9 @@ FTRACE_ENTRY_PACKED(fgraph_retaddr_entry, fgraph_retaddr_ent_entry,
>  
>  	F_STRUCT(
>  		__field_struct(	struct fgraph_retaddr_ent,	graph_rent	)
> -		__field_packed(	unsigned long,	graph_rent.ent,	func		)
> -		__field_packed(	unsigned long,	graph_rent.ent,	depth		)
> -		__field_packed(	unsigned long,	graph_rent,	retaddr		)
> +		__field_desc_packed(	unsigned long,	graph_rent.ent,	func	)
> +		__field_desc_packed(	unsigned long,	graph_rent.ent,	depth	)
> +		__field_desc_packed(	unsigned long,	graph_rent,	retaddr	)
>  		__dynamic_array(unsigned long,	args				)
>  	),
>  
> @@ -123,12 +123,12 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
>  
>  	F_STRUCT(
>  		__field_struct(	struct ftrace_graph_ret,	ret	)
> -		__field_packed(	unsigned long,	ret,		func	)
> -		__field_packed(	unsigned long,	ret,		retval	)
> -		__field_packed(	unsigned int,	ret,		depth	)
> -		__field_packed(	unsigned int,	ret,		overrun	)
> -		__field(unsigned long long,	calltime		)
> -		__field(unsigned long long,	rettime			)
> +		__field_desc_packed(	unsigned long,	ret,	func	)
> +		__field_desc_packed(	unsigned long,	ret,	retval	)
> +		__field_desc_packed(	unsigned int,	ret,	depth	)
> +		__field_desc_packed(	unsigned int,	ret,	overrun	)
> +		__field_packed(unsigned long long,	calltime)
> +		__field_packed(unsigned long long,	rettime	)
>  	),
>  
>  	F_printk("<-- %ps (%u) (start: %llx  end: %llx) over: %u retval: %lx",
> @@ -146,11 +146,11 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
>  
>  	F_STRUCT(
>  		__field_struct(	struct ftrace_graph_ret,	ret	)
> -		__field_packed(	unsigned long,	ret,		func	)
> -		__field_packed(	unsigned int,	ret,		depth	)
> -		__field_packed(	unsigned int,	ret,		overrun	)
> -		__field(unsigned long long,	calltime		)
> -		__field(unsigned long long,	rettime			)
> +		__field_desc_packed(	unsigned long,	ret,	func	)
> +		__field_desc_packed(	unsigned int,	ret,	depth	)
> +		__field_desc_packed(	unsigned int,	ret,	overrun	)
> +		__field_packed(unsigned long long,	calltime	)
> +		__field_packed(unsigned long long,	rettime		)
>  	),
>  
>  	F_printk("<-- %ps (%u) (start: %llx  end: %llx) over: %u",
> diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
> index 1698fc22afa0..32a42ef31855 100644
> --- a/kernel/trace/trace_export.c
> +++ b/kernel/trace/trace_export.c
> @@ -42,11 +42,14 @@ static int ftrace_event_register(struct trace_event_call *call,
>  #undef __field_fn
>  #define __field_fn(type, item)				type item;
>  
> +#undef __field_packed
> +#define __field_packed(type, item)			type item;
> +
>  #undef __field_desc
>  #define __field_desc(type, container, item)		type item;
>  
> -#undef __field_packed
> -#define __field_packed(type, container, item)		type item;
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item)	type item;
>  
>  #undef __array
>  #define __array(type, item, size)			type item[size];
> @@ -104,11 +107,14 @@ static void __always_unused ____ftrace_check_##name(void)		\
>  #undef __field_fn
>  #define __field_fn(_type, _item) __field_ext(_type, _item, FILTER_TRACE_FN)
>  
> +#undef __field_packed
> +#define __field_packed(_type, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
> +
>  #undef __field_desc
>  #define __field_desc(_type, _container, _item) __field_ext(_type, _item, FILTER_OTHER)
>  
> -#undef __field_packed
> -#define __field_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
> +#undef __field_desc_packed
> +#define __field_desc_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
>  
>  #undef __array
>  #define __array(_type, _item, _len) {					\
> @@ -146,11 +152,14 @@ static struct trace_event_fields ftrace_event_fields_##name[] = {	\
>  #undef __field_fn
>  #define __field_fn(type, item)
>  
> +#undef __field_packed
> +#define __field_packed(type, item)
> +
>  #undef __field_desc
>  #define __field_desc(type, container, item)
>  
> -#undef __field_packed
> -#define __field_packed(type, container, item)
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item)
>  
>  #undef __array
>  #define __array(type, item, len)
> -- 
> 2.51.0
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>