Re: [PATCH v2 3/3] interconnect: Add basic tracepoints

From: Steven Rostedt
Date: Wed Nov 13 2019 - 15:23:22 EST


On Fri, 1 Nov 2019 15:00:31 +0200
Georgi Djakov <georgi.djakov@xxxxxxxxxx> 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@xxxxxxxxxx>
> + */
> +
> +#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>