Skip to contentSkip to conversation

tracing: Fix ftrace_boot_snapshot command line

PATCH New Steven Rostedt 1 participant 3 messages Apr 5, 2023

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

v2 cover tracing: Fix ftrace_boot_snapshot command line

While debugging some boot up code, I found that the snapshots were constantly triggering at boot up, even though there was no boot snapshot specified. Looking into it, I found there were too bugs.

1) It would trigger a snapshot on any instance if one was created from the kernel command line.

2) The error handling would only affect the top level instance. So the fact that a snapshot was done on a instance that didn't allocate a buffer triggered a warning written into the top level buffer, and worse yet, disabled the top level buffer.

Changes since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/

- My tests failed due to tr->allocated_snapshot only being a field when CONFIG_TRACE_MAX_TRACE is defined. And it doesn't make sense doing any of that logic if it is not, as snapshots depend on that config.

This addresses both of the above bugs.

Steven Rostedt (Google) (2):
tracing: Have tracing_snapshot_instance_cond() write errors to the appropriate instance
tracing: Fix ftrace_boot_snapshot command line logic

----

 kernel/trace/trace.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
author maintainer
#1 lore raw

PATCH v2 1/2 tracing: Have tracing_snapshot_instance_cond() write errors to the appropriate instance

From: "Steven Rostedt (Google)" <[email protected]>

If a trace instance has a failure with its snapshot code, the error message is to be written to that instance's buffer. But currently, the message is written to the top level buffer. Worse yet, it may also disable the top level buffer and not the instance that had the issue.

Fixes 2824f50332486 ("tracing: Make the snapshot trigger work with instances")
---
 kernel/trace/trace.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
kernel/trace/trace.c +7 -7
index 0a7ea02c9f08..93740a9370c6 100644
@@ -1149,22 +1149,22 @@ static void tracing_snapshot_instance_cond(struct trace_array *tr,
 	unsigned long flags;
 
 	if (in_nmi()) {
-		internal_trace_puts("*** SNAPSHOT CALLED FROM NMI CONTEXT ***\n");
-		internal_trace_puts("*** snapshot is being ignored        ***\n");
+		trace_array_puts(tr, "*** SNAPSHOT CALLED FROM NMI CONTEXT ***\n");
+		trace_array_puts(tr, "*** snapshot is being ignored        ***\n");
 		return;
 	}
 
 	if (!tr->allocated_snapshot) {
-		internal_trace_puts("*** SNAPSHOT NOT ALLOCATED ***\n");
-		internal_trace_puts("*** stopping trace here!   ***\n");
-		tracing_off();
+		trace_array_puts(tr, "*** SNAPSHOT NOT ALLOCATED ***\n");
+		trace_array_puts(tr, "*** stopping trace here!   ***\n");
+		tracer_tracing_off(tr);
 		return;
 	}
 
 	/* Note, snapshot can not be used when the tracer uses it */
 	if (tracer->use_max_tr) {
-		internal_trace_puts("*** LATENCY TRACER ACTIVE ***\n");
-		internal_trace_puts("*** Can not use snapshot (sorry) ***\n");
+		trace_array_puts(tr, "*** LATENCY TRACER ACTIVE ***\n");
+		trace_array_puts(tr, "*** Can not use snapshot (sorry) ***\n");
 		return;
 	}
 
Signature · 1 line
2.39.2
author maintainer
#2 lore raw

PATCH v2 2/2 tracing: Fix ftrace_boot_snapshot command line logic

In reply to Steven Rostedt

From: "Steven Rostedt (Google)" <[email protected]>

The kernel command line ftrace_boot_snapshot by itself is supposed to trigger a snapshot at the end of boot up of the main top level trace buffer. A ftrace_boot_snapshot=foo will do the same for an instance called foo that was created by trace_instance=foo,...

The logic was broken where if ftrace_boot_snapshot was by itself, it would trigger a snapshot for all instances that had tracing enabled, regardless if it asked for a snapshot or not.

When a snapshot is requested for a buffer, the buffer's tr->allocated_snapshot is set to true. Use that to know if a trace buffer wants a snapshot at boot up or not.

Since the top level buffer is part of the ftrace_trace_arrays list, there's no reason to treat it differently than the other buffers. Just iterate the list if ftrace_boot_snapshot was specified.

Fixes 9c1c251d670bc ("tracing: Allow boot instances to have snapshot buffers")

---
Changes since v1: https://lkml.kernel.org/r/[email protected]

- Protect use of tr->allocated_snapshot around #ifdef TRACER_MAX_TRACE

 kernel/trace/trace.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
kernel/trace/trace.c +7 -6
index 93740a9370c6..36a6037823cd 100644
@@ -10394,19 +10394,20 @@ __init static int tracer_alloc_buffers(void)
 
 void __init ftrace_boot_snapshot(void)
 {
+#ifdef CONFIG_TRACER_MAX_TRACE
 	struct trace_array *tr;
 
-	if (snapshot_at_boot) {
-		tracing_snapshot();
-		internal_trace_puts("** Boot snapshot taken **\n");
-	}
+	if (!snapshot_at_boot)
+		return;
 
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (tr == &global_trace)
+		if (!tr->allocated_snapshot)
 			continue;
-		trace_array_puts(tr, "** Boot snapshot taken **\n");
+
 		tracing_snapshot_instance(tr);
+		trace_array_puts(tr, "** Boot snapshot taken **\n");
 	}
+#endif
 }
 
 void __init early_trace_init(void)
Signature · 1 line
2.39.2