Re: [PATCH bpf-next 08/13] tracing: parse the vmlinux BTF when BTF-typed output is enabled

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] tracing: missing trace_btf_prepare() in irqsoff and wakeup tracers
- [Medium] tracing: missing trace_btf_prepare() for persistent buffers and latency tracers
--

commit b7e170f2972f546cc8806104f942bd820ff8ecbb
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

tracing: parse the vmlinux BTF when BTF-typed output is enabled

This commit prepares the vmlinux BTF when BTF-typed function or function_graph
tracer output is enabled or when reading an event's btf_ids file, and changes
print_function_args() and btf_trim_retval() to peek at the vmlinux BTF rather
than parsing it on demand while holding trace read locks.

> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index a5ad76175d10d..cdb926c9d4e21 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -739,6 +739,9 @@ void print_function_args(struct trace_seq *s, unsigned long *args,
>  	if (lookup_symbol_name(func, name))
>  		goto out;
>  
> +	/* see trace_btf_prepare() */
> +	if (IS_ERR_OR_NULL(bpf_peek_btf_vmlinux()))
> +		goto out;

[Severity: Medium]
Can this cause recorded function arguments to be omitted (printed as empty
parentheses) for latency tracers and persistent boot ring buffers when
CONFIG_DEBUG_INFO_BTF_LAZY is enabled and nothing else has triggered vmlinux
BTF parsing yet?

First, when CONFIG_FUNCTION_TRACE_ARGS is enabled, the irqsoff, preemptoff,
preemptirqsoff, wakeup, wakeup_rt, and wakeup_dl latency tracers record
function arguments via fregs into TRACE_FN entries since commit 76fe0337c219
("ftrace: Add arguments to function tracer"):

kernel/trace/trace_irqsoff.c:irqsoff_tracer_call() {
	...
	trace_function(tr, ip, parent_ip, trace_ctx, fregs);
}

kernel/trace/trace_sched_wakeup.c:wakeup_tracer_call() {
	...
	local_irq_save(flags);
	trace_function(tr, ip, parent_ip, trace_ctx, fregs);
	local_irq_restore(flags);
}

When reading trace or trace_pipe in default (non-graph) mode,
irqsoff_print_line() and wakeup_print_line() return TRACE_TYPE_UNHANDLED,
which dispatches TRACE_FN entries through trace_fn_trace() ->
print_fn_trace() -> print_function_args():

kernel/trace/trace_output.c:trace_fn_trace() {
	...
	args_size = iter->ent_size - offsetof(struct ftrace_entry, args);
	if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long))
		args = field->args;
	else
		args = NULL;

	print_fn_trace(s, field->ip, field->parent_ip, args, iter->tr, flags);
}

However, neither __irqsoff_tracer_init() / register_irqsoff_function() nor
__wakeup_tracer_init() / register_wakeup_function() calls trace_btf_prepare().

Second, when a persistent trace_array instance (TRACE_ARRAY_FL_LAST_BOOT) is
set up across boots via trace_instance= on the command line (using reserve_mem
or backup= in enable_instances()), allocate_trace_buffer() and
setup_trace_scratch() preserve the ring buffer events and address deltas from
the previous boot while the instance starts in the new boot with nop_trace and
default tracer flags. Selecting a tracer in the new boot via
tracing_set_tracer() calls update_last_data(tr), which resets the ring buffer
and clears the previous boot data (and backup= instances are marked
TRACE_ARRAY_FL_RDONLY):

kernel/trace/trace.c:tracing_set_tracer() {
	...
	guard(mutex)(&trace_types_lock);

	update_last_data(tr);
	...
}

When reading trace or trace_pipe from that persistent instance to inspect the
previous boot's TRACE_FN or TRACE_GRAPH_ENT entries, print_trace_fmt()
dispatches them to trace_fn_trace() -> print_fn_trace() ->
print_function_args() or print_graph_function_event() ->
print_graph_entry_leaf() / print_graph_entry_nested() ->
print_function_args() without trace_btf_prepare() having been called in the
current boot:

kernel/trace/trace_output.c:print_fn_trace() {
	...
	ip = trace_adjust_address(tr, ip);
	parent_ip = trace_adjust_address(tr, parent_ip);

	seq_print_ip_sym(s, ip, flags);
	if (args)
		print_function_args(s, args, ip);
	...
}

Should trace_btf_prepare() also be called when initializing the irqsoff and
wakeup tracers and when setting up a persistent last-boot trace buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031130959.23472-1-wanjay@amazon.com?part=8