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 <[email protected]>

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/[email protected]?part=8

Reply via email to