Re: [PATCH] tracing: Do not free snapshot if tracer is on cmdline

[Google]

On Tue, 4 Oct 2022 18:04:52 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
> 
> The ftrace_boot_snapshot and alloc_snapshot cmdline options allocate the
> snapshot buffer at boot up for use later. The ftrace_boot_snapshot in
> particular requires the snapshot to be allocated because it will take a
> snapshot at the end of boot up allowing to see the traces that happened
> during boot so that it's not lost when user space takes over.
> 
> When a tracer is registered (started) there's a path that checks if it
> requires the snapshot buffer or not, and if it does not and it was
> allocated it will do a synchronization and free the snapshot buffer.
> 
> This is only required if the previous tracer was using it for "max
> latency" snapshots, as it needs to make sure all max snapshots are
> complete before freeing. But this is only needed if the previous tracer
> was using the snapshot buffer for latency (like irqoff tracer and
> friends). But it does not make sense to free it, if the previous tracer
> was not using it, and the snapshot was allocated by the cmdline
> parameters. This basically takes away the point of allocating it in the
> first place!
> 
> Note, the allocated snapshot worked fine for just trace events, but fails
> when a tracer is enabled on the cmdline.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 55034cd6e6481 ("tracing: Add alloc_snapshot kernel command line parameter")
> Reported-by: Ross Zwisler <zwisler@xxxxxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

> ---
>  kernel/trace/trace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index def721de68a0..871e2b592969 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6432,7 +6432,7 @@ int tracing_set_tracer(struct trace_array *tr, const char *buf)
>  	tr->current_trace = &nop_trace;
>  
>  #ifdef CONFIG_TRACER_MAX_TRACE
> -	had_max_tr = tr->allocated_snapshot;
> +	had_max_tr = tr->current_trace->use_max_tr;
>  
>  	if (had_max_tr && !t->use_max_tr) {
>  		/*
> -- 
> 2.35.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>