[PATCH 0/1] tracing/boot: Add alloc_snapshot for instance option

[PATCH 0/1] tracing/boot: Add alloc_snapshot for instance option

[Masami Hiramatsu]

Hi,

Here is a patch to add ftrace[.instance.INSTANCE].alloc_snapshot option
to allocate snapshot buffer for specific isntance. Actually, this has
been described in Documentation/trace/boottime-trace.rst but I forgot
to implement it. (Maybe I confused it with kernel.alloc_snapshot)

Anyway, it is better to have this option with currnet ftrace implementation,
because if user sets ftrace.instance.X.event.*.*.actions = "snapshot" and
ftrace.instance.X.tracer at same bootconfig, the snapshot buffer always
removed (even if the tracer is not a max-tracer).
This seems buggy, but I'm not sure the reason why.

So if someone wants to use snapshot event action with e.g. function tracer,
they must use this option. For example,

ftrace.instance.foo {
	tracer = function
	event.signal.signal_deliver {
		enable
		actions = "snapshot if sig = 7"
	}
	alloc_snapshot
}

Steve and Tom, would you know why tracing_set_tracer() removes
snapshot buffer even if the tracer is not a max-tracer?

int tracing_set_tracer(struct trace_array *tr, const char *buf)
{
...
#ifdef CONFIG_TRACER_MAX_TRACE
        had_max_tr = tr->allocated_snapshot;

        if (had_max_tr && !t->use_max_tr) {
                /*
                 * We need to make sure that the update_max_tr sees that
                 * current_trace changed to nop_trace to keep it from
                 * swapping the buffers after we resize it.
                 * The update_max_tr is called from interrupts disabled
                 * so a synchronized_sched() is sufficient.
                 */
                synchronize_rcu();
                free_snapshot(tr);
        }
#endif
}

It seems had_max_tr is true even if the previous tracer is nop and
snapshot buffer was traced by snapshot trigger action.

This actually causes a problem except for the boot time tracing,
for example,

1) boot kernel with nop tracer
2) add snapshot trigger action to an event
   (snapshot buffer is allocated here)
3) set function tracer to current tracer
   (snapshot buffer is freed)
4) run the tracer
   -> the event trigger can not take a snapshot.


Thank you,

---

Masami Hiramatsu (1):
      tracing/boot: Add ftrace.instance.*.alloc_snapshot option


 kernel/trace/trace_boot.c |    6 ++++++
 1 file changed, 6 insertions(+)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/1] tracing/boot: Add ftrace.instance.*.alloc_snapshot option

[Masami Hiramatsu]

Add ftrace.instance.*.alloc_snapshot option.

This option has been described in Documentation/trace/boottime-trace.rst
but not implemented yet.

ftrace.[instance.INSTANCE.]alloc_snapshot
   Allocate snapshot buffer.

The difference from kernel.alloc_snapshot is that the kernel.alloc_snapshot
will allocate the buffer only for the main instance, but this can allocate
buffer for any new instances.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_boot.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 754e3cf2df3a..c22a152ef0b4 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -284,6 +284,12 @@ trace_boot_enable_tracer(struct trace_array *tr, struct xbc_node *node)
 		if (tracing_set_tracer(tr, p) < 0)
 			pr_err("Failed to set given tracer: %s\n", p);
 	}
+
+	/* Since tracer can free snapshot buffer, allocate snapshot here.*/
+	if (xbc_node_find_value(node, "alloc_snapshot", NULL)) {
+		if (tracing_alloc_snapshot_instance(tr) < 0)
+			pr_err("Failed to allocate snapshot buffer\n");
+	}
 }
 
 static void __init

Re: [PATCH 1/1] tracing/boot: Add ftrace.instance.*.alloc_snapshot option

[Steven Rostedt]

On Sun, 11 Oct 2020 00:28:09 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Add ftrace.instance.*.alloc_snapshot option.
> 
> This option has been described in Documentation/trace/boottime-trace.rst
> but not implemented yet.
> 
> ftrace.[instance.INSTANCE.]alloc_snapshot
>    Allocate snapshot buffer.
> 
> The difference from kernel.alloc_snapshot is that the kernel.alloc_snapshot
> will allocate the buffer only for the main instance, but this can allocate
> buffer for any new instances.

I queued this up. Thanks Masami!

-- Steve

> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_boot.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index 754e3cf2df3a..c22a152ef0b4 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -284,6 +284,12 @@ trace_boot_enable_tracer(struct trace_array *tr, struct xbc_node *node)
>  		if (tracing_set_tracer(tr, p) < 0)
>  			pr_err("Failed to set given tracer: %s\n", p);
>  	}
> +
> +	/* Since tracer can free snapshot buffer, allocate snapshot here.*/
> +	if (xbc_node_find_value(node, "alloc_snapshot", NULL)) {
> +		if (tracing_alloc_snapshot_instance(tr) < 0)
> +			pr_err("Failed to allocate snapshot buffer\n");
> +	}
>  }
>  
>  static void __init