On Mon, 15 Dec 2025 11:41:52 +0800 Donglin Peng <[email protected]> wrote:
> From: pengdonglin <[email protected]> > > The current funcgraph-retval implementation suffers from two accuracy > issues: > > 1. Void-returning functions still print a return value, creating > misleading noise in the trace output. > > 2. For functions returning narrower types (e.g., char, short), the > displayed value can be incorrect because high bits of the register > may contain undefined data. > > This patch addresses both problems by leveraging BTF to obtain the exact > return type of each traced kernel function. The key changes are: > > 1. Void function filtering: Functions with void return type no longer > display any return value in the trace output, eliminating unnecessary > clutter. > > 2. Type-aware value formatting: The return value is now properly truncated > to match the actual width of the return type before being displayed. > Additionally, the value is formatted according to its type for better > human readability. > > Here is an output comparison: > > Before: > # perf ftrace -G vfs_read --graph-opts retval > ... > 1) | touch_atime() { > 1) | atime_needs_update() { > 1) 0.069 us | make_vfsuid(); /* ret=0x0 */ > 1) 0.067 us | make_vfsgid(); /* ret=0x0 */ > 1) | current_time() { > 1) 0.197 us | ktime_get_coarse_real_ts64_mg(); /* > ret=0x187f886aec3ed6f5 */ > 1) 0.352 us | } /* current_time ret=0x69380753 */ > 1) 0.792 us | } /* atime_needs_update ret=0x0 */ > 1) 0.937 us | } /* touch_atime ret=0x0 */ > > After: > # perf ftrace -G vfs_read --graph-opts retval > ... > 2) | touch_atime() { > 2) | atime_needs_update() { > 2) 0.070 us | make_vfsuid(); /* ret=0x0 */ > 2) 0.070 us | make_vfsgid(); /* ret=0x0 */ > 2) | current_time() { > 2) 0.162 us | ktime_get_coarse_real_ts64_mg(); > 2) 0.312 us | } /* current_time ret=0x69380649(trunc) */ > 2) 0.753 us | } /* atime_needs_update ret=false */ > 2) 0.899 us | } /* touch_atime */ > > Cc: Steven Rostedt (Google) <[email protected]> > Cc: Masami Hiramatsu <[email protected]> > Cc: Xiaoqin Zhang <[email protected]> > Signed-off-by: pengdonglin <[email protected]> > --- > kernel/trace/trace_functions_graph.c | 124 ++++++++++++++++++++++++--- > 1 file changed, 111 insertions(+), 13 deletions(-) > > diff --git a/kernel/trace/trace_functions_graph.c > b/kernel/trace/trace_functions_graph.c > index 17c75cf2348e..46b66b1cfc16 100644 > --- a/kernel/trace/trace_functions_graph.c > +++ b/kernel/trace/trace_functions_graph.c > @@ -15,6 +15,7 @@ > > #include "trace.h" > #include "trace_output.h" > +#include "trace_btf.h" > > /* When set, irq functions might be ignored */ > static int ftrace_graph_skip_irqs; > @@ -120,6 +121,13 @@ enum { > FLAGS_FILL_END = 3 << TRACE_GRAPH_PRINT_FILL_SHIFT, > }; > > +enum { > + RETVAL_FMT_HEX = BIT(0), > + RETVAL_FMT_DEC = BIT(1), > + RETVAL_FMT_BOOL = BIT(2), > + RETVAL_FMT_TRUNC = BIT(3), > +}; > + > static void > print_graph_duration(struct trace_array *tr, unsigned long long duration, > struct trace_seq *s, u32 flags); > @@ -865,6 +873,73 @@ static void print_graph_retaddr(struct trace_seq *s, > struct fgraph_retaddr_ent_e > > #if defined(CONFIG_FUNCTION_GRAPH_RETVAL) || > defined(CONFIG_FUNCTION_GRAPH_RETADDR) > > +static void trim_retval(unsigned long func, unsigned long *retval, bool > *print_retval, > + int *fmt) This function should really be in trace_btf.c and a stub when btf is not enabled. -- Steve > +{ > + const struct btf_type *t; > + char name[KSYM_NAME_LEN]; > + struct btf *btf; > + u32 v, msb; > + int kind; > + > + if (!IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) > + return; > + > + if (lookup_symbol_name(func, name)) > + return; > + > + t = btf_find_func_proto(name, &btf); > + if (IS_ERR_OR_NULL(t)) > + return; > + > + t = btf_type_skip_modifiers(btf, t->type, NULL); > + kind = t ? BTF_INFO_KIND(t->info) : BTF_KIND_UNKN; > + switch (kind) { > + case BTF_KIND_UNKN: > + *print_retval = false; > + break; > + case BTF_KIND_STRUCT: > + case BTF_KIND_UNION: > + case BTF_KIND_ENUM: > + case BTF_KIND_ENUM64: > + if (kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION) > + *fmt = RETVAL_FMT_HEX; > + else > + *fmt = RETVAL_FMT_DEC; > + > + if (t->size > sizeof(unsigned long)) { > + *fmt |= RETVAL_FMT_TRUNC; > + } else { > + msb = BITS_PER_BYTE * t->size - 1; > + *retval &= GENMASK(msb, 0); > + } > + break; > + case BTF_KIND_INT: > + v = *(u32 *)(t + 1); > + if (BTF_INT_ENCODING(v) == BTF_INT_BOOL) { > + *fmt = RETVAL_FMT_BOOL; > + msb = 0; > + } else { > + if (BTF_INT_ENCODING(v) == BTF_INT_SIGNED) > + *fmt = RETVAL_FMT_DEC; > + else > + *fmt = RETVAL_FMT_HEX; > + > + if (t->size > sizeof(unsigned long)) { > + *fmt |= RETVAL_FMT_TRUNC; > + msb = BITS_PER_LONG - 1; > + } else { > + msb = BTF_INT_BITS(v) - 1; > + } > + } > + *retval &= GENMASK(msb, 0); > + break; > + default: > + *fmt = RETVAL_FMT_HEX; > + break; > + } > +} > +
