* [PATCH v1 2/3] perf pmu: Improve name/comments, avoid a memory allocation
2023-04-06 23:52 [PATCH v1 1/3] perf pmu: Fewer const casts Ian Rogers
@ 2023-04-06 23:52 ` Ian Rogers
2023-04-06 23:52 ` [PATCH v1 3/3] perf pmu: Sort and remove duplicates using json PMU name Ian Rogers
1 sibling, 0 replies; 3+ messages in thread
From: Ian Rogers @ 2023-04-06 23:52 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, Suzuki Poulouse, James Clark,
Sean Christopherson, Ravi Bangoria, Rob Herring, linux-perf-users,
linux-kernel
Improve documentation around perf_pmu_alias pmu_name and on
functions. Reduce the scope of pmu_uncore_alias_match to just
file. Rename perf_pmu__valid_suffix to the more revealing
perf_pmu__match_ignoring_suffix. Add a short-cut to
perf_pmu__match_ignoring_suffix for PMU names that don't also have a
socket value, and can therefore avoid a memory allocation.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/pmu.c | 27 ++++++++++++++++++++-------
tools/perf/util/pmu.h | 8 +++++---
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 63a33fcfd42d..00714f560643 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -668,11 +668,14 @@ __weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
return perf_pmu__find_metrics_table(NULL);
}
-/*
- * Suffix must be in form tok_{digits}, or tok{digits}, or same as pmu_name
- * to be valid.
+/**
+ * perf_pmu__match_ignoring_suffix - Does the pmu_name match tok ignoring any
+ * trailing suffix? The Suffix must be in form
+ * tok_{digits}, or tok{digits}.
+ * @pmu_name: The pmu_name with possible suffix.
+ * @tok: The possible match to pmu_name without suffix.
*/
-static bool perf_pmu__valid_suffix(const char *pmu_name, char *tok)
+static bool perf_pmu__match_ignoring_suffix(const char *pmu_name, const char *tok)
{
const char *p;
@@ -697,11 +700,21 @@ static bool perf_pmu__valid_suffix(const char *pmu_name, char *tok)
return true;
}
-bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
+/**
+ * pmu_uncore_alias_match - does name match the PMU name?
+ * @pmu_name: the json struct pmu_event name. This may lack a suffix (which
+ * matches) or be of the form "socket,pmuname" which will match
+ * "socketX_pmunameY".
+ * @name: a real full PMU name as from sysfs.
+ */
+static bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
{
char *tmp = NULL, *tok, *str;
bool res;
+ if (strchr(pmu_name, ',') == NULL)
+ return perf_pmu__match_ignoring_suffix(name, pmu_name);
+
str = strdup(pmu_name);
if (!str)
return false;
@@ -728,7 +741,7 @@ bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
name = strstr(name, tok);
if (!name ||
- (!next_tok && !perf_pmu__valid_suffix(name, tok))) {
+ (!next_tok && !perf_pmu__match_ignoring_suffix(name, tok))) {
res = false;
goto out;
}
@@ -1939,7 +1952,7 @@ int perf_pmu__match(char *pattern, char *name, char *tok)
if (fnmatch(pattern, name, 0))
return -1;
- if (tok && !perf_pmu__valid_suffix(name, tok))
+ if (tok && !perf_pmu__match_ignoring_suffix(name, tok))
return -1;
return 0;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 32c3a75bca0e..6be75594d11e 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -35,7 +35,7 @@ struct perf_pmu_caps {
};
/**
- * struct perf_pmu - hi
+ * struct perf_pmu
*/
struct perf_pmu {
/** @name: The name of the PMU such as "cpu". */
@@ -186,7 +186,10 @@ struct perf_pmu_alias {
* default.
*/
bool deprecated;
- /** @pmu_name: The name copied from struct perf_pmu. */
+ /**
+ * @pmu_name: The name copied from the json struct pmu_event. This can
+ * differ from the PMU name as it won't have suffixes.
+ */
char *pmu_name;
};
@@ -238,7 +241,6 @@ 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_alias_match(const char *pmu_name, const char *name);
void perf_pmu_free_alias(struct perf_pmu_alias *alias);
int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
--
2.40.0.577.gac1e443424-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v1 3/3] perf pmu: Sort and remove duplicates using json PMU name
2023-04-06 23:52 [PATCH v1 1/3] perf pmu: Fewer const casts Ian Rogers
2023-04-06 23:52 ` [PATCH v1 2/3] perf pmu: Improve name/comments, avoid a memory allocation Ian Rogers
@ 2023-04-06 23:52 ` Ian Rogers
1 sibling, 0 replies; 3+ messages in thread
From: Ian Rogers @ 2023-04-06 23:52 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, Suzuki Poulouse, James Clark,
Sean Christopherson, Ravi Bangoria, Rob Herring, linux-perf-users,
linux-kernel
We may have a lot of copies of a particular uncore PMU, such as
uncore_cha_0 to uncore_cha_59 on Intel sapphirerapids. The json events
may match each of PMUs and so the events are copied to it. In perf
list this means we see the same json event 60 times as events on
different PMUs don't have duplicates removed. There are 284 uncore_cha
events on sapphirerapids. Rather than use the PMU's name to sort and
remove duplicates, use the json PMU name. This reduces the 60 copies
back down to 1 and has the side effect of speeding things like the
"perf all PMU test" shell test.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/pmu.c | 47 ++++++++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 00714f560643..03da1cdfa869 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1556,7 +1556,7 @@ static int cmp_sevent(const void *a, const void *b)
{
const struct sevent *as = a;
const struct sevent *bs = b;
- const char *a_pmu_name, *b_pmu_name;
+ const char *a_pmu_name = NULL, *b_pmu_name = NULL;
const char *a_name = "//", *a_desc = NULL, *a_topic = "";
const char *b_name = "//", *b_desc = NULL, *b_topic = "";
int ret;
@@ -1565,11 +1565,13 @@ static int cmp_sevent(const void *a, const void *b)
a_name = as->event->name;
a_desc = as->event->desc;
a_topic = as->event->topic ?: "";
+ a_pmu_name = as->event->pmu_name;
}
if (bs->event) {
b_name = bs->event->name;
b_desc = bs->event->desc;
b_topic = bs->event->topic ?: "";
+ b_pmu_name = bs->event->pmu_name;
}
/* Put extra events last. */
if (!!a_desc != !!b_desc)
@@ -1585,11 +1587,13 @@ static int cmp_sevent(const void *a, const void *b)
return as->is_cpu ? -1 : 1;
/* Order by PMU name. */
- a_pmu_name = as->pmu->name ?: "";
- b_pmu_name = bs->pmu->name ?: "";
- ret = strcmp(a_pmu_name, b_pmu_name);
- if (ret)
- return ret;
+ if (as->pmu != bs->pmu) {
+ a_pmu_name = a_pmu_name ?: (as->pmu->name ?: "");
+ b_pmu_name = b_pmu_name ?: (bs->pmu->name ?: "");
+ ret = strcmp(a_pmu_name, b_pmu_name);
+ if (ret)
+ return ret;
+ }
/* Order by event name. */
return strcmp(a_name, b_name);
@@ -1603,17 +1607,26 @@ bool is_pmu_core(const char *name)
static bool pmu_alias_is_duplicate(struct sevent *alias_a,
struct sevent *alias_b)
{
- const char *a_pmu_name, *b_pmu_name;
- const char *a_name = alias_a->event ? alias_a->event->name : "//";
- const char *b_name = alias_b->event ? alias_b->event->name : "//";
+ const char *a_pmu_name = NULL, *b_pmu_name = NULL;
+ const char *a_name = "//", *b_name = "//";
+
+
+ if (alias_a->event) {
+ a_name = alias_a->event->name;
+ a_pmu_name = alias_a->event->pmu_name;
+ }
+ if (alias_b->event) {
+ b_name = alias_b->event->name;
+ b_pmu_name = alias_b->event->pmu_name;
+ }
/* Different names -> never duplicates */
if (strcmp(a_name, b_name))
return false;
/* Don't remove duplicates for different PMUs */
- a_pmu_name = alias_a->pmu->name ?: "";
- b_pmu_name = alias_b->pmu->name ?: "";
+ a_pmu_name = a_pmu_name ?: (alias_a->pmu->name ?: "");
+ b_pmu_name = b_pmu_name ?: (alias_b->pmu->name ?: "");
return strcmp(a_pmu_name, b_pmu_name) == 0;
}
@@ -1662,7 +1675,8 @@ void print_pmu_events(const struct print_callbacks *print_cb, void *print_state)
for (j = 0; j < len; j++) {
const char *name, *alias = NULL, *scale_unit = NULL,
*desc = NULL, *long_desc = NULL,
- *encoding_desc = NULL, *topic = NULL;
+ *encoding_desc = NULL, *topic = NULL,
+ *pmu_name = NULL;
bool deprecated = false;
size_t buf_used;
@@ -1672,7 +1686,8 @@ void print_pmu_events(const struct print_callbacks *print_cb, void *print_state)
if (!aliases[j].event) {
/* A selectable event. */
- buf_used = snprintf(buf, sizeof(buf), "%s//", aliases[j].pmu->name) + 1;
+ pmu_name = aliases[j].pmu->name;
+ buf_used = snprintf(buf, sizeof(buf), "%s//", pmu_name) + 1;
name = buf;
} else {
if (aliases[j].event->desc) {
@@ -1687,6 +1702,7 @@ void print_pmu_events(const struct print_callbacks *print_cb, void *print_state)
}
buf_used = strlen(buf) + 1;
}
+ pmu_name = aliases[j].event->pmu_name ?: (aliases[j].pmu->name ?: "");
if (strlen(aliases[j].event->unit) || aliases[j].event->scale != 1.0) {
scale_unit = buf + buf_used;
buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
@@ -1698,12 +1714,11 @@ void print_pmu_events(const struct print_callbacks *print_cb, void *print_state)
topic = aliases[j].event->topic;
encoding_desc = buf + buf_used;
buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
- "%s/%s/", aliases[j].pmu->name,
- aliases[j].event->str) + 1;
+ "%s/%s/", pmu_name, aliases[j].event->str) + 1;
deprecated = aliases[j].event->deprecated;
}
print_cb->print_event(print_state,
- aliases[j].pmu->name,
+ pmu_name,
topic,
name,
alias,
--
2.40.0.577.gac1e443424-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread