Re: [PATCH v2 1/2] fgraph: Use BTF to trim and filter return values

On Tue, Dec 9, 2025 at 8:48 AM Masami Hiramatsu [off-list ref] wrote:

On Mon,  8 Dec 2025 21:19:16 +0800
Donglin Peng [off-list ref] wrote:

quoted

From: pengdonglin <redacted>

The current funcgraph-retval implementation has two limitations:

1. It prints a return value even when the traced function returns void.
2. When the return type is narrower than a register, the printed value may
   be incorrect because high bits can contain undefined data.

Both issues are addressed by using BTF to obtain the precise return type
of each traced function:

- Return values are now printed only for functions whose return type is
  not void.
- The value is truncated to the actual width of the return type, ensuring
  correct representation.

These changes make the funcgraph-retval output more accurate and remove
noise from void functions.

Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Xiaoqin Zhang <redacted>
Signed-off-by: pengdonglin <redacted>
---
 kernel/trace/trace_functions_graph.c | 64 +++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 17c75cf2348e..9e63665c81e2 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;

@@ -865,6 +866,46 @@ 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)
+{
+     const struct btf_type *t;
+     char name[KSYM_NAME_LEN];
+     struct btf *btf;
+     u32 v, msb;
+
+     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);
+     switch (t ? BTF_INFO_KIND(t->info) : BTF_KIND_UNKN) {
+     case BTF_KIND_UNKN:
+             *print_retval = false;
+             break;
+     case BTF_KIND_ENUM:
+     case BTF_KIND_ENUM64:
+             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)
+                     msb = 0;
+             else
+                     msb = BTF_INT_BITS(v) - 1;
+             *retval &= GENMASK(msb, 0);
+             break;
+     default:
+             break;
+     }
+}

Hmm, I think we should have another flag which directly tells the print
format of retval from BTF.

Thanks, I thought about it and the format flag could have the following values:

1. HEX - hexadecimal (%lx for pointers/unsigned)
2. DEC - decimal (%ld for signed/error_code)
3. BOOL - boolean strings ("true"/"false")

WDYT?

Thanks,
Donglin

quoted

+
 static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entry *entry,
                              struct ftrace_graph_ret *graph_ret, void *func,
                              u32 opt_flags, u32 trace_flags, int args_size)

@@ -884,17 +925,20 @@ 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;

The original code just guesses the type (signed/unsigned) from
the retval, but we don't need to do that with BTF.

Thanks. I will fix it in the next version.

Thanks,
Donglin

Thank you,

quoted

+     if (print_retval) {
+             trim_retval((unsigned long)func, &retval, &print_retval);
+             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 (!IS_ERR_VALUE(err_code))
+                             err_code = 0;
              }
-
-             if (!IS_ERR_VALUE(err_code))
-                     err_code = 0;
      }

      if (entry) {
--
2.34.1

--
Masami Hiramatsu (Google) [off-list ref]