linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/4] Integration of function trace with System Trace IP blocks
@ 2016-06-22  2:46 Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM Chunyan Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-22  2:46 UTC (permalink / raw)
  To: rostedt, mathieu.poirier, alexander.shishkin, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

IP blocks allowing a variety of trace sources to log debugging
information to a pre-defined area have been introduced on a couple of
architecture [1][2]. These system trace blocks (also known as STM)
typically follow the MIPI STPv2 protocol [3] and provide a system wide
logging facility to any device, running a kernel or not, with access
to the block's log entry port(s).  Since each trace message has a
timestamp, it is possible to correlate events happening in the entire
system rather than being confined to the logging facility of a single
entity.

This patchset is trying to use STM IP blocks to collect some function
tracing information produced by Ftrace.  Since writing STM is relatively
time-consuming, my thought is that export as much useful information as
possible while using a limited bytes, in this patchset only function
pointers were copied to STM.

That way logging information generated by the function trace subsystem
and gathered in the coresight sink can be used in conjunction with
trace data from other board components, also collected in the same
trace sink.  This example is using ARM coresight STM but the same would
apply to any architecture wishing to do the same.

Comments and advice would be greatly appreciated.

Thanks,
Chunyan

[1]. https://lwn.net/Articles/674746/
[2]. http://lxr.free-electrons.com/source/drivers/hwtracing/intel_th/
[3]. http://mipi.org/specifications/debug#STP
[4]. http://lxr.free-electrons.com/source/include/linux/stm.h

Changes v2:
* Addressed comments from Alexander Shishkin:
 - Modified some ambiguous change logs.
 - Decoupled stm_ftrace and trace_output interface to STM.
 - Changed the file name from stm_ftrace.c to stm/ftrace.c.
 - Implemented link/unlink hooks for stm_ftrace.
* Removed useless header file include from stm/ftrace.c
* Added Acked-by from Steven Rostedt on 4/4.

Chunyan Zhang (4):
  trace: Introduce an output interface from ftrace to STM
  STM Ftrace: Adding generic buffer interface driver
  trace: duplicate function pointer to STM
  stm: Mark the functions of writing buffer with notrace

 drivers/hwtracing/coresight/coresight-stm.c |  2 +-
 drivers/hwtracing/intel_th/sth.c            | 11 +++--
 drivers/hwtracing/stm/Kconfig               | 10 ++++
 drivers/hwtracing/stm/Makefile              |  2 +
 drivers/hwtracing/stm/core.c                |  7 +--
 drivers/hwtracing/stm/dummy_stm.c           |  2 +-
 drivers/hwtracing/stm/ftrace.c              | 77 +++++++++++++++++++++++++++++
 include/linux/stm.h                         | 11 ++++-
 include/linux/trace_output_stm.h            | 17 +++++++
 kernel/trace/Makefile                       |  1 +
 kernel/trace/trace.c                        |  5 +-
 kernel/trace/trace_output_stm.c             | 41 +++++++++++++++
 12 files changed, 174 insertions(+), 12 deletions(-)
 create mode 100644 drivers/hwtracing/stm/ftrace.c
 create mode 100644 include/linux/trace_output_stm.h
 create mode 100644 kernel/trace/trace_output_stm.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM
  2016-06-22  2:46 [PATCH V2 0/4] Integration of function trace with System Trace IP blocks Chunyan Zhang
@ 2016-06-22  2:46 ` Chunyan Zhang
  2016-06-22  5:56   ` Alexander Shishkin
  2016-06-22  2:46 ` [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver Chunyan Zhang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-22  2:46 UTC (permalink / raw)
  To: rostedt, mathieu.poirier, alexander.shishkin, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

This patch is introducing a new output to print Ftrace messages
to STM buffer when the traces happen.  In order to reduce the
effect on timing overhead as much as possible, only the current
function and its parent's instruction pointer will be recorded
into STM. This idea was first introduced by Philippe Langlais
at ST-Microelectronics a long time ago[1].

[1] http://thread.gmane.org/gmane.linux.kernel/1114274/focus=1114275

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 include/linux/stm.h              |  7 +++++++
 include/linux/trace_output_stm.h | 17 +++++++++++++++++
 kernel/trace/Makefile            |  1 +
 kernel/trace/trace_output_stm.c  | 41 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 include/linux/trace_output_stm.h
 create mode 100644 kernel/trace/trace_output_stm.c

diff --git a/include/linux/stm.h b/include/linux/stm.h
index 8369d8a..2b6639a 100644
--- a/include/linux/stm.h
+++ b/include/linux/stm.h
@@ -129,6 +129,13 @@ struct stm_source_data {
 	void			(*unlink)(struct stm_source_data *data);
 };
 
+struct stm_ftrace {
+	struct stm_source_data	data;
+	void (*write)(struct stm_source_data *data, const char *buf,
+		      unsigned int len, unsigned int chan);
+	bool available;
+};
+
 int stm_source_register_device(struct device *parent,
 			       struct stm_source_data *data);
 void stm_source_unregister_device(struct stm_source_data *data);
diff --git a/include/linux/trace_output_stm.h b/include/linux/trace_output_stm.h
new file mode 100644
index 0000000..7edc680
--- /dev/null
+++ b/include/linux/trace_output_stm.h
@@ -0,0 +1,17 @@
+#ifndef __TRACE_OUTPUT_STM_H
+#define __TRACE_OUTPUT_STM_H
+
+#include <linux/module.h>
+
+#if IS_ENABLED(CONFIG_STM_SOURCE_FTRACE)
+struct stm_ftrace;
+extern void
+trace_func_to_stm(unsigned long ip, unsigned long parent_ip);
+extern void trace_add_output(struct stm_ftrace *stm);
+extern void trace_rm_output(void);
+#else
+static inline void
+trace_func_to_stm(unsigned long ip, unsigned long parent_ip) {}
+#endif
+
+#endif /* __TRACE_OUTPUT_STM_H */
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 979e7bf..5330803 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -69,4 +69,5 @@ obj-$(CONFIG_UPROBE_EVENT) += trace_uprobe.o
 
 obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
 
+obj-$(CONFIG_TRACING) += trace_output_stm.o
 libftrace-y := ftrace.o
diff --git a/kernel/trace/trace_output_stm.c b/kernel/trace/trace_output_stm.c
new file mode 100644
index 0000000..dc04952
--- /dev/null
+++ b/kernel/trace/trace_output_stm.c
@@ -0,0 +1,41 @@
+/*
+ * Output interface from Ftrace to STM buffer
+ * Copyright (c) 2016, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/stm.h>
+
+/* Offset above the start channel number */
+#define STM_FTRACE_CHAN 0
+
+static struct stm_ftrace *trace_output;
+
+void trace_func_to_stm(unsigned long ip, unsigned long parent_ip)
+{
+	unsigned long ip_array[2] = {ip, parent_ip};
+
+	if (trace_output)
+		trace_output->write(&trace_output->data, (char *)ip_array,
+				    sizeof(unsigned long) * 2, STM_FTRACE_CHAN);
+}
+
+void trace_add_output(struct stm_ftrace *stm)
+{
+	trace_output = stm;
+}
+EXPORT_SYMBOL_GPL(trace_add_output);
+
+void trace_rm_output(void)
+{
+	trace_output = NULL;
+}
+EXPORT_SYMBOL_GPL(trace_rm_output);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver
  2016-06-22  2:46 [PATCH V2 0/4] Integration of function trace with System Trace IP blocks Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM Chunyan Zhang
@ 2016-06-22  2:46 ` Chunyan Zhang
  2016-06-24  1:21   ` kbuild test robot
  2016-06-24  2:18   ` kbuild test robot
  2016-06-22  2:46 ` [PATCH V2 3/4] trace: duplicate function pointer to STM Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 4/4] stm: Mark the functions of writing buffer with notrace Chunyan Zhang
  3 siblings, 2 replies; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-22  2:46 UTC (permalink / raw)
  To: rostedt, mathieu.poirier, alexander.shishkin, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

This patch adds a driver that models itself as an stm_source and
who's sole purpose is to be an interface to the rest of the
kernel.  Once the stm and stm_source have been linked via sysfs,
everything that is passed to the interface will endup in the STM
trace engine.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 drivers/hwtracing/stm/Kconfig  | 10 ++++++
 drivers/hwtracing/stm/Makefile |  2 ++
 drivers/hwtracing/stm/ftrace.c | 77 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 drivers/hwtracing/stm/ftrace.c

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 847a39b..f4c69d2 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -39,4 +39,14 @@ config STM_SOURCE_HEARTBEAT
 	  If you want to send heartbeat messages over STM devices,
 	  say Y.
 
+config STM_SOURCE_FTRACE
+	tristate "Copy the output from kernel Ftrace to STM engine"
+	help
+	  This option can be used to copy the output from kernel Ftrace
+	  to STM engine. Enabling this option will introduce a slight
+	  timing effect.
+
+	  If you want to send kernel Ftrace messages over STM devices,
+	  say Y.
+
 endif
diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
index a9ce3d4..3abd84c 100644
--- a/drivers/hwtracing/stm/Makefile
+++ b/drivers/hwtracing/stm/Makefile
@@ -6,6 +6,8 @@ obj-$(CONFIG_STM_DUMMY)	+= dummy_stm.o
 
 obj-$(CONFIG_STM_SOURCE_CONSOLE)	+= stm_console.o
 obj-$(CONFIG_STM_SOURCE_HEARTBEAT)	+= stm_heartbeat.o
+obj-$(CONFIG_STM_SOURCE_FTRACE)		+= stm_ftrace.o
 
 stm_console-y		:= console.o
 stm_heartbeat-y		:= heartbeat.o
+stm_ftrace-y		:= ftrace.o
diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
new file mode 100644
index 0000000..ccce91e
--- /dev/null
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -0,0 +1,77 @@
+/*
+ * Simple kernel driver to link kernel Ftrace and an STM device
+ * Copyright (c) 2016, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/stm.h>
+#include <linux/trace_output_stm.h>
+
+#define STM_FTRACE_NR_CHANNELS 1
+
+static int stm_ftrace_link(struct stm_source_data *data);
+static void stm_ftrace_unlink(struct stm_source_data *data);
+static void stm_ftrace_write(struct stm_source_data *data, const char *buf,
+			     unsigned int len, unsigned int chan);
+
+static struct stm_ftrace ftrace = {
+	.data	= {
+		.name		= "ftrace",
+		.nr_chans	= STM_FTRACE_NR_CHANNELS,
+		.link		= stm_ftrace_link,
+		.unlink		= stm_ftrace_unlink,
+	},
+	.write			= stm_ftrace_write,
+};
+
+/**
+ * stm_ftrace_write() - write data to STM via 'stm_ftrace' source
+ * @buf:	buffer containing the data packet
+ * @len:	length of the data packet
+ * @chan:	offset above the start channel number allocated to 'stm_ftrace'
+ */
+static void notrace stm_ftrace_write(struct stm_source_data *data,
+				     const char *buf, unsigned int len,
+				     unsigned int chan)
+{
+	stm_source_write(data, chan, buf, len);
+}
+
+static int stm_ftrace_link(struct stm_source_data *data)
+{
+	struct stm_ftrace *sf = container_of(data, struct stm_ftrace, data);
+
+	trace_add_output(sf);
+
+	return 0;
+}
+
+static void stm_ftrace_unlink(struct stm_source_data *data)
+{
+	trace_rm_output();
+}
+
+static int __init stm_ftrace_init(void)
+{
+	return stm_source_register_device(NULL, &ftrace.data);
+}
+
+static void __exit stm_ftrace_exit(void)
+{
+	stm_source_unregister_device(&ftrace.data);
+}
+
+module_init(stm_ftrace_init);
+module_exit(stm_ftrace_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("stm_ftrace driver");
+MODULE_AUTHOR("Chunyan Zhang <zhang.chunyan@linaro.org>");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V2 3/4] trace: duplicate function pointer to STM
  2016-06-22  2:46 [PATCH V2 0/4] Integration of function trace with System Trace IP blocks Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver Chunyan Zhang
@ 2016-06-22  2:46 ` Chunyan Zhang
  2016-06-22  2:46 ` [PATCH V2 4/4] stm: Mark the functions of writing buffer with notrace Chunyan Zhang
  3 siblings, 0 replies; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-22  2:46 UTC (permalink / raw)
  To: rostedt, mathieu.poirier, alexander.shishkin, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

This patch inserts an output from Ftrace to STM, logs the pointer of
the functions and their parents to STM.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
 kernel/trace/trace.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8a4bd6b..ed9e1b4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -41,6 +41,7 @@
 #include <linux/nmi.h>
 #include <linux/fs.h>
 #include <linux/sched/rt.h>
+#include <linux/trace_output_stm.h>
 
 #include "trace.h"
 #include "trace_output.h"
@@ -1884,8 +1885,10 @@ trace_function(struct trace_array *tr,
 	entry->ip			= ip;
 	entry->parent_ip		= parent_ip;
 
-	if (!call_filter_check_discard(call, entry, buffer, event))
+	if (!call_filter_check_discard(call, entry, buffer, event)) {
 		__buffer_unlock_commit(buffer, event);
+		trace_func_to_stm(ip, parent_ip);
+	}
 }
 
 #ifdef CONFIG_STACKTRACE
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V2 4/4] stm: Mark the functions of writing buffer with notrace
  2016-06-22  2:46 [PATCH V2 0/4] Integration of function trace with System Trace IP blocks Chunyan Zhang
                   ` (2 preceding siblings ...)
  2016-06-22  2:46 ` [PATCH V2 3/4] trace: duplicate function pointer to STM Chunyan Zhang
@ 2016-06-22  2:46 ` Chunyan Zhang
  3 siblings, 0 replies; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-22  2:46 UTC (permalink / raw)
  To: rostedt, mathieu.poirier, alexander.shishkin, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

If CONFIG_STM_FTRACE is selected, Function trace data would be
writen to STM buffer, all  functions that related to writing data
packets to STM buffer should be marked 'notrace' to avoid being
traced by Ftrace, otherwise the program would stall into an endless loop.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/hwtracing/coresight/coresight-stm.c |  2 +-
 drivers/hwtracing/intel_th/sth.c            | 11 +++++++----
 drivers/hwtracing/stm/core.c                |  7 ++++---
 drivers/hwtracing/stm/dummy_stm.c           |  2 +-
 include/linux/stm.h                         |  4 ++--
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 73be58a..345c81e 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -386,7 +386,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
 	return 0;
 }
 
-static ssize_t stm_generic_packet(struct stm_data *stm_data,
+static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
 				  unsigned int master,
 				  unsigned int channel,
 				  unsigned int packet,
diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
index e1aee61..b034446 100644
--- a/drivers/hwtracing/intel_th/sth.c
+++ b/drivers/hwtracing/intel_th/sth.c
@@ -67,10 +67,13 @@ static void sth_iowrite(void __iomem *dest, const unsigned char *payload,
 	}
 }
 
-static ssize_t sth_stm_packet(struct stm_data *stm_data, unsigned int master,
-			      unsigned int channel, unsigned int packet,
-			      unsigned int flags, unsigned int size,
-			      const unsigned char *payload)
+static ssize_t notrace sth_stm_packet(struct stm_data *stm_data,
+				      unsigned int master,
+				      unsigned int channel,
+				      unsigned int packet,
+				      unsigned int flags,
+				      unsigned int size,
+				      const unsigned char *payload)
 {
 	struct sth_device *sth = container_of(stm_data, struct sth_device, stm);
 	struct intel_th_channel __iomem *out =
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index ff31108..3139b01 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -424,7 +424,7 @@ static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
 	return ret;
 }
 
-static ssize_t stm_write(struct stm_data *data, unsigned int master,
+static ssize_t notrace stm_write(struct stm_data *data, unsigned int master,
 			  unsigned int channel, const char *buf, size_t count)
 {
 	unsigned int flags = STP_PACKET_TIMESTAMPED;
@@ -1069,8 +1069,9 @@ void stm_source_unregister_device(struct stm_source_data *data)
 }
 EXPORT_SYMBOL_GPL(stm_source_unregister_device);
 
-int stm_source_write(struct stm_source_data *data, unsigned int chan,
-		     const char *buf, size_t count)
+int notrace stm_source_write(struct stm_source_data *data,
+			     unsigned int chan,
+			     const char *buf, size_t count)
 {
 	struct stm_source_device *src = data->src;
 	struct stm_device *stm;
diff --git a/drivers/hwtracing/stm/dummy_stm.c b/drivers/hwtracing/stm/dummy_stm.c
index a86612d..c5f94ca 100644
--- a/drivers/hwtracing/stm/dummy_stm.c
+++ b/drivers/hwtracing/stm/dummy_stm.c
@@ -21,7 +21,7 @@
 #include <linux/slab.h>
 #include <linux/stm.h>
 
-static ssize_t
+static ssize_t notrace
 dummy_stm_packet(struct stm_data *stm_data, unsigned int master,
 		 unsigned int channel, unsigned int packet, unsigned int flags,
 		 unsigned int size, const unsigned char *payload)
diff --git a/include/linux/stm.h b/include/linux/stm.h
index 2b6639a..80c60da 100644
--- a/include/linux/stm.h
+++ b/include/linux/stm.h
@@ -140,7 +140,7 @@ int stm_source_register_device(struct device *parent,
 			       struct stm_source_data *data);
 void stm_source_unregister_device(struct stm_source_data *data);
 
-int stm_source_write(struct stm_source_data *data, unsigned int chan,
-		     const char *buf, size_t count);
+int notrace stm_source_write(struct stm_source_data *data, unsigned int chan,
+			     const char *buf, size_t count);
 
 #endif /* _STM_H_ */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM
  2016-06-22  2:46 ` [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM Chunyan Zhang
@ 2016-06-22  5:56   ` Alexander Shishkin
  2016-06-28  6:59     ` Chunyan Zhang
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Shishkin @ 2016-06-22  5:56 UTC (permalink / raw)
  To: Chunyan Zhang, rostedt, mathieu.poirier, mingo
  Cc: mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel,
	felipe.balbi

[adding Felipe for his sudden interest in the subject matter]

Chunyan Zhang <zhang.chunyan@linaro.org> writes:

> +static struct stm_ftrace *trace_output;

What you want is a possibility to have different ftrace outputs, not
different STM outputs for ftrace (again, STM core already does this).
In other words, here, you want to have the notion of "output" be
stm-agnostic, but be a generalized output driver object.

> +
> +void trace_func_to_stm(unsigned long ip, unsigned long parent_ip)
> +{
> +	unsigned long ip_array[2] = {ip, parent_ip};
> +
> +	if (trace_output)
> +		trace_output->write(&trace_output->data, (char *)ip_array,
> +				    sizeof(unsigned long) * 2, STM_FTRACE_CHAN);
> +}

The ip+parent_ip pair is still not a useful output from ftrace
data. Moreover, doing this is basically like inventing another binary
protocol for ftrace data over stm, where ftrace is in and of itself
already a binary protocol, why not just use that? The decoder will
basically depend on the kernel binary from whence the traces are
coming, but this is a requirement even if we want to decypher the
ip+parent_ip data you're proposing.

We would need to take some time to think this through. What we might
consider is:

 * bypassing ftrace ring buffer, sending data directly to an "output",
   which has a drawback of ending up in a driver callback, which needs
   to serialize on its driver stuff and write registers (I did try to
   make stm_write as light as possible when I wrote it, though); the
   good part is that data goes into the wire as soon as it is produced
   instead of being buffered along the way;
 * starting a work (or multiple works) that would traverse new data in
   ftrace buffer and feed it to an "output", such as stm; this has a
   problem of producers being potentially faster than consumers
   (consider 'function' tracer, for example) and hogging the cpus by
   simply exporting trace data; this also botches the stm timestamps,
   which will then be representative of nothing in particular.

> +
> +void trace_add_output(struct stm_ftrace *stm)
> +{
> +	trace_output = stm;
> +}
> +EXPORT_SYMBOL_GPL(trace_add_output);
> +
> +void trace_rm_output(void)
> +{
> +	trace_output = NULL;
> +}
> +EXPORT_SYMBOL_GPL(trace_rm_output);

These, of course, only work because they are implicitly serialized on
stm core's link locks.

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver
  2016-06-22  2:46 ` [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver Chunyan Zhang
@ 2016-06-24  1:21   ` kbuild test robot
  2016-06-24  2:18   ` kbuild test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2016-06-24  1:21 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: kbuild-all, rostedt, mathieu.poirier, alexander.shishkin, mingo,
	mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

Hi,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.7-rc4 next-20160623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chunyan-Zhang/Integration-of-function-trace-with-System-Trace-IP-blocks/20160622-115305
config: m32r-allmodconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m32r 

All errors (new ones prefixed by >>):

   ERROR: "bad_dma_ops" [sound/soc/fsl/snd-soc-fsl-asrc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/soc/atmel/snd-soc-atmel-pcm-pdc.ko] undefined!
   ERROR: "bad_dma_ops" [sound/core/snd-pcm.ko] undefined!
   ERROR: "dma_common_mmap" [sound/core/snd-pcm.ko] undefined!
   ERROR: "__ucmpdi2" [lib/842/842_decompress.ko] undefined!
   ERROR: "__ucmpdi2" [fs/btrfs/btrfs.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/media/i2c/adv7842.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/md/bcache/bcache.ko] undefined!
   ERROR: "__ucmpdi2" [drivers/iio/imu/inv_mpu6050/inv-mpu6050.ko] undefined!
>> ERROR: "trace_rm_output" [drivers/hwtracing/stm/stm_ftrace.ko] undefined!
>> ERROR: "trace_add_output" [drivers/hwtracing/stm/stm_ftrace.ko] undefined!
   ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 35694 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver
  2016-06-22  2:46 ` [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver Chunyan Zhang
  2016-06-24  1:21   ` kbuild test robot
@ 2016-06-24  2:18   ` kbuild test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2016-06-24  2:18 UTC (permalink / raw)
  To: Chunyan Zhang
  Cc: kbuild-all, rostedt, mathieu.poirier, alexander.shishkin, mingo,
	mike.leach, tor, maxime.coquelin, philippe.langlais,
	nicolas.guion, zhang.lyra, linux-kernel, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

Hi,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.7-rc4 next-20160623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chunyan-Zhang/Integration-of-function-trace-with-System-Trace-IP-blocks/20160622-115305
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `stm_ftrace_unlink':
>> ftrace.c:(.text+0x12197ac): undefined reference to `trace_rm_output'
   drivers/built-in.o: In function `stm_ftrace_link':
>> ftrace.c:(.text+0x12197be): undefined reference to `trace_add_output'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37199 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM
  2016-06-22  5:56   ` Alexander Shishkin
@ 2016-06-28  6:59     ` Chunyan Zhang
  0 siblings, 0 replies; 9+ messages in thread
From: Chunyan Zhang @ 2016-06-28  6:59 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Steven Rostedt, Mathieu Poirier, mingo, Mike Leach,
	Tor Jeremiassen, Maxime Coquelin, philippe.langlais,
	Nicolas GUION, Lyra Zhang, linux-kernel, linux-arm-kernel,
	felipe.balbi

On Wed, Jun 22, 2016 at 1:56 PM, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> [adding Felipe for his sudden interest in the subject matter]
>
> Chunyan Zhang <zhang.chunyan@linaro.org> writes:
>
>> +static struct stm_ftrace *trace_output;
>
> What you want is a possibility to have different ftrace outputs, not
> different STM outputs for ftrace (again, STM core already does this).
> In other words, here, you want to have the notion of "output" be
> stm-agnostic, but be a generalized output driver object.

Ok, I will make it more generic, i.e. an agnostic of the exact output devices.

>
>> +
>> +void trace_func_to_stm(unsigned long ip, unsigned long parent_ip)
>> +{
>> +     unsigned long ip_array[2] = {ip, parent_ip};
>> +
>> +     if (trace_output)
>> +             trace_output->write(&trace_output->data, (char *)ip_array,
>> +                                 sizeof(unsigned long) * 2, STM_FTRACE_CHAN);
>> +}
>
> The ip+parent_ip pair is still not a useful output from ftrace
> data. Moreover, doing this is basically like inventing another binary
> protocol for ftrace data over stm, where ftrace is in and of itself
> already a binary protocol, why not just use that? The decoder will
> basically depend on the kernel binary from whence the traces are
> coming, but this is a requirement even if we want to decypher the
> ip+parent_ip data you're proposing.
>
> We would need to take some time to think this through. What we might
> consider is:
>
>  * bypassing ftrace ring buffer, sending data directly to an "output",
>    which has a drawback of ending up in a driver callback, which needs
>    to serialize on its driver stuff and write registers (I did try to
>    make stm_write as light as possible when I wrote it, though); the
>    good part is that data goes into the wire as soon as it is produced
>    instead of being buffered along the way;

I want to try this way, if I remember correctly, Steven Rostedt said
the similar solution before, use a jump label to switch which "output"
traces should be sent to.  And yes like you said, the drawback is
exporting trace data to STM by writing registers is slower than
writing ring buffer, it would slow down the process.

>  * starting a work (or multiple works) that would traverse new data in
>    ftrace buffer and feed it to an "output", such as stm; this has a
>    problem of producers being potentially faster than consumers
>    (consider 'function' tracer, for example) and hogging the cpus by
>    simply exporting trace data; this also botches the stm timestamps,

With stm timestamps on each trace data, we can sync up every events
happened on the system, not only on CPU but also on other processors,
this is useful if only the timestamps are nearly real-time, from this
view, the second way seems not so more feasible than the first one.

Please correct me if I'm missing something.

Thanks for your comments,
Chunyan

>    which will then be representative of nothing in particular.
>
>> +
>> +void trace_add_output(struct stm_ftrace *stm)
>> +{
>> +     trace_output = stm;
>> +}
>> +EXPORT_SYMBOL_GPL(trace_add_output);
>> +
>> +void trace_rm_output(void)
>> +{
>> +     trace_output = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(trace_rm_output);
>
> These, of course, only work because they are implicitly serialized on
> stm core's link locks.
>
> Regards,
> --
> Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-28  6:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-22  2:46 [PATCH V2 0/4] Integration of function trace with System Trace IP blocks Chunyan Zhang
2016-06-22  2:46 ` [PATCH V2 1/4] trace: Introduce an output interface from ftrace to STM Chunyan Zhang
2016-06-22  5:56   ` Alexander Shishkin
2016-06-28  6:59     ` Chunyan Zhang
2016-06-22  2:46 ` [PATCH V2 2/4] STM Ftrace: Adding generic buffer interface driver Chunyan Zhang
2016-06-24  1:21   ` kbuild test robot
2016-06-24  2:18   ` kbuild test robot
2016-06-22  2:46 ` [PATCH V2 3/4] trace: duplicate function pointer to STM Chunyan Zhang
2016-06-22  2:46 ` [PATCH V2 4/4] stm: Mark the functions of writing buffer with notrace Chunyan Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).