From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753484AbdJST5e (ORCPT ); Thu, 19 Oct 2017 15:57:34 -0400 Received: from mail.kernel.org ([198.145.29.99]:52344 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751831AbdJST5d (ORCPT ); Thu, 19 Oct 2017 15:57:33 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 766A72187D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Thu, 19 Oct 2017 16:57:30 -0300 From: Arnaldo Carvalho de Melo To: Jack Henschel Cc: Peter Zijlstra , Ingo Molnar , Alexander Shishkin , Jiri Olsa , Andi Kleen , Taeung Song , Mathieu Poirier , linux-kernel@vger.kernel.org Subject: Re: [PATCH] perf parser: Improve error message for PMU address filters Message-ID: <20171019195730.GB4059@kernel.org> References: <20170905090839.1619-1-jackdev@mailbox.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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