From: pengdonglin <pengdonglin@xiaomi.com>
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 <zhangxiaoqin@xiaomi.com>
Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
---
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;
+ }
+}
+
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;
+ 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.1On Mon, 8 Dec 2025 21:19:16 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> From: pengdonglin <pengdonglin@xiaomi.com>
>
> 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 <zhangxiaoqin@xiaomi.com>
> Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
> ---
> 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.
> +
> 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.
Thank you,
> + 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) <mhiramat@kernel.org>
On Tue, Dec 9, 2025 at 8:48 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Mon, 8 Dec 2025 21:19:16 +0800
> Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> > From: pengdonglin <pengdonglin@xiaomi.com>
> >
> > 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 <zhangxiaoqin@xiaomi.com>
> > Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
> > ---
> > 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
>
> > +
> > 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,
>
> > + 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) <mhiramat@kernel.org>
On Tue, Dec 9, 2025 at 9:42 AM Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> On Tue, Dec 9, 2025 at 8:48 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Mon, 8 Dec 2025 21:19:16 +0800
> > Donglin Peng <dolinux.peng@gmail.com> wrote:
> >
> > > From: pengdonglin <pengdonglin@xiaomi.com>
> > >
> > > 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 <zhangxiaoqin@xiaomi.com>
> > > Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
> > > ---
> > > 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
> >
> > > +
> > > 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
> >
> > Thank you,
> >
> > > + 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) <mhiramat@kernel.org>
© 2016 - 2025 Red Hat, Inc.