* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-12 17:53 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-12 17:53 UTC (permalink / raw)
To: acme, adrian.hunter
Cc: mingo, peterz, alexander.shishkin, jolsa, acme, linux-kernel,
linux-arm-kernel
This patch makes it possible to use the current filter
framework with address filters. That way address filters for
HW tracers such as CoreSight and IntelPT can be communicated
to the kernel drivers.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
Changes for V5:
- Modified perf_evsel__append_filter() to take a string format
rather than an operation.
Changes for V4:
- Added support for address filters over more than one
nibble.
- Removed Jiri's ack, this version is too different from
what was reviewed.
Changes for V3:
- Added Jiri's ack.
- Rebased to v4.8-rc5.
Changes for V2:
- Rebased to v4.8-rc4.
- Revisited error path.
tools/perf/util/evsel.c | 4 ++--
tools/perf/util/evsel.h | 2 +-
tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d40f852d2de2..a9bb277f221f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
}
int perf_evsel__append_filter(struct perf_evsel *evsel,
- const char *op, const char *filter)
+ const char *fmt, const char *filter)
{
char *new_filter;
if (evsel->filter == NULL)
return perf_evsel__set_filter(evsel, filter);
- if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
+ if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
free(evsel->filter);
evsel->filter = new_filter;
return 0;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 8ceb7ebb51f5..50595c8c7207 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
int perf_evsel__append_filter(struct perf_evsel *evsel,
- const char *op, const char *filter);
+ const char *fmt, const char *filter);
int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
const char *filter);
int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2eb8b1ed4cc8..8e683979ccd8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
static int set_filter(struct perf_evsel *evsel, const void *arg)
{
const char *str = arg;
+ bool found = false;
+ int nr_addr_filters = 0;
+ struct perf_pmu *pmu = NULL;
- if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
- fprintf(stderr,
- "--filter option should follow a -e tracepoint option\n");
- return -1;
+ if (evsel == NULL)
+ goto err;
+
+ if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
+ if (perf_evsel__append_filter(evsel,
+ "(%s) && (%s)", str) < 0) {
+ fprintf(stderr,
+ "not enough memory to hold filter string\n");
+ return -1;
+ }
+
+ return 0;
}
- if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
+ while ((pmu = perf_pmu__scan(pmu)) != NULL)
+ if (pmu->type == evsel->attr.type) {
+ found = true;
+ break;
+ }
+
+ if (found)
+ perf_pmu__scan_file(pmu, "nr_addr_filters",
+ "%d", &nr_addr_filters);
+
+ if (!nr_addr_filters)
+ goto err;
+
+ if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
fprintf(stderr,
"not enough memory to hold filter string\n");
return -1;
}
return 0;
+
+err:
+ fprintf(stderr,
+ "--filter option should follow a -e tracepoint or HW tracer option\n");
+
+ return -1;
}
int parse_filter(const struct option *opt, const char *str,
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-12 17:53 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-12 17:53 UTC (permalink / raw)
To: linux-arm-kernel
This patch makes it possible to use the current filter
framework with address filters. That way address filters for
HW tracers such as CoreSight and IntelPT can be communicated
to the kernel drivers.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
Changes for V5:
- Modified perf_evsel__append_filter() to take a string format
rather than an operation.
Changes for V4:
- Added support for address filters over more than one
nibble.
- Removed Jiri's ack, this version is too different from
what was reviewed.
Changes for V3:
- Added Jiri's ack.
- Rebased to v4.8-rc5.
Changes for V2:
- Rebased to v4.8-rc4.
- Revisited error path.
tools/perf/util/evsel.c | 4 ++--
tools/perf/util/evsel.h | 2 +-
tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d40f852d2de2..a9bb277f221f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
}
int perf_evsel__append_filter(struct perf_evsel *evsel,
- const char *op, const char *filter)
+ const char *fmt, const char *filter)
{
char *new_filter;
if (evsel->filter == NULL)
return perf_evsel__set_filter(evsel, filter);
- if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
+ if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
free(evsel->filter);
evsel->filter = new_filter;
return 0;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 8ceb7ebb51f5..50595c8c7207 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
int perf_evsel__append_filter(struct perf_evsel *evsel,
- const char *op, const char *filter);
+ const char *fmt, const char *filter);
int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
const char *filter);
int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2eb8b1ed4cc8..8e683979ccd8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
static int set_filter(struct perf_evsel *evsel, const void *arg)
{
const char *str = arg;
+ bool found = false;
+ int nr_addr_filters = 0;
+ struct perf_pmu *pmu = NULL;
- if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
- fprintf(stderr,
- "--filter option should follow a -e tracepoint option\n");
- return -1;
+ if (evsel == NULL)
+ goto err;
+
+ if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
+ if (perf_evsel__append_filter(evsel,
+ "(%s) && (%s)", str) < 0) {
+ fprintf(stderr,
+ "not enough memory to hold filter string\n");
+ return -1;
+ }
+
+ return 0;
}
- if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
+ while ((pmu = perf_pmu__scan(pmu)) != NULL)
+ if (pmu->type == evsel->attr.type) {
+ found = true;
+ break;
+ }
+
+ if (found)
+ perf_pmu__scan_file(pmu, "nr_addr_filters",
+ "%d", &nr_addr_filters);
+
+ if (!nr_addr_filters)
+ goto err;
+
+ if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
fprintf(stderr,
"not enough memory to hold filter string\n");
return -1;
}
return 0;
+
+err:
+ fprintf(stderr,
+ "--filter option should follow a -e tracepoint or HW tracer option\n");
+
+ return -1;
}
int parse_filter(const struct option *opt, const char *str,
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-12 17:53 ` Mathieu Poirier
@ 2016-09-13 10:01 ` Adrian Hunter
-1 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-13 10:01 UTC (permalink / raw)
To: Mathieu Poirier, acme
Cc: mingo, peterz, alexander.shishkin, jolsa, acme, linux-kernel,
linux-arm-kernel
On 12/09/16 20:53, Mathieu Poirier wrote:
> This patch makes it possible to use the current filter
> framework with address filters. That way address filters for
> HW tracers such as CoreSight and IntelPT can be communicated
> to the kernel drivers.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> ---
> Changes for V5:
> - Modified perf_evsel__append_filter() to take a string format
> rather than an operation.
Hope I'm not being a pain, but aren't there other places calling
perf_evsel__append_filter() that need to be changed. Might make
sense as a separate patch.
>
> Changes for V4:
> - Added support for address filters over more than one
> nibble.
> - Removed Jiri's ack, this version is too different from
> what was reviewed.
>
> Changes for V3:
> - Added Jiri's ack.
> - Rebased to v4.8-rc5.
>
> Changes for V2:
> - Rebased to v4.8-rc4.
> - Revisited error path.
>
>
> tools/perf/util/evsel.c | 4 ++--
> tools/perf/util/evsel.h | 2 +-
> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> 3 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d40f852d2de2..a9bb277f221f 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> }
>
> int perf_evsel__append_filter(struct perf_evsel *evsel,
> - const char *op, const char *filter)
> + const char *fmt, const char *filter)
> {
> char *new_filter;
>
> if (evsel->filter == NULL)
> return perf_evsel__set_filter(evsel, filter);
>
> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> free(evsel->filter);
> evsel->filter = new_filter;
> return 0;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 8ceb7ebb51f5..50595c8c7207 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>
> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> int perf_evsel__append_filter(struct perf_evsel *evsel,
> - const char *op, const char *filter);
> + const char *fmt, const char *filter);
> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> const char *filter);
> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 2eb8b1ed4cc8..8e683979ccd8 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> static int set_filter(struct perf_evsel *evsel, const void *arg)
> {
> const char *str = arg;
> + bool found = false;
> + int nr_addr_filters = 0;
> + struct perf_pmu *pmu = NULL;
>
> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> - fprintf(stderr,
> - "--filter option should follow a -e tracepoint option\n");
> - return -1;
> + if (evsel == NULL)
> + goto err;
> +
> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> + if (perf_evsel__append_filter(evsel,
> + "(%s) && (%s)", str) < 0) {
> + fprintf(stderr,
> + "not enough memory to hold filter string\n");
> + return -1;
> + }
> +
> + return 0;
> }
>
> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> + if (pmu->type == evsel->attr.type) {
> + found = true;
> + break;
> + }
> +
> + if (found)
> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> + "%d", &nr_addr_filters);
> +
> + if (!nr_addr_filters)
> + goto err;
> +
> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> fprintf(stderr,
> "not enough memory to hold filter string\n");
> return -1;
> }
>
> return 0;
> +
> +err:
> + fprintf(stderr,
> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> +
> + return -1;
> }
>
> int parse_filter(const struct option *opt, const char *str,
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-13 10:01 ` Adrian Hunter
0 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-13 10:01 UTC (permalink / raw)
To: linux-arm-kernel
On 12/09/16 20:53, Mathieu Poirier wrote:
> This patch makes it possible to use the current filter
> framework with address filters. That way address filters for
> HW tracers such as CoreSight and IntelPT can be communicated
> to the kernel drivers.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> ---
> Changes for V5:
> - Modified perf_evsel__append_filter() to take a string format
> rather than an operation.
Hope I'm not being a pain, but aren't there other places calling
perf_evsel__append_filter() that need to be changed. Might make
sense as a separate patch.
>
> Changes for V4:
> - Added support for address filters over more than one
> nibble.
> - Removed Jiri's ack, this version is too different from
> what was reviewed.
>
> Changes for V3:
> - Added Jiri's ack.
> - Rebased to v4.8-rc5.
>
> Changes for V2:
> - Rebased to v4.8-rc4.
> - Revisited error path.
>
>
> tools/perf/util/evsel.c | 4 ++--
> tools/perf/util/evsel.h | 2 +-
> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> 3 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d40f852d2de2..a9bb277f221f 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> }
>
> int perf_evsel__append_filter(struct perf_evsel *evsel,
> - const char *op, const char *filter)
> + const char *fmt, const char *filter)
> {
> char *new_filter;
>
> if (evsel->filter == NULL)
> return perf_evsel__set_filter(evsel, filter);
>
> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> free(evsel->filter);
> evsel->filter = new_filter;
> return 0;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 8ceb7ebb51f5..50595c8c7207 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>
> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> int perf_evsel__append_filter(struct perf_evsel *evsel,
> - const char *op, const char *filter);
> + const char *fmt, const char *filter);
> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> const char *filter);
> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 2eb8b1ed4cc8..8e683979ccd8 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> static int set_filter(struct perf_evsel *evsel, const void *arg)
> {
> const char *str = arg;
> + bool found = false;
> + int nr_addr_filters = 0;
> + struct perf_pmu *pmu = NULL;
>
> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> - fprintf(stderr,
> - "--filter option should follow a -e tracepoint option\n");
> - return -1;
> + if (evsel == NULL)
> + goto err;
> +
> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> + if (perf_evsel__append_filter(evsel,
> + "(%s) && (%s)", str) < 0) {
> + fprintf(stderr,
> + "not enough memory to hold filter string\n");
> + return -1;
> + }
> +
> + return 0;
> }
>
> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> + if (pmu->type == evsel->attr.type) {
> + found = true;
> + break;
> + }
> +
> + if (found)
> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> + "%d", &nr_addr_filters);
> +
> + if (!nr_addr_filters)
> + goto err;
> +
> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> fprintf(stderr,
> "not enough memory to hold filter string\n");
> return -1;
> }
>
> return 0;
> +
> +err:
> + fprintf(stderr,
> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> +
> + return -1;
> }
>
> int parse_filter(const struct option *opt, const char *str,
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-13 10:01 ` Adrian Hunter
@ 2016-09-13 14:18 ` Mathieu Poirier
-1 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-13 14:18 UTC (permalink / raw)
To: Adrian Hunter
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Alexander Shishkin, jolsa, Arnaldo Carvalho de Melo,
linux-kernel, linux-arm-kernel
On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 12/09/16 20:53, Mathieu Poirier wrote:
>> This patch makes it possible to use the current filter
>> framework with address filters. That way address filters for
>> HW tracers such as CoreSight and IntelPT can be communicated
>> to the kernel drivers.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>> ---
>> Changes for V5:
>> - Modified perf_evsel__append_filter() to take a string format
>> rather than an operation.
>
> Hope I'm not being a pain, but aren't there other places calling
> perf_evsel__append_filter() that need to be changed. Might make
> sense as a separate patch.
No no, you're right - I completely overlooked that.
But shouldn't it be in the same patch? That way a git bisect would
stay consistent...
>
>>
>> Changes for V4:
>> - Added support for address filters over more than one
>> nibble.
>> - Removed Jiri's ack, this version is too different from
>> what was reviewed.
>>
>> Changes for V3:
>> - Added Jiri's ack.
>> - Rebased to v4.8-rc5.
>>
>> Changes for V2:
>> - Rebased to v4.8-rc4.
>> - Revisited error path.
>>
>>
>> tools/perf/util/evsel.c | 4 ++--
>> tools/perf/util/evsel.h | 2 +-
>> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>> 3 files changed, 38 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index d40f852d2de2..a9bb277f221f 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>> }
>>
>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> - const char *op, const char *filter)
>> + const char *fmt, const char *filter)
>> {
>> char *new_filter;
>>
>> if (evsel->filter == NULL)
>> return perf_evsel__set_filter(evsel, filter);
>>
>> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>> free(evsel->filter);
>> evsel->filter = new_filter;
>> return 0;
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index 8ceb7ebb51f5..50595c8c7207 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>>
>> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> - const char *op, const char *filter);
>> + const char *fmt, const char *filter);
>> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>> const char *filter);
>> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 2eb8b1ed4cc8..8e683979ccd8 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>> static int set_filter(struct perf_evsel *evsel, const void *arg)
>> {
>> const char *str = arg;
>> + bool found = false;
>> + int nr_addr_filters = 0;
>> + struct perf_pmu *pmu = NULL;
>>
>> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>> - fprintf(stderr,
>> - "--filter option should follow a -e tracepoint option\n");
>> - return -1;
>> + if (evsel == NULL)
>> + goto err;
>> +
>> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>> + if (perf_evsel__append_filter(evsel,
>> + "(%s) && (%s)", str) < 0) {
>> + fprintf(stderr,
>> + "not enough memory to hold filter string\n");
>> + return -1;
>> + }
>> +
>> + return 0;
>> }
>>
>> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>> + if (pmu->type == evsel->attr.type) {
>> + found = true;
>> + break;
>> + }
>> +
>> + if (found)
>> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>> + "%d", &nr_addr_filters);
>> +
>> + if (!nr_addr_filters)
>> + goto err;
>> +
>> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>> fprintf(stderr,
>> "not enough memory to hold filter string\n");
>> return -1;
>> }
>>
>> return 0;
>> +
>> +err:
>> + fprintf(stderr,
>> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>> +
>> + return -1;
>> }
>>
>> int parse_filter(const struct option *opt, const char *str,
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-13 14:18 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-13 14:18 UTC (permalink / raw)
To: linux-arm-kernel
On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 12/09/16 20:53, Mathieu Poirier wrote:
>> This patch makes it possible to use the current filter
>> framework with address filters. That way address filters for
>> HW tracers such as CoreSight and IntelPT can be communicated
>> to the kernel drivers.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>> ---
>> Changes for V5:
>> - Modified perf_evsel__append_filter() to take a string format
>> rather than an operation.
>
> Hope I'm not being a pain, but aren't there other places calling
> perf_evsel__append_filter() that need to be changed. Might make
> sense as a separate patch.
No no, you're right - I completely overlooked that.
But shouldn't it be in the same patch? That way a git bisect would
stay consistent...
>
>>
>> Changes for V4:
>> - Added support for address filters over more than one
>> nibble.
>> - Removed Jiri's ack, this version is too different from
>> what was reviewed.
>>
>> Changes for V3:
>> - Added Jiri's ack.
>> - Rebased to v4.8-rc5.
>>
>> Changes for V2:
>> - Rebased to v4.8-rc4.
>> - Revisited error path.
>>
>>
>> tools/perf/util/evsel.c | 4 ++--
>> tools/perf/util/evsel.h | 2 +-
>> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>> 3 files changed, 38 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index d40f852d2de2..a9bb277f221f 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>> }
>>
>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> - const char *op, const char *filter)
>> + const char *fmt, const char *filter)
>> {
>> char *new_filter;
>>
>> if (evsel->filter == NULL)
>> return perf_evsel__set_filter(evsel, filter);
>>
>> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>> free(evsel->filter);
>> evsel->filter = new_filter;
>> return 0;
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index 8ceb7ebb51f5..50595c8c7207 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>>
>> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> - const char *op, const char *filter);
>> + const char *fmt, const char *filter);
>> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>> const char *filter);
>> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 2eb8b1ed4cc8..8e683979ccd8 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>> static int set_filter(struct perf_evsel *evsel, const void *arg)
>> {
>> const char *str = arg;
>> + bool found = false;
>> + int nr_addr_filters = 0;
>> + struct perf_pmu *pmu = NULL;
>>
>> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>> - fprintf(stderr,
>> - "--filter option should follow a -e tracepoint option\n");
>> - return -1;
>> + if (evsel == NULL)
>> + goto err;
>> +
>> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>> + if (perf_evsel__append_filter(evsel,
>> + "(%s) && (%s)", str) < 0) {
>> + fprintf(stderr,
>> + "not enough memory to hold filter string\n");
>> + return -1;
>> + }
>> +
>> + return 0;
>> }
>>
>> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>> + if (pmu->type == evsel->attr.type) {
>> + found = true;
>> + break;
>> + }
>> +
>> + if (found)
>> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>> + "%d", &nr_addr_filters);
>> +
>> + if (!nr_addr_filters)
>> + goto err;
>> +
>> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>> fprintf(stderr,
>> "not enough memory to hold filter string\n");
>> return -1;
>> }
>>
>> return 0;
>> +
>> +err:
>> + fprintf(stderr,
>> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>> +
>> + return -1;
>> }
>>
>> int parse_filter(const struct option *opt, const char *str,
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-13 14:18 ` Mathieu Poirier
@ 2016-09-13 23:25 ` Masami Hiramatsu
-1 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-13 23:25 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Adrian Hunter, Arnaldo Carvalho de Melo, Ingo Molnar,
Peter Zijlstra, Alexander Shishkin, jolsa,
Arnaldo Carvalho de Melo, linux-kernel, linux-arm-kernel
On Tue, 13 Sep 2016 08:18:10 -0600
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> > On 12/09/16 20:53, Mathieu Poirier wrote:
> >> This patch makes it possible to use the current filter
> >> framework with address filters. That way address filters for
> >> HW tracers such as CoreSight and IntelPT can be communicated
> >> to the kernel drivers.
> >>
> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>
> >> ---
> >> Changes for V5:
> >> - Modified perf_evsel__append_filter() to take a string format
> >> rather than an operation.
> >
> > Hope I'm not being a pain, but aren't there other places calling
> > perf_evsel__append_filter() that need to be changed. Might make
> > sense as a separate patch.
>
> No no, you're right - I completely overlooked that.
>
> But shouldn't it be in the same patch? That way a git bisect would
> stay consistent...
You're right. Caller and callee should be changed in atomic.
BTW, could you add document updates how the perf command line
will be changed, and also show the result in the patch description?
Thank you,
>
> >
> >>
> >> Changes for V4:
> >> - Added support for address filters over more than one
> >> nibble.
> >> - Removed Jiri's ack, this version is too different from
> >> what was reviewed.
> >>
> >> Changes for V3:
> >> - Added Jiri's ack.
> >> - Rebased to v4.8-rc5.
> >>
> >> Changes for V2:
> >> - Rebased to v4.8-rc4.
> >> - Revisited error path.
> >>
> >>
> >> tools/perf/util/evsel.c | 4 ++--
> >> tools/perf/util/evsel.h | 2 +-
> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> >> 3 files changed, 38 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> >> index d40f852d2de2..a9bb277f221f 100644
> >> --- a/tools/perf/util/evsel.c
> >> +++ b/tools/perf/util/evsel.c
> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> >> }
> >>
> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> - const char *op, const char *filter)
> >> + const char *fmt, const char *filter)
> >> {
> >> char *new_filter;
> >>
> >> if (evsel->filter == NULL)
> >> return perf_evsel__set_filter(evsel, filter);
> >>
> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> >> free(evsel->filter);
> >> evsel->filter = new_filter;
> >> return 0;
> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> >> index 8ceb7ebb51f5..50595c8c7207 100644
> >> --- a/tools/perf/util/evsel.h
> >> +++ b/tools/perf/util/evsel.h
> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
> >>
> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> - const char *op, const char *filter);
> >> + const char *fmt, const char *filter);
> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> >> const char *filter);
> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
> >> --- a/tools/perf/util/parse-events.c
> >> +++ b/tools/perf/util/parse-events.c
> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
> >> {
> >> const char *str = arg;
> >> + bool found = false;
> >> + int nr_addr_filters = 0;
> >> + struct perf_pmu *pmu = NULL;
> >>
> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> >> - fprintf(stderr,
> >> - "--filter option should follow a -e tracepoint option\n");
> >> - return -1;
> >> + if (evsel == NULL)
> >> + goto err;
> >> +
> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> >> + if (perf_evsel__append_filter(evsel,
> >> + "(%s) && (%s)", str) < 0) {
> >> + fprintf(stderr,
> >> + "not enough memory to hold filter string\n");
> >> + return -1;
> >> + }
> >> +
> >> + return 0;
> >> }
> >>
> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> >> + if (pmu->type == evsel->attr.type) {
> >> + found = true;
> >> + break;
> >> + }
> >> +
> >> + if (found)
> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> >> + "%d", &nr_addr_filters);
> >> +
> >> + if (!nr_addr_filters)
> >> + goto err;
> >> +
> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> >> fprintf(stderr,
> >> "not enough memory to hold filter string\n");
> >> return -1;
> >> }
> >>
> >> return 0;
> >> +
> >> +err:
> >> + fprintf(stderr,
> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> >> +
> >> + return -1;
> >> }
> >>
> >> int parse_filter(const struct option *opt, const char *str,
> >>
> >
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-13 23:25 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-13 23:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 13 Sep 2016 08:18:10 -0600
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> > On 12/09/16 20:53, Mathieu Poirier wrote:
> >> This patch makes it possible to use the current filter
> >> framework with address filters. That way address filters for
> >> HW tracers such as CoreSight and IntelPT can be communicated
> >> to the kernel drivers.
> >>
> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>
> >> ---
> >> Changes for V5:
> >> - Modified perf_evsel__append_filter() to take a string format
> >> rather than an operation.
> >
> > Hope I'm not being a pain, but aren't there other places calling
> > perf_evsel__append_filter() that need to be changed. Might make
> > sense as a separate patch.
>
> No no, you're right - I completely overlooked that.
>
> But shouldn't it be in the same patch? That way a git bisect would
> stay consistent...
You're right. Caller and callee should be changed in atomic.
BTW, could you add document updates how the perf command line
will be changed, and also show the result in the patch description?
Thank you,
>
> >
> >>
> >> Changes for V4:
> >> - Added support for address filters over more than one
> >> nibble.
> >> - Removed Jiri's ack, this version is too different from
> >> what was reviewed.
> >>
> >> Changes for V3:
> >> - Added Jiri's ack.
> >> - Rebased to v4.8-rc5.
> >>
> >> Changes for V2:
> >> - Rebased to v4.8-rc4.
> >> - Revisited error path.
> >>
> >>
> >> tools/perf/util/evsel.c | 4 ++--
> >> tools/perf/util/evsel.h | 2 +-
> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> >> 3 files changed, 38 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> >> index d40f852d2de2..a9bb277f221f 100644
> >> --- a/tools/perf/util/evsel.c
> >> +++ b/tools/perf/util/evsel.c
> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> >> }
> >>
> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> - const char *op, const char *filter)
> >> + const char *fmt, const char *filter)
> >> {
> >> char *new_filter;
> >>
> >> if (evsel->filter == NULL)
> >> return perf_evsel__set_filter(evsel, filter);
> >>
> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> >> free(evsel->filter);
> >> evsel->filter = new_filter;
> >> return 0;
> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> >> index 8ceb7ebb51f5..50595c8c7207 100644
> >> --- a/tools/perf/util/evsel.h
> >> +++ b/tools/perf/util/evsel.h
> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
> >>
> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> - const char *op, const char *filter);
> >> + const char *fmt, const char *filter);
> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> >> const char *filter);
> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
> >> --- a/tools/perf/util/parse-events.c
> >> +++ b/tools/perf/util/parse-events.c
> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
> >> {
> >> const char *str = arg;
> >> + bool found = false;
> >> + int nr_addr_filters = 0;
> >> + struct perf_pmu *pmu = NULL;
> >>
> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> >> - fprintf(stderr,
> >> - "--filter option should follow a -e tracepoint option\n");
> >> - return -1;
> >> + if (evsel == NULL)
> >> + goto err;
> >> +
> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> >> + if (perf_evsel__append_filter(evsel,
> >> + "(%s) && (%s)", str) < 0) {
> >> + fprintf(stderr,
> >> + "not enough memory to hold filter string\n");
> >> + return -1;
> >> + }
> >> +
> >> + return 0;
> >> }
> >>
> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> >> + if (pmu->type == evsel->attr.type) {
> >> + found = true;
> >> + break;
> >> + }
> >> +
> >> + if (found)
> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> >> + "%d", &nr_addr_filters);
> >> +
> >> + if (!nr_addr_filters)
> >> + goto err;
> >> +
> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> >> fprintf(stderr,
> >> "not enough memory to hold filter string\n");
> >> return -1;
> >> }
> >>
> >> return 0;
> >> +
> >> +err:
> >> + fprintf(stderr,
> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> >> +
> >> + return -1;
> >> }
> >>
> >> int parse_filter(const struct option *opt, const char *str,
> >>
> >
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-13 14:18 ` Mathieu Poirier
@ 2016-09-14 13:41 ` Adrian Hunter
-1 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-14 13:41 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Alexander Shishkin, jolsa, Arnaldo Carvalho de Melo,
linux-kernel, linux-arm-kernel
On 13/09/16 17:18, Mathieu Poirier wrote:
> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 12/09/16 20:53, Mathieu Poirier wrote:
>>> This patch makes it possible to use the current filter
>>> framework with address filters. That way address filters for
>>> HW tracers such as CoreSight and IntelPT can be communicated
>>> to the kernel drivers.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>
>>> ---
>>> Changes for V5:
>>> - Modified perf_evsel__append_filter() to take a string format
>>> rather than an operation.
>>
>> Hope I'm not being a pain, but aren't there other places calling
>> perf_evsel__append_filter() that need to be changed. Might make
>> sense as a separate patch.
>
> No no, you're right - I completely overlooked that.
>
> But shouldn't it be in the same patch? That way a git bisect would
> stay consistent...
I meant changing perf_evsel__append_filter() and callers in a separate
preparatory patch. Perhaps add and use:
int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter)
{
return perf_evsel__append_filter(evsel, "(%s) && (%s)", filter);
}
>
>>
>>>
>>> Changes for V4:
>>> - Added support for address filters over more than one
>>> nibble.
>>> - Removed Jiri's ack, this version is too different from
>>> what was reviewed.
>>>
>>> Changes for V3:
>>> - Added Jiri's ack.
>>> - Rebased to v4.8-rc5.
>>>
>>> Changes for V2:
>>> - Rebased to v4.8-rc4.
>>> - Revisited error path.
>>>
>>>
>>> tools/perf/util/evsel.c | 4 ++--
>>> tools/perf/util/evsel.h | 2 +-
>>> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>>> 3 files changed, 38 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> index d40f852d2de2..a9bb277f221f 100644
>>> --- a/tools/perf/util/evsel.c
>>> +++ b/tools/perf/util/evsel.c
>>> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>>> }
>>>
>>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>>> - const char *op, const char *filter)
>>> + const char *fmt, const char *filter)
>>> {
>>> char *new_filter;
>>>
>>> if (evsel->filter == NULL)
>>> return perf_evsel__set_filter(evsel, filter);
>>>
>>> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>>> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>>> free(evsel->filter);
>>> evsel->filter = new_filter;
>>> return 0;
>>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>>> index 8ceb7ebb51f5..50595c8c7207 100644
>>> --- a/tools/perf/util/evsel.h
>>> +++ b/tools/perf/util/evsel.h
>>> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>>>
>>> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>>> - const char *op, const char *filter);
>>> + const char *fmt, const char *filter);
>>> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>>> const char *filter);
>>> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>> index 2eb8b1ed4cc8..8e683979ccd8 100644
>>> --- a/tools/perf/util/parse-events.c
>>> +++ b/tools/perf/util/parse-events.c
>>> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>>> static int set_filter(struct perf_evsel *evsel, const void *arg)
>>> {
>>> const char *str = arg;
>>> + bool found = false;
>>> + int nr_addr_filters = 0;
>>> + struct perf_pmu *pmu = NULL;
>>>
>>> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>>> - fprintf(stderr,
>>> - "--filter option should follow a -e tracepoint option\n");
>>> - return -1;
>>> + if (evsel == NULL)
>>> + goto err;
>>> +
>>> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>>> + if (perf_evsel__append_filter(evsel,
>>> + "(%s) && (%s)", str) < 0) {
>>> + fprintf(stderr,
>>> + "not enough memory to hold filter string\n");
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> }
>>>
>>> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>>> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>>> + if (pmu->type == evsel->attr.type) {
>>> + found = true;
>>> + break;
>>> + }
>>> +
>>> + if (found)
>>> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>>> + "%d", &nr_addr_filters);
>>> +
>>> + if (!nr_addr_filters)
>>> + goto err;
>>> +
>>> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>>> fprintf(stderr,
>>> "not enough memory to hold filter string\n");
>>> return -1;
>>> }
>>>
>>> return 0;
>>> +
>>> +err:
>>> + fprintf(stderr,
>>> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>>> +
>>> + return -1;
>>> }
>>>
>>> int parse_filter(const struct option *opt, const char *str,
>>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-14 13:41 ` Adrian Hunter
0 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-14 13:41 UTC (permalink / raw)
To: linux-arm-kernel
On 13/09/16 17:18, Mathieu Poirier wrote:
> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 12/09/16 20:53, Mathieu Poirier wrote:
>>> This patch makes it possible to use the current filter
>>> framework with address filters. That way address filters for
>>> HW tracers such as CoreSight and IntelPT can be communicated
>>> to the kernel drivers.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>
>>> ---
>>> Changes for V5:
>>> - Modified perf_evsel__append_filter() to take a string format
>>> rather than an operation.
>>
>> Hope I'm not being a pain, but aren't there other places calling
>> perf_evsel__append_filter() that need to be changed. Might make
>> sense as a separate patch.
>
> No no, you're right - I completely overlooked that.
>
> But shouldn't it be in the same patch? That way a git bisect would
> stay consistent...
I meant changing perf_evsel__append_filter() and callers in a separate
preparatory patch. Perhaps add and use:
int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter)
{
return perf_evsel__append_filter(evsel, "(%s) && (%s)", filter);
}
>
>>
>>>
>>> Changes for V4:
>>> - Added support for address filters over more than one
>>> nibble.
>>> - Removed Jiri's ack, this version is too different from
>>> what was reviewed.
>>>
>>> Changes for V3:
>>> - Added Jiri's ack.
>>> - Rebased to v4.8-rc5.
>>>
>>> Changes for V2:
>>> - Rebased to v4.8-rc4.
>>> - Revisited error path.
>>>
>>>
>>> tools/perf/util/evsel.c | 4 ++--
>>> tools/perf/util/evsel.h | 2 +-
>>> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>>> 3 files changed, 38 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> index d40f852d2de2..a9bb277f221f 100644
>>> --- a/tools/perf/util/evsel.c
>>> +++ b/tools/perf/util/evsel.c
>>> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>>> }
>>>
>>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>>> - const char *op, const char *filter)
>>> + const char *fmt, const char *filter)
>>> {
>>> char *new_filter;
>>>
>>> if (evsel->filter == NULL)
>>> return perf_evsel__set_filter(evsel, filter);
>>>
>>> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>>> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>>> free(evsel->filter);
>>> evsel->filter = new_filter;
>>> return 0;
>>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>>> index 8ceb7ebb51f5..50595c8c7207 100644
>>> --- a/tools/perf/util/evsel.h
>>> +++ b/tools/perf/util/evsel.h
>>> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>>>
>>> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>>> int perf_evsel__append_filter(struct perf_evsel *evsel,
>>> - const char *op, const char *filter);
>>> + const char *fmt, const char *filter);
>>> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>>> const char *filter);
>>> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>> index 2eb8b1ed4cc8..8e683979ccd8 100644
>>> --- a/tools/perf/util/parse-events.c
>>> +++ b/tools/perf/util/parse-events.c
>>> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>>> static int set_filter(struct perf_evsel *evsel, const void *arg)
>>> {
>>> const char *str = arg;
>>> + bool found = false;
>>> + int nr_addr_filters = 0;
>>> + struct perf_pmu *pmu = NULL;
>>>
>>> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>>> - fprintf(stderr,
>>> - "--filter option should follow a -e tracepoint option\n");
>>> - return -1;
>>> + if (evsel == NULL)
>>> + goto err;
>>> +
>>> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>>> + if (perf_evsel__append_filter(evsel,
>>> + "(%s) && (%s)", str) < 0) {
>>> + fprintf(stderr,
>>> + "not enough memory to hold filter string\n");
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> }
>>>
>>> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>>> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>>> + if (pmu->type == evsel->attr.type) {
>>> + found = true;
>>> + break;
>>> + }
>>> +
>>> + if (found)
>>> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>>> + "%d", &nr_addr_filters);
>>> +
>>> + if (!nr_addr_filters)
>>> + goto err;
>>> +
>>> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>>> fprintf(stderr,
>>> "not enough memory to hold filter string\n");
>>> return -1;
>>> }
>>>
>>> return 0;
>>> +
>>> +err:
>>> + fprintf(stderr,
>>> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>>> +
>>> + return -1;
>>> }
>>>
>>> int parse_filter(const struct option *opt, const char *str,
>>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-13 23:25 ` Masami Hiramatsu
@ 2016-09-16 15:32 ` Mathieu Poirier
-1 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-16 15:32 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Adrian Hunter, Arnaldo Carvalho de Melo, Ingo Molnar,
Peter Zijlstra, Alexander Shishkin, jolsa,
Arnaldo Carvalho de Melo, linux-kernel, linux-arm-kernel
On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Tue, 13 Sep 2016 08:18:10 -0600
> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> > On 12/09/16 20:53, Mathieu Poirier wrote:
>> >> This patch makes it possible to use the current filter
>> >> framework with address filters. That way address filters for
>> >> HW tracers such as CoreSight and IntelPT can be communicated
>> >> to the kernel drivers.
>> >>
>> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> >>
>> >> ---
>> >> Changes for V5:
>> >> - Modified perf_evsel__append_filter() to take a string format
>> >> rather than an operation.
>> >
>> > Hope I'm not being a pain, but aren't there other places calling
>> > perf_evsel__append_filter() that need to be changed. Might make
>> > sense as a separate patch.
>>
>> No no, you're right - I completely overlooked that.
>>
>> But shouldn't it be in the same patch? That way a git bisect would
>> stay consistent...
>
> You're right. Caller and callee should be changed in atomic.
>
> BTW, could you add document updates how the perf command line
> will be changed, and also show the result in the patch description?
This patch does not change anything on the perf command line. It
simply allows current options to succeed (as they should) rather than
fail.
Thanks,
Mathieu
>
> Thank you,
>
>>
>> >
>> >>
>> >> Changes for V4:
>> >> - Added support for address filters over more than one
>> >> nibble.
>> >> - Removed Jiri's ack, this version is too different from
>> >> what was reviewed.
>> >>
>> >> Changes for V3:
>> >> - Added Jiri's ack.
>> >> - Rebased to v4.8-rc5.
>> >>
>> >> Changes for V2:
>> >> - Rebased to v4.8-rc4.
>> >> - Revisited error path.
>> >>
>> >>
>> >> tools/perf/util/evsel.c | 4 ++--
>> >> tools/perf/util/evsel.h | 2 +-
>> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>> >> 3 files changed, 38 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> >> index d40f852d2de2..a9bb277f221f 100644
>> >> --- a/tools/perf/util/evsel.c
>> >> +++ b/tools/perf/util/evsel.c
>> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>> >> }
>> >>
>> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> >> - const char *op, const char *filter)
>> >> + const char *fmt, const char *filter)
>> >> {
>> >> char *new_filter;
>> >>
>> >> if (evsel->filter == NULL)
>> >> return perf_evsel__set_filter(evsel, filter);
>> >>
>> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>> >> free(evsel->filter);
>> >> evsel->filter = new_filter;
>> >> return 0;
>> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> >> index 8ceb7ebb51f5..50595c8c7207 100644
>> >> --- a/tools/perf/util/evsel.h
>> >> +++ b/tools/perf/util/evsel.h
>> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>> >>
>> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> >> - const char *op, const char *filter);
>> >> + const char *fmt, const char *filter);
>> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>> >> const char *filter);
>> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
>> >> --- a/tools/perf/util/parse-events.c
>> >> +++ b/tools/perf/util/parse-events.c
>> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
>> >> {
>> >> const char *str = arg;
>> >> + bool found = false;
>> >> + int nr_addr_filters = 0;
>> >> + struct perf_pmu *pmu = NULL;
>> >>
>> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>> >> - fprintf(stderr,
>> >> - "--filter option should follow a -e tracepoint option\n");
>> >> - return -1;
>> >> + if (evsel == NULL)
>> >> + goto err;
>> >> +
>> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>> >> + if (perf_evsel__append_filter(evsel,
>> >> + "(%s) && (%s)", str) < 0) {
>> >> + fprintf(stderr,
>> >> + "not enough memory to hold filter string\n");
>> >> + return -1;
>> >> + }
>> >> +
>> >> + return 0;
>> >> }
>> >>
>> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>> >> + if (pmu->type == evsel->attr.type) {
>> >> + found = true;
>> >> + break;
>> >> + }
>> >> +
>> >> + if (found)
>> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>> >> + "%d", &nr_addr_filters);
>> >> +
>> >> + if (!nr_addr_filters)
>> >> + goto err;
>> >> +
>> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>> >> fprintf(stderr,
>> >> "not enough memory to hold filter string\n");
>> >> return -1;
>> >> }
>> >>
>> >> return 0;
>> >> +
>> >> +err:
>> >> + fprintf(stderr,
>> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>> >> +
>> >> + return -1;
>> >> }
>> >>
>> >> int parse_filter(const struct option *opt, const char *str,
>> >>
>> >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-16 15:32 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2016-09-16 15:32 UTC (permalink / raw)
To: linux-arm-kernel
On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Tue, 13 Sep 2016 08:18:10 -0600
> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> > On 12/09/16 20:53, Mathieu Poirier wrote:
>> >> This patch makes it possible to use the current filter
>> >> framework with address filters. That way address filters for
>> >> HW tracers such as CoreSight and IntelPT can be communicated
>> >> to the kernel drivers.
>> >>
>> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> >>
>> >> ---
>> >> Changes for V5:
>> >> - Modified perf_evsel__append_filter() to take a string format
>> >> rather than an operation.
>> >
>> > Hope I'm not being a pain, but aren't there other places calling
>> > perf_evsel__append_filter() that need to be changed. Might make
>> > sense as a separate patch.
>>
>> No no, you're right - I completely overlooked that.
>>
>> But shouldn't it be in the same patch? That way a git bisect would
>> stay consistent...
>
> You're right. Caller and callee should be changed in atomic.
>
> BTW, could you add document updates how the perf command line
> will be changed, and also show the result in the patch description?
This patch does not change anything on the perf command line. It
simply allows current options to succeed (as they should) rather than
fail.
Thanks,
Mathieu
>
> Thank you,
>
>>
>> >
>> >>
>> >> Changes for V4:
>> >> - Added support for address filters over more than one
>> >> nibble.
>> >> - Removed Jiri's ack, this version is too different from
>> >> what was reviewed.
>> >>
>> >> Changes for V3:
>> >> - Added Jiri's ack.
>> >> - Rebased to v4.8-rc5.
>> >>
>> >> Changes for V2:
>> >> - Rebased to v4.8-rc4.
>> >> - Revisited error path.
>> >>
>> >>
>> >> tools/perf/util/evsel.c | 4 ++--
>> >> tools/perf/util/evsel.h | 2 +-
>> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
>> >> 3 files changed, 38 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> >> index d40f852d2de2..a9bb277f221f 100644
>> >> --- a/tools/perf/util/evsel.c
>> >> +++ b/tools/perf/util/evsel.c
>> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>> >> }
>> >>
>> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> >> - const char *op, const char *filter)
>> >> + const char *fmt, const char *filter)
>> >> {
>> >> char *new_filter;
>> >>
>> >> if (evsel->filter == NULL)
>> >> return perf_evsel__set_filter(evsel, filter);
>> >>
>> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
>> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>> >> free(evsel->filter);
>> >> evsel->filter = new_filter;
>> >> return 0;
>> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> >> index 8ceb7ebb51f5..50595c8c7207 100644
>> >> --- a/tools/perf/util/evsel.h
>> >> +++ b/tools/perf/util/evsel.h
>> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>> >>
>> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
>> >> - const char *op, const char *filter);
>> >> + const char *fmt, const char *filter);
>> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>> >> const char *filter);
>> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
>> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
>> >> --- a/tools/perf/util/parse-events.c
>> >> +++ b/tools/perf/util/parse-events.c
>> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
>> >> {
>> >> const char *str = arg;
>> >> + bool found = false;
>> >> + int nr_addr_filters = 0;
>> >> + struct perf_pmu *pmu = NULL;
>> >>
>> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
>> >> - fprintf(stderr,
>> >> - "--filter option should follow a -e tracepoint option\n");
>> >> - return -1;
>> >> + if (evsel == NULL)
>> >> + goto err;
>> >> +
>> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>> >> + if (perf_evsel__append_filter(evsel,
>> >> + "(%s) && (%s)", str) < 0) {
>> >> + fprintf(stderr,
>> >> + "not enough memory to hold filter string\n");
>> >> + return -1;
>> >> + }
>> >> +
>> >> + return 0;
>> >> }
>> >>
>> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
>> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
>> >> + if (pmu->type == evsel->attr.type) {
>> >> + found = true;
>> >> + break;
>> >> + }
>> >> +
>> >> + if (found)
>> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
>> >> + "%d", &nr_addr_filters);
>> >> +
>> >> + if (!nr_addr_filters)
>> >> + goto err;
>> >> +
>> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
>> >> fprintf(stderr,
>> >> "not enough memory to hold filter string\n");
>> >> return -1;
>> >> }
>> >>
>> >> return 0;
>> >> +
>> >> +err:
>> >> + fprintf(stderr,
>> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
>> >> +
>> >> + return -1;
>> >> }
>> >>
>> >> int parse_filter(const struct option *opt, const char *str,
>> >>
>> >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-16 15:32 ` Mathieu Poirier
@ 2016-09-17 13:33 ` Masami Hiramatsu
-1 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-17 13:33 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Adrian Hunter, Arnaldo Carvalho de Melo, Ingo Molnar,
Peter Zijlstra, Alexander Shishkin, jolsa,
Arnaldo Carvalho de Melo, linux-kernel, linux-arm-kernel
On Fri, 16 Sep 2016 09:32:43 -0600
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > On Tue, 13 Sep 2016 08:18:10 -0600
> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >
> >> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >> > On 12/09/16 20:53, Mathieu Poirier wrote:
> >> >> This patch makes it possible to use the current filter
> >> >> framework with address filters. That way address filters for
> >> >> HW tracers such as CoreSight and IntelPT can be communicated
> >> >> to the kernel drivers.
> >> >>
> >> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >> >>
> >> >> ---
> >> >> Changes for V5:
> >> >> - Modified perf_evsel__append_filter() to take a string format
> >> >> rather than an operation.
> >> >
> >> > Hope I'm not being a pain, but aren't there other places calling
> >> > perf_evsel__append_filter() that need to be changed. Might make
> >> > sense as a separate patch.
> >>
> >> No no, you're right - I completely overlooked that.
> >>
> >> But shouldn't it be in the same patch? That way a git bisect would
> >> stay consistent...
> >
> > You're right. Caller and callee should be changed in atomic.
> >
> > BTW, could you add document updates how the perf command line
> > will be changed, and also show the result in the patch description?
>
> This patch does not change anything on the perf command line. It
> simply allows current options to succeed (as they should) rather than
> fail.
Yes, and it will make perf acceptable to pass --filter to CoreSight or
Intel PT events, or am I misunderstand?
If it is correct, could you give us an example how to pass it, since
tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
Thank you,
>
> Thanks,
> Mathieu
>
> >
> > Thank you,
> >
> >>
> >> >
> >> >>
> >> >> Changes for V4:
> >> >> - Added support for address filters over more than one
> >> >> nibble.
> >> >> - Removed Jiri's ack, this version is too different from
> >> >> what was reviewed.
> >> >>
> >> >> Changes for V3:
> >> >> - Added Jiri's ack.
> >> >> - Rebased to v4.8-rc5.
> >> >>
> >> >> Changes for V2:
> >> >> - Rebased to v4.8-rc4.
> >> >> - Revisited error path.
> >> >>
> >> >>
> >> >> tools/perf/util/evsel.c | 4 ++--
> >> >> tools/perf/util/evsel.h | 2 +-
> >> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> >> >> 3 files changed, 38 insertions(+), 8 deletions(-)
> >> >>
> >> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> >> >> index d40f852d2de2..a9bb277f221f 100644
> >> >> --- a/tools/perf/util/evsel.c
> >> >> +++ b/tools/perf/util/evsel.c
> >> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> >> >> }
> >> >>
> >> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> >> - const char *op, const char *filter)
> >> >> + const char *fmt, const char *filter)
> >> >> {
> >> >> char *new_filter;
> >> >>
> >> >> if (evsel->filter == NULL)
> >> >> return perf_evsel__set_filter(evsel, filter);
> >> >>
> >> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> >> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> >> >> free(evsel->filter);
> >> >> evsel->filter = new_filter;
> >> >> return 0;
> >> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> >> >> index 8ceb7ebb51f5..50595c8c7207 100644
> >> >> --- a/tools/perf/util/evsel.h
> >> >> +++ b/tools/perf/util/evsel.h
> >> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
> >> >>
> >> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> >> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> >> - const char *op, const char *filter);
> >> >> + const char *fmt, const char *filter);
> >> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> >> >> const char *filter);
> >> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> >> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
> >> >> --- a/tools/perf/util/parse-events.c
> >> >> +++ b/tools/perf/util/parse-events.c
> >> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> >> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
> >> >> {
> >> >> const char *str = arg;
> >> >> + bool found = false;
> >> >> + int nr_addr_filters = 0;
> >> >> + struct perf_pmu *pmu = NULL;
> >> >>
> >> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> >> >> - fprintf(stderr,
> >> >> - "--filter option should follow a -e tracepoint option\n");
> >> >> - return -1;
> >> >> + if (evsel == NULL)
> >> >> + goto err;
> >> >> +
> >> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> >> >> + if (perf_evsel__append_filter(evsel,
> >> >> + "(%s) && (%s)", str) < 0) {
> >> >> + fprintf(stderr,
> >> >> + "not enough memory to hold filter string\n");
> >> >> + return -1;
> >> >> + }
> >> >> +
> >> >> + return 0;
> >> >> }
> >> >>
> >> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> >> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> >> >> + if (pmu->type == evsel->attr.type) {
> >> >> + found = true;
> >> >> + break;
> >> >> + }
> >> >> +
> >> >> + if (found)
> >> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> >> >> + "%d", &nr_addr_filters);
> >> >> +
> >> >> + if (!nr_addr_filters)
> >> >> + goto err;
> >> >> +
> >> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> >> >> fprintf(stderr,
> >> >> "not enough memory to hold filter string\n");
> >> >> return -1;
> >> >> }
> >> >>
> >> >> return 0;
> >> >> +
> >> >> +err:
> >> >> + fprintf(stderr,
> >> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> >> >> +
> >> >> + return -1;
> >> >> }
> >> >>
> >> >> int parse_filter(const struct option *opt, const char *str,
> >> >>
> >> >
> >
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel.org>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-17 13:33 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-17 13:33 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Sep 2016 09:32:43 -0600
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > On Tue, 13 Sep 2016 08:18:10 -0600
> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >
> >> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >> > On 12/09/16 20:53, Mathieu Poirier wrote:
> >> >> This patch makes it possible to use the current filter
> >> >> framework with address filters. That way address filters for
> >> >> HW tracers such as CoreSight and IntelPT can be communicated
> >> >> to the kernel drivers.
> >> >>
> >> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >> >>
> >> >> ---
> >> >> Changes for V5:
> >> >> - Modified perf_evsel__append_filter() to take a string format
> >> >> rather than an operation.
> >> >
> >> > Hope I'm not being a pain, but aren't there other places calling
> >> > perf_evsel__append_filter() that need to be changed. Might make
> >> > sense as a separate patch.
> >>
> >> No no, you're right - I completely overlooked that.
> >>
> >> But shouldn't it be in the same patch? That way a git bisect would
> >> stay consistent...
> >
> > You're right. Caller and callee should be changed in atomic.
> >
> > BTW, could you add document updates how the perf command line
> > will be changed, and also show the result in the patch description?
>
> This patch does not change anything on the perf command line. It
> simply allows current options to succeed (as they should) rather than
> fail.
Yes, and it will make perf acceptable to pass --filter to CoreSight or
Intel PT events, or am I misunderstand?
If it is correct, could you give us an example how to pass it, since
tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
Thank you,
>
> Thanks,
> Mathieu
>
> >
> > Thank you,
> >
> >>
> >> >
> >> >>
> >> >> Changes for V4:
> >> >> - Added support for address filters over more than one
> >> >> nibble.
> >> >> - Removed Jiri's ack, this version is too different from
> >> >> what was reviewed.
> >> >>
> >> >> Changes for V3:
> >> >> - Added Jiri's ack.
> >> >> - Rebased to v4.8-rc5.
> >> >>
> >> >> Changes for V2:
> >> >> - Rebased to v4.8-rc4.
> >> >> - Revisited error path.
> >> >>
> >> >>
> >> >> tools/perf/util/evsel.c | 4 ++--
> >> >> tools/perf/util/evsel.h | 2 +-
> >> >> tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++++++++-----
> >> >> 3 files changed, 38 insertions(+), 8 deletions(-)
> >> >>
> >> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> >> >> index d40f852d2de2..a9bb277f221f 100644
> >> >> --- a/tools/perf/util/evsel.c
> >> >> +++ b/tools/perf/util/evsel.c
> >> >> @@ -1047,14 +1047,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
> >> >> }
> >> >>
> >> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> >> - const char *op, const char *filter)
> >> >> + const char *fmt, const char *filter)
> >> >> {
> >> >> char *new_filter;
> >> >>
> >> >> if (evsel->filter == NULL)
> >> >> return perf_evsel__set_filter(evsel, filter);
> >> >>
> >> >> - if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> >> >> + if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
> >> >> free(evsel->filter);
> >> >> evsel->filter = new_filter;
> >> >> return 0;
> >> >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> >> >> index 8ceb7ebb51f5..50595c8c7207 100644
> >> >> --- a/tools/perf/util/evsel.h
> >> >> +++ b/tools/perf/util/evsel.h
> >> >> @@ -236,7 +236,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
> >> >>
> >> >> int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> >> >> int perf_evsel__append_filter(struct perf_evsel *evsel,
> >> >> - const char *op, const char *filter);
> >> >> + const char *fmt, const char *filter);
> >> >> int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
> >> >> const char *filter);
> >> >> int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
> >> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >> >> index 2eb8b1ed4cc8..8e683979ccd8 100644
> >> >> --- a/tools/perf/util/parse-events.c
> >> >> +++ b/tools/perf/util/parse-events.c
> >> >> @@ -1760,20 +1760,50 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> >> >> static int set_filter(struct perf_evsel *evsel, const void *arg)
> >> >> {
> >> >> const char *str = arg;
> >> >> + bool found = false;
> >> >> + int nr_addr_filters = 0;
> >> >> + struct perf_pmu *pmu = NULL;
> >> >>
> >> >> - if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> >> >> - fprintf(stderr,
> >> >> - "--filter option should follow a -e tracepoint option\n");
> >> >> - return -1;
> >> >> + if (evsel == NULL)
> >> >> + goto err;
> >> >> +
> >> >> + if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> >> >> + if (perf_evsel__append_filter(evsel,
> >> >> + "(%s) && (%s)", str) < 0) {
> >> >> + fprintf(stderr,
> >> >> + "not enough memory to hold filter string\n");
> >> >> + return -1;
> >> >> + }
> >> >> +
> >> >> + return 0;
> >> >> }
> >> >>
> >> >> - if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> >> >> + while ((pmu = perf_pmu__scan(pmu)) != NULL)
> >> >> + if (pmu->type == evsel->attr.type) {
> >> >> + found = true;
> >> >> + break;
> >> >> + }
> >> >> +
> >> >> + if (found)
> >> >> + perf_pmu__scan_file(pmu, "nr_addr_filters",
> >> >> + "%d", &nr_addr_filters);
> >> >> +
> >> >> + if (!nr_addr_filters)
> >> >> + goto err;
> >> >> +
> >> >> + if (perf_evsel__append_filter(evsel, "%s,%s", str) < 0) {
> >> >> fprintf(stderr,
> >> >> "not enough memory to hold filter string\n");
> >> >> return -1;
> >> >> }
> >> >>
> >> >> return 0;
> >> >> +
> >> >> +err:
> >> >> + fprintf(stderr,
> >> >> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> >> >> +
> >> >> + return -1;
> >> >> }
> >> >>
> >> >> int parse_filter(const struct option *opt, const char *str,
> >> >>
> >> >
> >
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel.org>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-17 13:33 ` Masami Hiramatsu
@ 2016-09-19 6:15 ` Adrian Hunter
-1 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-19 6:15 UTC (permalink / raw)
To: Masami Hiramatsu, Mathieu Poirier
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Alexander Shishkin, jolsa, Arnaldo Carvalho de Melo,
linux-kernel, linux-arm-kernel
On 17/09/16 16:33, Masami Hiramatsu wrote:
> On Fri, 16 Sep 2016 09:32:43 -0600
> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
>> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>> On Tue, 13 Sep 2016 08:18:10 -0600
>>> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>>>
>>>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 12/09/16 20:53, Mathieu Poirier wrote:
>>>>>> This patch makes it possible to use the current filter
>>>>>> framework with address filters. That way address filters for
>>>>>> HW tracers such as CoreSight and IntelPT can be communicated
>>>>>> to the kernel drivers.
>>>>>>
>>>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>>>
>>>>>> ---
>>>>>> Changes for V5:
>>>>>> - Modified perf_evsel__append_filter() to take a string format
>>>>>> rather than an operation.
>>>>>
>>>>> Hope I'm not being a pain, but aren't there other places calling
>>>>> perf_evsel__append_filter() that need to be changed. Might make
>>>>> sense as a separate patch.
>>>>
>>>> No no, you're right - I completely overlooked that.
>>>>
>>>> But shouldn't it be in the same patch? That way a git bisect would
>>>> stay consistent...
>>>
>>> You're right. Caller and callee should be changed in atomic.
>>>
>>> BTW, could you add document updates how the perf command line
>>> will be changed, and also show the result in the patch description?
>>
>> This patch does not change anything on the perf command line. It
>> simply allows current options to succeed (as they should) rather than
>> fail.
>
> Yes, and it will make perf acceptable to pass --filter to CoreSight or
> Intel PT events, or am I misunderstand?
> If it is correct, could you give us an example how to pass it, since
> tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
I am adding support for symbols in the address filter. I will send the
patches soon, but the documentation will be:
--filter=<filter>::
Event filter. This option should follow a event selector (-e) which
selects either tracepoint event(s) or a hardware trace PMU
(e.g. Intel PT or CoreSight).
- tracepoint filters
In the case of tracepoints, multiple '--filter' options are combined
using '&&'.
- address filters
A hardware trace PMU advertises its ability to accept a number of
address filters by specifying a non-zero value in
/sys/bus/event_source/devices/<pmu>/nr_addr_filters.
Address filters have the format:
filter|start|stop|tracestop <start> [/ <size>] [@<file name>]
Where:
- 'filter': defines a region that will be traced.
- 'start': defines an address at which tracing will begin.
- 'stop': defines an address at which tracing will stop.
- 'tracestop': defines a region in which tracing will stop.
<file name> is the name of the object file, <start> is the offset to the
code to trace in that file, and <size> is the size of the region to
trace. 'start' and 'stop' filters need not specify a <size>.
If no object file is specified then the kernel is assumed, in which case
the start address must be a current kernel memory address.
<start> can also be specified by providing the name of a symbol. If the
symbol name is not unique, it can be disambiguated by inserting #n where
'n' selects the n'th symbol in address order. Alternately #0, #g or #G
select only a global symbol. <size> can also be specified by providing
the name of a symbol, in which case the size is calculated to the end
of that symbol. For 'filter' and 'tracestop' filters, if <size> is
omitted and <start> is a symbol, then the size is calculated to the end
of that symbol.
If <size> is omitted and <start> is '*', then the start and size will
be calculated from the first and last symbols, i.e. to trace the whole
file.
If symbol names (or "*") are provided, they must be surrounded by white
space.
The filter passed to the kernel is not necessarily the same as entered.
To see the filter that is passed, use the -v option.
The kernel may not be able to configure a trace region if it is not
within a single mapping. MMAP events (or /proc/<pid>/maps) can be
examined to determine if that is a possibility.
Multiple filters can be separated with space or comma.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-19 6:15 ` Adrian Hunter
0 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2016-09-19 6:15 UTC (permalink / raw)
To: linux-arm-kernel
On 17/09/16 16:33, Masami Hiramatsu wrote:
> On Fri, 16 Sep 2016 09:32:43 -0600
> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
>> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>> On Tue, 13 Sep 2016 08:18:10 -0600
>>> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>>>
>>>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 12/09/16 20:53, Mathieu Poirier wrote:
>>>>>> This patch makes it possible to use the current filter
>>>>>> framework with address filters. That way address filters for
>>>>>> HW tracers such as CoreSight and IntelPT can be communicated
>>>>>> to the kernel drivers.
>>>>>>
>>>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>>>
>>>>>> ---
>>>>>> Changes for V5:
>>>>>> - Modified perf_evsel__append_filter() to take a string format
>>>>>> rather than an operation.
>>>>>
>>>>> Hope I'm not being a pain, but aren't there other places calling
>>>>> perf_evsel__append_filter() that need to be changed. Might make
>>>>> sense as a separate patch.
>>>>
>>>> No no, you're right - I completely overlooked that.
>>>>
>>>> But shouldn't it be in the same patch? That way a git bisect would
>>>> stay consistent...
>>>
>>> You're right. Caller and callee should be changed in atomic.
>>>
>>> BTW, could you add document updates how the perf command line
>>> will be changed, and also show the result in the patch description?
>>
>> This patch does not change anything on the perf command line. It
>> simply allows current options to succeed (as they should) rather than
>> fail.
>
> Yes, and it will make perf acceptable to pass --filter to CoreSight or
> Intel PT events, or am I misunderstand?
> If it is correct, could you give us an example how to pass it, since
> tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
I am adding support for symbols in the address filter. I will send the
patches soon, but the documentation will be:
--filter=<filter>::
Event filter. This option should follow a event selector (-e) which
selects either tracepoint event(s) or a hardware trace PMU
(e.g. Intel PT or CoreSight).
- tracepoint filters
In the case of tracepoints, multiple '--filter' options are combined
using '&&'.
- address filters
A hardware trace PMU advertises its ability to accept a number of
address filters by specifying a non-zero value in
/sys/bus/event_source/devices/<pmu>/nr_addr_filters.
Address filters have the format:
filter|start|stop|tracestop <start> [/ <size>] [@<file name>]
Where:
- 'filter': defines a region that will be traced.
- 'start': defines an address at which tracing will begin.
- 'stop': defines an address at which tracing will stop.
- 'tracestop': defines a region in which tracing will stop.
<file name> is the name of the object file, <start> is the offset to the
code to trace in that file, and <size> is the size of the region to
trace. 'start' and 'stop' filters need not specify a <size>.
If no object file is specified then the kernel is assumed, in which case
the start address must be a current kernel memory address.
<start> can also be specified by providing the name of a symbol. If the
symbol name is not unique, it can be disambiguated by inserting #n where
'n' selects the n'th symbol in address order. Alternately #0, #g or #G
select only a global symbol. <size> can also be specified by providing
the name of a symbol, in which case the size is calculated to the end
of that symbol. For 'filter' and 'tracestop' filters, if <size> is
omitted and <start> is a symbol, then the size is calculated to the end
of that symbol.
If <size> is omitted and <start> is '*', then the start and size will
be calculated from the first and last symbols, i.e. to trace the whole
file.
If symbol names (or "*") are provided, they must be surrounded by white
space.
The filter passed to the kernel is not necessarily the same as entered.
To see the filter that is passed, use the -v option.
The kernel may not be able to configure a trace region if it is not
within a single mapping. MMAP events (or /proc/<pid>/maps) can be
examined to determine if that is a possibility.
Multiple filters can be separated with space or comma.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V5] perf tools: adding support for address filters
2016-09-19 6:15 ` Adrian Hunter
@ 2016-09-19 22:44 ` Masami Hiramatsu
-1 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-19 22:44 UTC (permalink / raw)
To: Adrian Hunter
Cc: Mathieu Poirier, Arnaldo Carvalho de Melo, Ingo Molnar,
Peter Zijlstra, Alexander Shishkin, jolsa,
Arnaldo Carvalho de Melo, linux-kernel, linux-arm-kernel
On Mon, 19 Sep 2016 09:15:40 +0300
Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 17/09/16 16:33, Masami Hiramatsu wrote:
> > On Fri, 16 Sep 2016 09:32:43 -0600
> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >
> >> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >>> On Tue, 13 Sep 2016 08:18:10 -0600
> >>> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >>>
> >>>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>>> On 12/09/16 20:53, Mathieu Poirier wrote:
> >>>>>> This patch makes it possible to use the current filter
> >>>>>> framework with address filters. That way address filters for
> >>>>>> HW tracers such as CoreSight and IntelPT can be communicated
> >>>>>> to the kernel drivers.
> >>>>>>
> >>>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>>>>>
> >>>>>> ---
> >>>>>> Changes for V5:
> >>>>>> - Modified perf_evsel__append_filter() to take a string format
> >>>>>> rather than an operation.
> >>>>>
> >>>>> Hope I'm not being a pain, but aren't there other places calling
> >>>>> perf_evsel__append_filter() that need to be changed. Might make
> >>>>> sense as a separate patch.
> >>>>
> >>>> No no, you're right - I completely overlooked that.
> >>>>
> >>>> But shouldn't it be in the same patch? That way a git bisect would
> >>>> stay consistent...
> >>>
> >>> You're right. Caller and callee should be changed in atomic.
> >>>
> >>> BTW, could you add document updates how the perf command line
> >>> will be changed, and also show the result in the patch description?
> >>
> >> This patch does not change anything on the perf command line. It
> >> simply allows current options to succeed (as they should) rather than
> >> fail.
> >
> > Yes, and it will make perf acceptable to pass --filter to CoreSight or
> > Intel PT events, or am I misunderstand?
> > If it is correct, could you give us an example how to pass it, since
> > tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
>
> I am adding support for symbols in the address filter. I will send the
> patches soon, but the documentation will be:
>
> --filter=<filter>::
> Event filter. This option should follow a event selector (-e) which
> selects either tracepoint event(s) or a hardware trace PMU
> (e.g. Intel PT or CoreSight).
>
> - tracepoint filters
>
> In the case of tracepoints, multiple '--filter' options are combined
> using '&&'.
>
> - address filters
>
> A hardware trace PMU advertises its ability to accept a number of
> address filters by specifying a non-zero value in
> /sys/bus/event_source/devices/<pmu>/nr_addr_filters.
>
> Address filters have the format:
>
> filter|start|stop|tracestop <start> [/ <size>] [@<file name>]
>
> Where:
> - 'filter': defines a region that will be traced.
> - 'start': defines an address at which tracing will begin.
> - 'stop': defines an address at which tracing will stop.
> - 'tracestop': defines a region in which tracing will stop.
>
> <file name> is the name of the object file, <start> is the offset to the
> code to trace in that file, and <size> is the size of the region to
> trace. 'start' and 'stop' filters need not specify a <size>.
>
> If no object file is specified then the kernel is assumed, in which case
> the start address must be a current kernel memory address.
>
> <start> can also be specified by providing the name of a symbol. If the
> symbol name is not unique, it can be disambiguated by inserting #n where
> 'n' selects the n'th symbol in address order. Alternately #0, #g or #G
> select only a global symbol. <size> can also be specified by providing
> the name of a symbol, in which case the size is calculated to the end
> of that symbol. For 'filter' and 'tracestop' filters, if <size> is
> omitted and <start> is a symbol, then the size is calculated to the end
> of that symbol.
>
> If <size> is omitted and <start> is '*', then the start and size will
> be calculated from the first and last symbols, i.e. to trace the whole
> file.
>
> If symbol names (or "*") are provided, they must be surrounded by white
> space.
>
> The filter passed to the kernel is not necessarily the same as entered.
> To see the filter that is passed, use the -v option.
>
> The kernel may not be able to configure a trace region if it is not
> within a single mapping. MMAP events (or /proc/<pid>/maps) can be
> examined to determine if that is a possibility.
>
> Multiple filters can be separated with space or comma.
Perfect! :)
OK, I'll wait your patch.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V5] perf tools: adding support for address filters
@ 2016-09-19 22:44 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2016-09-19 22:44 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 19 Sep 2016 09:15:40 +0300
Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 17/09/16 16:33, Masami Hiramatsu wrote:
> > On Fri, 16 Sep 2016 09:32:43 -0600
> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >
> >> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >>> On Tue, 13 Sep 2016 08:18:10 -0600
> >>> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >>>
> >>>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>>> On 12/09/16 20:53, Mathieu Poirier wrote:
> >>>>>> This patch makes it possible to use the current filter
> >>>>>> framework with address filters. That way address filters for
> >>>>>> HW tracers such as CoreSight and IntelPT can be communicated
> >>>>>> to the kernel drivers.
> >>>>>>
> >>>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>>>>>
> >>>>>> ---
> >>>>>> Changes for V5:
> >>>>>> - Modified perf_evsel__append_filter() to take a string format
> >>>>>> rather than an operation.
> >>>>>
> >>>>> Hope I'm not being a pain, but aren't there other places calling
> >>>>> perf_evsel__append_filter() that need to be changed. Might make
> >>>>> sense as a separate patch.
> >>>>
> >>>> No no, you're right - I completely overlooked that.
> >>>>
> >>>> But shouldn't it be in the same patch? That way a git bisect would
> >>>> stay consistent...
> >>>
> >>> You're right. Caller and callee should be changed in atomic.
> >>>
> >>> BTW, could you add document updates how the perf command line
> >>> will be changed, and also show the result in the patch description?
> >>
> >> This patch does not change anything on the perf command line. It
> >> simply allows current options to succeed (as they should) rather than
> >> fail.
> >
> > Yes, and it will make perf acceptable to pass --filter to CoreSight or
> > Intel PT events, or am I misunderstand?
> > If it is correct, could you give us an example how to pass it, since
> > tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
>
> I am adding support for symbols in the address filter. I will send the
> patches soon, but the documentation will be:
>
> --filter=<filter>::
> Event filter. This option should follow a event selector (-e) which
> selects either tracepoint event(s) or a hardware trace PMU
> (e.g. Intel PT or CoreSight).
>
> - tracepoint filters
>
> In the case of tracepoints, multiple '--filter' options are combined
> using '&&'.
>
> - address filters
>
> A hardware trace PMU advertises its ability to accept a number of
> address filters by specifying a non-zero value in
> /sys/bus/event_source/devices/<pmu>/nr_addr_filters.
>
> Address filters have the format:
>
> filter|start|stop|tracestop <start> [/ <size>] [@<file name>]
>
> Where:
> - 'filter': defines a region that will be traced.
> - 'start': defines an address at which tracing will begin.
> - 'stop': defines an address at which tracing will stop.
> - 'tracestop': defines a region in which tracing will stop.
>
> <file name> is the name of the object file, <start> is the offset to the
> code to trace in that file, and <size> is the size of the region to
> trace. 'start' and 'stop' filters need not specify a <size>.
>
> If no object file is specified then the kernel is assumed, in which case
> the start address must be a current kernel memory address.
>
> <start> can also be specified by providing the name of a symbol. If the
> symbol name is not unique, it can be disambiguated by inserting #n where
> 'n' selects the n'th symbol in address order. Alternately #0, #g or #G
> select only a global symbol. <size> can also be specified by providing
> the name of a symbol, in which case the size is calculated to the end
> of that symbol. For 'filter' and 'tracestop' filters, if <size> is
> omitted and <start> is a symbol, then the size is calculated to the end
> of that symbol.
>
> If <size> is omitted and <start> is '*', then the start and size will
> be calculated from the first and last symbols, i.e. to trace the whole
> file.
>
> If symbol names (or "*") are provided, they must be surrounded by white
> space.
>
> The filter passed to the kernel is not necessarily the same as entered.
> To see the filter that is passed, use the -v option.
>
> The kernel may not be able to configure a trace region if it is not
> within a single mapping. MMAP events (or /proc/<pid>/maps) can be
> examined to determine if that is a possibility.
>
> Multiple filters can be separated with space or comma.
Perfect! :)
OK, I'll wait your patch.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-09-19 22:45 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12 17:53 [PATCH V5] perf tools: adding support for address filters Mathieu Poirier
2016-09-12 17:53 ` Mathieu Poirier
2016-09-13 10:01 ` Adrian Hunter
2016-09-13 10:01 ` Adrian Hunter
2016-09-13 14:18 ` Mathieu Poirier
2016-09-13 14:18 ` Mathieu Poirier
2016-09-13 23:25 ` Masami Hiramatsu
2016-09-13 23:25 ` Masami Hiramatsu
2016-09-16 15:32 ` Mathieu Poirier
2016-09-16 15:32 ` Mathieu Poirier
2016-09-17 13:33 ` Masami Hiramatsu
2016-09-17 13:33 ` Masami Hiramatsu
2016-09-19 6:15 ` Adrian Hunter
2016-09-19 6:15 ` Adrian Hunter
2016-09-19 22:44 ` Masami Hiramatsu
2016-09-19 22:44 ` Masami Hiramatsu
2016-09-14 13:41 ` Adrian Hunter
2016-09-14 13:41 ` Adrian Hunter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.