tracing: export event trace and trace_marker

PATCH Superseded Tingwei Zhang 3 participants 10 messages Jul 28, 2020 – Aug 11, 2020 2 messages not in the archive

Subsystems (4)

Tracing Arm STM32 Hardware tracing facilities System trace module class

lore

Share & tools

lore

Tingwei Zhang

author

2020-07-28 01:34 UTC #0 lore raw

v3 cover tracing: export event trace and trace_marker

Ftrace has ability to export trace packets to other destination. 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 separate channel for each CPU to avoid mixing up packets from different CPUs together.

Change from v2:
Change flag definition to BIT(). (Steven)
Add comment in stm_ftrace_write() to clarify it's safe to use smp_processor_id() here since preempt is disabled. (Steven)

Change from v1:
All changes are suggested by Steven Rostedt.
User separate flag to control function trace, event trace and trace mark. Allocate channels according to num_possible_cpu() dynamically. Move ftrace_exports routines up so all ftrace can use them.

Tingwei Zhang (6):
stm class: ftrace: change dependency to TRACING
tracing: add flag to control different traces
tracing: add trace_export support for event trace
tracing: add trace_export support for trace_marker stm class: ftrace: enable supported trace export flag stm class: ftrace: use different channel accroding to CPU

 drivers/hwtracing/stm/Kconfig  |   2 +-
 drivers/hwtracing/stm/ftrace.c |   7 +-
 include/linux/trace.h          |   7 +
 kernel/trace/trace.c           | 270 ++++++++++++++++++---------------
 4 files changed, 159 insertions(+), 127 deletions(-)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Tingwei Zhang

author

2020-07-28 01:34 UTC #1 lore raw

PATCH v3 1/6 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.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 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

Tingwei Zhang

author

2020-07-28 01:34 UTC #2 lore raw

PATCH v3 2/6 tracing: add flag to control different traces

In reply to Tingwei Zhang

More traces like event trace or trace marker will be supported. Add flag for difference traces, so that they can be controlled separately. Move current function trace to it's own flag instead of global ftrace enable flag.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 include/linux/trace.h |  5 +++++
 kernel/trace/trace.c  | 36 +++++++++++++++++++-----------------
 2 files changed, 24 insertions(+), 17 deletions(-)

include/linux/trace.h +5 -0

index 7fd86d3c691f..bc93063dda39 100644
@@ -3,6 +3,9 @@
 #define _LINUX_TRACE_H
 
 #ifdef CONFIG_TRACING
+
+#define TRACE_EXPORT_FUNCTION	BIT(0)
+
 /*
  * The trace export - an export of Ftrace output. The trace_export
  * can process traces and export them to a registered destination as

@@ -15,10 +18,12 @@
  * next		- pointer to the next trace_export
  * write	- copy traces which have been delt with ->commit() to
  *		  the destination
+ * flags	- which ftrace to be exported
  */
 struct trace_export {
 	struct trace_export __rcu	*next;
 	void (*write)(struct trace_export *, const void *, unsigned int);
+	int flags;
 };
 
 int register_ftrace_export(struct trace_export *export);

kernel/trace/trace.c +19 ~~-17~~

index bb62269724d5..8f1e66831e9e 100644
@@ -2747,33 +2747,37 @@ trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,
 
 static void
 trace_process_export(struct trace_export *export,
-	       struct ring_buffer_event *event)
+	       struct ring_buffer_event *event, int flag)
 {
 	struct trace_entry *entry;
 	unsigned int size = 0;
 
-	entry = ring_buffer_event_data(event);
-	size = ring_buffer_event_length(event);
-	export->write(export, entry, size);
+	if (export->flags & flag) {
+		entry = ring_buffer_event_data(event);
+		size = ring_buffer_event_length(event);
+		export->write(export, entry, size);
+	}
 }
 
 static DEFINE_MUTEX(ftrace_export_lock);
 
 static struct trace_export __rcu *ftrace_exports_list __read_mostly;
 
-static DEFINE_STATIC_KEY_FALSE(ftrace_exports_enabled);
+static DEFINE_STATIC_KEY_FALSE(trace_function_exports_enabled);
 
-static inline void ftrace_exports_enable(void)
+static inline void ftrace_exports_enable(struct trace_export *export)
 {
-	static_branch_enable(&ftrace_exports_enabled);
+	if (export->flags & TRACE_EXPORT_FUNCTION)
+		static_branch_inc(&trace_function_exports_enabled);
 }
 
-static inline void ftrace_exports_disable(void)
+static inline void ftrace_exports_disable(struct trace_export *export)
 {
-	static_branch_disable(&ftrace_exports_enabled);
+	if (export->flags & TRACE_EXPORT_FUNCTION)
+		static_branch_dec(&trace_function_exports_enabled);
 }
 
-static void ftrace_exports(struct ring_buffer_event *event)
+static void ftrace_exports(struct ring_buffer_event *event, int flag)
 {
 	struct trace_export *export;

@@ -2781,7 +2785,7 @@ static void ftrace_exports(struct ring_buffer_event *event)
 
 	export = rcu_dereference_raw_check(ftrace_exports_list);
 	while (export) {
-		trace_process_export(export, event);
+		trace_process_export(export, event, flag);
 		export = rcu_dereference_raw_check(export->next);
 	}

@@ -2821,8 +2825,7 @@ rm_trace_export(struct trace_export **list, struct trace_export *export)
 static inline void
 add_ftrace_export(struct trace_export **list, struct trace_export *export)
 {
-	if (*list == NULL)
-		ftrace_exports_enable();
+	ftrace_exports_enable(export);
 
 	add_trace_export(list, export);
 }

@@ -2833,8 +2836,7 @@ rm_ftrace_export(struct trace_export **list, struct trace_export *export)
 	int ret;
 
 	ret = rm_trace_export(list, export);
-	if (*list == NULL)
-		ftrace_exports_disable();
+	ftrace_exports_disable(export);
 
 	return ret;
 }

@@ -2887,8 +2889,8 @@ trace_function(struct trace_array *tr,
 	entry->parent_ip		= parent_ip;
 
 	if (!call_filter_check_discard(call, entry, buffer, event)) {
-		if (static_branch_unlikely(&ftrace_exports_enabled))
-			ftrace_exports(event);
+		if (static_branch_unlikely(&trace_function_exports_enabled))
+			ftrace_exports(event, TRACE_EXPORT_FUNCTION);
 		__buffer_unlock_commit(buffer, event);
 	}
 }

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Tingwei Zhang

author

2020-07-28 01:34 UTC #3 lore raw

PATCH v3 3/6 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. Move trace export related function to the beginning of file so other trace can call trace_process_export() to export.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 include/linux/trace.h |   1 +
 kernel/trace/trace.c  | 259 ++++++++++++++++++++++--------------------
 2 files changed, 135 insertions(+), 125 deletions(-)

include/linux/trace.h +1 -0

index bc93063dda39..5a01eeffb254 100644
@@ -5,6 +5,7 @@
 #ifdef CONFIG_TRACING
 
 #define TRACE_EXPORT_FUNCTION	BIT(0)
+#define TRACE_EXPORT_EVENT	BIT(1)
 
 /*
  * The trace export - an export of Ftrace output. The trace_export

kernel/trace/trace.c +134 ~~-125~~

277 lines not on this page · raw message

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Tingwei Zhang

author

2020-07-28 01:35 UTC #4 lore raw

PATCH v3 4/6 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.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 include/linux/trace.h | 1 +
 kernel/trace/trace.c  | 9 +++++++++
 2 files changed, 10 insertions(+)

include/linux/trace.h +1 -0

index 5a01eeffb254..80b7df68bc0f 100644
@@ -6,6 +6,7 @@
 
 #define TRACE_EXPORT_FUNCTION	BIT(0)
 #define TRACE_EXPORT_EVENT	BIT(1)
+#define TRACE_EXPORT_MARKER	BIT(2)
 
 /*
  * The trace export - an export of Ftrace output. The trace_export

kernel/trace/trace.c +9 -0

index 2f9302a8b322..67993abda394 100644
@@ -271,6 +271,7 @@ static struct trace_export __rcu *ftrace_exports_list __read_mostly;
 
 static DEFINE_STATIC_KEY_FALSE(trace_function_exports_enabled);
 static DEFINE_STATIC_KEY_FALSE(trace_event_exports_enabled);
+static DEFINE_STATIC_KEY_FALSE(trace_marker_exports_enabled);
 
 static inline void ftrace_exports_enable(struct trace_export *export)
 {

@@ -279,6 +280,9 @@ static inline void ftrace_exports_enable(struct trace_export *export)
 
 	if (export->flags & TRACE_EXPORT_EVENT)
 		static_branch_inc(&trace_event_exports_enabled);
+
+	if (export->flags & TRACE_EXPORT_MARKER)
+		static_branch_inc(&trace_marker_exports_enabled);
 }
 
 static inline void ftrace_exports_disable(struct trace_export *export)

@@ -288,6 +292,9 @@ static inline void ftrace_exports_disable(struct trace_export *export)
 
 	if (export->flags & TRACE_EXPORT_EVENT)
 		static_branch_dec(&trace_event_exports_enabled);
+
+	if (export->flags & TRACE_EXPORT_MARKER)
+		static_branch_dec(&trace_marker_exports_enabled);
 }
 
 static void ftrace_exports(struct ring_buffer_event *event, int flag)

@@ -6648,6 +6655,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	} else
 		entry->buf[cnt] = '\0';
 
+	if (static_branch_unlikely(&trace_marker_exports_enabled))
+		ftrace_exports(event, TRACE_EXPORT_MARKER);
 	__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

Tingwei Zhang

author

2020-07-28 01:34 UTC #5 lore raw

PATCH v3 5/6 stm class: ftrace: enable supported trace export flag

In reply to Tingwei Zhang

Set flags for trace_export. Export function trace, event trace and trace marker to stm.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 drivers/hwtracing/stm/ftrace.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/hwtracing/stm/ftrace.c +2 -0

index ce868e095410..c694a6e692d1 100644
@@ -46,6 +46,8 @@ static int stm_ftrace_link(struct stm_source_data *data)
 	struct stm_ftrace *sf = container_of(data, struct stm_ftrace, data);
 
 	sf->ftrace.write = stm_ftrace_write;
+	sf->ftrace.flags = TRACE_EXPORT_FUNCTION | TRACE_EXPORT_EVENT
+			| TRACE_EXPORT_MARKER;
 
 	return register_ftrace_export(&sf->ftrace);
 }

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Tingwei Zhang

author

2020-07-28 01:34 UTC #6 lore raw

PATCH v3 6/6 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.

Signed-off-by Tingwei Zhang <[email protected]>

Reviewed-by Steven Rostedt (VMware) <[email protected]>

---
 drivers/hwtracing/stm/ftrace.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/hwtracing/stm/ftrace.c +4 -1

index c694a6e692d1..ebf29489919c 100644
@@ -37,8 +37,10 @@ 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);
+	/* This is called from trace system with preemption disabled */
+	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)

@@ -63,6 +65,7 @@ static int __init stm_ftrace_init(void)
 {
 	int ret;
 
+	stm_ftrace.data.nr_chans = num_possible_cpus();
 	ret = stm_source_register_device(NULL, &stm_ftrace.data);
 	if (ret)
 		pr_err("Failed to register stm_source - ftrace.\n");

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Tingwei Zhang

2020-08-11 03:04 UTC #7 lore raw

In reply to Tingwei Zhang

Hi Steven,

Quoted 11 lines · Tingwei Zhang

Ftrace has ability to export trace packets to other destination. 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 separate channel for each CPU to avoid mixing up packets
from different CPUs together.
Change from v2:
Change flag definition to BIT(). (Steven)
Add comment in stm_ftrace_write() to clarify it's safe to use smp_processor_id() here since preempt is disabled. (Steven)

Thanks for your comments, Steven. I've addressed all your comments in v3. Do you have more comments on v3? Is there anything I need to do to merge this series to Linux Kernel?

Quoted 25 lines

Change from v1:
All changes are suggested by Steven Rostedt.
User separate flag to control function trace, event trace and trace mark. Allocate channels according to num_possible_cpu() dynamically. Move ftrace_exports routines up so all ftrace can use them.
Tingwei Zhang (6):
stm class: ftrace: change dependency to TRACING
tracing: add flag to control different traces
tracing: add trace_export support for event trace
tracing: add trace_export support for trace_marker stm class: ftrace: enable supported trace export flag stm class: ftrace: use different channel accroding to CPU
 drivers/hwtracing/stm/Kconfig  |   2 +-
 drivers/hwtracing/stm/ftrace.c |   7 +-
 include/linux/trace.h          |   7 +
 kernel/trace/trace.c           | 270 ++++++++++++++++++---------------
 4 files changed, 159 insertions(+), 127 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Steven Rostedt

maintainer

2020-08-11 03:20 UTC #8 lore raw

Quoted 3 lines · On Tue, 11 Aug 2020 11:04:18 +0800 Tingwei Zhang <[email protected]>…

Thanks for your comments, Steven. I've addressed all your comments in v3. Do you have more comments on v3? Is there anything I need to do to merge this series to Linux Kernel?

I gave my Reviewed-by tag on each of the patches that touch my tree. It should go in via whoever maintains the drivers/hwtracing tree. Is that Greg KH?

-- Steve

A message that is not in the archive (sent off-list or not yet received): [email protected]

Tingwei Zhang

2020-08-11 06:45 UTC #11 lore raw

Quoted 31 lines · Steven Rostedt

Quoted 5 lines · On Tue, 11 Aug 2020 11:49:46 +0800 Tingwei Zhang <[email protected]>…
Quoted 4 lines · Steven Rostedt
Quoted 1 line · On Tue, 11 Aug 2020 11:04:18 +0800 Tingwei Zhang <[email protected]>…
Thanks for your comments, Steven. I've addressed all your comments
in
Quoted 2 lines
Quoted 2 lines
v3.
Quoted 1 line
Do you have more comments on v3? Is there anything I need to do to
merge
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
this series to Linux Kernel?
I gave my Reviewed-by tag on each of the patches that touch my tree.
It
Quoted 3 lines
Quoted 2 lines
should go in via whoever maintains the drivers/hwtracing tree. Is that Greg KH?
I thought it will go to tracing tree since majority of the changes are
in
Quoted 5 lines
kernel/trace.
Maintainers of drviers/hwtracing are Mathieu and Suzuki. I'll add them into review list.
As I didn't have reviews or acks from them. I couldn't take the code. When touching two subsystems, it usually requires one of the subsystem maintainers to ack the changes to their subsystem, so that the other subsystem maintainer can take the rest of the code through their tree.

Quoted 5 lines

And it usually goes through the tree that has the interface that is changing. That is, the changes to tracing was the infrastructure needed for the changes in the hwtrace subsystem. And I don't test that subsystem, so I wouldn't really be able to test this code.

Thanks a lot for detail clarification, Steven.

Thanks,
Tingwei

Quoted 1 line

-- Steve