On Mon, 8 Dec 2025 21:19:16 +0800 Donglin Peng <[email protected]> wrote:
> From: pengdonglin <[email protected]> > > 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) <[email protected]> > Cc: Masami Hiramatsu <[email protected]> > Cc: Xiaoqin Zhang <[email protected]> > Signed-off-by: pengdonglin <[email protected]> > --- > 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) <[email protected]>
