Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1472606 > unrolled thread

[PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-08-30 18:20 +0200
    Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm  PMU Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-08-31 11:40 +0200
      Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-08-31 16:20 +0200
        Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm  PMU Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-09-01 11:10 +0200
          Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-09-01 17:20 +0200

#1472606 — [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU

Using the PMU::set_drv_config() callback to enable the CoreSight
sink that will be used for the trace session.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/arch/arm/util/cs-etm.c | 56 +++++++++++++++++++++++++++++++++++++++
 tools/perf/arch/arm/util/cs-etm.h |  3 +++
 tools/perf/arch/arm/util/pmu.c    |  3 +++
 3 files changed, 62 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 829c479614f1..dfea6b635525 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -27,12 +27,16 @@
 #include "../../util/auxtrace.h"
 #include "../../util/cpumap.h"
 #include "../../util/evlist.h"
+#include "../../util/evsel.h"
 #include "../../util/pmu.h"
 #include "../../util/thread_map.h"
 #include "../../util/cs-etm.h"
 
 #include <stdlib.h>
 
+#define ENABLE_SINK_MAX	128
+#define CS_BUS_DEVICE_PATH "/bus/coresight/devices/"
+
 struct cs_etm_recording {
 	struct auxtrace_record	itr;
 	struct perf_pmu		*cs_etm_pmu;
@@ -557,3 +561,55 @@ struct auxtrace_record *cs_etm_record_init(int *err)
 out:
 	return NULL;
 }
+
+static FILE *cs_device__open_file(const char *name)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	const char *sysfs;
+
+	sysfs = sysfs__mountpoint();
+	if (!sysfs)
+		return NULL;
+
+	snprintf(path, PATH_MAX,
+		 "%s" CS_BUS_DEVICE_PATH "%s", sysfs, name);
+
+	if (stat(path, &st) < 0)
+		return NULL;
+
+	return fopen(path, "w");
+
+}
+
+static __attribute__((format(printf, 2, 3)))
+int cs_device__print_file(const char *name, const char *fmt, ...)
+{
+	va_list args;
+	FILE *file;
+	int ret = -EINVAL;
+
+	va_start(args, fmt);
+	file = cs_device__open_file(name);
+	if (file) {
+		ret = vfprintf(file, fmt, args);
+		fclose(file);
+	}
+	va_end(args);
+	return ret;
+}
+
+int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
+{
+	int ret;
+	char enable_sink[ENABLE_SINK_MAX];
+
+	snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
+		 term->val.drv_cfg, "enable_sink");
+
+	ret = cs_device__print_file(enable_sink, "%d", 1);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
diff --git a/tools/perf/arch/arm/util/cs-etm.h b/tools/perf/arch/arm/util/cs-etm.h
index 909f486d02d1..5256741be549 100644
--- a/tools/perf/arch/arm/util/cs-etm.h
+++ b/tools/perf/arch/arm/util/cs-etm.h
@@ -18,6 +18,9 @@
 #ifndef INCLUDE__PERF_CS_ETM_H__
 #define INCLUDE__PERF_CS_ETM_H__
 
+#include "../../util/evsel.h"
+
 struct auxtrace_record *cs_etm_record_init(int *err);
+int cs_etm_set_drv_config(struct perf_evsel_config_term *term);
 
 #endif
diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
index af9fb666b44f..c11f8a13d14d 100644
--- a/tools/perf/arch/arm/util/pmu.c
+++ b/tools/perf/arch/arm/util/pmu.c
@@ -19,8 +19,10 @@
 #include <linux/coresight-pmu.h>
 #include <linux/perf_event.h>
 
+#include "cs-etm.h"
 #include "../../util/pmu.h"
 
+
 struct perf_event_attr
 *perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
@@ -28,6 +30,7 @@ struct perf_event_attr
 	if (!strcmp(pmu->name, CORESIGHT_ETM_PMU_NAME)) {
 		/* add ETM default config here */
 		pmu->selectable = true;
+		pmu->set_drv_config = cs_etm_set_drv_config;
 	}
 #endif
 	return NULL;
-- 
2.7.4

[toc] | [next] | [standalone]

#1473232 — Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU

On 30/08/16 17:19, Mathieu Poirier wrote:
> Using the PMU::set_drv_config() callback to enable the CoreSight
> sink that will be used for the trace session.

> +int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
> +{
> +	int ret;
> +	char enable_sink[ENABLE_SINK_MAX];
> +
> +	snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
> +		 term->val.drv_cfg, "enable_sink");
> +
> +	ret = cs_device__print_file(enable_sink, "%d", 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}


Don't we have to disable the sink at the end of the session ? How is that
taken care of ? Did I miss that ?

Suzuki

[toc] | [prev] | [next] | [standalone]

#1473520

On 31 August 2016 at 03:37, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 30/08/16 17:19, Mathieu Poirier wrote:
>>
>> Using the PMU::set_drv_config() callback to enable the CoreSight
>> sink that will be used for the trace session.
>
>
>> +int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
>> +{
>> +       int ret;
>> +       char enable_sink[ENABLE_SINK_MAX];
>> +
>> +       snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
>> +                term->val.drv_cfg, "enable_sink");
>> +
>> +       ret = cs_device__print_file(enable_sink, "%d", 1);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>
>
>
> Don't we have to disable the sink at the end of the session ? How is that
> taken care of ? Did I miss that ?
>

Correct - the sink has to be disabled once it is no longer needed.  It
is a little tricky to do and I haven't decided on the best way to
proceed.  Fortunately that aspect doesn't affect this patchset.

Mathieu

> Suzuki

[toc] | [prev] | [next] | [standalone]

#1474200 — Re: [PATCH V7 5/5] perf tools: adding sink configuration for cs_etm PMU

On 31/08/16 15:14, Mathieu Poirier wrote:
> On 31 August 2016 at 03:37, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
>> On 30/08/16 17:19, Mathieu Poirier wrote:
>>>
>>> Using the PMU::set_drv_config() callback to enable the CoreSight
>>> sink that will be used for the trace session.
>>
>>
>>> +int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
>>> +{
>>> +       int ret;
>>> +       char enable_sink[ENABLE_SINK_MAX];
>>> +
>>> +       snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
>>> +                term->val.drv_cfg, "enable_sink");
>>> +
>>> +       ret = cs_device__print_file(enable_sink, "%d", 1);
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       return 0;
>>> +}
>>
>>
>>
>> Don't we have to disable the sink at the end of the session ? How is that
>> taken care of ? Did I miss that ?
>>
>
> Correct - the sink has to be disabled once it is no longer needed.  It
> is a little tricky to do and I haven't decided on the best way to
> proceed.  Fortunately that aspect doesn't affect this patchset.

Well, this patchset when used, could leave a sink enabled. If we a choose
a different sink (say an ETF) from the perf, which occurs before the
previous sink (say an ETR) in the coresight path, the perf wouldn't get any
trace data, without any clue.

May be we could register an atexit() handler for clearing the sink ? So that
it is guaranteed to clear it irrespective of the path taken by perf to exit ?

Cheers
Suzuki

[toc] | [prev] | [next] | [standalone]

#1474467

On 1 September 2016 at 03:04, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 31/08/16 15:14, Mathieu Poirier wrote:
>>
>> On 31 August 2016 at 03:37, Suzuki K Poulose <Suzuki.Poulose@arm.com>
>> wrote:
>>>
>>> On 30/08/16 17:19, Mathieu Poirier wrote:
>>>>
>>>>
>>>> Using the PMU::set_drv_config() callback to enable the CoreSight
>>>> sink that will be used for the trace session.
>>>
>>>
>>>
>>>> +int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
>>>> +{
>>>> +       int ret;
>>>> +       char enable_sink[ENABLE_SINK_MAX];
>>>> +
>>>> +       snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
>>>> +                term->val.drv_cfg, "enable_sink");
>>>> +
>>>> +       ret = cs_device__print_file(enable_sink, "%d", 1);
>>>> +       if (ret < 0)
>>>> +               return ret;
>>>> +
>>>> +       return 0;
>>>> +}
>>>
>>>
>>>
>>>
>>> Don't we have to disable the sink at the end of the session ? How is that
>>> taken care of ? Did I miss that ?
>>>
>>
>> Correct - the sink has to be disabled once it is no longer needed.  It
>> is a little tricky to do and I haven't decided on the best way to
>> proceed.  Fortunately that aspect doesn't affect this patchset.
>
>
> Well, this patchset when used, could leave a sink enabled.

At this time it does yes, but that is a problem inherent to the
CoreSight framework rather than perf's ability to push driver specific
configuration to the PMU drivers.  That is the reason why I decided to
address this issue in a later patch.  There is also the fact that I
haven't heard back from Arnaldo on this approach - if I get asked to
proceed differently then the method for disabling sinks will have to
change.

> If we a choose
> a different sink (say an ETF) from the perf, which occurs before the
> previous sink (say an ETR) in the coresight path, the perf wouldn't get any
> trace data, without any clue.
>
> May be we could register an atexit() handler for clearing the sink ? So that
> it is guaranteed to clear it irrespective of the path taken by perf to exit
> ?

Yes, that would be the most intuitive solution but it won't work for
CoreSight.  The PMU driver allows multiple traces session to exist
concurrently and that, with different sinks for each session (if the
HW topology supports it).  As such the sink for a session needs to be
disabled at driver level as soon as it got recognised in the
configuration of a path.  That way the next session, which may start
before the previous one ended, doesn't trip on the sink that was
selected earlier.

I thought about a solution and I think it can work but before moving
ahead with it, I need to have an idea of how this patchset will go.

Thanks for the comments,
Mathieu


>
> Cheers
> Suzuki
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web