diff mbox series

[v2,3/3] interconnect: Add basic tracepoints

Message ID 20191101130031.27996-4-georgi.djakov@linaro.org
State New
Headers show
Series None | expand

Commit Message

Georgi Djakov Nov. 1, 2019, 1 p.m. UTC 
The tracepoints can help with understanding the system behavior of a
given interconnect path when the consumer drivers change their bandwidth
demands. This might be interesting when we want to monitor the requested
interconnect bandwidth for each client driver. The paths may share the
same nodes and this will help to understand "who and when is requesting
what". All this is useful for subsystem drivers developers and may also
provide hints when optimizing the power and performance profile of the
system.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

---
 MAINTAINERS                         |  1 +
 drivers/interconnect/core.c         |  7 +++
 include/trace/events/interconnect.h | 81 +++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 include/trace/events/interconnect.h

Comments

Steven Rostedt Nov. 13, 2019, 8:23 p.m. UTC | #1 
On Fri,  1 Nov 2019 15:00:31 +0200
Georgi Djakov <georgi.djakov@linaro.org> wrote:

> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c

> index df44ef713db5..15e11e22ddf7 100644

> --- a/drivers/interconnect/core.c

> +++ b/drivers/interconnect/core.c

> @@ -26,6 +26,9 @@ static LIST_HEAD(icc_providers);

>  static DEFINE_MUTEX(icc_lock);

>  static struct dentry *icc_debugfs_dir;

>  

> +#define CREATE_TRACE_POINTS

> +#include <trace/events/interconnect.h>

> +

>  static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)

>  {

>  	if (!n)

> @@ -435,6 +438,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)

>  

>  		/* aggregate requests for this node */

>  		aggregate_requests(node);

> +

> +		trace_icc_set_bw(path, node, i, avg_bw, peak_bw);

>  	}

>  

>  	ret = apply_constraints(path);

> @@ -453,6 +458,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)

>  

>  	mutex_unlock(&icc_lock);

>  

> +	trace_icc_set_bw_end(path, ret);

> +

>  	return ret;

>  }

>  EXPORT_SYMBOL_GPL(icc_set_bw);

> diff --git a/include/trace/events/interconnect.h b/include/trace/events/interconnect.h

> new file mode 100644

> index 000000000000..64b646aa7bd3

> --- /dev/null

> +++ b/include/trace/events/interconnect.h

> @@ -0,0 +1,81 @@

> +/* SPDX-License-Identifier: GPL-2.0 */

> +/*

> + * Copyright (c) 2019, Linaro Ltd.

> + * Author: Georgi Djakov <georgi.djakov@linaro.org>

> + */

> +

> +#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ)

> +#define _TRACE_INTERCONNECT_H

> +

> +#include <linux/tracepoint.h>

> +

> +#undef TRACE_SYSTEM

> +#define TRACE_SYSTEM interconnect

> +

> +#include "../../../drivers/interconnect/internal.h"


Ug.

I would recommend placing this file in with drivers/interconnect, and
call it simply trace.h, or interconnect_trace.h. Then you can include
"internal.h" nicely.

To do so, you need to add to drivers/interconnect/Makefile:

 CFLAGS_core.o := -I$(src)

To have core.c include this file.

Rest looks fine.

-- Steve


> +

> +TRACE_EVENT(icc_set_bw,

> +

> +	TP_PROTO(struct icc_path *p, struct icc_node *n, int i,

> +		 u32 avg_bw, u32 peak_bw),

> +

> +	TP_ARGS(p, n, i, avg_bw, peak_bw),

> +

> +	TP_STRUCT__entry(

> +		__string(path_name, p->name)

> +		__string(dev, dev_name(p->reqs[i].dev))

> +		__string(node_name, n->name)

> +		__field(u32, avg_bw)

> +		__field(u32, peak_bw)

> +		__field(u32, node_avg_bw)

> +		__field(u32, node_peak_bw)

> +	),

> +

> +	TP_fast_assign(

> +		__assign_str(path_name, p->name);

> +		__assign_str(dev, dev_name(p->reqs[i].dev));

> +		__assign_str(node_name, n->name);

> +		__entry->avg_bw = avg_bw;

> +		__entry->peak_bw = peak_bw;

> +		__entry->node_avg_bw = n->avg_bw;

> +		__entry->node_peak_bw = n->peak_bw;

> +	),

> +

> +	TP_printk("path=%s dev=%s node=%s avg_bw=%u peak_bw=%u agg_avg=%u agg_peak=%u",

> +		  __get_str(path_name),

> +		  __get_str(dev),

> +		  __get_str(node_name),

> +		  __entry->avg_bw,

> +		  __entry->peak_bw,

> +		  __entry->node_avg_bw,

> +		  __entry->node_peak_bw)

> +);

> +

> +TRACE_EVENT(icc_set_bw_end,

> +

> +	TP_PROTO(struct icc_path *p, int ret),

> +

> +	TP_ARGS(p, ret),

> +

> +	TP_STRUCT__entry(

> +		__string(path_name, p->name)

> +		__string(dev, dev_name(p->reqs[0].dev))

> +		__field(int, ret)

> +	),

> +

> +	TP_fast_assign(

> +		__assign_str(path_name, p->name);

> +		__assign_str(dev, dev_name(p->reqs[0].dev));

> +		__entry->ret = ret;

> +	),

> +

> +	TP_printk("path=%s dev=%s ret=%d",

> +		  __get_str(path_name),

> +		  __get_str(dev),

> +		  __entry->ret)

> +);

> +

> +#endif /* _TRACE_INTERCONNECT_H */

> +

> +/* This part must be outside protection */

> +#include <trace/define_trace.h>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index a69e6db80c79..16f28fa515f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8522,6 +8522,7 @@  F:	drivers/interconnect/
 F:	include/dt-bindings/interconnect/
 F:	include/linux/interconnect-provider.h
 F:	include/linux/interconnect.h
+F:	include/trace/events/interconnect.h
 
 INVENSENSE MPU-3050 GYROSCOPE DRIVER
 M:	Linus Walleij <linus.walleij@linaro.org>
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index df44ef713db5..15e11e22ddf7 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -26,6 +26,9 @@  static LIST_HEAD(icc_providers);
 static DEFINE_MUTEX(icc_lock);
 static struct dentry *icc_debugfs_dir;
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/interconnect.h>
+
 static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)
 {
 	if (!n)
@@ -435,6 +438,8 @@  int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
 
 		/* aggregate requests for this node */
 		aggregate_requests(node);
+
+		trace_icc_set_bw(path, node, i, avg_bw, peak_bw);
 	}
 
 	ret = apply_constraints(path);
@@ -453,6 +458,8 @@  int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
 
 	mutex_unlock(&icc_lock);
 
+	trace_icc_set_bw_end(path, ret);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(icc_set_bw);
diff --git a/include/trace/events/interconnect.h b/include/trace/events/interconnect.h
new file mode 100644
index 000000000000..64b646aa7bd3
--- /dev/null
+++ b/include/trace/events/interconnect.h
@@ -0,0 +1,81 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2019, Linaro Ltd.
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INTERCONNECT_H
+
+#include <linux/tracepoint.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM interconnect
+
+#include "../../../drivers/interconnect/internal.h"
+
+TRACE_EVENT(icc_set_bw,
+
+	TP_PROTO(struct icc_path *p, struct icc_node *n, int i,
+		 u32 avg_bw, u32 peak_bw),
+
+	TP_ARGS(p, n, i, avg_bw, peak_bw),
+
+	TP_STRUCT__entry(
+		__string(path_name, p->name)
+		__string(dev, dev_name(p->reqs[i].dev))
+		__string(node_name, n->name)
+		__field(u32, avg_bw)
+		__field(u32, peak_bw)
+		__field(u32, node_avg_bw)
+		__field(u32, node_peak_bw)
+	),
+
+	TP_fast_assign(
+		__assign_str(path_name, p->name);
+		__assign_str(dev, dev_name(p->reqs[i].dev));
+		__assign_str(node_name, n->name);
+		__entry->avg_bw = avg_bw;
+		__entry->peak_bw = peak_bw;
+		__entry->node_avg_bw = n->avg_bw;
+		__entry->node_peak_bw = n->peak_bw;
+	),
+
+	TP_printk("path=%s dev=%s node=%s avg_bw=%u peak_bw=%u agg_avg=%u agg_peak=%u",
+		  __get_str(path_name),
+		  __get_str(dev),
+		  __get_str(node_name),
+		  __entry->avg_bw,
+		  __entry->peak_bw,
+		  __entry->node_avg_bw,
+		  __entry->node_peak_bw)
+);
+
+TRACE_EVENT(icc_set_bw_end,
+
+	TP_PROTO(struct icc_path *p, int ret),
+
+	TP_ARGS(p, ret),
+
+	TP_STRUCT__entry(
+		__string(path_name, p->name)
+		__string(dev, dev_name(p->reqs[0].dev))
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		__assign_str(path_name, p->name);
+		__assign_str(dev, dev_name(p->reqs[0].dev));
+		__entry->ret = ret;
+	),
+
+	TP_printk("path=%s dev=%s ret=%d",
+		  __get_str(path_name),
+		  __get_str(dev),
+		  __entry->ret)
+);
+
+#endif /* _TRACE_INTERCONNECT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>