From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Hunter Subject: Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule Date: Thu, 3 May 2018 14:38:15 +0300 Message-ID: <0c33d3f9-4b76-c94c-7306-e93e8cd8d4aa@intel.com> References: <20180425160008.3407-1-acme@kernel.org> <20180425160008.3407-6-acme@kernel.org> <448c4e21-8232-3d04-cac4-49b95c8bca3a@intel.com> <20180503103717.GA14776@krava> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180503103717.GA14776@krava> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Jiri Olsa Cc: Arnaldo Carvalho de Melo , Ingo Molnar , Clark Williams , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Jiri Olsa , Alexander Shishkin , David Ahern , Namhyung Kim , Peter Zijlstra , Arnaldo Carvalho de Melo List-Id: linux-perf-users.vger.kernel.org On 03/05/18 13:37, Jiri Olsa wrote: > On Thu, May 03, 2018 at 11:25:16AM +0300, Adrian Hunter wrote: >> Hi >> >> This breaks Intel PT i.e. >> >> $ perf record -e intel_pt//u uname >> event syntax error: 'intel_pt//u' >> \___ parser error >> Run 'perf list' for a list of valid events >> >> Usage: perf record [] [] >> or: perf record [] -- [] >> >> -e, --event event selector. use 'perf list' to list available events >> >> See below for cause. >> >> On 25/04/18 19:00, Arnaldo Carvalho de Melo wrote: >>> From: Jiri Olsa >>> >>> Currently all the event parsing fails end up in the event_pmu rule, and >>> display misleading help like: >>> >>> $ perf stat -e inst kill >>> event syntax error: 'inst' >>> \___ Cannot find PMU `inst'. Missing kernel support? >>> ... >>> >>> The reason is that the event_pmu is too strong and match also single >>> string. Changing it to force the '/' separators to be part of the rule, >>> and getting the proper error now: >>> >>> $ perf stat -e inst kill >>> event syntax error: 'inst' >>> \___ parser error >>> Run 'perf list' for a list of valid events >>> ... >>> >>> Signed-off-by: Jiri Olsa >>> Reported-by: Ingo Molnar >>> Tested-by: Arnaldo Carvalho de Melo >>> Cc: Alexander Shishkin >>> Cc: David Ahern >>> Cc: Namhyung Kim >>> Cc: Peter Zijlstra >>> Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org >>> Signed-off-by: Arnaldo Carvalho de Melo >>> --- >>> tools/perf/util/parse-events.y | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y >>> index 7afeb80cc39e..d14464c42714 100644 >>> --- a/tools/perf/util/parse-events.y >>> +++ b/tools/perf/util/parse-events.y >>> @@ -224,15 +224,15 @@ event_def: event_pmu | >>> event_bpf_file >>> >>> event_pmu: >>> -PE_NAME opt_event_config >>> +PE_NAME '/' event_config '/' >> >> These are not equivalent because opt_event_config allows '//' >> but event_config cannot be an empty string. > > yep, overlooked this one, how about patch below Seems to work but gives build warnings: util/parse-events.y: warning: 1 shift/reduce conflict [-Wconflicts-sr] util/parse-events.y: warning: 1 reduce/reduce conflict [-Wconflicts-rr] > > jirka > > --- > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index d14464c42714..1ed2befeca8a 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -523,6 +523,10 @@ event_term > list_add_tail(&term->list, head); > $$ = head; > } > +| > +{ > + $$ = NULL; > +} > > event_term: > PE_NAME '=' PE_NAME >