Received: by 2002:a05:7412:31a9:b0:e2:908c:2ebd with SMTP id et41csp2985841rdb; Tue, 12 Sep 2023 20:15:07 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHqvuVRb0LoMOj5pwyUO/AoLmunOxniGxS7z/VWae15EixQE/W4xhjL43m/Don9HxbNIW39 X-Received: by 2002:a05:6870:9585:b0:1bf:e2a6:b287 with SMTP id k5-20020a056870958500b001bfe2a6b287mr1624033oao.0.1694574907267; Tue, 12 Sep 2023 20:15:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694574907; cv=none; d=google.com; s=arc-20160816; b=KxNKnzC+S8m5WZZ6JAoibEKsiyFQbWTlBtQSDlWIGep9sOrdvmr/K1vcsXqKrDocQx sg/WEBrSv+Qa/MwUEISgpET5P2TCxDSSHNru+B5d6VAQk6nzqx4guilz/nW3hjWSAliP lF0Hu/cPVMbal40anjWtGoFuSt5K4i/aQJJDQYLg73KfIGf5yYIOeouhNAxIPSh8kWOc WYhicwaJPDjG6ibRtTAPNEjsZVov/ptWrLG5U2FIS3rYFaBxzN8k32X++jCOQcQ3kmKm SwvqyIV/Okaz0Ms2PwMmoA04N1GSsFXzC+bQm2QKEfU8CBBMpkLU76ROmVvbtzvrOwxf uqVQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:cc:to:subject:user-agent:mime-version:date:message-id; bh=nRVEdvMrekzy04+Pl3f7wtB5hh98BadoN4GVUXors6I=; fh=lIlTeq0oiELndf6343Fi20K0hVEYEaY0WHJs02YKlPk=; b=avDqhtEz//UiZ/WLDOoU+gII7ul05gPICHy7uZ1r6YY7sVVegJWZtJEnMKIucGDdY3 urFw/NKqM9+GZ2LKkTX9fmSx9NpCkBC68rn4v1Cm/dXiTa6yTyTub5LAhbjFNFVT88F8 zNtrd1CIpbQxIqNccgCr5HIpiC3AFGQ2ztL3s4MRvwoxoM6GhDy0ww8FJg0f0jvIXccS foVoptB/0LimOwfrSUml3Il93qy/JHfb6TzfYAUVqWAy/6+uAnKhAm88Bb1bT6UR8LyY 9dUhVjAPUAn7e3ENKK6/+10LUCK4hnbIKZj+BtV1MEVnajwR7aqWkPAS3mFSlcyfRs23 +Vag== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Return-Path: Received: from agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id r142-20020a632b94000000b0056fc3ceaba4si9088525pgr.432.2023.09.12.20.15.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Sep 2023 20:15:07 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 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 (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 300448028F9D; Tue, 12 Sep 2023 20:12:54 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231181AbjIMDMl (ORCPT + 99 others); Tue, 12 Sep 2023 23:12:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57452 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229948AbjIMDMj (ORCPT ); Tue, 12 Sep 2023 23:12:39 -0400 Received: from out30-101.freemail.mail.aliyun.com (out30-101.freemail.mail.aliyun.com [115.124.30.101]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4B1AD1719; Tue, 12 Sep 2023 20:12:34 -0700 (PDT) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R161e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045168;MF=renyu.zj@linux.alibaba.com;NM=1;PH=DS;RN=20;SR=0;TI=SMTPD_---0VrymIl3_1694574747; Received: from 30.221.149.124(mailfrom:renyu.zj@linux.alibaba.com fp:SMTPD_---0VrymIl3_1694574747) by smtp.aliyun-inc.com; Wed, 13 Sep 2023 11:12:29 +0800 Message-ID: Date: Wed, 13 Sep 2023 11:12:24 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH v8 1/8] perf pmu: "Compat" supports matching multiple identifiers To: Ian Rogers Cc: John Garry , Will Deacon , James Clark , Arnaldo Carvalho de Melo , Mark Rutland , Mike Leach , Leo Yan , Namhyung Kim , Peter Zijlstra , Ingo Molnar , Alexander Shishkin , Jiri Olsa , Adrian Hunter , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-doc@vger.kernel.org, Zhuo Song , Shuai Xue References: <1694087913-46144-1-git-send-email-renyu.zj@linux.alibaba.com> <1694087913-46144-2-git-send-email-renyu.zj@linux.alibaba.com> <8bab7404-8e24-8606-558c-db3495429f2f@linux.alibaba.com> From: Jing Zhang In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 12 Sep 2023 20:12:54 -0700 (PDT) X-Spam-Status: No, score=-2.2 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, UNPARSEABLE_RELAY autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email 在 2023/9/12 上午1:32, Ian Rogers 写道: > On Sun, Sep 10, 2023 at 7:32 PM Jing Zhang wrote: >> >> >> >> 在 2023/9/9 上午5:33, Ian Rogers 写道: >>> On Thu, Sep 7, 2023 at 4:58 AM 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. >>>> 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 >>>> Reviewed-by: John Garry >>>> --- >>>> tools/perf/util/pmu.c | 28 ++++++++++++++++++++++++++-- >>>> tools/perf/util/pmu.h | 1 + >>>> 2 files changed, 27 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >>>> index e215985..c3c3818 100644 >>>> --- a/tools/perf/util/pmu.c >>>> +++ b/tools/perf/util/pmu.c >>>> @@ -875,6 +875,30 @@ 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 = false; >>>> + >>>> + /* >>>> + * 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); >>> >>> Did the comma vs semicolon difference get explained? It seems to add >>> inconsistency to use a semicolon. >>> >> >> Hi Ian, >> >> Yes, I explained the reason for using semicolons instead of commas in v7. >> >> 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 Unit, commas act as wildcards, and in “Compat”, the semicolon means “or”. So >> I think semicolons are more appropriate. >> >> John also raised this issue earlier, and we finally agreed to use semicolons. >> What do you think? > > I'm okay with it, but thanks for capturing the why of this. I'd like > at some point to make the wildcarding of things less ad hoc. For > example, on x86 we use regular expressions to match cpuid: > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/mapfile.csv?h=perf-tools-next Thank you for the example. I was not aware that regular expressions were already being used for matching in tools/perf. > but file name style matching for pmus: > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next#n1974 > Given that we're okay with regular expressions then I don't see why > everything shouldn't be a regular expression. This could, for example, > make matching PMUs more specific than just adding a star and doing a > file name match. For an example of why this is weird, on my laptop: > ``` > $ perf stat -e i/actual-frequency/ true > > Performance counter stats for 'system wide': > > 0 i/actual-frequency/ > > 0.001168195 seconds time elapsed > ``` > The PMU I used here as 'i' is /sys/devices/i915 as we allow arbitrary > numbers after a PMU name for cases of multiple uncore PMUs. > > My feeling longer term is that the matching distinction of Unit and > Compat, comma and semi-colon, would be better captured with regular > expressions as I think they show the intent in the matching more > clearly. > Yes, using regular expressions is indeed a better choice for consistency and clarity, and I will try using regular expressions for Compat matching in the next version. Thanks, Jing > Thanks, > Ian > > >> Thanks, >> Jing >> >>> Thanks, >>> Ian >>> >>>> + for (; tok; tok = strtok_r(NULL, ";", &tmp)) { >>>> + if (!fnmatch(tok, id, FNM_CASEFOLD)) { >>>> + res = true; >>>> + break; >>>> + } >>>> + } >>>> + free(str); >>>> + return res; >>>> +} >>>> + >>>> 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 +939,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(pmu, >>>> pe->name, >>>> pe->desc, >>>> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h >>>> index bd5d804..1bf5cf1 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 *id, const char *compat); >>>> >>>> int perf_pmu__convert_scale(const char *scale, char **end, double *sval); >>>> >>>> -- >>>> 1.8.3.1 >>>>