* [PATCH] perf tool: Add event name to error message for filters @ 2015-03-24 16:10 David Ahern 2015-03-24 21:29 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 6+ messages in thread From: David Ahern @ 2015-03-24 16:10 UTC (permalink / raw) To: acme; +Cc: linux-kernel, David Ahern, Jiri Olsa, Namhyung Kim Use of a bad filter currently generates the message: Error: failed to set filter with 22 (Invalid argument) Add the event name to make it clear to which event the filter failed to apply: Error: Failed to set filter on event sched:sg_lb_stats: 22: Invalid argument Signed-off-by: David Ahern <david.ahern@oracle.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> --- tools/perf/builtin-record.c | 2 -- tools/perf/builtin-stat.c | 5 +---- tools/perf/util/evsel.c | 16 +++++++++++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 091868288d29..b4acd0933c7c 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -162,8 +162,6 @@ static int record__open(struct record *rec) } if (perf_evlist__apply_filters(evlist)) { - error("failed to set filter with %d (%s)\n", errno, - strerror_r(errno, msg, sizeof(msg))); rc = -1; goto out; } diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index d4d1b77da0bd..2aceb9296e8f 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -684,11 +684,8 @@ static int __run_perf_stat(int argc, const char **argv) unit_width = l; } - if (perf_evlist__apply_filters(evsel_list)) { - error("failed to set filter with %d (%s)\n", errno, - strerror_r(errno, msg, sizeof(msg))); + if (perf_evlist__apply_filters(evsel_list)) return -1; - } /* * Enable counters and exec the command: diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index bb4eff28869e..daa053d9c638 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -807,9 +807,19 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthrea int perf_evsel__set_filter(struct perf_evsel *evsel, int ncpus, int nthreads, const char *filter) { - return perf_evsel__run_ioctl(evsel, ncpus, nthreads, - PERF_EVENT_IOC_SET_FILTER, - (void *)filter); + int err; + char msg[512]; + + err = perf_evsel__run_ioctl(evsel, ncpus, nthreads, + PERF_EVENT_IOC_SET_FILTER, + (void *)filter); + if (err) { + error("Failed to set filter on event %s: %d: %s\n", + perf_evsel__name(evsel), errno, + strerror_r(errno, msg, sizeof(msg))); + } + + return err; } int perf_evsel__enable(struct perf_evsel *evsel, int ncpus, int nthreads) -- 2.2.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tool: Add event name to error message for filters 2015-03-24 16:10 [PATCH] perf tool: Add event name to error message for filters David Ahern @ 2015-03-24 21:29 ` Arnaldo Carvalho de Melo 2015-03-24 21:57 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-03-24 21:29 UTC (permalink / raw) To: David Ahern; +Cc: linux-kernel, Jiri Olsa, Namhyung Kim Em Tue, Mar 24, 2015 at 12:10:17PM -0400, David Ahern escreveu: > Use of a bad filter currently generates the message: > Error: failed to set filter with 22 (Invalid argument) > > Add the event name to make it clear to which event the filter > failed to apply: > Error: Failed to set filter on event sched:sg_lb_stats: 22: Invalid argument > > Signed-off-by: David Ahern <david.ahern@oracle.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Namhyung Kim <namhyung@kernel.org> > +++ b/tools/perf/util/evsel.c > @@ -807,9 +807,19 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthrea > int perf_evsel__set_filter(struct perf_evsel *evsel, int ncpus, int nthreads, > const char *filter) > { > - return perf_evsel__run_ioctl(evsel, ncpus, nthreads, > - PERF_EVENT_IOC_SET_FILTER, > - (void *)filter); > + int err; > + char msg[512]; > + > + err = perf_evsel__run_ioctl(evsel, ncpus, nthreads, > + PERF_EVENT_IOC_SET_FILTER, > + (void *)filter); > + if (err) { > + error("Failed to set filter on event %s: %d: %s\n", > + perf_evsel__name(evsel), errno, > + strerror_r(errno, msg, sizeof(msg))); Humm, probably this will be the only call to error() from evsel.c, making it require the error() routine, which in turn will break the python binding, lemme check... If that is the case we'll have to somehow propagate to the caller of perf_evlist__apply_filters what was the evsel that had a filter that caused the problem. - Arnaldo > + } > + > + return err; > } > > int perf_evsel__enable(struct perf_evsel *evsel, int ncpus, int nthreads) > -- > 2.2.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tool: Add event name to error message for filters 2015-03-24 21:29 ` Arnaldo Carvalho de Melo @ 2015-03-24 21:57 ` Arnaldo Carvalho de Melo 2015-03-24 22:00 ` David Ahern 0 siblings, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-03-24 21:57 UTC (permalink / raw) To: David Ahern; +Cc: linux-kernel, Jiri Olsa, Namhyung Kim Em Tue, Mar 24, 2015 at 06:29:56PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Tue, Mar 24, 2015 at 12:10:17PM -0400, David Ahern escreveu: > > Use of a bad filter currently generates the message: > > Error: failed to set filter with 22 (Invalid argument) > > > > Add the event name to make it clear to which event the filter > > failed to apply: > > Error: Failed to set filter on event sched:sg_lb_stats: 22: Invalid argument > > > > Signed-off-by: David Ahern <david.ahern@oracle.com> > > Cc: Jiri Olsa <jolsa@kernel.org> > > Cc: Namhyung Kim <namhyung@kernel.org> > > > > +++ b/tools/perf/util/evsel.c > > @@ -807,9 +807,19 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthrea > > int perf_evsel__set_filter(struct perf_evsel *evsel, int ncpus, int nthreads, > > const char *filter) > > { > > - return perf_evsel__run_ioctl(evsel, ncpus, nthreads, > > - PERF_EVENT_IOC_SET_FILTER, > > - (void *)filter); > > + int err; > > + char msg[512]; > > + > > + err = perf_evsel__run_ioctl(evsel, ncpus, nthreads, > > + PERF_EVENT_IOC_SET_FILTER, > > + (void *)filter); > > + if (err) { > > + error("Failed to set filter on event %s: %d: %s\n", > > + perf_evsel__name(evsel), errno, > > + strerror_r(errno, msg, sizeof(msg))); > > Humm, probably this will be the only call to error() from evsel.c, > making it require the error() routine, which in turn will break the > python binding, lemme check... > > If that is the case we'll have to somehow propagate to the caller of > perf_evlist__apply_filters what was the evsel that had a filter that > caused the problem. With the patch below, that leaves printing something, using whatever UI, to the tool, acceptable? [root@ssdandy linux]# perf record -e sched:sched_switch -e sched:*fork --filter parent_pid==1 -e sched:*wait* --filter bla usleep 1 Error: failed to set filter "bla" on event sched:sched_stat_iowait with 22 (Invalid argument) [root@ssdandy linux]# diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 091868288d29..15670aa986b9 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -161,8 +161,9 @@ try_again: } } - if (perf_evlist__apply_filters(evlist)) { - error("failed to set filter with %d (%s)\n", errno, + if (perf_evlist__apply_filters(evlist, &pos)) { + error("failed to set filter \"%s\" on event %s with %d (%s)\n", + pos->filter, perf_evsel__name(pos), errno, strerror_r(errno, msg, sizeof(msg))); rc = -1; goto out; diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index d4d1b77da0bd..f7b8218785f6 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -684,8 +684,9 @@ static int __run_perf_stat(int argc, const char **argv) unit_width = l; } - if (perf_evlist__apply_filters(evsel_list)) { - error("failed to set filter with %d (%s)\n", errno, + if (perf_evlist__apply_filters(evsel_list, &counter)) { + error("failed to set filter \"%s\" on event %s with %d (%s)\n", + counter->filter, perf_evsel__name(counter), errno, strerror_r(errno, msg, sizeof(msg))); return -1; } diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 8d0b62361129..82bf224bbee9 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1050,7 +1050,7 @@ out_delete_threads: return -1; } -int perf_evlist__apply_filters(struct perf_evlist *evlist) +int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel) { struct perf_evsel *evsel; int err = 0; @@ -1062,8 +1062,10 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist) continue; err = perf_evsel__set_filter(evsel, ncpus, nthreads, evsel->filter); - if (err) + if (err) { + *err_evsel = evsel; break; + } } return err; diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index f07c984465f0..fb19c47b8aac 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -152,7 +152,7 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist, } int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target); -int perf_evlist__apply_filters(struct perf_evlist *evlist); +int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel); void __perf_evlist__set_leader(struct list_head *list); void perf_evlist__set_leader(struct perf_evlist *evlist); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tool: Add event name to error message for filters 2015-03-24 21:57 ` Arnaldo Carvalho de Melo @ 2015-03-24 22:00 ` David Ahern 2015-03-24 22:23 ` Arnaldo Carvalho de Melo 2015-03-25 11:19 ` Jiri Olsa 0 siblings, 2 replies; 6+ messages in thread From: David Ahern @ 2015-03-24 22:00 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, Jiri Olsa, Namhyung Kim On 3/24/15 3:57 PM, Arnaldo Carvalho de Melo wrote: > With the patch below, that leaves printing something, using whatever UI, to the > tool, acceptable? I keep forgetting about the python bindings. Your patch serves the purpose. Thanks, Arnaldo. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tool: Add event name to error message for filters 2015-03-24 22:00 ` David Ahern @ 2015-03-24 22:23 ` Arnaldo Carvalho de Melo 2015-03-25 11:19 ` Jiri Olsa 1 sibling, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-03-24 22:23 UTC (permalink / raw) To: David Ahern; +Cc: linux-kernel, Jiri Olsa, Namhyung Kim Em Tue, Mar 24, 2015 at 04:00:36PM -0600, David Ahern escreveu: > On 3/24/15 3:57 PM, Arnaldo Carvalho de Melo wrote: > >With the patch below, that leaves printing something, using whatever UI, to the > >tool, acceptable? > > I keep forgetting about the python bindings. Oh, its not just about the python bindings, but kinda layering violation, i.e. we should leave the error reporting to the actual tool, that may be using GTK, Qt, some custom made TUI, whatever UI. The python bindings, were not a problem somehow, at least 'perf test python' hasn't failed when I tried. > Your patch serves the purpose. Thanks, Arnaldo. Thanks, Applying with your Acked-by and Based-on-a-patch-by. - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tool: Add event name to error message for filters 2015-03-24 22:00 ` David Ahern 2015-03-24 22:23 ` Arnaldo Carvalho de Melo @ 2015-03-25 11:19 ` Jiri Olsa 1 sibling, 0 replies; 6+ messages in thread From: Jiri Olsa @ 2015-03-25 11:19 UTC (permalink / raw) To: David Ahern Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim On Tue, Mar 24, 2015 at 04:00:36PM -0600, David Ahern wrote: > On 3/24/15 3:57 PM, Arnaldo Carvalho de Melo wrote: > >With the patch below, that leaves printing something, using whatever UI, to the > >tool, acceptable? > > I keep forgetting about the python bindings. # perf test for each change ;-) jirka ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-25 11:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-03-24 16:10 [PATCH] perf tool: Add event name to error message for filters David Ahern 2015-03-24 21:29 ` Arnaldo Carvalho de Melo 2015-03-24 21:57 ` Arnaldo Carvalho de Melo 2015-03-24 22:00 ` David Ahern 2015-03-24 22:23 ` Arnaldo Carvalho de Melo 2015-03-25 11:19 ` Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox