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

Steven Rostedt
2024-12-25T22:02:27Z

On Thu, 26 Dec 2024 00:41:52 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> 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