Skip to contentSkip to conversation

tracing: export event trace and trace_marker

PATCH Superseded Tingwei Zhang 2 participants 10 messages Jul 20, 2020 – Jul 21, 2020

Subsystems (4)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover tracing: export event trace and trace_marker

Ftrace has ability to export trace packets to other destionation. Currently, only function trace can be exported. This series extends the support to event trace and trace_maker. STM is one possible destination to export ftrace. Use seperate channel for each CPU to avoid mixing up packets from different CPUs together.

Tingwei Zhang (4):
stm class: ftrace: change dependency to TRACING
tracing: add trace_export support for event trace
tracing: add trace_export support for trace_marker stm class: ftrace: use different channel accroding to CPU

 drivers/hwtracing/stm/Kconfig  |  2 +-
 drivers/hwtracing/stm/ftrace.c |  5 +++--
 kernel/trace/trace.c           | 26 +++++++++++++++-----------
 3 files changed, 19 insertions(+), 14 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

PATCH 1/4 stm class: ftrace: change dependency to TRACING

We will support copying event trace to STM. Change STM_SOURCE_FTRACE to depend on TRACING since we will support multiple tracers.

---
 drivers/hwtracing/stm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwtracing/stm/Kconfig +1 -1
index d0e92a8a045c..aad594fe79cc 100644
@@ -71,7 +71,7 @@ config STM_SOURCE_HEARTBEAT
 
 config STM_SOURCE_FTRACE
 	tristate "Copy the output from kernel Ftrace to STM engine"
-	depends on FUNCTION_TRACER
+	depends on TRACING
 	help
 	  This option can be used to copy the output from kernel Ftrace
 	  to STM engine. Enabling this option will introduce a slight
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#2 lore raw

PATCH 2/4 tracing: add trace_export support for event trace

In reply to Tingwei Zhang

Only function traces can be exported to other destinations currently. This patch exports event trace as well.

---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
kernel/trace/trace.c +13 -11
index bb62269724d5..aef6330836e2 100644
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct trace_export *export)
 }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);
+	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
+}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned long flags,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#3 lore raw

Re: [PATCH 2/4] tracing: add trace_export support for event trace

Quoted 17 lines · On Mon, 20 Jul 2020 10:21:15 +0800 Tingwei Zhang <[email protected]> wrote:

Only function traces can be exported to other destinations currently. This patch exports event trace as well.

---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
kernel/trace/trace.c +0 -1
index bb62269724d5..aef6330836e2 100644
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write, · line 2710

Quoted 10 lines
kernel/trace/trace.c +0 -10
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-

Please move the ftrace_exports routines up, instead of moving the trace_event_buffer_commit() down. As it fits better where it is (next to the other buffer_commit code).

-- Steve

Quoted 25 lines
kernel/trace/trace.c +12 -0
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
 trace_export *export) }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);
+	event_trigger_unlock_commit_regs(fbuffer->trace_file,
 fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc,
 fbuffer->regs); +}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned

long flags,

maintainer
#4 lore raw

Re: [PATCH 2/4] tracing: add trace_export support for event trace

Quoted 45 lines · On Tue, 21 Jul 2020 12:37:33 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 17 lines · On Mon, 20 Jul 2020 10:21:15 +0800 Tingwei Zhang <[email protected]> wrote:

Only function traces can be exported to other destinations currently. This patch exports event trace as well.

---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
kernel/trace/trace.c +0 -1
index bb62269724d5..aef6330836e2 100644
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write, · line 2710

Quoted 10 lines
kernel/trace/trace.c +0 -10
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-  

Please move the ftrace_exports routines up, instead of moving the trace_event_buffer_commit() down. As it fits better where it is (next to the other buffer_commit code).

-- Steve

Quoted 7 lines
kernel/trace/trace.c +0 -0
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
 trace_export *export) }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
Quoted 7 lines

kernel/trace/trace.c int unregister_ftrace_export(struct trace_export *export) · line 2866

Quoted 7 lines
kernel/trace/trace.c +7 -0
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);

I would also recommend making each static branch a separate variable. That way you could pick and choose what you want to enable. If you only want events, and functions are being traced, it will add a bit of overhead to handle the functions to just ignore them.

That is:

ftrace_exports_enabled, trace_event_exports_enabled, trace_marker_exports_enabled.

-- Steve

Quoted 12 lines
Quoted 11 lines
kernel/trace/trace.c +5 -0
+	event_trigger_unlock_commit_regs(fbuffer->trace_file,
 fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc,
 fbuffer->regs); +}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned

long flags,

#5 lore raw

Re: [PATCH 2/4] tracing: add trace_export support for event trace

Quoted 64 lines · Steven Rostedt
Quoted 11 lines · On Tue, 21 Jul 2020 12:37:33 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 8 lines · On Mon, 20 Jul 2020 10:21:15 +0800 Tingwei Zhang <[email protected]> wrote:

Only function traces can be exported to other destinations currently. This patch exports event trace as well.

---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
Quoted 5 lines

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table

Quoted 5 lines
kernel/trace/trace.c +0 -0
index bb62269724d5..aef6330836e2 100644
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table

*table, int write,

Quoted 9 lines

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write,

Quoted 9 lines
kernel/trace/trace.c +0 -6
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file,

fbuffer->buffer,

Quoted 2 lines

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table · line 2707

Quoted 2 lines
kernel/trace/trace.c +0 -2
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc,

fbuffer->regs);

Quoted 18 lines

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write, · line 2710

Quoted 3 lines
kernel/trace/trace.c +0 -3
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-

Please move the ftrace_exports routines up, instead of moving the trace_event_buffer_commit() down. As it fits better where it is (next to the other buffer_commit code).

-- Steve

Quoted 7 lines
kernel/trace/trace.c +0 -0
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
 trace_export *export) }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
Quoted 7 lines

kernel/trace/trace.c int unregister_ftrace_export(struct trace_export *export) · line 2866

Quoted 7 lines
kernel/trace/trace.c +7 -0
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);

I would also recommend making each static branch a separate variable. That way you could pick and choose what you want to enable. If you only want events, and functions are being traced, it will add a bit of overhead to handle the functions to just ignore them.

Quoted 7 lines

That is:

ftrace_exports_enabled, trace_event_exports_enabled, trace_marker_exports_enabled.

-- Steve

That's a good idea. Let me add that to next revision.

Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
kernel/trace/trace.c +5 -0
+	event_trigger_unlock_commit_regs(fbuffer->trace_file,
 fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc,
 fbuffer->regs); +}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned

long flags,

#6 lore raw

Re: [PATCH 2/4] tracing: add trace_export support for event trace

In reply to Steven Rostedt
Quoted 38 lines · Steven Rostedt

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table

Quoted 13 lines · On Mon, 20 Jul 2020 10:21:15 +0800 Tingwei Zhang <[email protected]> wrote:

Only function traces can be exported to other destinations currently. This patch exports event trace as well.

---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
kernel/trace/trace.c +0 -0
index bb62269724d5..aef6330836e2 100644
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table

*table, int write,

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write,

Quoted 9 lines
kernel/trace/trace.c +0 -6
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file,

fbuffer->buffer,

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table · line 2707

Quoted 2 lines
kernel/trace/trace.c +0 -2
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc,

fbuffer->regs);

kernel/trace/trace.c int tracepoint_printk_sysctl(struct ctl_table *table, int write, · line 2710

Quoted 3 lines
kernel/trace/trace.c +0 -3
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-

Please move the ftrace_exports routines up, instead of moving the trace_event_buffer_commit() down. As it fits better where it is (next to the other buffer_commit code).

Quoted 2 lines

-- Steve

Thanks, Steve. I'll fix this.

Quoted 26 lines
Quoted 26 lines
kernel/trace/trace.c +12 -0
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
 trace_export *export) }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);
+	event_trigger_unlock_commit_regs(fbuffer->trace_file,
 fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc,
 fbuffer->regs); +}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned

long flags,

#7 lore raw

PATCH 3/4 tracing: add trace_export support for trace_marker

In reply to Tingwei Zhang

Add the support to route trace_marker buffer to other destination via trace_export.

---
 kernel/trace/trace.c | 2 ++
 1 file changed, 2 insertions(+)
kernel/trace/trace.c +2 -0
index aef6330836e2..ac18e0ee9246 100644
@@ -6639,6 +6639,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	} else
 		entry->buf[cnt] = '\0';
 
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(event);
 	__buffer_unlock_commit(buffer, event);
 
 	if (tt)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#8 lore raw

PATCH 4/4 stm class: ftrace: use different channel accroding to CPU

In reply to Tingwei Zhang

To avoid mixup of packets from differnt ftrace packets simultaneously, use different channel for packets from different CPU.

---
 drivers/hwtracing/stm/ftrace.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/hwtracing/stm/ftrace.c +3 -2
index ce868e095410..6c3bce11c8d3 100644
@@ -10,7 +10,7 @@
 #include <linux/stm.h>
 #include <linux/trace.h>
 
-#define STM_FTRACE_NR_CHANNELS 1
+#define STM_FTRACE_NR_CHANNELS NR_CPUS
 #define STM_FTRACE_CHAN 0
 
 static int stm_ftrace_link(struct stm_source_data *data);
@@ -37,8 +37,9 @@ static void notrace
 stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
 {
 	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
+	unsigned int cpu = smp_processor_id();
 
-	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
+	stm_source_write(&stm->data, STM_FTRACE_CHAN + cpu, buf, len);
 }
 
 static int stm_ftrace_link(struct stm_source_data *data)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#9 lore raw

Re: [PATCH 4/4] stm class: ftrace: use different channel accroding to CPU

Quoted 13 lines · On Mon, 20 Jul 2020 10:21:17 +0800 Tingwei Zhang <[email protected]> wrote:

To avoid mixup of packets from differnt ftrace packets simultaneously, use different channel for packets from different CPU.

---
 drivers/hwtracing/stm/ftrace.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/hwtracing/stm/ftrace.c +0 -0
index ce868e095410..6c3bce11c8d3 100644

drivers/hwtracing/stm/ftrace.c line 13

Quoted 5 lines
drivers/hwtracing/stm/ftrace.c +1 -1
@@ -10,7 +10,7 @@
 #include <linux/stm.h>
 #include <linux/trace.h>
 
-#define STM_FTRACE_NR_CHANNELS 1
+#define STM_FTRACE_NR_CHANNELS NR_CPUS

I would make this dynamic. If this causes any kind of allocation, you do not want NR_CPUS. That's hardcoded by the distribution and is the max number of CPUs. With machines with hundreds of CPUs today, that could easily be 256 or 512 or more.

Instead, in the stm_ftrace_init() I would just assign it to nr_online_cpus().

	stm_ftrace.data.nr_chans = nr_online_cpus();

Or nr_possible_cpus().

-- Steve

Quoted 14 lines
drivers/hwtracing/stm/ftrace.c +2 -1
 #define STM_FTRACE_CHAN 0
 
 static int stm_ftrace_link(struct stm_source_data *data);
@@ -37,8 +37,9 @@ static void notrace
 stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
 {
 	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
+	unsigned int cpu = smp_processor_id();
 
-	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
+	stm_source_write(&stm->data, STM_FTRACE_CHAN + cpu, buf, len);
 }
 
 static int stm_ftrace_link(struct stm_source_data *data)