Re: [PATCH v2 2/4] ftrace: Add support for function argument to graph tracer

[Steven Rostedt]

On Thu, 26 Dec 2024 00:41:52 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:
> > diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> > index d0e4f412c298..c8eda9bebdf4 100644
> > --- a/kernel/trace/trace_functions_graph.c
> > +++ b/kernel/trace/trace_functions_graph.c
> > @@ -12,6 +12,8 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/slab.h>
> >  #include <linux/fs.h>
> > +#include <linux/btf.h>
> > +#include <linux/bpf.h>  
> 
> Do we need these headers? I think it is wrapped by print_function_args().

Oh, probably not. This is just leftovers from the original patch.

> > @@ -814,7 +853,14 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> >  		if (entry->ent.type != TRACE_GRAPH_RETADDR_ENT)
> >  			print_retaddr = false;
> >  
> > -		trace_seq_printf(s, "%ps();", func);
> > +		trace_seq_printf(s, "%ps", func);
> > +
> > +		if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
> > +			print_function_args(s, entry->args, (unsigned long)func);
> > +			trace_seq_putc(s, ';');
> > +		} else
> > +			trace_seq_puts(s, "();");
> > +
> >  		if (print_retval || print_retaddr)
> >  			trace_seq_puts(s, " /*");
> >  		else
> > @@ -836,12 +882,13 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> >  	}
> >  
> >  	if (!entry || print_retval || print_retaddr)
> > -		trace_seq_puts(s, " */\n");
> > +		trace_seq_puts(s, " */");  
> 
> Do we need this change?

Hmm, maybe not. It may have been caused by added (and removed) debugging.

Thanks for the review.

-- Steve