* [PATCH] perf parser: Improve error message for PMU address filters
@ 2017-09-05 9:08 Jack Henschel
2017-10-19 19:37 ` Jack Henschel
0 siblings, 1 reply; 5+ messages in thread
From: Jack Henschel @ 2017-09-05 9:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
Andi Kleen, Taeung Song, Mathieu Poirier, linux-kernel,
Jack Henschel
This patch improves the error message of the perf events parser
when the PMU hardware does not support address filters.
Previously, the perf returned the following error:
> --filter option should follow a -e tracepoint or HW tracer option
This implies there is some syntax error present in the command line,
which is not true. Rather, notify the user that the CPU does not have
support for this feature.
For example, Intel chips based on the Broadwell micro-archticture have
the Intel PT PMU, but do not support address filtering.
Signed-off-by: Jack Henschel <jackdev@mailbox.org>
---
tools/perf/util/parse-events.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f44aeba51d1f..672b6d9423e9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1833,8 +1833,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
int nr_addr_filters = 0;
struct perf_pmu *pmu = NULL;
- if (evsel == NULL)
- goto err;
+ if (evsel == NULL) {
+ fprintf(stderr,
+ "--filter option should follow a -e tracepoint or HW tracer option\n");
+ return -1;
+ }
if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
if (perf_evsel__append_tp_filter(evsel, str) < 0) {
@@ -1856,8 +1859,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
perf_pmu__scan_file(pmu, "nr_addr_filters",
"%d", &nr_addr_filters);
- if (!nr_addr_filters)
- goto err;
+ if (!nr_addr_filters) {
+ fprintf(stderr,
+ "This CPU does not support address filtering\n");
+ return -1;
+ }
if (perf_evsel__append_addr_filter(evsel, str) < 0) {
fprintf(stderr,
@@ -1866,12 +1872,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
}
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.14.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf parser: Improve error message for PMU address filters
2017-09-05 9:08 [PATCH] perf parser: Improve error message for PMU address filters Jack Henschel
@ 2017-10-19 19:37 ` Jack Henschel
2017-10-19 19:57 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 5+ messages in thread
From: Jack Henschel @ 2017-10-19 19:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
Andi Kleen, Taeung Song, Mathieu Poirier, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2318 bytes --]
On 09/05/2017 11:08 AM, Jack Henschel wrote:
> This patch improves the error message of the perf events parser
> when the PMU hardware does not support address filters.
>
> Previously, the perf returned the following error:
>> --filter option should follow a -e tracepoint or HW tracer option
> This implies there is some syntax error present in the command line,
> which is not true. Rather, notify the user that the CPU does not have
> support for this feature.
>
> For example, Intel chips based on the Broadwell micro-archticture have
> the Intel PT PMU, but do not support address filtering.
>
> Signed-off-by: Jack Henschel <jackdev@mailbox.org>
> ---
> tools/perf/util/parse-events.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index f44aeba51d1f..672b6d9423e9 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1833,8 +1833,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> int nr_addr_filters = 0;
> struct perf_pmu *pmu = NULL;
>
> - if (evsel == NULL)
> - goto err;
> + if (evsel == NULL) {
> + fprintf(stderr,
> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> + return -1;
> + }
>
> if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> if (perf_evsel__append_tp_filter(evsel, str) < 0) {
> @@ -1856,8 +1859,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> perf_pmu__scan_file(pmu, "nr_addr_filters",
> "%d", &nr_addr_filters);
>
> - if (!nr_addr_filters)
> - goto err;
> + if (!nr_addr_filters) {
> + fprintf(stderr,
> + "This CPU does not support address filtering\n");
> + return -1;
> + }
>
> if (perf_evsel__append_addr_filter(evsel, str) < 0) {
> fprintf(stderr,
> @@ -1866,12 +1872,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> }
>
> 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,
>
Is there any interest in this patch? Did it get lost?
Greetings
Jack
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf parser: Improve error message for PMU address filters
2017-10-19 19:37 ` Jack Henschel
@ 2017-10-19 19:57 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-10-19 19:57 UTC (permalink / raw)
To: Jack Henschel
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
Andi Kleen, Taeung Song, Mathieu Poirier, linux-kernel
Em Thu, Oct 19, 2017 at 09:37:13PM +0200, Jack Henschel escreveu:
> On 09/05/2017 11:08 AM, Jack Henschel wrote:
> > This patch improves the error message of the perf events parser
> > when the PMU hardware does not support address filters.
> >
> > Previously, the perf returned the following error:
> >> --filter option should follow a -e tracepoint or HW tracer option
> > This implies there is some syntax error present in the command line,
> > which is not true. Rather, notify the user that the CPU does not have
> > support for this feature.
> >
> > For example, Intel chips based on the Broadwell micro-archticture have
> > the Intel PT PMU, but do not support address filtering.
> >
> > Signed-off-by: Jack Henschel <jackdev@mailbox.org>
> > ---
> > tools/perf/util/parse-events.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index f44aeba51d1f..672b6d9423e9 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -1833,8 +1833,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> > int nr_addr_filters = 0;
> > struct perf_pmu *pmu = NULL;
> >
> > - if (evsel == NULL)
> > - goto err;
> > + if (evsel == NULL) {
> > + fprintf(stderr,
> > + "--filter option should follow a -e tracepoint or HW tracer option\n");
> > + return -1;
> > + }
> >
> > if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> > if (perf_evsel__append_tp_filter(evsel, str) < 0) {
> > @@ -1856,8 +1859,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> > perf_pmu__scan_file(pmu, "nr_addr_filters",
> > "%d", &nr_addr_filters);
> >
> > - if (!nr_addr_filters)
> > - goto err;
> > + if (!nr_addr_filters) {
> > + fprintf(stderr,
> > + "This CPU does not support address filtering\n");
> > + return -1;
> > + }
> >
> > if (perf_evsel__append_addr_filter(evsel, str) < 0) {
> > fprintf(stderr,
> > @@ -1866,12 +1872,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> > }
> >
> > 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,
> >
>
> Is there any interest in this patch? Did it get lost?
> > For example, Intel chips based on the Broadwell micro-archticture have
> > the Intel PT PMU, but do not support address filtering.
Can you provide this example with the tool output before and after? I
happen to have a Broadwell machine as my current notebook and would just
use this to test your patch.
And generally before/after command lines + output is good to quickly
show, in more concrete terms what the problem is and how it gets fixed.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf parser: Improve error message for PMU address filters
2018-07-04 9:54 Jack Henschel
@ 2018-07-04 10:24 ` Jiri Olsa
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2018-07-04 10:24 UTC (permalink / raw)
To: Jack Henschel
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, linux-kernel, linux-perf-users
On Wed, Jul 04, 2018 at 11:54:09AM +0200, Jack Henschel wrote:
> This patch improves the error message of the perf events parser
> when the PMU hardware does not support address filters.
>
> Previously, the perf returned the following error:
> > --filter option should follow a -e tracepoint or HW tracer option
> This implies there is some syntax error present in the command line,
> which is not true. Rather, notify the user that the CPU does not have
> support for this feature.
>
> For example, Intel chips based on the Broadwell micro-archticture have
> the Intel PT PMU, but do not support address filtering.
looks ok, but please provide real examples of those error outputs in changelog
thanks,
jirka
>
> Signed-off-by: Jack Henschel <jackdev@mailbox.org>
> ---
> tools/perf/util/parse-events.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 15eec49e71a1..f8cd3e7c9186 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1991,8 +1991,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> int nr_addr_filters = 0;
> struct perf_pmu *pmu = NULL;
>
> - if (evsel == NULL)
> - goto err;
> + if (evsel == NULL) {
> + fprintf(stderr,
> + "--filter option should follow a -e tracepoint or HW tracer option\n");
> + return -1;
> + }
>
> if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> if (perf_evsel__append_tp_filter(evsel, str) < 0) {
> @@ -2014,8 +2017,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> perf_pmu__scan_file(pmu, "nr_addr_filters",
> "%d", &nr_addr_filters);
>
> - if (!nr_addr_filters)
> - goto err;
> + if (!nr_addr_filters) {
> + fprintf(stderr,
> + "This CPU does not support address filtering\n");
> + return -1;
> + }
>
> if (perf_evsel__append_addr_filter(evsel, str) < 0) {
> fprintf(stderr,
> @@ -2024,12 +2030,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
> }
>
> 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.18.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] perf parser: Improve error message for PMU address filters
@ 2018-07-04 9:54 Jack Henschel
2018-07-04 10:24 ` Jiri Olsa
0 siblings, 1 reply; 5+ messages in thread
From: Jack Henschel @ 2018-07-04 9:54 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa
Cc: linux-kernel, linux-perf-users, Jack Henschel
This patch improves the error message of the perf events parser
when the PMU hardware does not support address filters.
Previously, the perf returned the following error:
> --filter option should follow a -e tracepoint or HW tracer option
This implies there is some syntax error present in the command line,
which is not true. Rather, notify the user that the CPU does not have
support for this feature.
For example, Intel chips based on the Broadwell micro-archticture have
the Intel PT PMU, but do not support address filtering.
Signed-off-by: Jack Henschel <jackdev@mailbox.org>
---
tools/perf/util/parse-events.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 15eec49e71a1..f8cd3e7c9186 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1991,8 +1991,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
int nr_addr_filters = 0;
struct perf_pmu *pmu = NULL;
- if (evsel == NULL)
- goto err;
+ if (evsel == NULL) {
+ fprintf(stderr,
+ "--filter option should follow a -e tracepoint or HW tracer option\n");
+ return -1;
+ }
if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
if (perf_evsel__append_tp_filter(evsel, str) < 0) {
@@ -2014,8 +2017,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
perf_pmu__scan_file(pmu, "nr_addr_filters",
"%d", &nr_addr_filters);
- if (!nr_addr_filters)
- goto err;
+ if (!nr_addr_filters) {
+ fprintf(stderr,
+ "This CPU does not support address filtering\n");
+ return -1;
+ }
if (perf_evsel__append_addr_filter(evsel, str) < 0) {
fprintf(stderr,
@@ -2024,12 +2030,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
}
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.18.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-04 10:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 9:08 [PATCH] perf parser: Improve error message for PMU address filters Jack Henschel
2017-10-19 19:37 ` Jack Henschel
2017-10-19 19:57 ` Arnaldo Carvalho de Melo
2018-07-04 9:54 Jack Henschel
2018-07-04 10:24 ` Jiri Olsa
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.