| Message ID | 1460128045-97310-2-git-send-email-wangnan0@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 2016/4/8 23:22, Arnaldo Carvalho de Melo wrote: > Em Fri, Apr 08, 2016 at 03:07:22PM +0000, Wang Nan escreveu: >> Before this patch, strange error message is provided if passed a >> non-tracepoint event to 'perf trace': >> >> # perf trace -a --ev cycles sleep 1 >> Failed to set filter "common_pid != 27500" on event cycles with 22 (Invalid argument) >> >> This is because 'perf trace' accepts all valid events during cmdline >> parsing, but in fact user can only provide tracepoints, because it >> needs filter. >> >> This patch validate evlist, report error earlier: >> >> # ./perf trace -a --ev cycles sleep 1 >> Only support tracepoint events! > Humm, perhaps we should instead refrain from setting filters to non > tracepoint events? I.e. I don't see why we whouldn't support, say, > software events... > > /me trying some now, i.e.: > > # trace --ev minor-faults --no-syscalls > > But it has some issues... > > - Arnaldo > We already have commit fdf14720fbd02 ("perf tools: Only set filter for tracepoints events") so you won't see the ugly error message again. However, I think parsing non-tracepoint events in 'perf trace' is still a challange. We never support it in 'perf trace' and I'm not too much sure who will need this feature and how to use them, and why he/she can't use 'perf record' instead. Thank you.
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 11290b5..6fbed86 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -2680,6 +2680,17 @@ out_enomem: goto out; } +static int validate_evlist(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel; + + evlist__for_each(evlist, evsel) { + if (evsel->attr.type != PERF_TYPE_TRACEPOINT) + return -EINVAL; + } + return 0; +} + static int trace__run(struct trace *trace, int argc, const char **argv) { struct perf_evlist *evlist = trace->evlist; @@ -3273,6 +3284,11 @@ int cmd_trace(int argc, const char **argv, const char *prefix __maybe_unused) argc = parse_options_subcommand(argc, argv, trace_options, trace_subcommands, trace_usage, PARSE_OPT_STOP_AT_NON_OPTION); + if (validate_evlist(trace.evlist)) { + pr_err("Only support tracepoint events!\n"); + return -EINVAL; + } + if (trace.trace_pgfaults) { trace.opts.sample_address = true; trace.opts.sample_time = true;
Before this patch, strange error message is provided if passed a non-tracepoint event to 'perf trace': # perf trace -a --ev cycles sleep 1 Failed to set filter "common_pid != 27500" on event cycles with 22 (Invalid argument) This is because 'perf trace' accepts all valid events during cmdline parsing, but in fact user can only provide tracepoints, because it needs filter. This patch validate evlist, report error earlier: # ./perf trace -a --ev cycles sleep 1 Only support tracepoint events! Signed-off-by: Wang Nan <wangnan0@huawei.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Li Zefan <lizefan@huawei.com> Cc: pi3orama@163.com --- tools/perf/builtin-trace.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) -- 1.8.3.4