linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] interconnect: Add basic tracepoints
@ 2019-11-28 14:18 Georgi Djakov
  2019-11-28 14:18 ` [PATCH v4 1/3] interconnect: Move internal structs into a separate file Georgi Djakov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Georgi Djakov @ 2019-11-28 14:18 UTC (permalink / raw)
  To: linux-pm
  Cc: rostedt, mingo, bjorn.andersson, vincent.guittot, daidavid1,
	okukatla, evgreen, mka, linux-arm-msm, linux-kernel,
	georgi.djakov

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.

v4:
- Move CREATE_TRACE_POINTS below all other headers and remove internal.h
  from trace.h
- Picked Steven's Reviewed-by.

v3: http://lore.kernel.org/r/20191122152712.19105-1-georgi.djakov@linaro.org
- In order to avoid #including a file with relative path, move the trace.h
  header into drivers/interconnect/. (Steven)

v2: https://lore.kernel.org/r/20191101130031.27996-1-georgi.djakov@linaro.org
- Moved dev_name() into TP_fast_assign() to reduce cache footprint. (Steven)
- Added path name to traces (Bjorn)
- Added trace for path, device and ret. (Bjorn)

v1: https://lore.kernel.org/r/20191018140224.15087-1-georgi.djakov@linaro.org

Georgi Djakov (3):
  interconnect: Move internal structs into a separate file
  interconnect: Add a name to struct icc_path
  interconnect: Add basic tracepoints

 drivers/interconnect/Makefile   |  1 +
 drivers/interconnect/core.c     | 55 +++++++++------------
 drivers/interconnect/internal.h | 42 ++++++++++++++++
 drivers/interconnect/trace.h    | 88 +++++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 31 deletions(-)
 create mode 100644 drivers/interconnect/internal.h
 create mode 100644 drivers/interconnect/trace.h


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

* [PATCH v4 1/3] interconnect: Move internal structs into a separate file
  2019-11-28 14:18 [PATCH v4 0/3] interconnect: Add basic tracepoints Georgi Djakov
@ 2019-11-28 14:18 ` Georgi Djakov
  2019-11-28 18:08   ` Bjorn Andersson
  2019-11-28 14:18 ` [PATCH v4 2/3] interconnect: Add a name to struct icc_path Georgi Djakov
  2019-11-28 14:18 ` [PATCH v4 3/3] interconnect: Add basic tracepoints Georgi Djakov
  2 siblings, 1 reply; 7+ messages in thread
From: Georgi Djakov @ 2019-11-28 14:18 UTC (permalink / raw)
  To: linux-pm
  Cc: rostedt, mingo, bjorn.andersson, vincent.guittot, daidavid1,
	okukatla, evgreen, mka, linux-arm-msm, linux-kernel,
	georgi.djakov

Move the interconnect framework internal structs into a separate file,
so that it can be included and used by ftrace code. This will allow us
to expose some more useful information in the traces.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/interconnect/core.c     | 30 ++-----------------------
 drivers/interconnect/internal.h | 40 +++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 28 deletions(-)
 create mode 100644 drivers/interconnect/internal.h

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 1b811423020a..f30a326dc7ce 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -19,39 +19,13 @@
 #include <linux/of.h>
 #include <linux/overflow.h>
 
+#include "internal.h"
+
 static DEFINE_IDR(icc_idr);
 static LIST_HEAD(icc_providers);
 static DEFINE_MUTEX(icc_lock);
 static struct dentry *icc_debugfs_dir;
 
-/**
- * struct icc_req - constraints that are attached to each node
- * @req_node: entry in list of requests for the particular @node
- * @node: the interconnect node to which this constraint applies
- * @dev: reference to the device that sets the constraints
- * @tag: path tag (optional)
- * @avg_bw: an integer describing the average bandwidth in kBps
- * @peak_bw: an integer describing the peak bandwidth in kBps
- */
-struct icc_req {
-	struct hlist_node req_node;
-	struct icc_node *node;
-	struct device *dev;
-	u32 tag;
-	u32 avg_bw;
-	u32 peak_bw;
-};
-
-/**
- * struct icc_path - interconnect path structure
- * @num_nodes: number of hops (nodes)
- * @reqs: array of the requests applicable to this path of nodes
- */
-struct icc_path {
-	size_t num_nodes;
-	struct icc_req reqs[];
-};
-
 static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)
 {
 	if (!n)
diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h
new file mode 100644
index 000000000000..5853e8faf223
--- /dev/null
+++ b/drivers/interconnect/internal.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework internal structs
+ *
+ * Copyright (c) 2019, Linaro Ltd.
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#ifndef __DRIVERS_INTERCONNECT_INTERNAL_H
+#define __DRIVERS_INTERCONNECT_INTERNAL_H
+
+/**
+ * struct icc_req - constraints that are attached to each node
+ * @req_node: entry in list of requests for the particular @node
+ * @node: the interconnect node to which this constraint applies
+ * @dev: reference to the device that sets the constraints
+ * @tag: path tag (optional)
+ * @avg_bw: an integer describing the average bandwidth in kBps
+ * @peak_bw: an integer describing the peak bandwidth in kBps
+ */
+struct icc_req {
+	struct hlist_node req_node;
+	struct icc_node *node;
+	struct device *dev;
+	u32 tag;
+	u32 avg_bw;
+	u32 peak_bw;
+};
+
+/**
+ * struct icc_path - interconnect path structure
+ * @num_nodes: number of hops (nodes)
+ * @reqs: array of the requests applicable to this path of nodes
+ */
+struct icc_path {
+	size_t num_nodes;
+	struct icc_req reqs[];
+};
+
+#endif

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

* [PATCH v4 2/3] interconnect: Add a name to struct icc_path
  2019-11-28 14:18 [PATCH v4 0/3] interconnect: Add basic tracepoints Georgi Djakov
  2019-11-28 14:18 ` [PATCH v4 1/3] interconnect: Move internal structs into a separate file Georgi Djakov
@ 2019-11-28 14:18 ` Georgi Djakov
  2019-11-28 18:04   ` Bjorn Andersson
  2019-11-28 14:18 ` [PATCH v4 3/3] interconnect: Add basic tracepoints Georgi Djakov
  2 siblings, 1 reply; 7+ messages in thread
From: Georgi Djakov @ 2019-11-28 14:18 UTC (permalink / raw)
  To: linux-pm
  Cc: rostedt, mingo, bjorn.andersson, vincent.guittot, daidavid1,
	okukatla, evgreen, mka, linux-arm-msm, linux-kernel,
	georgi.djakov

When debugging interconnect things, it turned out that saving the path
name and including it in the traces is quite useful, especially for
devices with multiple paths.

For the path name we use the one specified in DT, or if we use platform
data, the name is based on the source and destination node names.

Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/interconnect/core.c     | 18 +++++++++++++++---
 drivers/interconnect/internal.h |  2 ++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index f30a326dc7ce..c9e16bc1331e 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -356,9 +356,17 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
 
 	mutex_lock(&icc_lock);
 	path = path_find(dev, src_node, dst_node);
-	if (IS_ERR(path))
-		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
 	mutex_unlock(&icc_lock);
+	if (IS_ERR(path)) {
+		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
+		return path;
+	}
+
+	if (name)
+		path->name = kstrdup(name, GFP_KERNEL);
+	else
+		path->name = kasprintf(GFP_KERNEL, "%s-%s",
+				       src_node->name, dst_node->name);
 
 	return path;
 }
@@ -481,9 +489,12 @@ struct icc_path *icc_get(struct device *dev, const int src_id, const int dst_id)
 		goto out;
 
 	path = path_find(dev, src, dst);
-	if (IS_ERR(path))
+	if (IS_ERR(path)) {
 		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
+		goto out;
+	}
 
+	path->name = kasprintf(GFP_KERNEL, "%s-%s", src->name, dst->name);
 out:
 	mutex_unlock(&icc_lock);
 	return path;
@@ -519,6 +530,7 @@ void icc_put(struct icc_path *path)
 	}
 	mutex_unlock(&icc_lock);
 
+	kfree(path->name);
 	kfree(path);
 }
 EXPORT_SYMBOL_GPL(icc_put);
diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h
index 5853e8faf223..bf18cb7239df 100644
--- a/drivers/interconnect/internal.h
+++ b/drivers/interconnect/internal.h
@@ -29,10 +29,12 @@ struct icc_req {
 
 /**
  * struct icc_path - interconnect path structure
+ * @name: a string name of the path (useful for ftrace)
  * @num_nodes: number of hops (nodes)
  * @reqs: array of the requests applicable to this path of nodes
  */
 struct icc_path {
+	const char *name;
 	size_t num_nodes;
 	struct icc_req reqs[];
 };

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

* [PATCH v4 3/3] interconnect: Add basic tracepoints
  2019-11-28 14:18 [PATCH v4 0/3] interconnect: Add basic tracepoints Georgi Djakov
  2019-11-28 14:18 ` [PATCH v4 1/3] interconnect: Move internal structs into a separate file Georgi Djakov
  2019-11-28 14:18 ` [PATCH v4 2/3] interconnect: Add a name to struct icc_path Georgi Djakov
@ 2019-11-28 14:18 ` Georgi Djakov
  2019-11-28 18:08   ` Bjorn Andersson
  2 siblings, 1 reply; 7+ messages in thread
From: Georgi Djakov @ 2019-11-28 14:18 UTC (permalink / raw)
  To: linux-pm
  Cc: rostedt, mingo, bjorn.andersson, vincent.guittot, daidavid1,
	okukatla, evgreen, mka, linux-arm-msm, linux-kernel,
	georgi.djakov

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.

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/interconnect/Makefile |  1 +
 drivers/interconnect/core.c   |  7 +++
 drivers/interconnect/trace.h  | 88 +++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 drivers/interconnect/trace.h

diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 28f2ab0824d5..725029ae7a2c 100644
--- a/drivers/interconnect/Makefile
+++ b/drivers/interconnect/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
+CFLAGS_core.o				:= -I$(src)
 icc-core-objs				:= core.o
 
 obj-$(CONFIG_INTERCONNECT)		+= icc-core.o
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index c9e16bc1331e..0e4852feb395 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -21,6 +21,9 @@
 
 #include "internal.h"
 
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
 static DEFINE_IDR(icc_idr);
 static LIST_HEAD(icc_providers);
 static DEFINE_MUTEX(icc_lock);
@@ -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/drivers/interconnect/trace.h b/drivers/interconnect/trace.h
new file mode 100644
index 000000000000..3d668ff566bf
--- /dev/null
+++ b/drivers/interconnect/trace.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework tracepoints
+ * Copyright (c) 2019, Linaro Ltd.
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM interconnect
+
+#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INTERCONNECT_H
+
+#include <linux/interconnect.h>
+#include <linux/tracepoint.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 */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+
+#include <trace/define_trace.h>

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

* Re: [PATCH v4 2/3] interconnect: Add a name to struct icc_path
  2019-11-28 14:18 ` [PATCH v4 2/3] interconnect: Add a name to struct icc_path Georgi Djakov
@ 2019-11-28 18:04   ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2019-11-28 18:04 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: linux-pm, rostedt, mingo, vincent.guittot, daidavid1, okukatla,
	evgreen, mka, linux-arm-msm, linux-kernel

On Thu 28 Nov 06:18 PST 2019, Georgi Djakov wrote:

> When debugging interconnect things, it turned out that saving the path
> name and including it in the traces is quite useful, especially for
> devices with multiple paths.
> 
> For the path name we use the one specified in DT, or if we use platform
> data, the name is based on the source and destination node names.
> 
> Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
>  drivers/interconnect/core.c     | 18 +++++++++++++++---
>  drivers/interconnect/internal.h |  2 ++
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index f30a326dc7ce..c9e16bc1331e 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -356,9 +356,17 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
>  
>  	mutex_lock(&icc_lock);
>  	path = path_find(dev, src_node, dst_node);
> -	if (IS_ERR(path))
> -		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
>  	mutex_unlock(&icc_lock);
> +	if (IS_ERR(path)) {
> +		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
> +		return path;
> +	}
> +
> +	if (name)
> +		path->name = kstrdup(name, GFP_KERNEL);

path->name is declared as const and name is likely to be rodata, so
using kstrdup_const() here instead have a good chance of avoiding an
unnecessary allocation.

> +	else
> +		path->name = kasprintf(GFP_KERNEL, "%s-%s",
> +				       src_node->name, dst_node->name);
>  
>  	return path;
>  }
> @@ -481,9 +489,12 @@ struct icc_path *icc_get(struct device *dev, const int src_id, const int dst_id)
>  		goto out;
>  
>  	path = path_find(dev, src, dst);
> -	if (IS_ERR(path))
> +	if (IS_ERR(path)) {
>  		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
> +		goto out;
> +	}
>  
> +	path->name = kasprintf(GFP_KERNEL, "%s-%s", src->name, dst->name);
>  out:
>  	mutex_unlock(&icc_lock);
>  	return path;
> @@ -519,6 +530,7 @@ void icc_put(struct icc_path *path)
>  	}
>  	mutex_unlock(&icc_lock);
>  
> +	kfree(path->name);

And then kfree_const() here (which will handle both the rodata and
dynamically allocated case).


Apart from this I think the patch looks good.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

>  	kfree(path);
>  }
>  EXPORT_SYMBOL_GPL(icc_put);
> diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h
> index 5853e8faf223..bf18cb7239df 100644
> --- a/drivers/interconnect/internal.h
> +++ b/drivers/interconnect/internal.h
> @@ -29,10 +29,12 @@ struct icc_req {
>  
>  /**
>   * struct icc_path - interconnect path structure
> + * @name: a string name of the path (useful for ftrace)
>   * @num_nodes: number of hops (nodes)
>   * @reqs: array of the requests applicable to this path of nodes
>   */
>  struct icc_path {
> +	const char *name;
>  	size_t num_nodes;
>  	struct icc_req reqs[];
>  };

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

* Re: [PATCH v4 3/3] interconnect: Add basic tracepoints
  2019-11-28 14:18 ` [PATCH v4 3/3] interconnect: Add basic tracepoints Georgi Djakov
@ 2019-11-28 18:08   ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2019-11-28 18:08 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: linux-pm, rostedt, mingo, vincent.guittot, daidavid1, okukatla,
	evgreen, mka, linux-arm-msm, linux-kernel

On Thu 28 Nov 06:18 PST 2019, Georgi Djakov wrote:

> 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.
> 
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
>  drivers/interconnect/Makefile |  1 +
>  drivers/interconnect/core.c   |  7 +++
>  drivers/interconnect/trace.h  | 88 +++++++++++++++++++++++++++++++++++
>  3 files changed, 96 insertions(+)
>  create mode 100644 drivers/interconnect/trace.h
> 
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 28f2ab0824d5..725029ae7a2c 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> +CFLAGS_core.o				:= -I$(src)
>  icc-core-objs				:= core.o
>  
>  obj-$(CONFIG_INTERCONNECT)		+= icc-core.o
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index c9e16bc1331e..0e4852feb395 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -21,6 +21,9 @@
>  
>  #include "internal.h"
>  
> +#define CREATE_TRACE_POINTS
> +#include "trace.h"
> +
>  static DEFINE_IDR(icc_idr);
>  static LIST_HEAD(icc_providers);
>  static DEFINE_MUTEX(icc_lock);
> @@ -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/drivers/interconnect/trace.h b/drivers/interconnect/trace.h
> new file mode 100644
> index 000000000000..3d668ff566bf
> --- /dev/null
> +++ b/drivers/interconnect/trace.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework tracepoints
> + * Copyright (c) 2019, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM interconnect
> +
> +#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_INTERCONNECT_H
> +
> +#include <linux/interconnect.h>
> +#include <linux/tracepoint.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 */
> +
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH .
> +
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE trace
> +
> +#include <trace/define_trace.h>

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

* Re: [PATCH v4 1/3] interconnect: Move internal structs into a separate file
  2019-11-28 14:18 ` [PATCH v4 1/3] interconnect: Move internal structs into a separate file Georgi Djakov
@ 2019-11-28 18:08   ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2019-11-28 18:08 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: linux-pm, rostedt, mingo, vincent.guittot, daidavid1, okukatla,
	evgreen, mka, linux-arm-msm, linux-kernel

On Thu 28 Nov 06:18 PST 2019, Georgi Djakov wrote:

> Move the interconnect framework internal structs into a separate file,
> so that it can be included and used by ftrace code. This will allow us
> to expose some more useful information in the traces.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
>  drivers/interconnect/core.c     | 30 ++-----------------------
>  drivers/interconnect/internal.h | 40 +++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/interconnect/internal.h
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 1b811423020a..f30a326dc7ce 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -19,39 +19,13 @@
>  #include <linux/of.h>
>  #include <linux/overflow.h>
>  
> +#include "internal.h"
> +
>  static DEFINE_IDR(icc_idr);
>  static LIST_HEAD(icc_providers);
>  static DEFINE_MUTEX(icc_lock);
>  static struct dentry *icc_debugfs_dir;
>  
> -/**
> - * struct icc_req - constraints that are attached to each node
> - * @req_node: entry in list of requests for the particular @node
> - * @node: the interconnect node to which this constraint applies
> - * @dev: reference to the device that sets the constraints
> - * @tag: path tag (optional)
> - * @avg_bw: an integer describing the average bandwidth in kBps
> - * @peak_bw: an integer describing the peak bandwidth in kBps
> - */
> -struct icc_req {
> -	struct hlist_node req_node;
> -	struct icc_node *node;
> -	struct device *dev;
> -	u32 tag;
> -	u32 avg_bw;
> -	u32 peak_bw;
> -};
> -
> -/**
> - * struct icc_path - interconnect path structure
> - * @num_nodes: number of hops (nodes)
> - * @reqs: array of the requests applicable to this path of nodes
> - */
> -struct icc_path {
> -	size_t num_nodes;
> -	struct icc_req reqs[];
> -};
> -
>  static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)
>  {
>  	if (!n)
> diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h
> new file mode 100644
> index 000000000000..5853e8faf223
> --- /dev/null
> +++ b/drivers/interconnect/internal.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework internal structs
> + *
> + * Copyright (c) 2019, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef __DRIVERS_INTERCONNECT_INTERNAL_H
> +#define __DRIVERS_INTERCONNECT_INTERNAL_H
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @dev: reference to the device that sets the constraints
> + * @tag: path tag (optional)
> + * @avg_bw: an integer describing the average bandwidth in kBps
> + * @peak_bw: an integer describing the peak bandwidth in kBps
> + */
> +struct icc_req {
> +	struct hlist_node req_node;
> +	struct icc_node *node;
> +	struct device *dev;
> +	u32 tag;
> +	u32 avg_bw;
> +	u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> +	size_t num_nodes;
> +	struct icc_req reqs[];
> +};
> +
> +#endif

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

end of thread, other threads:[~2019-11-28 18:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 14:18 [PATCH v4 0/3] interconnect: Add basic tracepoints Georgi Djakov
2019-11-28 14:18 ` [PATCH v4 1/3] interconnect: Move internal structs into a separate file Georgi Djakov
2019-11-28 18:08   ` Bjorn Andersson
2019-11-28 14:18 ` [PATCH v4 2/3] interconnect: Add a name to struct icc_path Georgi Djakov
2019-11-28 18:04   ` Bjorn Andersson
2019-11-28 14:18 ` [PATCH v4 3/3] interconnect: Add basic tracepoints Georgi Djakov
2019-11-28 18:08   ` Bjorn Andersson

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