Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp215109pxj; Thu, 10 Jun 2021 19:59:01 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxhP6Bc9KiitxWQ77rbS/+NYcshnG+q2silQDFNBRZMyaPtyhHBenQ8pKHyTt9veSVF4Bgy X-Received: by 2002:a17:906:26cb:: with SMTP id u11mr1456925ejc.385.1623380341443; Thu, 10 Jun 2021 19:59:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623380341; cv=none; d=google.com; s=arc-20160816; b=JIjb9jspmf5kY9NVr1axMgC5RIpBscEITmKTwYELgXzurwVUaIqM9PvHRiqGxGYYfO aQf4piswLP1YwGAg/INWoK1givZRtDq5Tv5aYlPFvtcGf1U8BgAzh4dqmeHKtydZ6DTA JmTmKqpjyfCvf/QaLqBwnP4eAkD4dnptHdye+uVyJPMW14Cl1tl/NYaEgZe5CLXipigQ 4zyrrcqWr1/qUq4sTcDbtDbxLMDxReY+o/eqbkK8TDGDsEb5cX4cyfajeOipdefICi/x kKAqxSbTWWnPxkDeX8TuDiaZsqpdU4GuezOK0fxcpGTkCVmos6qoVawr+fk6zli+4lAs 39Zw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:ironport-sdr:ironport-sdr; bh=czL5UdO+Z5PaFesofaOaC4NbvDt4GLNCqG9S8Mcnh+w=; b=VcwF1MFq8HAM70ZdRFVGdRLVDF8yyiZytfNObpucDHLQtr+m2hPv1bXRM0j7ULLqQv 4LOc6+I/K917nkJ31CnDtYmTj7IbT1mFpMEDcpkCoybWauqFxXP8/A37rgilDwe6v60k RW733/eAttfX/jDwuoacjD9d+eD5EEXCvZZujeuKTjtXQZ6sFjvcCET5mKnMRmQs5iVy BxwnMS6SGIRCq1udGuSuJR0/On2aZE/oHJRiwfMrqqEjHBaYSXwq8sxcGlCssafQm/+J Pi9TaW6atW97dSuuFGkzUIehYEksIRJfj9zBvk1IeplM0yoBgywKqucxT6wVwdsZh6fI e9oQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j18si3975546ejo.733.2021.06.10.19.58.20; Thu, 10 Jun 2021 19:59:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230307AbhFKC4p (ORCPT + 99 others); Thu, 10 Jun 2021 22:56:45 -0400 Received: from mga02.intel.com ([134.134.136.20]:34878 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230035AbhFKC4o (ORCPT ); Thu, 10 Jun 2021 22:56:44 -0400 IronPort-SDR: rwD/alEc6eW7iQaOQ5TriehezBu2UAm040mXbWYkIw8fRQmN0szrEshoiG8ZHoRVWjPAW8FSTN y2l+zGGORH2A== X-IronPort-AV: E=McAfee;i="6200,9189,10011"; a="192560478" X-IronPort-AV: E=Sophos;i="5.83,265,1616482800"; d="scan'208";a="192560478" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Jun 2021 19:54:43 -0700 IronPort-SDR: v5GE+bgtrmK7ZmlK/3vsFSZJqTuEwhRCrny7NeLTzGCVmR2rCvJBRp82fE6njwhQ3FrJdIM3Fd azzJkwJzlAVw== X-IronPort-AV: E=Sophos;i="5.83,265,1616482800"; d="scan'208";a="448959865" Received: from yjin15-mobl1.ccr.corp.intel.com (HELO [10.238.4.82]) ([10.238.4.82]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Jun 2021 19:54:40 -0700 Subject: Re: [PATCH v1] perf tools: Fix pattern matching for same substring used in different pmu type To: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com Cc: Linux-kernel@vger.kernel.org, ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com References: <20210609045738.1051-1-yao.jin@linux.intel.com> From: "Jin, Yao" Message-ID: <982714a5-8a5d-8f8a-4e30-bd9a497ffa40@linux.intel.com> Date: Fri, 11 Jun 2021 10:54:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210609045738.1051-1-yao.jin@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org And, though this patch is to fix the uncore_imc/uncore_imc_free_running mismatching issue. We are also surprised to see it can solve another hybrid PMU issue on Alderlake. On Alderlake, # ./perf stat -e cpu/event=0xc2,umask=0x2/ true Performance counter stats for 'true': 702,246 cpu_core/event=0xc2,umask=0x2/ cpu_atom/event=0xc2,umask=0x2/ It should error out with the wrong PMU rather than using 'cpu_core' and'cpu_atom' instead. This is still the pattern matching issue. The pattern is "cpu*". Both "cpu_core" and "cpu_atom" can match the pattern "cpu*", so the parser wrongly thinks they are the same PMU type. Now with this patch, # ./perf stat -e cpu/cpu-cycles/ true event syntax error: 'cpu/cpu-cycles/' \___ Cannot find PMU `cpu'. Missing kernel support? Run 'perf list' for a list of valid events Thanks Jin Yao On 6/9/2021 12:57 PM, Jin Yao wrote: > Some different pmu types may have same substring. For example, > on Icelake server, we have pmu types "uncore_imc" and > "uncore_imc_free_running". Both pmu types have substring "uncore_imc". > But the parser would wrongly think they are the same pmu type. > > We enable an imc event, > perf stat -e uncore_imc/event=0xe3/ -a -- sleep 1 > > Perf actually expands the event to: > uncore_imc_0/event=0xe3/ > uncore_imc_1/event=0xe3/ > uncore_imc_2/event=0xe3/ > uncore_imc_3/event=0xe3/ > uncore_imc_4/event=0xe3/ > uncore_imc_5/event=0xe3/ > uncore_imc_6/event=0xe3/ > uncore_imc_7/event=0xe3/ > uncore_imc_free_running_0/event=0xe3/ > uncore_imc_free_running_1/event=0xe3/ > uncore_imc_free_running_3/event=0xe3/ > uncore_imc_free_running_4/event=0xe3/ > > That's because the "uncore_imc_free_running" matches the > pattern "uncore_imc*". > > Now we check that the last characters of pmu name is > '_'. > > Fixes: b2b9d3a3f021 ("perf pmu: Support wildcards on pmu name in dynamic pmu events") > Signed-off-by: Jin Yao > --- > tools/perf/util/parse-events.y | 2 ++ > tools/perf/util/pmu.c | 25 ++++++++++++++++++++++++- > tools/perf/util/pmu.h | 1 + > 3 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index aba12a4d488e..7a694c7f7f1a 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -317,6 +317,8 @@ event_pmu_name opt_pmu_config > strncmp($1, "uncore_", 7)) > name += 7; > if (!fnmatch(pattern, name, 0)) { > + if (!perf_pmu__valid_suffix($1, name)) > + continue; > if (parse_events_copy_term_list(orig_terms, &terms)) > CLEANUP_YYABORT; > if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false)) > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 88c8ecdc60b0..78af01959830 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -3,6 +3,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -768,7 +769,7 @@ bool pmu_uncore_alias_match(const char *pmu_name, const char *name) > */ > for (; tok; name += strlen(tok), tok = strtok_r(NULL, ",", &tmp)) { > name = strstr(name, tok); > - if (!name) { > + if (!name || !perf_pmu__valid_suffix(tok, (char *)name)) { > res = false; > goto out; > } > @@ -1872,3 +1873,25 @@ bool perf_pmu__has_hybrid(void) > > return !list_empty(&perf_pmu__hybrid_pmus); > } > + > +bool perf_pmu__valid_suffix(char *tok, char *pmu_name) > +{ > + char *p; > + > + /* > + * The pmu_name has substring tok. If the format of > + * pmu_name is or _, return true. > + */ > + p = pmu_name + strlen(tok); > + if (*p == 0) > + return true; > + > + if (*p != '_') > + return false; > + > + ++p; > + if (*p == 0 || !isdigit(*p)) > + return false; > + > + return true; > +} > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index a790ef758171..ebfd2b71532b 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -133,5 +133,6 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config, > char *name); > > bool perf_pmu__has_hybrid(void); > +bool perf_pmu__valid_suffix(char *tok, char *pmu_name); > > #endif /* __PMU_H */ >