Messages in this thread Patch in this message | | | Subject | Re: [PATCH] perf tools: Fix parser for empty pmu terms case | From | "Liang, Kan" <> | Date | Sun, 6 May 2018 12:34:48 -0400 |
| |
On 5/6/2018 10:28 AM, Jiri Olsa wrote: > On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote: >> Jiri Olsa <jolsa@redhat.com> writes: >> >> Please fix this quickly, PT is currently totally non functional in Linus >> mainline. > > attached.. Kan, could you please test it wrt your latest changes? > > thanks, > jirka > > > --- > Adrian reported broken event parsing for Intel PT: > > $ perf record -e intel_pt//u uname > event syntax error: 'intel_pt//u' > \___ parser error > Run 'perf list' for a list of valid events > > It's caused by recent change in parsing grammar > (see Fixes: for commit). > > Adding special rule with empty terms config to handle > the reported case and moving the common rule code into > new parse_events_pmu function. > > Fixes: 9a4a931ce847 ("perf pmu: Fix pmu events parsing rule") > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > Cc: Andi Kleen <ak@linux.intel.com> > Link: http://lkml.kernel.org/n/tip-uorb0azuem7b7ydace7cf6vc@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
The patch looks good to me. I tested my latest change and Intel PT. Both looks good.
./perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000 # time counts unit events 1.001556622 95,686 unc_m_cas_count.all 1.001556622 1,900,544,149 unc_m_clockticks 2.002722048 159,598 unc_m_cas_count.all 2.002722048 2,339,028,614 unc_m_clockticks
./perf record -e intel_pt//u [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.044 MB perf.data ]
Tested-by: Kan Liang <kan.liang@linux.intel.com>
> --- > tools/perf/util/parse-events.c | 46 +++++++++++++++++++++++++++++++++++++ > tools/perf/util/parse-events.h | 3 +++ > tools/perf/util/parse-events.y | 51 +++++++++++++----------------------------- > 3 files changed, 65 insertions(+), 35 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 3576aaaa9e4c..7cf326a8effe 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -8,6 +8,7 @@ > #include <sys/stat.h> > #include <fcntl.h> > #include <sys/param.h> > +#include <fnmatch.h> > #include "term.h" > #include "../perf.h" > #include "evlist.h" > @@ -1294,6 +1295,51 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, > return evsel ? 0 : -ENOMEM; > } > > +int parse_events_pmu(struct parse_events_state *parse_state, > + struct list_head *list, char *name, > + struct list_head *head_config) > +{ > + struct list_head *orig_terms; > + struct perf_pmu *pmu = NULL; > + char *pattern = NULL; > + int ok = 0; > + > + if (!parse_events_add_pmu(parse_state, list, name, > + head_config, false, false)) > + return 0; > + > + if (parse_events_copy_term_list(head_config, &orig_terms)) > + return -1; > + > + if (asprintf(&pattern, "%s*", name) < 0) > + goto out; > + > + while ((pmu = perf_pmu__scan(pmu)) != NULL) { > + char *tmp = pmu->name; > + > + if (!strncmp(tmp, "uncore_", 7) && > + strncmp(name, "uncore_", 7)) > + tmp += 7; > + if (!fnmatch(pattern, tmp, 0)) { > + struct list_head *terms; > + > + if (parse_events_copy_term_list(orig_terms, &terms)) { > + ok = 0; > + goto out; > + } > + if (!parse_events_add_pmu(parse_state, list, pmu->name, > + terms, true, false)) > + ok++; > + parse_events_terms__delete(terms); > + } > + } > + > +out: > + parse_events_terms__delete(orig_terms); > + free(pattern); > + return ok ? 0 : -1; > +} > + > int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > char *str, struct list_head **listp) > { > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index 4473dac27aee..553fcaf5d23e 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -170,6 +170,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, > struct list_head *head_config, > bool auto_merge_stats, > bool use_alias); > +int parse_events_pmu(struct parse_events_state *parse_state, > + struct list_head *list, char *name, > + struct list_head *head_config); > > int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > char *str, > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index 47f6399a309a..d44599bece43 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -8,7 +8,6 @@ > > #define YYDEBUG 1 > > -#include <fnmatch.h> > #include <linux/compiler.h> > #include <linux/list.h> > #include <linux/types.h> > @@ -226,44 +225,26 @@ event_def: event_pmu | > event_pmu: > PE_NAME '/' event_config '/' > { > - struct list_head *list, *orig_terms, *terms; > + struct list_head *list; > + > + ALLOC_LIST(list); > > - if (parse_events_copy_term_list($3, &orig_terms)) > + if (parse_events_pmu(_parse_state, list, $1, $3)) > YYABORT; > > - ALLOC_LIST(list); > - if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) { > - struct perf_pmu *pmu = NULL; > - int ok = 0; > - char *pattern; > - > - if (asprintf(&pattern, "%s*", $1) < 0) > - YYABORT; > - > - while ((pmu = perf_pmu__scan(pmu)) != NULL) { > - char *name = pmu->name; > - > - if (!strncmp(name, "uncore_", 7) && > - strncmp($1, "uncore_", 7)) > - name += 7; > - if (!fnmatch(pattern, name, 0)) { > - if (parse_events_copy_term_list(orig_terms, &terms)) { > - free(pattern); > - YYABORT; > - } > - if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false)) > - ok++; > - parse_events_terms__delete(terms); > - } > - } > - > - free(pattern); > - > - if (!ok) > - YYABORT; > - } > parse_events_terms__delete($3); > - parse_events_terms__delete(orig_terms); > + $$ = list; > +} > +| > +PE_NAME '/' '/' > +{ > + struct list_head *list; > + > + ALLOC_LIST(list); > + > + if (parse_events_pmu(_parse_state, list, $1, NULL)) > + YYABORT; > + > $$ = list; > } > | >
| |