On Wed, May 13, 2026 at 10:47 PM Steven Rostedt <[email protected]> wrote:
>
> 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.
Thanks, I will fix it.
>
> -- 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;
> > + }
> > +}
> > +
