Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758707AbcDHPXO (ORCPT ); Fri, 8 Apr 2016 11:23:14 -0400 Received: from mail.kernel.org ([198.145.29.136]:39147 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754266AbcDHPXK (ORCPT ); Fri, 8 Apr 2016 11:23:10 -0400 Date: Fri, 8 Apr 2016 12:22:57 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: pi3orama@163.com, linux-kernel@vger.kernel.org, lizefan@huawei.com, Arnaldo Carvalho de Melo , Jiri Olsa Subject: Re: [PATCH 1/4] perf trace: Improve error message when receive non-tracepoint events Message-ID: <20160408152257.GO5945@kernel.org> References: <1460128045-97310-1-git-send-email-wangnan0@huawei.com> <1460128045-97310-2-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460128045-97310-2-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2232 Lines: 73 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 > Signed-off-by: Wang Nan > Cc: Arnaldo Carvalho de Melo > Cc: Jiri Olsa > Cc: Li Zefan > Cc: pi3orama@163.com > --- > tools/perf/builtin-trace.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > 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; > -- > 1.8.3.4