| Message ID | 1695037955-107983-2-git-send-email-renyu.zj@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add metrics for Arm CMN | expand |
On Mon, Sep 18, 2023 at 4:52 AM Jing Zhang <renyu.zj@linux.alibaba.com> 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" support using regular expression to match identifiers > for uncore PMU alias. For example, if the "Compat" value is set to > "43401|43c01", it would be able to match PMU identifiers such as "43401" > or "43c01", which correspond to CMN600_r0p0 or CMN700_r0p0. > > Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> > --- > tools/perf/util/pmu.c | 23 +++++++++++++++++++++-- > tools/perf/util/pmu.h | 1 + > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index e215985..7e2242f 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -28,6 +28,7 @@ > #include "strbuf.h" > #include "fncache.h" > #include "util/evsel_config.h" > +#include <regex.h> > > struct perf_pmu perf_pmu__fake = { > .name = "fake", > @@ -875,6 +876,24 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) > return res; > } > > +bool pmu_uncore_identifier_match(const char *compat, const char *id) > +{ > + regex_t re; > + regmatch_t pmatch[1]; > + int match; > + > + if (regcomp(&re, compat, REG_EXTENDED) != 0) { > + /* Warn unable to generate match particular string. */ > + pr_info("Invalid regular expression %s\n", compat); > + return false; > + } > + > + match = !regexec(&re, id, 1, pmatch, 0); I wonder if we can make the regular expressions like "^(434|436|43c|43a)" more like "(434|436|43c|43a).*", so that we fully match the id string, by here doing: if (match) { /* Ensure a full match. */ match = pmatch[0].rm_so == 0 && pmatch[0].rm_eo == strlen(id); } I think longer term we can use jevents.py to generate a pmu-events.l, which would have a contents something like: (434|436|43c|43a).* { return PMU_....;} That should make the matching faster but may add some restrictions onto the regular expression. Thanks, Ian > + regfree(&re); > + > + return match; > +} > + > static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe, > const struct pmu_events_table *table __maybe_unused, > void *vdata) > @@ -915,8 +934,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(pe->compat, pmu->id)) { > perf_pmu__new_alias(pmu, > pe->name, > pe->desc, > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index bd5d804..fc155ce 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(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 *compat, const char *id); > > int perf_pmu__convert_scale(const char *scale, char **end, double *sval); > > -- > 1.8.3.1 >
在 2023/9/21 上午2:36, Ian Rogers 写道: > On Mon, Sep 18, 2023 at 4:52 AM Jing Zhang <renyu.zj@linux.alibaba.com> 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" support using regular expression to match identifiers >> for uncore PMU alias. For example, if the "Compat" value is set to >> "43401|43c01", it would be able to match PMU identifiers such as "43401" >> or "43c01", which correspond to CMN600_r0p0 or CMN700_r0p0. >> >> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> >> --- >> tools/perf/util/pmu.c | 23 +++++++++++++++++++++-- >> tools/perf/util/pmu.h | 1 + >> 2 files changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >> index e215985..7e2242f 100644 >> --- a/tools/perf/util/pmu.c >> +++ b/tools/perf/util/pmu.c >> @@ -28,6 +28,7 @@ >> #include "strbuf.h" >> #include "fncache.h" >> #include "util/evsel_config.h" >> +#include <regex.h> >> >> struct perf_pmu perf_pmu__fake = { >> .name = "fake", >> @@ -875,6 +876,24 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) >> return res; >> } >> >> +bool pmu_uncore_identifier_match(const char *compat, const char *id) >> +{ >> + regex_t re; >> + regmatch_t pmatch[1]; >> + int match; >> + >> + if (regcomp(&re, compat, REG_EXTENDED) != 0) { >> + /* Warn unable to generate match particular string. */ >> + pr_info("Invalid regular expression %s\n", compat); >> + return false; >> + } >> + >> + match = !regexec(&re, id, 1, pmatch, 0); > > I wonder if we can make the regular expressions like > "^(434|436|43c|43a)" more like "(434|436|43c|43a).*", so that we fully > match the id string, by here doing: > > if (match) { > /* Ensure a full match. */ > match = pmatch[0].rm_so == 0 && pmatch[0].rm_eo == strlen(id); > } > Ok, will do. > I think longer term we can use jevents.py to generate a pmu-events.l, > which would have a contents something like: > > (434|436|43c|43a).* { return PMU_....;} > > That should make the matching faster but may add some restrictions > onto the regular expression. Could you please describe the function of pmu-event.l in more detail? I may not fully understand it. Thanks, Jing > > Thanks, > Ian > >> + regfree(&re); >> + >> + return match; >> +} >> + >> static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe, >> const struct pmu_events_table *table __maybe_unused, >> void *vdata) >> @@ -915,8 +934,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(pe->compat, pmu->id)) { >> perf_pmu__new_alias(pmu, >> pe->name, >> pe->desc, >> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h >> index bd5d804..fc155ce 100644 >> --- a/tools/perf/util/pmu.h >> +++ b/tools/perf/util/pmu.h >> @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(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 *compat, const char *id); >> >> int perf_pmu__convert_scale(const char *scale, char **end, double *sval); >> >> -- >> 1.8.3.1 >>
On Fri, Sep 22, 2023 at 1:19 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote: > > > > 在 2023/9/21 上午2:36, Ian Rogers 写道: > > On Mon, Sep 18, 2023 at 4:52 AM Jing Zhang <renyu.zj@linux.alibaba.com> 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" support using regular expression to match identifiers > >> for uncore PMU alias. For example, if the "Compat" value is set to > >> "43401|43c01", it would be able to match PMU identifiers such as "43401" > >> or "43c01", which correspond to CMN600_r0p0 or CMN700_r0p0. > >> > >> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> > >> --- > >> tools/perf/util/pmu.c | 23 +++++++++++++++++++++-- > >> tools/perf/util/pmu.h | 1 + > >> 2 files changed, 22 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > >> index e215985..7e2242f 100644 > >> --- a/tools/perf/util/pmu.c > >> +++ b/tools/perf/util/pmu.c > >> @@ -28,6 +28,7 @@ > >> #include "strbuf.h" > >> #include "fncache.h" > >> #include "util/evsel_config.h" > >> +#include <regex.h> > >> > >> struct perf_pmu perf_pmu__fake = { > >> .name = "fake", > >> @@ -875,6 +876,24 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) > >> return res; > >> } > >> > >> +bool pmu_uncore_identifier_match(const char *compat, const char *id) > >> +{ > >> + regex_t re; > >> + regmatch_t pmatch[1]; > >> + int match; > >> + > >> + if (regcomp(&re, compat, REG_EXTENDED) != 0) { > >> + /* Warn unable to generate match particular string. */ > >> + pr_info("Invalid regular expression %s\n", compat); > >> + return false; > >> + } > >> + > >> + match = !regexec(&re, id, 1, pmatch, 0); > > > > I wonder if we can make the regular expressions like > > "^(434|436|43c|43a)" more like "(434|436|43c|43a).*", so that we fully > > match the id string, by here doing: > > > > if (match) { > > /* Ensure a full match. */ > > match = pmatch[0].rm_so == 0 && pmatch[0].rm_eo == strlen(id); > > } > > > > Ok, will do. > > > > I think longer term we can use jevents.py to generate a pmu-events.l, > > which would have a contents something like: > > > > (434|436|43c|43a).* { return PMU_....;} > > > > That should make the matching faster but may add some restrictions > > onto the regular expression. > > Could you please describe the function of pmu-event.l in more detail? I may not fully understand it. So for now there's no need for a pmu-event.l, I'm fine with the code as-is. The issue for using regular expressions is that we need to compile (regcomp) then use them (regexec), and in this new code the result of the parsing is discarded - perhaps we can save on some compiling with a 1 element cache, let's wait to see performance data saying it is an issue. If we were to compile the regular expressions at build time with flex then the runtime cost, any caching, etc. is unnecessary. Hope this makes sense. Thanks, Ian > Thanks, > Jing > > > > > Thanks, > > Ian > > > >> + regfree(&re); > >> + > >> + return match; > >> +} > >> + > >> static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe, > >> const struct pmu_events_table *table __maybe_unused, > >> void *vdata) > >> @@ -915,8 +934,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(pe->compat, pmu->id)) { > >> perf_pmu__new_alias(pmu, > >> pe->name, > >> pe->desc, > >> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > >> index bd5d804..fc155ce 100644 > >> --- a/tools/perf/util/pmu.h > >> +++ b/tools/perf/util/pmu.h > >> @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(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 *compat, const char *id); > >> > >> int perf_pmu__convert_scale(const char *scale, char **end, double *sval); > >> > >> -- > >> 1.8.3.1 > >>
在 2023/9/22 下午10:58, Ian Rogers 写道: > On Fri, Sep 22, 2023 at 1:19 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote: >> >> >> >> 在 2023/9/21 上午2:36, Ian Rogers 写道: >>> On Mon, Sep 18, 2023 at 4:52 AM Jing Zhang <renyu.zj@linux.alibaba.com> 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" support using regular expression to match identifiers >>>> for uncore PMU alias. For example, if the "Compat" value is set to >>>> "43401|43c01", it would be able to match PMU identifiers such as "43401" >>>> or "43c01", which correspond to CMN600_r0p0 or CMN700_r0p0. >>>> >>>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> >>>> --- >>>> tools/perf/util/pmu.c | 23 +++++++++++++++++++++-- >>>> tools/perf/util/pmu.h | 1 + >>>> 2 files changed, 22 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >>>> index e215985..7e2242f 100644 >>>> --- a/tools/perf/util/pmu.c >>>> +++ b/tools/perf/util/pmu.c >>>> @@ -28,6 +28,7 @@ >>>> #include "strbuf.h" >>>> #include "fncache.h" >>>> #include "util/evsel_config.h" >>>> +#include <regex.h> >>>> >>>> struct perf_pmu perf_pmu__fake = { >>>> .name = "fake", >>>> @@ -875,6 +876,24 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) >>>> return res; >>>> } >>>> >>>> +bool pmu_uncore_identifier_match(const char *compat, const char *id) >>>> +{ >>>> + regex_t re; >>>> + regmatch_t pmatch[1]; >>>> + int match; >>>> + >>>> + if (regcomp(&re, compat, REG_EXTENDED) != 0) { >>>> + /* Warn unable to generate match particular string. */ >>>> + pr_info("Invalid regular expression %s\n", compat); >>>> + return false; >>>> + } >>>> + >>>> + match = !regexec(&re, id, 1, pmatch, 0); >>> >>> I wonder if we can make the regular expressions like >>> "^(434|436|43c|43a)" more like "(434|436|43c|43a).*", so that we fully >>> match the id string, by here doing: >>> >>> if (match) { >>> /* Ensure a full match. */ >>> match = pmatch[0].rm_so == 0 && pmatch[0].rm_eo == strlen(id); >>> } >>> >> >> Ok, will do. >> >> >>> I think longer term we can use jevents.py to generate a pmu-events.l, >>> which would have a contents something like: >>> >>> (434|436|43c|43a).* { return PMU_....;} >>> >>> That should make the matching faster but may add some restrictions >>> onto the regular expression. >> >> Could you please describe the function of pmu-event.l in more detail? I may not fully understand it. > > So for now there's no need for a pmu-event.l, I'm fine with the code > as-is. The issue for using regular expressions is that we need to > compile (regcomp) then use them (regexec), and in this new code the > result of the parsing is discarded - perhaps we can save on some > compiling with a 1 element cache, let's wait to see performance data > saying it is an issue. If we were to compile the regular expressions > at build time with flex then the runtime cost, any caching, etc. is > unnecessary. > I see, thanks for the explanation. Thanks, Jing > Hope this makes sense. Thanks, > Ian > >> Thanks, >> Jing >> >>> >>> Thanks, >>> Ian >>> >>>> + regfree(&re); >>>> + >>>> + return match; >>>> +} >>>> + >>>> static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe, >>>> const struct pmu_events_table *table __maybe_unused, >>>> void *vdata) >>>> @@ -915,8 +934,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(pe->compat, pmu->id)) { >>>> perf_pmu__new_alias(pmu, >>>> pe->name, >>>> pe->desc, >>>> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h >>>> index bd5d804..fc155ce 100644 >>>> --- a/tools/perf/util/pmu.h >>>> +++ b/tools/perf/util/pmu.h >>>> @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(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 *compat, const char *id); >>>> >>>> int perf_pmu__convert_scale(const char *scale, char **end, double *sval); >>>> >>>> -- >>>> 1.8.3.1 >>>>
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index e215985..7e2242f 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -28,6 +28,7 @@ #include "strbuf.h" #include "fncache.h" #include "util/evsel_config.h" +#include <regex.h> struct perf_pmu perf_pmu__fake = { .name = "fake", @@ -875,6 +876,24 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name) return res; } +bool pmu_uncore_identifier_match(const char *compat, const char *id) +{ + regex_t re; + regmatch_t pmatch[1]; + int match; + + if (regcomp(&re, compat, REG_EXTENDED) != 0) { + /* Warn unable to generate match particular string. */ + pr_info("Invalid regular expression %s\n", compat); + return false; + } + + match = !regexec(&re, id, 1, pmatch, 0); + regfree(&re); + + return match; +} + static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe, const struct pmu_events_table *table __maybe_unused, void *vdata) @@ -915,8 +934,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(pe->compat, pmu->id)) { perf_pmu__new_alias(pmu, pe->name, pe->desc, diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h index bd5d804..fc155ce 100644 --- a/tools/perf/util/pmu.h +++ b/tools/perf/util/pmu.h @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(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 *compat, const char *id); int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
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" support using regular expression to match identifiers for uncore PMU alias. For example, if the "Compat" value is set to "43401|43c01", it would be able to match PMU identifiers such as "43401" or "43c01", which correspond to CMN600_r0p0 or CMN700_r0p0. Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> --- tools/perf/util/pmu.c | 23 +++++++++++++++++++++-- tools/perf/util/pmu.h | 1 + 2 files changed, 22 insertions(+), 2 deletions(-)