Re: [PATCH perf-tools-next v12 06/10] perf ftrace: Support display of inlined functions in function graph tracer

[Aaron Tomlin]

On Sat, Oct 31, 2026 at 12:11:34AM -0700, Namhyung Kim wrote:
> > @@ -664,11 +834,16 @@ static void extract_exit_func_name(const char *str, char *buf, size_t size)
> >   * @cpu: Pointer to store the CPU number
> >   *
> >   * Locate the context columns that precede the function call column of a
> > - * function_graph trace record.
> > + * function_graph trace record. Depending on the funcgraph-* options set
> > + * up by perf, a record looks like:
> > + *
> > + *   [ abstime |]  CPU)  [ comm-pid |]  [ duration |]  func
> > + *
> > + * The abstime and comm-pid columns are only present with --graph-opts
> > + * verbose.
> >   *
> >   * Return: A pointer to the last character of the context columns (the final
> > - * '|' delimiter, or the CPU ')' when no delimiter follows it). Returns NULL
> > - * if @line is not a trace record.
> > + * '|' delimiter). Returns NULL if @line is not a trace record.
> >   */
> 
> It'd be nice if you could move the comments to the original commit that
> introduced the function.

Agreed. This was an unintended artifact from an earlier rebase. I will move
the comment to patch 04/10 where find_func_column() is introduced.

> > diff --git a/tools/perf/util/ftrace.h b/tools/perf/util/ftrace.h
> > index 45f5090d0115..7b9e0919ae8d 100644
> > --- a/tools/perf/util/ftrace.h
> > +++ b/tools/perf/util/ftrace.h
> > @@ -42,6 +42,8 @@ struct perf_ftrace {
> >  	int			graph_verbose;
> >  	int			graph_thresh;
> >  	int			graph_tail;
> > +	bool			use_inline;
> > +	struct machine		*machine;
> >  };
> >  
> >  struct filter_entry {
> > @@ -96,44 +98,18 @@ perf_ftrace__latency_cleanup_bpf(struct perf_ftrace *ftrace __maybe_unused)
> >  
> >  #endif  /* HAVE_BPF_SKEL */
> >  
> > -#define FTRACE_MAX_CPUS 4096
> > -#define FTRACE_MAX_STACK 256
> > -#define FTRACE_PID_HASH_BITS 10
> > -#define FTRACE_PID_HASH_SIZE (1 << FTRACE_PID_HASH_BITS)
> > +struct machine;
> >  
> > -#define FTRACE_IDLE_PID(cpu)       ((pid_t)(-1 - (cpu)))
> > -#define FTRACE_UNKNOWN_PID(cpu)    ((pid_t)(-1 - FTRACE_MAX_CPUS - (cpu)))
> > -
> > -struct ftrace_stack_frame {
> > -	char *name;
> > -	bool inlined;
> > -	int indent;
> > -	int raw_indent;
> > -	u64 seq;
> > -};
> > -
> > -struct ftrace_indent_watermark {
> > -	u64 seq;
> > -	int raw_indent;
> > -};
> > -
> > -struct ftrace_task_state {
> > -	struct hlist_node	node;
> > -	pid_t			pid;
> > -	struct ftrace_stack_frame *stack;
> > -	int			max_stack;
> > -	int			depth;
> > -	int			inlined_depth;
> > -	int			overflow_depth;
> > -	u64			seq;
> > -	struct ftrace_indent_watermark *watermarks;
> > -	int			max_watermarks;
> > -	int			nr_watermarks;
> > -};
> 
> Hmm.. probably better to define the structs in the .c file in the first
> place?
> 
> Thanks,
> Namhyung

Agreed. Since these structures and macros are internal to
tools/perf/util/ftrace.c, I will define them directly in ftrace.c in patch
04/10 from the start.


Kind regards,
-- 
Aaron Tomlin