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_btf.c             | 68 ++++++++++++++++++++++++++++
 kernel/trace/trace_btf.h             | 16 +++++++
 kernel/trace/trace_functions_graph.c | 50 ++++++++++++++------
 3 files changed, 121 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace_btf.c b/kernel/trace/trace_btf.c
index 00172f301f25..38825f566235 100644
--- a/kernel/trace/trace_btf.c
+++ b/kernel/trace/trace_btf.c
@@ -2,6 +2,7 @@
 #include <linux/btf.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
+#include <linux/kallsyms.h>
 
 #include "trace_btf.h"
 
@@ -120,3 +121,70 @@ const struct btf_member *btf_find_struct_member(struct btf 
*btf,
        return member;
 }
 
+#ifdef CONFIG_DEBUG_INFO_BTF
+void btf_trim_retval(unsigned long func, unsigned long *retval, bool 
*print_retval,
+                       int *fmt)
+{
+       const struct btf_type *t;
+       char name[KSYM_NAME_LEN];
+       struct btf *btf;
+       u32 v, msb;
+       int kind;
+
+       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;
+       }
+
+       btf_put(btf);
+}
+#endif
diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
index 4bc44bc261e6..3675d99c22eb 100644
--- a/kernel/trace/trace_btf.h
+++ b/kernel/trace/trace_btf.h
@@ -1,6 +1,13 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #include <linux/btf.h>
 
+enum {
+       RETVAL_FMT_HEX   = BIT(0),
+       RETVAL_FMT_DEC   = BIT(1),
+       RETVAL_FMT_BOOL  = BIT(2),
+       RETVAL_FMT_TRUNC = BIT(3),
+};
+
 const struct btf_type *btf_find_func_proto(const char *func_name,
                                           struct btf **btf_p);
 const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
@@ -9,3 +16,12 @@ const struct btf_member *btf_find_struct_member(struct btf 
*btf,
                                                const struct btf_type *type,
                                                const char *member_name,
                                                u32 *anon_offset);
+#ifdef CONFIG_DEBUG_INFO_BTF
+void btf_trim_retval(unsigned long func, unsigned long *retval, bool 
*print_retval,
+                    int *fmt);
+#else
+static inline void btf_trim_retval(unsigned long func, unsigned long *retval,
+                                  bool *print_retval, int *fmt)
+{
+}
+#endif
diff --git a/kernel/trace/trace_functions_graph.c 
b/kernel/trace/trace_functions_graph.c
index ff7cb1a76b95..89a64982df2c 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;
@@ -877,7 +878,7 @@ static void print_graph_retval(struct trace_seq *s, struct 
ftrace_graph_ent_entr
        unsigned long retval = 0;
        bool print_retaddr = false;
        bool print_retval = false;
-       bool hex_format = !!(opt_flags & TRACE_GRAPH_PRINT_RETVAL_HEX);
+       int retval_fmt = 0;
 
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
        retval = graph_ret->retval;
@@ -888,17 +889,35 @@ static void print_graph_retval(struct trace_seq *s, 
struct ftrace_graph_ent_entr
        print_retaddr = !!(opt_flags & TRACE_GRAPH_PRINT_RETADDR);
 #endif
 
-       if (print_retval && retval && !hex_format) {
-               /* Check if the return value matches the negative format */
-               if (IS_ENABLED(CONFIG_64BIT) && (retval & BIT(31)) &&
-                       (((u64)retval) >> 32) == 0) {
-                       err_code = sign_extend64(retval, 31);
-               } else {
-                       err_code = retval;
+       if (print_retval) {
+               int fmt = RETVAL_FMT_HEX;
+
+               btf_trim_retval((unsigned long)func, &retval, &print_retval, 
&fmt);
+               if (print_retval) {
+                       if (opt_flags & TRACE_GRAPH_PRINT_RETVAL_HEX)
+                               retval_fmt = RETVAL_FMT_HEX;
+
+                       if (retval && retval_fmt != RETVAL_FMT_HEX) {
+                               /* Check if the return value matches the 
negative format */
+                               if (IS_ENABLED(CONFIG_64BIT) && (retval & 
BIT(31)) &&
+                                       (((u64)retval) >> 32) == 0) {
+                                       err_code = sign_extend64(retval, 31);
+                               } else {
+                                       err_code = retval;
+                               }
+
+                               if (!IS_ERR_VALUE(err_code))
+                                       err_code = 0;
+                       }
+
+                       if (retval_fmt == RETVAL_FMT_HEX) {
+                               retval_fmt |= (fmt & RETVAL_FMT_TRUNC);
+                       } else {
+                               if (err_code && fmt & RETVAL_FMT_HEX)
+                                       fmt = (fmt & ~RETVAL_FMT_HEX) | 
RETVAL_FMT_DEC;
+                               retval_fmt = fmt;
+                       }
                }
-
-               if (!IS_ERR_VALUE(err_code))
-                       err_code = 0;
        }
 
        if (entry) {
@@ -925,10 +944,15 @@ static void print_graph_retval(struct trace_seq *s, 
struct ftrace_graph_ent_entr
                                    trace_flags, false);
 
        if (print_retval) {
-               if (hex_format || (err_code == 0))
+               if (retval_fmt & RETVAL_FMT_HEX)
                        trace_seq_printf(s, " ret=0x%lx", retval);
+               else if (retval_fmt & RETVAL_FMT_BOOL)
+                       trace_seq_printf(s, " ret=%s", retval ? "true" : 
"false");
                else
-                       trace_seq_printf(s, " ret=%ld", err_code);
+                       trace_seq_printf(s, " ret=%ld", err_code ?: retval);
+
+               if (retval_fmt & RETVAL_FMT_TRUNC)
+                       trace_seq_printf(s, "(trunc)");
        }
 
        if (!entry || print_retval || print_retaddr)
-- 
2.34.1


Reply via email to