Message ID | 1690525040-77423-2-git-send-email-renyu.zj@linux.alibaba.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:918b:0:b0:3e4:2afc:c1 with SMTP id s11csp235960vqg; Thu, 27 Jul 2023 23:37:20 -0700 (PDT) X-Google-Smtp-Source: APBJJlGqJ0+ZtGW7I3vJcnMqHJZaCkoLALILnWxEXV4bGiZe5PZcNAyH1Ij39f8lVvXIeLR5xtKR X-Received: by 2002:a17:902:e546:b0:1b7:f64b:379b with SMTP id n6-20020a170902e54600b001b7f64b379bmr1347462plf.17.1690526239834; Thu, 27 Jul 2023 23:37:19 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690526239; cv=none; d=google.com; s=arc-20160816; b=uiFyj/58yPgzNBwCq00Mxjghi2VS/2la9TNKm5wi624y88QEd14zU3eoak16MnVBb2 /7h6YgFdlwVIMR3Fj8m97NJTIlOUhrsgdoTYb33yxqw/BJLslhL8Y6BrOWYGE0hAHCHY gIAzPf8T6si5h1by0+yT5phRX2MZY1ZTW+6gOVTOLKsh2ljqzrxqMcr3LDw3LwUNcbxP 0+VkajEqNZaegomiBHIHgCZ6h/PptYAofr03v1kWEfZdkmcI1shrKI3x+CIMJqWnjryk 3o52ZhxQ20my0t+IGBLDGujD5dxhmVzFD8UY07XgRAXDvEiqAxYA45XxBVNZiubTHTBc Okkg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from; bh=eZjRCHO2xQBGHysY3K60g3Ljy4wSGDYY5v2zylceSn4=; fh=6JOIFq++GFBr55qsbUomW9uDlSkbtTxkzfQq3YEDQ7Q=; b=ZL0MpZRXcXyD33Yg+XPeCEcRIJ8hmrxdQWij4vrU7FJHFY/iQekbRXW8FnZ3+phaAE k3SmWp/mw6Ntau4zGaBRlw2wZ7XaLgktkCOJlwKsfMfEIsaJeR2qY+Ngl5DI9e0LKjCE 4efPGnGDShvWoc3OcKS03XYm5+JzCeoG0lwoGuGJmXL66kK7C/a/HVK0NcIM59xuqLCk 9xucTwfVeseyQofVf8K/dUZo0o/oUbQ4irCHNvW4MYu0zaC4mQd8FO0EqddrfFAq36x2 zEqg31zR0MB59rXFW6l6u4hhPn2ctHjzE38uO02rW9lISDihXZ1DewD/4pBTGMzyby3a nyAQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b5-20020a170902d50500b001b7d2b55d8asi2638492plg.626.2023.07.27.23.37.06; Thu, 27 Jul 2023 23:37:19 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233493AbjG1GRn (ORCPT <rfc822;hanasaki@gmail.com> + 99 others); Fri, 28 Jul 2023 02:17:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233418AbjG1GRh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 28 Jul 2023 02:17:37 -0400 Received: from out30-112.freemail.mail.aliyun.com (out30-112.freemail.mail.aliyun.com [115.124.30.112]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 13230272C; Thu, 27 Jul 2023 23:17:34 -0700 (PDT) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R171e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045176;MF=renyu.zj@linux.alibaba.com;NM=1;PH=DS;RN=15;SR=0;TI=SMTPD_---0VoOxEHt_1690525049; Received: from srmbuffer011165236051.sqa.net(mailfrom:renyu.zj@linux.alibaba.com fp:SMTPD_---0VoOxEHt_1690525049) by smtp.aliyun-inc.com; Fri, 28 Jul 2023 14:17:30 +0800 From: Jing Zhang <renyu.zj@linux.alibaba.com> To: John Garry <john.g.garry@oracle.com>, Ian Rogers <irogers@google.com> Cc: Will Deacon <will@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Robin Murphy <robin.murphy@arm.com>, Ilkka Koskinen <ilkka@os.amperecomputing.com>, Namhyung Kim <namhyung@kernel.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-doc@vger.kernel.org, Zhuo Song <zhuo.song@linux.alibaba.com>, Jing Zhang <renyu.zj@linux.alibaba.com>, Shuai Xue <xueshuai@linux.alibaba.com> Subject: [PATCH v5 1/5] perf metric: Event "Compat" value supports matching multiple identifiers Date: Fri, 28 Jul 2023 14:17:16 +0800 Message-Id: <1690525040-77423-2-git-send-email-renyu.zj@linux.alibaba.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1690525040-77423-1-git-send-email-renyu.zj@linux.alibaba.com> References: <1690525040-77423-1-git-send-email-renyu.zj@linux.alibaba.com> X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,UNPARSEABLE_RELAY,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772645242704908535 X-GMAIL-MSGID: 1772645242704908535 |
Series |
Add aliases and metrics for Arm CMN
|
|
Commit Message
Jing Zhang
July 28, 2023, 6:17 a.m. UTC
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/metric. Since a
Compat value can only match one identifier, when adding the same event
alias and metric to PMUs with different identifiers, each identifier needs
to be defined once, which is not streamlined enough.
So let "Compat" value supports matching multiple identifiers. For example,
the Compat value {abcde;123*} can match the PMU identifier "abcde" and the
the PMU identifier with the prefix "123", where "*" is a wildcard.
Tokens in Unit field are delimited by ';' with no spaces.
Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
tools/perf/util/metricgroup.c | 2 +-
tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
tools/perf/util/pmu.h | 1 +
3 files changed, 28 insertions(+), 2 deletions(-)
Comments
On 28/07/2023 07:17, Jing Zhang 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/metric. Since a > Compat value can only match one identifier, when adding the same event > alias and metric to PMUs with different identifiers, each identifier needs > to be defined once, which is not streamlined enough. > > So let "Compat" value supports matching multiple identifiers. For example, > the Compat value {abcde;123*} why not use a comma-separated list? that is more common > can match the PMU identifier "abcde" and the > the PMU identifier with the prefix "123", I have to admit that this is not a great example as it does not match an expected real-life scenario. I mean, I would not expect a PMU identifier for the same PMU to be in either format "abcde" or "123*". I would expect to be in only ever one format. > where "*" is a wildcard. > Tokens in Unit field are delimited by ';' with no spaces. > > Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> > --- > tools/perf/util/metricgroup.c | 2 +- > tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++- > tools/perf/util/pmu.h | 1 + > 3 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > index 5e9c657..ff81bc5 100644 > --- a/tools/perf/util/metricgroup.c > +++ b/tools/perf/util/metricgroup.c > @@ -477,7 +477,7 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, > > while ((pmu = perf_pmu__scan(pmu))) { > > - if (!pmu->id || strcmp(pmu->id, pm->compat)) > + if (!pmu->id || !pmu_uncore_identifier_match(pmu->id, pm->compat)) > continue; > > return d->fn(pm, table, d->data); > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index ad209c8..3ae249b 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -776,6 +776,31 @@ 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) > +{ > + char *tmp = NULL, *tok, *str; > + bool res; > + int n; > + > + str = strdup(compat); why duplicate this? are you modifying something? > + 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)) { > + res = true; > + goto out; > + } > + } > + res = false; > +out: > + free(str); > + return res; > +} > + > struct pmu_add_cpu_aliases_map_data { > struct list_head *head; > const char *name; > @@ -847,7 +872,7 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe, This is not for metrics specifically. You are really doing 2x things here. I suggest that you split the patch out into 1st pmu.c change and 2nd metricgroup.c change > if (!pe->compat || !pe->pmu) > return 0; > > - if (!strcmp(pmu->id, pe->compat) && > + if (pmu_uncore_identifier_match(pmu->id, pe->compat) && > pmu_uncore_alias_match(pe->pmu, pmu->name)) { nit: let's change order to check alias and then identifier > __perf_pmu__new_alias(idata->head, -1, > (char *)pe->name, > 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); Thanks, John
在 2023/7/28 下午4:11, John Garry 写道: > On 28/07/2023 07:17, Jing Zhang 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/metric. Since a >> Compat value can only match one identifier, when adding the same event >> alias and metric to PMUs with different identifiers, each identifier needs >> to be defined once, which is not streamlined enough. >> >> So let "Compat" value supports matching multiple identifiers. For example, >> the Compat value {abcde;123*} > why not use a comma-separated list? that is more common > Hi John, I use a semicolon instead of a comma because I want to distinguish it from the function of the comma in "Unit" and avoid confusion between the use of commas in "Unit" and "Compat". Because in “Compat”, the semicolon means "or". So I think semicolons are more appropriate, what do you think? >> can match the PMU identifier "abcde" and the >> the PMU identifier with the prefix "123", > > I have to admit that this is not a great example as it does not match an expected real-life scenario. I mean, I would not expect a PMU identifier for the same PMU to be in either format "abcde" or "123*". I would expect to be in only ever one format. > Get, I'll pick a more appropriate example {43401;436*}(CMN600 r0p0 and all CMN650). >> where "*" is a wildcard. >> Tokens in Unit field are delimited by ';' with no spaces. >> >> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> >> --- >> tools/perf/util/metricgroup.c | 2 +- >> tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++- >> tools/perf/util/pmu.h | 1 + >> 3 files changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c >> index 5e9c657..ff81bc5 100644 >> --- a/tools/perf/util/metricgroup.c >> +++ b/tools/perf/util/metricgroup.c >> @@ -477,7 +477,7 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, >> while ((pmu = perf_pmu__scan(pmu))) { >> - if (!pmu->id || strcmp(pmu->id, pm->compat)) >> + if (!pmu->id || !pmu_uncore_identifier_match(pmu->id, pm->compat)) >> continue; >> return d->fn(pm, table, d->data); >> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >> index ad209c8..3ae249b 100644 >> --- a/tools/perf/util/pmu.c >> +++ b/tools/perf/util/pmu.c >> @@ -776,6 +776,31 @@ 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) >> +{ >> + char *tmp = NULL, *tok, *str; >> + bool res; >> + int n; >> + >> + str = strdup(compat); > > why duplicate this? are you modifying something? > This is really a redundant step, I will remove it. >> + 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)) { >> + res = true; >> + goto out; >> + } >> + } >> + res = false; >> +out: >> + free(str); >> + return res; >> +} >> + >> struct pmu_add_cpu_aliases_map_data { >> struct list_head *head; >> const char *name; >> @@ -847,7 +872,7 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe, > > This is not for metrics specifically. You are really doing 2x things here. I suggest that you split the patch out into 1st pmu.c change and 2nd metricgroup.c change > Ok, will do. >> if (!pe->compat || !pe->pmu) >> return 0; >> - if (!strcmp(pmu->id, pe->compat) && >> + if (pmu_uncore_identifier_match(pmu->id, pe->compat) && >> pmu_uncore_alias_match(pe->pmu, pmu->name)) { > > nit: let's change order to check alias and then identifier > Will do. Thanks, Jing >> __perf_pmu__new_alias(idata->head, -1, >> (char *)pe->name, >> 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); > > Thanks, > John
On 31/07/2023 11:59, Jing Zhang wrote: >> why not use a comma-separated list? that is more common >> > Hi John, > > I use a semicolon instead of a comma because I want to distinguish it from the function > of the comma in "Unit" and avoid confusion between the use of commas in "Unit" and "Compat". > Because in “Compat”, the semicolon means "or". So I think semicolons are more appropriate, > what do you think? > I suppose semicolon is ok in this case. Thanks, John
在 2023/7/31 下午6:59, Jing Zhang 写道: > > 在 2023/7/28 下午4:11, John Garry 写道: >> On 28/07/2023 07:17, Jing Zhang 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/metric. Since a >>> Compat value can only match one identifier, when adding the same event >>> alias and metric to PMUs with different identifiers, each identifier needs >>> to be defined once, which is not streamlined enough. >>> >>> So let "Compat" value supports matching multiple identifiers. For example, >>> the Compat value {abcde;123*} >> why not use a comma-separated list? that is more common >> > > Hi John, > > I use a semicolon instead of a comma because I want to distinguish it from the function > of the comma in "Unit" and avoid confusion between the use of commas in "Unit" and "Compat". > Because in “Compat”, the semicolon means "or". So I think semicolons are more appropriate, > what do you think? > >>> can match the PMU identifier "abcde" and the >>> the PMU identifier with the prefix "123", >> >> I have to admit that this is not a great example as it does not match an expected real-life scenario. I mean, I would not expect a PMU identifier for the same PMU to be in either format "abcde" or "123*". I would expect to be in only ever one format. >> > > Get, I'll pick a more appropriate example {43401;436*}(CMN600 r0p0 and all CMN650). > >>> where "*" is a wildcard. >>> Tokens in Unit field are delimited by ';' with no spaces. >>> >>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> >>> --- >>> tools/perf/util/metricgroup.c | 2 +- >>> tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++- >>> tools/perf/util/pmu.h | 1 + >>> 3 files changed, 28 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c >>> index 5e9c657..ff81bc5 100644 >>> --- a/tools/perf/util/metricgroup.c >>> +++ b/tools/perf/util/metricgroup.c >>> @@ -477,7 +477,7 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, >>> while ((pmu = perf_pmu__scan(pmu))) { >>> - if (!pmu->id || strcmp(pmu->id, pm->compat)) >>> + if (!pmu->id || !pmu_uncore_identifier_match(pmu->id, pm->compat)) >>> continue; >>> return d->fn(pm, table, d->data); >>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >>> index ad209c8..3ae249b 100644 >>> --- a/tools/perf/util/pmu.c >>> +++ b/tools/perf/util/pmu.c >>> @@ -776,6 +776,31 @@ 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) >>> +{ >>> + char *tmp = NULL, *tok, *str; >>> + bool res; >>> + int n; >>> + >>> + str = strdup(compat); >> >> why duplicate this? are you modifying something? >> > > This is really a redundant step, I will remove it. > Hi John, I reviewed this code again and found that it still needs to duplicate "compat" because "compat" is a const str* type and cannot be used as a parameter for the strtok_r function. If it is cast to char*, using "compat" as a parameter for strtok_r is also unsafe and can cause a "Segmentation fault" error. Therefore, let's keep the step of duplicating "compat". Thanks, Jing >>> + 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)) { >>> + res = true; >>> + goto out; >>> + } >>> + } >>> + res = false; >>> +out: >>> + free(str); >>> + return res; >>> +} >>> + >>> struct pmu_add_cpu_aliases_map_data { >>> struct list_head *head; >>> const char *name; >>> @@ -847,7 +872,7 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe, >> >> This is not for metrics specifically. You are really doing 2x things here. I suggest that you split the patch out into 1st pmu.c change and 2nd metricgroup.c change >> > > Ok, will do. > >>> if (!pe->compat || !pe->pmu) >>> return 0; >>> - if (!strcmp(pmu->id, pe->compat) && >>> + if (pmu_uncore_identifier_match(pmu->id, pe->compat) && >>> pmu_uncore_alias_match(pe->pmu, pmu->name)) { >> >> nit: let's change order to check alias and then identifier >> > > Will do. > > > Thanks, > Jing > >>> __perf_pmu__new_alias(idata->head, -1, >>> (char *)pe->name, >>> 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); >> >> Thanks, >> John
On 02/08/2023 10:38, Jing Zhang wrote: >>>> n; >>>> + >>>> + str = strdup(compat); >>> why duplicate this? are you modifying something? >>> >> This is really a redundant step, I will remove it. >> > Hi John, > > I reviewed this code again and found that it still needs to duplicate "compat" because "compat" is a > const str* type and cannot be used as a parameter for the strtok_r function. If it is cast to char*, > using "compat" as a parameter for strtok_r is also unsafe and can cause a "Segmentation fault" error. > Therefore, let's keep the step of duplicating "compat". ok, so then please add a small comment on why the strdup() call is needed. Thanks, John
在 2023/8/2 下午5:43, John Garry 写道: > On 02/08/2023 10:38, Jing Zhang wrote: >>>>> n; >>>>> + >>>>> + str = strdup(compat); >>>> why duplicate this? are you modifying something? >>>> >>> This is really a redundant step, I will remove it. >>> >> Hi John, >> >> I reviewed this code again and found that it still needs to duplicate "compat" because "compat" is a >> const str* type and cannot be used as a parameter for the strtok_r function. If it is cast to char*, >> using "compat" as a parameter for strtok_r is also unsafe and can cause a "Segmentation fault" error. >> Therefore, let's keep the step of duplicating "compat". > > ok, so then please add a small comment on why the strdup() call is needed. > No problem. Thanks, Jing
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 5e9c657..ff81bc5 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -477,7 +477,7 @@ static int metricgroup__sys_event_iter(const struct pmu_metric *pm, while ((pmu = perf_pmu__scan(pmu))) { - if (!pmu->id || strcmp(pmu->id, pm->compat)) + if (!pmu->id || !pmu_uncore_identifier_match(pmu->id, pm->compat)) continue; return d->fn(pm, table, d->data); diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index ad209c8..3ae249b 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -776,6 +776,31 @@ 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) +{ + char *tmp = NULL, *tok, *str; + bool res; + int n; + + 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)) { + res = true; + goto out; + } + } + res = false; +out: + free(str); + return res; +} + struct pmu_add_cpu_aliases_map_data { struct list_head *head; const char *name; @@ -847,7 +872,7 @@ 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) && + if (pmu_uncore_identifier_match(pmu->id, pe->compat) && pmu_uncore_alias_match(pe->pmu, pmu->name)) { __perf_pmu__new_alias(idata->head, -1, (char *)pe->name, 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);