Messages in this thread Patch in this message | | | Subject | Re: [PATCH] perf pmu: Create x86 specific perf_pmu__valid_suffix | From | John Garry <> | Date | Tue, 20 Jul 2021 09:56:57 +0100 |
| |
On 20/07/2021 09:20, Jin Yao wrote: > The commit c47a5599eda3 ("perf tools: Fix pattern matching for same > substring in different PMU type") breaks arm64 system because it > assumes the first token must be followed by a '_', but it is > possibly a numeric on arm64.
I wouldn't just say arm64. The core code should support matching multiple tokens, but it just so happens that only arm64 uses it.
> > For example, perf_pmu__valid_suffix("hisi_sccl3_l3c7", "hisi_sccl") > fails. "hisi_sccl3_l3c7" is pmu name and "hisi_sccl" is token. > "hisi_sccl" is followed by a digit but not followed by a '_' > ('3' in this example). > > Since the PMU alias format on arm64 has difference than the format > on x86. Create a x86 specific perf_pmu__valid_suffix. For other arch, > the weak function always returns true to keep original behavior > unchanged. > > Fixes: c47a5599eda3 ("perf tools: Fix pattern matching for same substring in different PMU type") > Signed-off-by: Jin Yao<yao.jin@linux.intel.com> > Reported-by: John Garry<john.garry@huawei.com> > ---
I tend not to agree with this solution. Let's not make it x86 vs non-x86 problem. Indeed, if we continue to support custom PMU alias matching per-arch, then it becomes a maintenance burden.
So this change fixes my initial problem:
-----8<------
From bbc9c7bf65c9cea756f0716dd717e9cfc767ba39 Mon Sep 17 00:00:00 2001 From: John Garry <john.garry@huawei.com> Date: Tue, 20 Jul 2021 09:48:22 +0100 Subject: [PATCH] perf pmu: Only check valid suffix for final token
Since commit 730670b1d108 ("perf pmu: Support more complex PMU event aliasing"), matching multiple alias tokens has been supported.
However, commit c47a5599eda32 ("perf tools: Fix pattern matching for same substring in different PMU type"), ignored the possibility of multiple tokens, and assumed that we can only match on valid suffix and a single token.
Fix multiple token support by only checking valid suffix from final token.
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 44b90d638ad5..6de2275b2ae2 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -789,12 +789,17 @@ bool pmu_uncore_alias_match(const char *pmu_name, const char *name) * match "socket" in "socketX_pmunameY" and then "pmuname" in * "pmunameY". */ - for (; tok; name += strlen(tok), tok = strtok_r(NULL, ",", &tmp)) { + for (; ; ) { + char *next_tok = strtok_r(NULL, ",", &tmp); name = strstr(name, tok); - if (!name || !perf_pmu__valid_suffix((char *)name, tok)) { + if (!name || (!perf_pmu__valid_suffix((char *)name, tok) && !next_tok)) { res = false; goto out; } + if (!next_tok) + break; + name += strlen(tok); + tok = next_tok; }
res = true; -- 2.26.2 ----->8------ Please check it.
I need to check the other place perf_pmu__valid_suffix() is referenced (perf_pmu__match()) to see if anything is broken there.
Thanks, John
| |