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

On Tue, Dec 9, 2025 at 9:42 AM Donglin Peng [off-list ref] wrote:

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

quoted

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

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.

Sorry, I think the above guessing process is necessary to display an error
code. In most cases, the return type is int, but retval is of size
sizeof(unsigned long).
We should perform sign extension to check whether it represents an error code.

Thanks,
Donglin

Thanks. I will fix it in the next version.

Thanks,
Donglin

quoted

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]