Re: [PATCH perf-tools-next v12 04/10] perf ftrace: Add basic task call stack management

[Namhyung Kim <namhyung@kernel.org>]

On Sun, Oct 25, 2026 at 10:57:29PM -0400, Aaron Tomlin wrote:
> The function_graph tracer records kernel function entries and exits with
> nested indentation. To perform call graph analysis and subsequent inlined
> function reconstruction, userspace must track the active call stack on a
> per-task and per-CPU basis.
> 
> Introduce basic call stack management for the function graph tracer:
> 
>     1.  Data structures
> 
>         Define struct ftrace_stack_frame to track function names,
>         indentation, and inlined status.
> 
>         Define struct ftrace_task_state with a dynamically resisable call
>         stack array, tracked in a hash table keyed by PID alongside per-CPU
>         active task bindings (ftrace_cpu_tasks).
> 
>     2.  Trace stream processing
> 
>         Implement find_func_column() to isolate context columns (CPU,
>         timestamps, duration) from function graph trace lines across
>         standard and verbose layouts.
> 
>         Implement ftrace_process_fgraph_line() to parse function entries
>         ("{") and exits ("}"), pushing and popping frames on the tracked
>         stack and matching bare closing braces against preceding frames.
> 
>     3.  Integration in __cmd_ftrace()
> 
>         Stream complete trace lines through ftrace_process_fgraph_line()
>         in __cmd_ftrace() and clean up allocated CPU states on exit via
>         ftrace_cpu_states__clear().
> 
> Tested-by: Ian Rogers <irogers@google.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
[SNIP]
> +/**
> + * ftrace_process_fgraph_line - Process and output a function graph trace line
> + * @ftrace: Perf ftrace context
> + * @line: The trace line to process
> + * @out: File stream to output to
> + *
> + * Parse function entries and exits, maintain the task's call stack, and
> + * output the trace line.
> + *
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FILE *out)
> +{
> +	const char *pipe_char;
> +	const char *after_pipe;
> +	const char *func_start;
> +	struct ftrace_task_state *cs;
> +	int cpu = -1;
> +	int leading_spaces;
> +	int top_raw_indent;
> +	int match_idx;
> +	int i;
> +	bool is_exit;
> +	bool has_open_brace;
> +	bool has_semicolon;
> +
> +	pipe_char = find_func_column(ftrace, line, &cpu);
> +	if (!pipe_char)
> +		return print_line(ftrace, line, out);
> +
> +	cs = get_cpu_state(ftrace, cpu);
> +	if (!cs)
> +		return print_line(ftrace, line, out);
> +
> +	after_pipe = pipe_char + 1;
> +	func_start = skip_spaces(after_pipe);
> +	leading_spaces = func_start - after_pipe;
> +
> +	if (*func_start == '\0')
> +		return print_line(ftrace, line, out);
> +
> +	is_exit = (*func_start == '}');
> +
> +	if (is_exit) {
> +		char exit_name[128];
> +
> +		extract_exit_func_name(func_start, exit_name, sizeof(exit_name));
> +
> +		top_raw_indent = -1;
> +		for (i = cs->depth - 1; i >= 0; i--) {
> +			top_raw_indent = cs->stack[i].raw_indent;
> +			break;
> +		}

This loop doesn't look natural.  Why not just use an if statement?

> +
> +		if (cs->overflow_depth > 0 &&
> +		    (top_raw_indent < 0 || leading_spaces > top_raw_indent)) {
> +			cs->overflow_depth--;
> +			return print_line(ftrace, line, out);
> +		}
> +
> +		match_idx = -1;
> +		if (exit_name[0] != '\0') {
> +			for (i = cs->depth - 1; i >= 0; i--) {
> +				if (cs->stack[i].name &&
> +				    cs->stack[i].raw_indent == leading_spaces &&
> +				    strcmp(cs->stack[i].name, exit_name) == 0) {
> +					match_idx = i;
> +					break;
> +				}
> +			}
> +		}
> +
> +		if (match_idx < 0 && exit_name[0] == '\0') {
> +			for (i = cs->depth - 1; i >= 0; i--) {
> +				if (cs->stack[i].raw_indent == leading_spaces) {
> +					match_idx = i;
> +					break;
> +				}
> +			}
> +		}
> +
> +		if (match_idx >= 0) {
> +			cs->overflow_depth = 0;
> +			while (cs->depth > match_idx) {
> +				free(cs->stack[cs->depth - 1].name);
> +				cs->depth--;
> +			}
> +		} else if (cs->overflow_depth > 0) {
> +			cs->overflow_depth--;
> +		}
> +
> +		return print_line(ftrace, line, out);
> +	}
> +
> +	has_open_brace = (strchr(func_start, '{') != NULL);
> +	has_semicolon = (strchr(func_start, ';') != NULL);
> +
> +	top_raw_indent = -1;
> +	for (i = cs->depth - 1; i >= 0; i--) {
> +		top_raw_indent = cs->stack[i].raw_indent;
> +		break;
> +	}

Ditto.

> +
> +	if (cs->overflow_depth > 0 && top_raw_indent >= 0 &&
> +	    leading_spaces <= top_raw_indent)
> +		cs->overflow_depth = 0;
> +
> +	if (has_open_brace && !has_semicolon) {
> +		char entry_name[128];
> +
> +		extract_entry_func_name(func_start, entry_name, sizeof(entry_name));
> +		if (cs->depth >= cs->max_stack &&
> +		    ftrace_task_state__grow_stack(cs) < 0) {
> +			cs->overflow_depth++;
> +		} else {
> +			cs->stack[cs->depth].name = strdup(entry_name);
> +			if (!cs->stack[cs->depth].name) {
> +				pr_err("Not enough memory\n");
> +				return -ENOMEM;
> +			}
> +			cs->stack[cs->depth].inlined = false;
> +			cs->stack[cs->depth].indent = leading_spaces;
> +			cs->stack[cs->depth].raw_indent = leading_spaces;
> +			cs->depth++;
> +		}
> +	}
> +
> +	return print_line(ftrace, line, out);
> +}
> diff --git a/tools/perf/util/ftrace.h b/tools/perf/util/ftrace.h
> index a3a679f74e9d..fa2974f46582 100644
> --- a/tools/perf/util/ftrace.h
> +++ b/tools/perf/util/ftrace.h
> @@ -1,6 +1,9 @@
>  #ifndef __PERF_FTRACE_H__
>  #define __PERF_FTRACE_H__
>  
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <sys/types.h>
>  #include <linux/list.h>
>  
>  #include "target.h"
> @@ -93,7 +96,34 @@ 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)
> +
> +#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;

Can you please add comments that explain why it has two indents?
Also inlined field may be added later.


> +};
> +
> +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;

Ditto.  What are these depths?

Thanks,
Namhyung


> +};
> +
>  bool ftrace_parse_retaddr(const char *str, char *sym_name, size_t sym_len, u64 *offset);
>  void ftrace_filter_retaddr(char *str);
> +void ftrace_cpu_states__clear(void);
> +int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FILE *out);
>  
>  #endif  /* __PERF_FTRACE_H__ */
> -- 
> 2.55.0
>