On Mon, Aug 21, 2023 at 1:36 AM Jing Zhang <renyu.zj@xxxxxxxxxxxxxxxxx> wrote: > > The jevent "Compat" is used for uncore PMU alias or metric definitions. > > The same PMU driver has different PMU identifiers due to different > hardware versions and types, but they may have some common PMU event. > Since a Compat value can only match one identifier, when adding the > same event alias to PMUs with different identifiers, each identifier > needs to be defined once, which is not streamlined enough. > > So let "Compat" supports matching multiple identifiers for uncore PMU > alias. For example, the Compat value {43401;436*} can match the PMU > identifier "43401", that is, CMN600_r0p0, and the PMU identifier with > the prefix "436", that is, all CMN650, where "*" is a wildcard. > Tokens in Unit field are delimited by ';' with no spaces. > > Signed-off-by: Jing Zhang <renyu.zj@xxxxxxxxxxxxxxxxx> > Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx> > --- > tools/perf/util/pmu.c | 33 +++++++++++++++++++++++++++++++-- > tools/perf/util/pmu.h | 1 + > 2 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index ad209c8..6402423 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -776,6 +776,35 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) > return res; > } > > +bool pmu_uncore_identifier_match(const char *id, const char *compat) static? > +{ > + char *tmp = NULL, *tok, *str; > + bool res; Initialize to false to avoid the goto. > + int n; Move into the scope of the for loop, to reduce the scope. > + > + /* > + * The strdup() call is necessary here because "compat" is a const str* > + * type and cannot be used as an argument to strtok_r(). > + */ > + str = strdup(compat); > + if (!str) > + return false; > + > + tok = strtok_r(str, ";", &tmp); > + for (; tok; tok = strtok_r(NULL, ";", &tmp)) { > + n = strlen(tok); > + if ((tok[n - 1] == '*' && !strncmp(id, tok, n - 1)) || > + !strcmp(id, tok)) { We use fnmatch for a similar check: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next#n1982 > + res = true; > + goto out; With "res=false;" above this can just be a regular break. Thanks, Ian > + } > + } > + res = false; > +out: > + free(str); > + return res; > +} > + > struct pmu_add_cpu_aliases_map_data { > struct list_head *head; > const char *name; > @@ -847,8 +876,8 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe, > if (!pe->compat || !pe->pmu) > return 0; > > - if (!strcmp(pmu->id, pe->compat) && > - pmu_uncore_alias_match(pe->pmu, pmu->name)) { > + if (pmu_uncore_alias_match(pe->pmu, pmu->name) && > + pmu_uncore_identifier_match(pmu->id, pe->compat)) { > __perf_pmu__new_alias(idata->head, -1, > (char *)pe->name, > (char *)pe->desc, > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index b9a02de..9d4385d 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -241,6 +241,7 @@ void pmu_add_cpu_aliases_table(struct list_head *head, struct perf_pmu *pmu, > char *perf_pmu__getcpuid(struct perf_pmu *pmu); > const struct pmu_events_table *pmu_events_table__find(void); > const struct pmu_metrics_table *pmu_metrics_table__find(void); > +bool pmu_uncore_identifier_match(const char *id, const char *compat); > void perf_pmu_free_alias(struct perf_pmu_alias *alias); > > int perf_pmu__convert_scale(const char *scale, char **end, double *sval); > -- > 1.8.3.1 >