* [PATCH] perf pmu: Create x86 specific perf_pmu__valid_suffix
@ 2021-07-20 8:20 Jin Yao
2021-07-20 8:56 ` John Garry
0 siblings, 1 reply; 2+ messages in thread
From: Jin Yao @ 2021-07-20 8:20 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, linux-perf-users, ak, kan.liang, yao.jin,
john.garry, Jin Yao
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.
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>
---
tools/perf/arch/x86/util/pmu.c | 22 ++++++++++++++++++++++
tools/perf/util/pmu.c | 27 ++++++---------------------
tools/perf/util/pmu.h | 1 +
3 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index d48d608517fd..519da0811308 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -3,6 +3,7 @@
#include <linux/stddef.h>
#include <linux/perf_event.h>
+#include <linux/ctype.h>
#include "../../../util/intel-pt.h"
#include "../../../util/intel-bts.h"
@@ -18,3 +19,24 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
#endif
return NULL;
}
+
+bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
+{
+ char *p;
+
+ if (strncmp(pmu_name, tok, strlen(tok)))
+ return false;
+
+ 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.c b/tools/perf/util/pmu.c
index 44b90d638ad5..a671b16f2d3e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -742,27 +742,6 @@ struct pmu_events_map *__weak pmu_events_map__find(void)
return perf_pmu__find_map(NULL);
}
-static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
-{
- char *p;
-
- if (strncmp(pmu_name, tok, strlen(tok)))
- return false;
-
- p = pmu_name + strlen(tok);
- if (*p == 0)
- return true;
-
- if (*p != '_')
- return false;
-
- ++p;
- if (*p == 0 || !isdigit(*p))
- return false;
-
- return true;
-}
-
bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
{
char *tmp = NULL, *tok, *str;
@@ -1906,3 +1885,9 @@ int perf_pmu__match(char *pattern, char *name, char *tok)
return 0;
}
+
+bool __weak perf_pmu__valid_suffix(char *pmu_name __maybe_unused,
+ char *tok __maybe_unused)
+{
+ return true;
+}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 926da483a141..901812987b79 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -134,5 +134,6 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
bool perf_pmu__has_hybrid(void);
int perf_pmu__match(char *pattern, char *name, char *tok);
+bool perf_pmu__valid_suffix(char *pmu_name, char *tok);
#endif /* __PMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] perf pmu: Create x86 specific perf_pmu__valid_suffix
2021-07-20 8:20 [PATCH] perf pmu: Create x86 specific perf_pmu__valid_suffix Jin Yao
@ 2021-07-20 8:56 ` John Garry
0 siblings, 0 replies; 2+ messages in thread
From: John Garry @ 2021-07-20 8:56 UTC (permalink / raw)
To: Jin Yao, acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, linux-perf-users, ak, kan.liang, yao.jin
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-20 8:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 8:20 [PATCH] perf pmu: Create x86 specific perf_pmu__valid_suffix Jin Yao
2021-07-20 8:56 ` John Garry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).