* [PATCH v2 0/5] perf hwmon related improvements
@ 2025-01-23 7:46 Ian Rogers
2025-01-23 7:46 ` [PATCH v2 1/5] perf evsel: Reduce scanning core PMUs in is_hybrid Ian Rogers
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
Try to do less scanning of files/directories with or without hwmon
PMUs. Don't merge all events with the same name, only merge those on
the same PMU ignoring suffix. Tidy PMU name matching to distinguish no
suffix or wildcard matching. Refactor uniquification so the evsels
with the same name as other evsels in the evlist are uniquified.
v2: Rename ignore suffix PMU name matching that is really a
wildcard/prefix match. Use a proper ignore suffix when not merging
counters purely on name.
Ian Rogers (5):
perf evsel: Reduce scanning core PMUs in is_hybrid
perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs
perf pmu: Rename name matching for no suffix or wildcard variants
perf stat: Don't merge counters purely on name
perf stat: Changes to event name uniquification
tools/perf/pmu-events/empty-pmu-events.c | 8 +-
tools/perf/pmu-events/jevents.py | 8 +-
tools/perf/tests/pmu.c | 85 ++++----
tools/perf/util/evsel.c | 4 +-
tools/perf/util/evsel.h | 1 +
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/pmu.c | 252 ++++++++++++++++-------
tools/perf/util/pmu.h | 7 +-
tools/perf/util/pmus.c | 144 ++++++++-----
tools/perf/util/stat-display.c | 111 +++++++---
tools/perf/util/stat.c | 13 +-
11 files changed, 418 insertions(+), 217 deletions(-)
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/5] perf evsel: Reduce scanning core PMUs in is_hybrid
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
@ 2025-01-23 7:46 ` Ian Rogers
2025-01-23 7:46 ` [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs Ian Rogers
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
evsel__is_hybrid returns true if there are multiple core PMUs and the
evsel is for a core PMU. Determining the number of core PMUs can
require loading/scanning PMUs. There's no point doing the scanning if
evsel for the is_hybrid test isn't core so reorder the tests to reduce
PMU scanning.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/evsel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bc144388f892..ae20691bbfae 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3856,10 +3856,10 @@ void evsel__zero_per_pkg(struct evsel *evsel)
*/
bool evsel__is_hybrid(const struct evsel *evsel)
{
- if (perf_pmus__num_core_pmus() == 1)
+ if (!evsel->core.is_pmu_core)
return false;
- return evsel->core.is_pmu_core;
+ return perf_pmus__num_core_pmus() > 1;
}
struct evsel *evsel__leader(const struct evsel *evsel)
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
2025-01-23 7:46 ` [PATCH v2 1/5] perf evsel: Reduce scanning core PMUs in is_hybrid Ian Rogers
@ 2025-01-23 7:46 ` Ian Rogers
2025-01-30 19:24 ` Liang, Kan
2025-01-23 7:46 ` [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants Ian Rogers
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
Rather than scanning core or all PMUs, allow pmu_read_sysfs to read
some combination of core, other, hwmon and tool PMUs. The PMUs that
should be read and are already read are held as bitmaps. It is known
that a "hwmon_" prefix is necessary for a hwmon PMU's name, similarly
with "tool", so only scan those PMUs in situations the PMU name or the
PMU's type number make sense to.
The number of openat system calls reduces from 276 to 98 for a hwmon
event. The number of openats for regular perf events isn't changed.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/pmu.h | 2 +
tools/perf/util/pmus.c | 144 ++++++++++++++++++++++++++---------------
2 files changed, 95 insertions(+), 51 deletions(-)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index dbed6c243a5e..edd36c20aedc 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -37,6 +37,8 @@ struct perf_pmu_caps {
};
enum {
+ PERF_PMU_TYPE_PE_START = 0,
+ PERF_PMU_TYPE_PE_END = 0xFFFEFFFF,
PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
PERF_PMU_TYPE_HWMON_END = 0xFFFFFFFD,
PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index b493da0d22ef..3e3ffafcad71 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -37,10 +37,23 @@
*/
static LIST_HEAD(core_pmus);
static LIST_HEAD(other_pmus);
-static bool read_sysfs_core_pmus;
-static bool read_sysfs_all_pmus;
+enum perf_tool_pmu_type {
+ PERF_TOOL_PMU_TYPE_PE_CORE,
+#define PERF_TOOL_PMU_TYPE_PE_CORE_MASK (1 << PERF_TOOL_PMU_TYPE_PE_CORE)
+ PERF_TOOL_PMU_TYPE_PE_OTHER,
+#define PERF_TOOL_PMU_TYPE_PE_OTHER_MASK (1 << PERF_TOOL_PMU_TYPE_PE_OTHER)
+ PERF_TOOL_PMU_TYPE_TOOL,
+#define PERF_TOOL_PMU_TYPE_TOOL_MASK (1 << PERF_TOOL_PMU_TYPE_TOOL)
+ PERF_TOOL_PMU_TYPE_HWMON,
+#define PERF_TOOL_PMU_TYPE_HWMON_MASK (1 << PERF_TOOL_PMU_TYPE_HWMON)
+#define PERF_TOOL_PMU_TYPE_ALL_MASK (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | \
+ PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | \
+ PERF_TOOL_PMU_TYPE_TOOL_MASK | \
+ PERF_TOOL_PMU_TYPE_HWMON_MASK)
+};
+static unsigned int read_pmu_types;
-static void pmu_read_sysfs(bool core_only);
+static void pmu_read_sysfs(unsigned int to_read_pmus);
size_t pmu_name_len_no_suffix(const char *str)
{
@@ -102,8 +115,7 @@ void perf_pmus__destroy(void)
perf_pmu__delete(pmu);
}
- read_sysfs_core_pmus = false;
- read_sysfs_all_pmus = false;
+ read_pmu_types = 0;
}
static struct perf_pmu *pmu_find(const char *name)
@@ -129,6 +141,7 @@ struct perf_pmu *perf_pmus__find(const char *name)
struct perf_pmu *pmu;
int dirfd;
bool core_pmu;
+ unsigned int to_read_pmus = 0;
/*
* Once PMU is loaded it stays in the list,
@@ -139,11 +152,11 @@ struct perf_pmu *perf_pmus__find(const char *name)
if (pmu)
return pmu;
- if (read_sysfs_all_pmus)
+ if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
return NULL;
core_pmu = is_pmu_core(name);
- if (core_pmu && read_sysfs_core_pmus)
+ if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
return NULL;
dirfd = perf_pmu__event_source_devices_fd();
@@ -151,15 +164,27 @@ struct perf_pmu *perf_pmus__find(const char *name)
/*eager_load=*/false);
close(dirfd);
- if (!pmu) {
- /*
- * Looking up an inidividual PMU failed. This may mean name is
- * an alias, so read the PMUs from sysfs and try to find again.
- */
- pmu_read_sysfs(core_pmu);
+ if (pmu)
+ return pmu;
+
+ /* Looking up an individual perf event PMU failed, check if a tool PMU should be read. */
+ if (!strncmp(name, "hwmon_", 6))
+ to_read_pmus |= PERF_TOOL_PMU_TYPE_HWMON_MASK;
+ else if (!strcmp(name, "tool"))
+ to_read_pmus |= PERF_TOOL_PMU_TYPE_TOOL_MASK;
+
+ if (to_read_pmus) {
+ pmu_read_sysfs(to_read_pmus);
pmu = pmu_find(name);
+ if (pmu)
+ return pmu;
}
- return pmu;
+ /* Read all necessary PMUs from sysfs and see if the PMU is found. */
+ to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK;
+ if (!core_pmu)
+ to_read_pmus |= PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
+ pmu_read_sysfs(to_read_pmus);
+ return pmu_find(name);
}
static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
@@ -176,11 +201,11 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
if (pmu)
return pmu;
- if (read_sysfs_all_pmus)
+ if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
return NULL;
core_pmu = is_pmu_core(name);
- if (core_pmu && read_sysfs_core_pmus)
+ if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
return NULL;
return perf_pmu__lookup(core_pmu ? &core_pmus : &other_pmus, dirfd, name,
@@ -197,52 +222,60 @@ static int pmus_cmp(void *priv __maybe_unused,
}
/* Add all pmus in sysfs to pmu list: */
-static void pmu_read_sysfs(bool core_only)
+static void pmu_read_sysfs(unsigned int to_read_types)
{
- int fd;
- DIR *dir;
- struct dirent *dent;
struct perf_pmu *tool_pmu;
- if (read_sysfs_all_pmus || (core_only && read_sysfs_core_pmus))
+ if ((read_pmu_types & to_read_types) == to_read_types) {
+ /* All requested PMU types have been read. */
return;
+ }
- fd = perf_pmu__event_source_devices_fd();
- if (fd < 0)
- return;
+ if (to_read_types & (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | PERF_TOOL_PMU_TYPE_PE_OTHER_MASK)) {
+ int fd = perf_pmu__event_source_devices_fd();
+ DIR *dir;
+ struct dirent *dent;
+ bool core_only = (to_read_types & PERF_TOOL_PMU_TYPE_PE_OTHER_MASK) == 0;
- dir = fdopendir(fd);
- if (!dir) {
- close(fd);
- return;
- }
+ if (fd < 0)
+ goto skip_pe_pmus;
- while ((dent = readdir(dir))) {
- if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
- continue;
- if (core_only && !is_pmu_core(dent->d_name))
- continue;
- /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
- perf_pmu__find2(fd, dent->d_name);
- }
+ dir = fdopendir(fd);
+ if (!dir) {
+ close(fd);
+ goto skip_pe_pmus;
+ }
- closedir(dir);
- if (list_empty(&core_pmus)) {
+ while ((dent = readdir(dir))) {
+ if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
+ continue;
+ if (core_only && !is_pmu_core(dent->d_name))
+ continue;
+ /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
+ perf_pmu__find2(fd, dent->d_name);
+ }
+
+ closedir(dir);
+ }
+skip_pe_pmus:
+ if ((to_read_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK) && list_empty(&core_pmus)) {
if (!perf_pmu__create_placeholder_core_pmu(&core_pmus))
pr_err("Failure to set up any core PMUs\n");
}
list_sort(NULL, &core_pmus, pmus_cmp);
- if (!core_only) {
+
+ if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) != 0 &&
+ (read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) == 0) {
tool_pmu = perf_pmus__tool_pmu();
list_add_tail(&tool_pmu->list, &other_pmus);
- perf_pmus__read_hwmon_pmus(&other_pmus);
}
+ if ((to_read_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) != 0 &&
+ (read_pmu_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) == 0)
+ perf_pmus__read_hwmon_pmus(&other_pmus);
+
list_sort(NULL, &other_pmus, pmus_cmp);
- if (!list_empty(&core_pmus)) {
- read_sysfs_core_pmus = true;
- if (!core_only)
- read_sysfs_all_pmus = true;
- }
+
+ read_pmu_types |= to_read_types;
}
static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
@@ -263,12 +296,21 @@ static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
struct perf_pmu *perf_pmus__find_by_type(unsigned int type)
{
+ unsigned int to_read_pmus;
struct perf_pmu *pmu = __perf_pmus__find_by_type(type);
- if (pmu || read_sysfs_all_pmus)
+ if (pmu || (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK))
return pmu;
- pmu_read_sysfs(/*core_only=*/false);
+ if (type >= PERF_PMU_TYPE_PE_START && type <= PERF_PMU_TYPE_PE_END) {
+ to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK |
+ PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
+ } else if (type >= PERF_PMU_TYPE_HWMON_START && type <= PERF_PMU_TYPE_HWMON_END) {
+ to_read_pmus = PERF_TOOL_PMU_TYPE_HWMON_MASK;
+ } else {
+ to_read_pmus = PERF_TOOL_PMU_TYPE_TOOL_MASK;
+ }
+ pmu_read_sysfs(to_read_pmus);
pmu = __perf_pmus__find_by_type(type);
return pmu;
}
@@ -282,7 +324,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
bool use_core_pmus = !pmu || pmu->is_core;
if (!pmu) {
- pmu_read_sysfs(/*core_only=*/false);
+ pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
pmu = list_prepare_entry(pmu, &core_pmus, list);
}
if (use_core_pmus) {
@@ -300,7 +342,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
{
if (!pmu) {
- pmu_read_sysfs(/*core_only=*/true);
+ pmu_read_sysfs(PERF_TOOL_PMU_TYPE_PE_CORE_MASK);
return list_first_entry_or_null(&core_pmus, typeof(*pmu), list);
}
list_for_each_entry_continue(pmu, &core_pmus, list)
@@ -316,7 +358,7 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
if (!pmu) {
- pmu_read_sysfs(/*core_only=*/false);
+ pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
pmu = list_prepare_entry(pmu, &core_pmus, list);
} else
last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
2025-01-23 7:46 ` [PATCH v2 1/5] perf evsel: Reduce scanning core PMUs in is_hybrid Ian Rogers
2025-01-23 7:46 ` [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs Ian Rogers
@ 2025-01-23 7:46 ` Ian Rogers
2025-01-30 19:29 ` Liang, Kan
2025-01-23 7:46 ` [PATCH v2 4/5] perf stat: Don't merge counters purely on name Ian Rogers
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
Wildcard PMU naming will match a name like pmu_1 to a PMU name like
pmu_10 but not to a PMU name like pmu_2 as the suffix forms part of
the match. No suffix matching will match pmu_10 to either pmu_1 or
pmu_2. Add or rename matching functions on PMU to make it clearer what
kind of matching is being performed.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/pmu-events/empty-pmu-events.c | 8 +-
tools/perf/pmu-events/jevents.py | 8 +-
tools/perf/tests/pmu.c | 85 ++++----
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/pmu.c | 252 ++++++++++++++++-------
tools/perf/util/pmu.h | 5 +-
6 files changed, 231 insertions(+), 129 deletions(-)
diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 1c7a2cfa321f..0cb7ba7912e8 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -422,7 +422,7 @@ int pmu_events_table__for_each_event(const struct pmu_events_table *table,
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
int ret;
- if (pmu && !pmu__name_match(pmu, pmu_name))
+ if (pmu && !perf_pmu__name_wildcard_match(pmu, pmu_name))
continue;
ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
@@ -443,7 +443,7 @@ int pmu_events_table__find_event(const struct pmu_events_table *table,
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
int ret;
- if (!pmu__name_match(pmu, pmu_name))
+ if (!perf_pmu__name_wildcard_match(pmu, pmu_name))
continue;
ret = pmu_events_table__find_event_pmu(table, table_pmu, name, fn, data);
@@ -462,7 +462,7 @@ size_t pmu_events_table__num_events(const struct pmu_events_table *table,
const struct pmu_table_entry *table_pmu = &table->pmus[i];
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
- if (pmu__name_match(pmu, pmu_name))
+ if (perf_pmu__name_wildcard_match(pmu, pmu_name))
count += table_pmu->num_entries;
}
return count;
@@ -581,7 +581,7 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
const struct pmu_table_entry *table_pmu = &map->event_table.pmus[i];
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
- if (pmu__name_match(pmu, pmu_name))
+ if (perf_pmu__name_wildcard_match(pmu, pmu_name))
return &map->event_table;
}
return NULL;
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 3e204700b59a..7499a35bfadd 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -945,7 +945,7 @@ int pmu_events_table__for_each_event(const struct pmu_events_table *table,
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
int ret;
- if (pmu && !pmu__name_match(pmu, pmu_name))
+ if (pmu && !perf_pmu__name_wildcard_match(pmu, pmu_name))
continue;
ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
@@ -966,7 +966,7 @@ int pmu_events_table__find_event(const struct pmu_events_table *table,
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
int ret;
- if (!pmu__name_match(pmu, pmu_name))
+ if (!perf_pmu__name_wildcard_match(pmu, pmu_name))
continue;
ret = pmu_events_table__find_event_pmu(table, table_pmu, name, fn, data);
@@ -985,7 +985,7 @@ size_t pmu_events_table__num_events(const struct pmu_events_table *table,
const struct pmu_table_entry *table_pmu = &table->pmus[i];
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
- if (pmu__name_match(pmu, pmu_name))
+ if (perf_pmu__name_wildcard_match(pmu, pmu_name))
count += table_pmu->num_entries;
}
return count;
@@ -1104,7 +1104,7 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
const struct pmu_table_entry *table_pmu = &map->event_table.pmus[i];
const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
- if (pmu__name_match(pmu, pmu_name))
+ if (perf_pmu__name_wildcard_match(pmu, pmu_name))
return &map->event_table;
}
return NULL;
diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 6a681e3fb552..4a9f8e090cf4 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -452,9 +452,9 @@ static int test__name_cmp(struct test_suite *test __maybe_unused, int subtest __
}
/**
- * Test perf_pmu__match() that's used to search for a PMU given a name passed
+ * Test perf_pmu__wildcard_match() that's used to search for a PMU given a name passed
* on the command line. The name that's passed may also be a filename type glob
- * match. If the name does not match, perf_pmu__match() attempts to match the
+ * match. If the name does not match, perf_pmu__wildcard_match() attempts to match the
* alias of the PMU, if provided.
*/
static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
@@ -463,41 +463,44 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
.name = "pmuname",
};
- TEST_ASSERT_EQUAL("Exact match", perf_pmu__match(&test_pmu, "pmuname"), true);
- TEST_ASSERT_EQUAL("Longer token", perf_pmu__match(&test_pmu, "longertoken"), false);
- TEST_ASSERT_EQUAL("Shorter token", perf_pmu__match(&test_pmu, "pmu"), false);
+#define TEST_PMU_MATCH(msg, to_match, expect) \
+ TEST_ASSERT_EQUAL(msg, perf_pmu__wildcard_match(&test_pmu, to_match), expect)
+
+ TEST_PMU_MATCH("Exact match", "pmuname", true);
+ TEST_PMU_MATCH("Longer token", "longertoken", false);
+ TEST_PMU_MATCH("Shorter token", "pmu", false);
test_pmu.name = "pmuname_10";
- TEST_ASSERT_EQUAL("Diff suffix_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
- TEST_ASSERT_EQUAL("Sub suffix_", perf_pmu__match(&test_pmu, "pmuname_1"), true);
- TEST_ASSERT_EQUAL("Same suffix_", perf_pmu__match(&test_pmu, "pmuname_10"), true);
- TEST_ASSERT_EQUAL("No suffix_", perf_pmu__match(&test_pmu, "pmuname"), true);
- TEST_ASSERT_EQUAL("Underscore_", perf_pmu__match(&test_pmu, "pmuname_"), true);
- TEST_ASSERT_EQUAL("Substring_", perf_pmu__match(&test_pmu, "pmuna"), false);
+ TEST_PMU_MATCH("Diff suffix_", "pmuname_2", false);
+ TEST_PMU_MATCH("Sub suffix_", "pmuname_1", true);
+ TEST_PMU_MATCH("Same suffix_", "pmuname_10", true);
+ TEST_PMU_MATCH("No suffix_", "pmuname", true);
+ TEST_PMU_MATCH("Underscore_", "pmuname_", true);
+ TEST_PMU_MATCH("Substring_", "pmuna", false);
test_pmu.name = "pmuname_ab23";
- TEST_ASSERT_EQUAL("Diff suffix hex_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
- TEST_ASSERT_EQUAL("Sub suffix hex_", perf_pmu__match(&test_pmu, "pmuname_ab"), true);
- TEST_ASSERT_EQUAL("Same suffix hex_", perf_pmu__match(&test_pmu, "pmuname_ab23"), true);
- TEST_ASSERT_EQUAL("No suffix hex_", perf_pmu__match(&test_pmu, "pmuname"), true);
- TEST_ASSERT_EQUAL("Underscore hex_", perf_pmu__match(&test_pmu, "pmuname_"), true);
- TEST_ASSERT_EQUAL("Substring hex_", perf_pmu__match(&test_pmu, "pmuna"), false);
+ TEST_PMU_MATCH("Diff suffix hex_", "pmuname_2", false);
+ TEST_PMU_MATCH("Sub suffix hex_", "pmuname_ab", true);
+ TEST_PMU_MATCH("Same suffix hex_", "pmuname_ab23", true);
+ TEST_PMU_MATCH("No suffix hex_", "pmuname", true);
+ TEST_PMU_MATCH("Underscore hex_", "pmuname_", true);
+ TEST_PMU_MATCH("Substring hex_", "pmuna", false);
test_pmu.name = "pmuname10";
- TEST_ASSERT_EQUAL("Diff suffix", perf_pmu__match(&test_pmu, "pmuname2"), false);
- TEST_ASSERT_EQUAL("Sub suffix", perf_pmu__match(&test_pmu, "pmuname1"), true);
- TEST_ASSERT_EQUAL("Same suffix", perf_pmu__match(&test_pmu, "pmuname10"), true);
- TEST_ASSERT_EQUAL("No suffix", perf_pmu__match(&test_pmu, "pmuname"), true);
- TEST_ASSERT_EQUAL("Underscore", perf_pmu__match(&test_pmu, "pmuname_"), false);
- TEST_ASSERT_EQUAL("Substring", perf_pmu__match(&test_pmu, "pmuna"), false);
+ TEST_PMU_MATCH("Diff suffix", "pmuname2", false);
+ TEST_PMU_MATCH("Sub suffix", "pmuname1", true);
+ TEST_PMU_MATCH("Same suffix", "pmuname10", true);
+ TEST_PMU_MATCH("No suffix", "pmuname", true);
+ TEST_PMU_MATCH("Underscore", "pmuname_", false);
+ TEST_PMU_MATCH("Substring", "pmuna", false);
test_pmu.name = "pmunameab23";
- TEST_ASSERT_EQUAL("Diff suffix hex", perf_pmu__match(&test_pmu, "pmuname2"), false);
- TEST_ASSERT_EQUAL("Sub suffix hex", perf_pmu__match(&test_pmu, "pmunameab"), true);
- TEST_ASSERT_EQUAL("Same suffix hex", perf_pmu__match(&test_pmu, "pmunameab23"), true);
- TEST_ASSERT_EQUAL("No suffix hex", perf_pmu__match(&test_pmu, "pmuname"), true);
- TEST_ASSERT_EQUAL("Underscore hex", perf_pmu__match(&test_pmu, "pmuname_"), false);
- TEST_ASSERT_EQUAL("Substring hex", perf_pmu__match(&test_pmu, "pmuna"), false);
+ TEST_PMU_MATCH("Diff suffix hex", "pmuname2", false);
+ TEST_PMU_MATCH("Sub suffix hex", "pmunameab", true);
+ TEST_PMU_MATCH("Same suffix hex", "pmunameab23", true);
+ TEST_PMU_MATCH("No suffix hex", "pmuname", true);
+ TEST_PMU_MATCH("Underscore hex", "pmuname_", false);
+ TEST_PMU_MATCH("Substring hex", "pmuna", false);
/*
* 2 hex chars or less are not considered suffixes so it shouldn't be
@@ -505,7 +508,7 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
* false results here than above.
*/
test_pmu.name = "pmuname_a3";
- TEST_ASSERT_EQUAL("Diff suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
+ TEST_PMU_MATCH("Diff suffix 2 hex_", "pmuname_2", false);
/*
* This one should be false, but because pmuname_a3 ends in 3 which is
* decimal, it's not possible to determine if it's a short hex suffix or
@@ -513,19 +516,19 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
* length of decimal suffix. Run the test anyway and expect the wrong
* result. And slightly fuzzy matching shouldn't do too much harm.
*/
- TEST_ASSERT_EQUAL("Sub suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_a"), true);
- TEST_ASSERT_EQUAL("Same suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_a3"), true);
- TEST_ASSERT_EQUAL("No suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname"), false);
- TEST_ASSERT_EQUAL("Underscore 2 hex_", perf_pmu__match(&test_pmu, "pmuname_"), false);
- TEST_ASSERT_EQUAL("Substring 2 hex_", perf_pmu__match(&test_pmu, "pmuna"), false);
+ TEST_PMU_MATCH("Sub suffix 2 hex_", "pmuname_a", true);
+ TEST_PMU_MATCH("Same suffix 2 hex_", "pmuname_a3", true);
+ TEST_PMU_MATCH("No suffix 2 hex_", "pmuname", false);
+ TEST_PMU_MATCH("Underscore 2 hex_", "pmuname_", false);
+ TEST_PMU_MATCH("Substring 2 hex_", "pmuna", false);
test_pmu.name = "pmuname_5";
- TEST_ASSERT_EQUAL("Glob 1", perf_pmu__match(&test_pmu, "pmu*"), true);
- TEST_ASSERT_EQUAL("Glob 2", perf_pmu__match(&test_pmu, "nomatch*"), false);
- TEST_ASSERT_EQUAL("Seq 1", perf_pmu__match(&test_pmu, "pmuname_[12345]"), true);
- TEST_ASSERT_EQUAL("Seq 2", perf_pmu__match(&test_pmu, "pmuname_[67890]"), false);
- TEST_ASSERT_EQUAL("? 1", perf_pmu__match(&test_pmu, "pmuname_?"), true);
- TEST_ASSERT_EQUAL("? 2", perf_pmu__match(&test_pmu, "pmuname_1?"), false);
+ TEST_PMU_MATCH("Glob 1", "pmu*", true);
+ TEST_PMU_MATCH("Glob 2", "nomatch*", false);
+ TEST_PMU_MATCH("Seq 1", "pmuname_[12345]", true);
+ TEST_PMU_MATCH("Seq 2", "pmuname_[67890]", false);
+ TEST_PMU_MATCH("? 1", "pmuname_?", true);
+ TEST_PMU_MATCH("? 2", "pmuname_1?", false);
return TEST_OK;
}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e23faa364b1..6c36b98875bc 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1660,7 +1660,7 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
/* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
while ((pmu = perf_pmus__scan(pmu)) != NULL) {
if (!parse_events__filter_pmu(parse_state, pmu) &&
- perf_pmu__match(pmu, event_or_pmu)) {
+ perf_pmu__wildcard_match(pmu, event_or_pmu)) {
bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
if (!parse_events_add_pmu(parse_state, *listp, pmu,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6206c8fe2bf9..4c6f9a1a9461 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -847,21 +847,23 @@ static size_t pmu_deduped_name_len(const struct perf_pmu *pmu, const char *name,
}
/**
- * 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}.
+ * perf_pmu__match_wildcard - Does the pmu_name start with tok and is then only
+ * followed by nothing or a suffix? tok may contain
+ * part of a suffix.
* @pmu_name: The pmu_name with possible suffix.
- * @tok: The possible match to pmu_name without suffix.
+ * @tok: The wildcard argument to match.
*/
-static bool perf_pmu__match_ignoring_suffix(const char *pmu_name, const char *tok)
+static bool perf_pmu__match_wildcard(const char *pmu_name, const char *tok)
{
const char *p, *suffix;
bool has_hex = false;
+ size_t tok_len = strlen(tok);
- if (strncmp(pmu_name, tok, strlen(tok)))
+ /* Check start of pmu_name for equality. */
+ if (strncmp(pmu_name, tok, tok_len))
return false;
- suffix = p = pmu_name + strlen(tok);
+ suffix = p = pmu_name + tok_len;
if (*p == 0)
return true;
@@ -887,60 +889,80 @@ static bool perf_pmu__match_ignoring_suffix(const char *pmu_name, const char *to
}
/**
- * pmu_uncore_alias_match - does name match the PMU name?
- * @pmu_name: the json struct pmu_event name. This may lack a suffix (which
+ * perf_pmu__match_ignoring_suffix_uncore - Does the pmu_name match tok ignoring
+ * any trailing suffix on pmu_name and
+ * tok? 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.
+ */
+static bool perf_pmu__match_ignoring_suffix_uncore(const char *pmu_name, const char *tok)
+{
+ size_t pmu_name_len, tok_len;
+
+ /* uncore_ prefixes are ignored. */
+ if (!strncmp(pmu_name, "uncore_", 7))
+ pmu_name += 7;
+ if (!strncmp(tok, "uncore_", 7))
+ tok += 7;
+
+ pmu_name_len = pmu_name_len_no_suffix(pmu_name);
+ tok_len = pmu_name_len_no_suffix(tok);
+ if (pmu_name_len != tok_len)
+ return false;
+
+ return strncmp(pmu_name, tok, pmu_name_len) == 0;
+}
+
+
+/**
+ * perf_pmu__match_wildcard_uncore - does to_match match the PMU's name?
+ * @pmu_name: The pmu->name or pmu->alias to match against.
+ * @to_match: 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)
+static bool perf_pmu__match_wildcard_uncore(const char *pmu_name, const char *to_match)
{
- char *tmp = NULL, *tok, *str;
- bool res;
+ char *mutable_to_match, *tok, *tmp;
- if (strchr(pmu_name, ',') == NULL)
- return perf_pmu__match_ignoring_suffix(name, pmu_name);
-
- str = strdup(pmu_name);
- if (!str)
+ if (!pmu_name)
return false;
- /*
- * uncore alias may be from different PMU with common prefix
- */
- tok = strtok_r(str, ",", &tmp);
- if (strncmp(pmu_name, tok, strlen(tok))) {
- res = false;
- goto out;
- }
+ /* uncore_ prefixes are ignored. */
+ if (!strncmp(pmu_name, "uncore_", 7))
+ pmu_name += 7;
+ if (!strncmp(to_match, "uncore_", 7))
+ to_match += 7;
- /*
- * Match more complex aliases where the alias name is a comma-delimited
- * list of tokens, orderly contained in the matching PMU name.
- *
- * Example: For alias "socket,pmuname" and PMU "socketX_pmunameY", we
- * match "socket" in "socketX_pmunameY" and then "pmuname" in
- * "pmunameY".
- */
- while (1) {
- char *next_tok = strtok_r(NULL, ",", &tmp);
+ if (strchr(to_match, ',') == NULL)
+ return perf_pmu__match_wildcard(pmu_name, to_match);
- name = strstr(name, tok);
- if (!name ||
- (!next_tok && !perf_pmu__match_ignoring_suffix(name, tok))) {
- res = false;
- goto out;
+ /* Process comma separated list of PMU name components. */
+ mutable_to_match = strdup(to_match);
+ if (!mutable_to_match)
+ return false;
+
+ tok = strtok_r(mutable_to_match, ",", &tmp);
+ while (tok) {
+ size_t tok_len = strlen(tok);
+
+ if (strncmp(pmu_name, tok, tok_len)) {
+ /* Mismatch between part of pmu_name and tok. */
+ free(mutable_to_match);
+ return false;
}
- if (!next_tok)
- break;
- tok = next_tok;
- name += strlen(tok);
+ /* Move pmu_name forward over tok and suffix. */
+ pmu_name += tok_len;
+ while (*pmu_name != '\0' && isdigit(*pmu_name))
+ pmu_name++;
+ if (*pmu_name == '_')
+ pmu_name++;
+
+ tok = strtok_r(NULL, ",", &tmp);
}
-
- res = true;
-out:
- free(str);
- return res;
+ free(mutable_to_match);
+ return *pmu_name == '\0';
}
bool pmu_uncore_identifier_match(const char *compat, const char *id)
@@ -1003,11 +1025,19 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe,
{
struct perf_pmu *pmu = vdata;
- if (!pe->compat || !pe->pmu)
+ if (!pe->compat || !pe->pmu) {
+ /* No data to match. */
return 0;
+ }
+
+ if (!perf_pmu__match_wildcard_uncore(pmu->name, pe->pmu) &&
+ !perf_pmu__match_wildcard_uncore(pmu->alias_name, pe->pmu)) {
+ /* PMU name/alias_name don't match. */
+ return 0;
+ }
- if (pmu_uncore_alias_match(pe->pmu, pmu->name) &&
- pmu_uncore_identifier_match(pe->compat, pmu->id)) {
+ if (pmu_uncore_identifier_match(pe->compat, pmu->id)) {
+ /* Id matched. */
perf_pmu__new_alias(pmu,
pe->name,
pe->desc,
@@ -1016,7 +1046,6 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe,
pe,
EVENT_SRC_SYS_JSON);
}
-
return 0;
}
@@ -1974,15 +2003,82 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
return ret;
}
-bool pmu__name_match(const struct perf_pmu *pmu, const char *pmu_name)
+static bool perf_pmu___name_match(const struct perf_pmu *pmu, const char *to_match, bool wildcard)
{
- return !strcmp(pmu->name, pmu_name) ||
- (pmu->is_uncore && pmu_uncore_alias_match(pmu_name, pmu->name)) ||
+ const char *names[2] = {
+ pmu->name,
+ pmu->alias_name,
+ };
+ if (pmu->is_core) {
+ for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
+ const char *name = names[i];
+
+ if (!name)
+ continue;
+
+ if (!strcmp(name, to_match)) {
+ /* Exact name match. */
+ return true;
+ }
+ }
+ if (!strcmp(to_match, "default_core")) {
+ /*
+ * jevents and tests use default_core as a marker for any core
+ * PMU as the PMU name varies across architectures.
+ */
+ return true;
+ }
+ return false;
+ }
+ if (!pmu->is_uncore) {
/*
- * jevents and tests use default_core as a marker for any core
- * PMU as the PMU name varies across architectures.
+ * PMU isn't core or uncore, some kind of broken CPU mask
+ * situation. Only match exact name.
*/
- (pmu->is_core && !strcmp(pmu_name, "default_core"));
+ for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
+ const char *name = names[i];
+
+ if (!name)
+ continue;
+
+ if (!strcmp(name, to_match)) {
+ /* Exact name match. */
+ return true;
+ }
+ }
+ return false;
+ }
+ for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
+ const char *name = names[i];
+
+ if (wildcard && perf_pmu__match_wildcard_uncore(name, to_match))
+ return true;
+ if (!wildcard && perf_pmu__match_ignoring_suffix_uncore(name, to_match))
+ return true;
+ }
+ return false;
+}
+
+/**
+ * perf_pmu__name_wildcard_match - Called by the jevents generated code to see
+ * if pmu matches the json to_match string.
+ * @pmu: The pmu whose name/alias to match.
+ * @to_match: The possible match to pmu_name.
+ */
+bool perf_pmu__name_wildcard_match(const struct perf_pmu *pmu, const char *to_match)
+{
+ return perf_pmu___name_match(pmu, to_match, /*wildcard=*/true);
+}
+
+/**
+ * perf_pmu__name_no_suffix_match - Does pmu's name match to_match ignoring any
+ * trailing suffix on the pmu_name and/or tok?
+ * @pmu: The pmu whose name/alias to match.
+ * @to_match: The possible match to pmu_name.
+ */
+bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_match)
+{
+ return perf_pmu___name_match(pmu, to_match, /*wildcard=*/false);
}
bool perf_pmu__is_software(const struct perf_pmu *pmu)
@@ -2229,29 +2325,31 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
name ?: "N/A", buf, config_name, config);
}
-bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok)
+bool perf_pmu__wildcard_match(const struct perf_pmu *pmu, const char *wildcard_to_match)
{
- const char *name = pmu->name;
- bool need_fnmatch = strisglob(tok);
+ const char *names[2] = {
+ pmu->name,
+ pmu->alias_name,
+ };
+ bool need_fnmatch = strisglob(wildcard_to_match);
- if (!strncmp(tok, "uncore_", 7))
- tok += 7;
- if (!strncmp(name, "uncore_", 7))
- name += 7;
+ if (!strncmp(wildcard_to_match, "uncore_", 7))
+ wildcard_to_match += 7;
- if (perf_pmu__match_ignoring_suffix(name, tok) ||
- (need_fnmatch && !fnmatch(tok, name, 0)))
- return true;
+ for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
+ const char *pmu_name = names[i];
- name = pmu->alias_name;
- if (!name)
- return false;
+ if (!pmu_name)
+ continue;
- if (!strncmp(name, "uncore_", 7))
- name += 7;
+ if (!strncmp(pmu_name, "uncore_", 7))
+ pmu_name += 7;
- return perf_pmu__match_ignoring_suffix(name, tok) ||
- (need_fnmatch && !fnmatch(tok, name, 0));
+ if (perf_pmu__match_wildcard(pmu_name, wildcard_to_match) ||
+ (need_fnmatch && !fnmatch(wildcard_to_match, pmu_name, 0)))
+ return true;
+ }
+ return false;
}
int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index edd36c20aedc..f5306428c03f 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -240,7 +240,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name);
size_t perf_pmu__num_events(struct perf_pmu *pmu);
int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
void *state, pmu_event_callback cb);
-bool pmu__name_match(const struct perf_pmu *pmu, const char *pmu_name);
+bool perf_pmu__name_wildcard_match(const struct perf_pmu *pmu, const char *to_match);
+bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_match);
/**
* perf_pmu_is_software - is the PMU a software PMU as in it uses the
@@ -275,7 +276,7 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
const char *config_name);
void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
-bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok);
+bool perf_pmu__wildcard_match(const struct perf_pmu *pmu, const char *wildcard_to_match);
int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size);
int perf_pmu__pathname_scnprintf(char *buf, size_t size,
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] perf stat: Don't merge counters purely on name
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
` (2 preceding siblings ...)
2025-01-23 7:46 ` [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants Ian Rogers
@ 2025-01-23 7:46 ` Ian Rogers
2025-01-23 7:46 ` [PATCH v2 5/5] perf stat: Changes to event name uniquification Ian Rogers
2025-01-30 4:29 ` [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
5 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
Counter merging was added in commit 942c5593393d ("perf stat: Add
perf_stat_merge_counters()"), however, it merges events with the same
name on different PMUs regardless of whether the different PMUs are
actually of the same type (ie they differ only in the suffix on the
PMU). For hwmon events there may be a temp1 event on every PMU, but
the PMU names are all unique and don't differ just by a suffix. The
merging is over eager and will merge all the hwmon counters together
meaning an aggregated and very large temp1 value is shown. The same
would be true for say cache events and memory controller events where
the PMUs differ but the event names are the same.
Fix the problem by correctly saying two PMUs alias when they differ
only by suffix.
Note, there is an overlap with evsel's merged_stat with aggregation
and the evsel's metric_leader where aggregation happens for metrics.
Fixes: 942c5593393d ("perf stat: Add perf_stat_merge_counters()")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/stat.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 7c2ccdcc3fdb..1f7abd8754c7 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -535,7 +535,10 @@ static int evsel__merge_aggr_counters(struct evsel *evsel, struct evsel *alias)
return 0;
}
-/* events should have the same name, scale, unit, cgroup but on different PMUs */
+/*
+ * Events should have the same name, scale, unit, cgroup but on different core
+ * PMUs or on different but matching uncore PMUs.
+ */
static bool evsel__is_alias(struct evsel *evsel_a, struct evsel *evsel_b)
{
if (strcmp(evsel__name(evsel_a), evsel__name(evsel_b)))
@@ -553,7 +556,13 @@ static bool evsel__is_alias(struct evsel *evsel_a, struct evsel *evsel_b)
if (evsel__is_clock(evsel_a) != evsel__is_clock(evsel_b))
return false;
- return evsel_a->pmu != evsel_b->pmu;
+ if (evsel_a->pmu == evsel_b->pmu || evsel_a->pmu == NULL || evsel_b->pmu == NULL)
+ return false;
+
+ if (evsel_a->pmu->is_core)
+ return evsel_b->pmu->is_core;
+
+ return perf_pmu__name_no_suffix_match(evsel_a->pmu, evsel_b->pmu->name);
}
static void evsel__merge_aliases(struct evsel *evsel)
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] perf stat: Changes to event name uniquification
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
` (3 preceding siblings ...)
2025-01-23 7:46 ` [PATCH v2 4/5] perf stat: Don't merge counters purely on name Ian Rogers
@ 2025-01-23 7:46 ` Ian Rogers
2025-01-30 4:29 ` [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
5 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-01-23 7:46 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
The existing logic would disable uniquification on an evlist or enable
it per evsel, this is unfortunate as uniquification is most needed
when events have the same name and so the whole evlist must be
considered. Change the initial disable uniquify on an evlist
processing to also set a needs_uniquify flag, for cases like the
matching event names. This must be done as an initial pass as
uniquification of an event name will change the behavior of the
check. Keep the per counter uniquification but now only uniquify event
names when the needs_uniquify flag is set.
Before this change a hwmon like temp1 wouldn't be uniquified and
afterwards it will (ie the PMU is added to the temp1 event's name).
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/evsel.h | 1 +
tools/perf/util/stat-display.c | 111 +++++++++++++++++++++++----------
2 files changed, 79 insertions(+), 33 deletions(-)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 5e789fa80590..d91621b6c8c2 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -119,6 +119,7 @@ struct evsel {
bool errored;
bool needs_auxtrace_mmap;
bool default_metricgroup; /* A member of the Default metricgroup */
+ bool needs_uniquify;
struct hashmap *per_pkg_mask;
int err;
int script_output_type;
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index ba79f73e1cf5..e65c7e9f15d1 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -929,12 +929,16 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
}
}
-static void uniquify_event_name(struct evsel *counter)
+static void evsel__uniquify_counter(struct evsel *counter)
{
const char *name, *pmu_name;
char *new_name, *config;
int ret;
+ /* No uniquification necessary. */
+ if (!counter->needs_uniquify)
+ return;
+
/* The evsel was already uniquified. */
if (counter->uniquified_name)
return;
@@ -942,19 +946,6 @@ static void uniquify_event_name(struct evsel *counter)
/* Avoid checking to uniquify twice. */
counter->uniquified_name = true;
- /* The evsel has a "name=" config term or is from libpfm. */
- if (counter->use_config_name || counter->is_libpfm_event)
- return;
-
- /* Legacy no PMU event, don't uniquify. */
- if (!counter->pmu ||
- (counter->pmu->type < PERF_TYPE_MAX && counter->pmu->type != PERF_TYPE_RAW))
- return;
-
- /* A sysfs or json event replacing a legacy event, don't uniquify. */
- if (counter->pmu->is_core && counter->alternate_hw_config != PERF_COUNT_HW_MAX)
- return;
-
name = evsel__name(counter);
pmu_name = counter->pmu->name;
/* Already prefixed by the PMU name. */
@@ -993,17 +984,6 @@ static void uniquify_event_name(struct evsel *counter)
}
}
-static bool hybrid_uniquify(struct evsel *evsel, struct perf_stat_config *config)
-{
- return evsel__is_hybrid(evsel) && !config->hybrid_merge;
-}
-
-static void uniquify_counter(struct perf_stat_config *config, struct evsel *counter)
-{
- if (config->aggr_mode == AGGR_NONE || hybrid_uniquify(counter, config))
- uniquify_event_name(counter);
-}
-
/**
* should_skip_zero_count() - Check if the event should print 0 values.
* @config: The perf stat configuration (including aggregation mode).
@@ -1089,7 +1069,7 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
if (counter->merged_stat)
return;
- uniquify_counter(config, counter);
+ evsel__uniquify_counter(counter);
val = aggr->counts.val;
ena = aggr->counts.ena;
@@ -1670,7 +1650,8 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist
print_metric_end(config, os);
}
-static void disable_uniquify(struct evlist *evlist)
+/* Should uniquify be disabled for the evlist? */
+static bool evlist__disable_uniquify(const struct evlist *evlist)
{
struct evsel *counter;
struct perf_pmu *last_pmu = NULL;
@@ -1679,20 +1660,84 @@ static void disable_uniquify(struct evlist *evlist)
evlist__for_each_entry(evlist, counter) {
/* If PMUs vary then uniquify can be useful. */
if (!first && counter->pmu != last_pmu)
- return;
+ return false;
first = false;
if (counter->pmu) {
/* Allow uniquify for uncore PMUs. */
if (!counter->pmu->is_core)
- return;
+ return false;
/* Keep hybrid event names uniquified for clarity. */
if (perf_pmus__num_core_pmus() > 1)
- return;
+ return false;
+ }
+ }
+ return true;
+}
+
+static void evsel__set_needs_uniquify(struct evsel *counter, const struct perf_stat_config *config)
+{
+ struct evsel *evsel;
+
+ if (counter->merged_stat) {
+ /* Counter won't be shown. */
+ return;
+ }
+
+ if (counter->use_config_name || counter->is_libpfm_event) {
+ /* Original name will be used. */
+ return;
+ }
+
+ if (counter->core.attr.type < PERF_TYPE_MAX && counter->core.attr.type != PERF_TYPE_RAW) {
+ /* Legacy event, don't uniquify. */
+ return;
+ }
+
+ if (counter->pmu && counter->pmu->is_core &&
+ counter->alternate_hw_config != PERF_COUNT_HW_MAX) {
+ /* A sysfs or json event replacing a legacy event, don't uniquify. */
+ return;
+ }
+
+ if (config->aggr_mode == AGGR_NONE) {
+ /* Always unique with no aggregation. */
+ counter->needs_uniquify = true;
+ return;
+ }
+
+ if (!config->hybrid_merge && evsel__is_hybrid(counter)) {
+ /* Unique hybrid counters necessary. */
+ counter->needs_uniquify = true;
+ return;
+ }
+
+ /*
+ * Do other non-merged events in the evlist have the same name? If so
+ * uniquify is necessary.
+ */
+ evlist__for_each_entry(counter->evlist, evsel) {
+ if (evsel == counter || evsel->merged_stat)
+ continue;
+
+ if (evsel__name_is(counter, evsel__name(evsel))) {
+ counter->needs_uniquify = true;
+ return;
}
}
- evlist__for_each_entry_continue(evlist, counter) {
- counter->uniquified_name = true;
+}
+
+static void evlist__set_needs_uniquify(struct evlist *evlist, const struct perf_stat_config *config)
+{
+ struct evsel *counter;
+
+ if (evlist__disable_uniquify(evlist)) {
+ evlist__for_each_entry(evlist, counter)
+ counter->uniquified_name = true;
+ return;
}
+
+ evlist__for_each_entry(evlist, counter)
+ evsel__set_needs_uniquify(counter, config);
}
void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *config,
@@ -1706,7 +1751,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
.first = true,
};
- disable_uniquify(evlist);
+ evlist__set_needs_uniquify(evlist, config);
if (config->iostat_run)
evlist->selected = evlist__first(evlist);
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/5] perf hwmon related improvements
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
` (4 preceding siblings ...)
2025-01-23 7:46 ` [PATCH v2 5/5] perf stat: Changes to event name uniquification Ian Rogers
@ 2025-01-30 4:29 ` Ian Rogers
5 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-01-30 4:29 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Ze Gao,
Weilin Wang, Jean-Philippe Romain, Junhao He, Yicong Yang,
linux-perf-users, linux-kernel
On Wed, Jan 22, 2025 at 11:47 PM Ian Rogers <irogers@google.com> wrote:
>
> Try to do less scanning of files/directories with or without hwmon
> PMUs. Don't merge all events with the same name, only merge those on
> the same PMU ignoring suffix. Tidy PMU name matching to distinguish no
> suffix or wildcard matching. Refactor uniquification so the evsels
> with the same name as other evsels in the evlist are uniquified.
>
> v2: Rename ignore suffix PMU name matching that is really a
> wildcard/prefix match. Use a proper ignore suffix when not merging
> counters purely on name.
>
> Ian Rogers (5):
> perf evsel: Reduce scanning core PMUs in is_hybrid
> perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs
> perf pmu: Rename name matching for no suffix or wildcard variants
> perf stat: Don't merge counters purely on name
> perf stat: Changes to event name uniquification
Hi, this series (v1 sent over 2 weeks ago) fixes broken event
aggregation where all events with the same name on different PMUs are
merged. For example, arm_cmn/cycles/ would be merged with
armv8_pmuv3/cycles/ if parsed through a glob. The bug was introduced
by:
https://lore.kernel.org/r/20221018020227.85905-16-namhyung@kernel.org
and the hope was this would be in v6.14 and the Fixes tag would see
the changes backported.
Thanks,
Ian
> tools/perf/pmu-events/empty-pmu-events.c | 8 +-
> tools/perf/pmu-events/jevents.py | 8 +-
> tools/perf/tests/pmu.c | 85 ++++----
> tools/perf/util/evsel.c | 4 +-
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/parse-events.c | 2 +-
> tools/perf/util/pmu.c | 252 ++++++++++++++++-------
> tools/perf/util/pmu.h | 7 +-
> tools/perf/util/pmus.c | 144 ++++++++-----
> tools/perf/util/stat-display.c | 111 +++++++---
> tools/perf/util/stat.c | 13 +-
> 11 files changed, 418 insertions(+), 217 deletions(-)
>
> --
> 2.48.1.262.g85cc9f2d1e-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs
2025-01-23 7:46 ` [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs Ian Rogers
@ 2025-01-30 19:24 ` Liang, Kan
2025-02-01 7:26 ` Ian Rogers
0 siblings, 1 reply; 11+ messages in thread
From: Liang, Kan @ 2025-01-30 19:24 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, James Clark, Ze Gao, Weilin Wang,
Jean-Philippe Romain, Junhao He, Yicong Yang, linux-perf-users,
linux-kernel
On 2025-01-23 2:46 a.m., Ian Rogers wrote:
> Rather than scanning core or all PMUs, allow pmu_read_sysfs to read
> some combination of core, other, hwmon and tool PMUs. The PMUs that
> should be read and are already read are held as bitmaps. It is known
> that a "hwmon_" prefix is necessary for a hwmon PMU's name, similarly
> with "tool", so only scan those PMUs in situations the PMU name or the
> PMU's type number make sense to.
>
> The number of openat system calls reduces from 276 to 98 for a hwmon
> event. The number of openats for regular perf events isn't changed.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/pmu.h | 2 +
> tools/perf/util/pmus.c | 144 ++++++++++++++++++++++++++---------------
> 2 files changed, 95 insertions(+), 51 deletions(-)
>
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index dbed6c243a5e..edd36c20aedc 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -37,6 +37,8 @@ struct perf_pmu_caps {
> };
>
> enum {
> + PERF_PMU_TYPE_PE_START = 0,
> + PERF_PMU_TYPE_PE_END = 0xFFFEFFFF,
> PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
> PERF_PMU_TYPE_HWMON_END = 0xFFFFFFFD,
> PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index b493da0d22ef..3e3ffafcad71 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -37,10 +37,23 @@
> */
> static LIST_HEAD(core_pmus);
> static LIST_HEAD(other_pmus);
> -static bool read_sysfs_core_pmus;
> -static bool read_sysfs_all_pmus;
> +enum perf_tool_pmu_type {
> + PERF_TOOL_PMU_TYPE_PE_CORE,
> +#define PERF_TOOL_PMU_TYPE_PE_CORE_MASK (1 << PERF_TOOL_PMU_TYPE_PE_CORE)
> + PERF_TOOL_PMU_TYPE_PE_OTHER,
What is the PE short for?
The terminology "other" confused me at first glance.
I think the PE_OTHER means the PMUs not core, hwmon or tool.
There is a "other_pmus" nearby, which means the PMUs not core, but
include hwmon and tool.
Please add some comments to explain the scope of the TYPE_PE_OTHER.
> +#define PERF_TOOL_PMU_TYPE_PE_OTHER_MASK (1 << PERF_TOOL_PMU_TYPE_PE_OTHER)
> + PERF_TOOL_PMU_TYPE_TOOL,
> +#define PERF_TOOL_PMU_TYPE_TOOL_MASK (1 << PERF_TOOL_PMU_TYPE_TOOL)
> + PERF_TOOL_PMU_TYPE_HWMON,
> +#define PERF_TOOL_PMU_TYPE_HWMON_MASK (1 << PERF_TOOL_PMU_TYPE_HWMON)
> +#define PERF_TOOL_PMU_TYPE_ALL_MASK (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | \
> + PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | \
> + PERF_TOOL_PMU_TYPE_TOOL_MASK | \
> + PERF_TOOL_PMU_TYPE_HWMON_MASK)
> +};
Nit: Maybe it's better to split the enum perf_tool_pmu_type and #define.
It's a little bit hard for me to locate how many types there are at
first glance.
> +static unsigned int read_pmu_types;
>
> -static void pmu_read_sysfs(bool core_only);
> +static void pmu_read_sysfs(unsigned int to_read_pmus);
>
> size_t pmu_name_len_no_suffix(const char *str)
> {
> @@ -102,8 +115,7 @@ void perf_pmus__destroy(void)
>
> perf_pmu__delete(pmu);
> }
> - read_sysfs_core_pmus = false;
> - read_sysfs_all_pmus = false;
> + read_pmu_types = 0;
> }
>
> static struct perf_pmu *pmu_find(const char *name)
> @@ -129,6 +141,7 @@ struct perf_pmu *perf_pmus__find(const char *name)
> struct perf_pmu *pmu;
> int dirfd;
> bool core_pmu;
> + unsigned int to_read_pmus = 0;
>
> /*
> * Once PMU is loaded it stays in the list,
> @@ -139,11 +152,11 @@ struct perf_pmu *perf_pmus__find(const char *name)
> if (pmu)
> return pmu;
>
> - if (read_sysfs_all_pmus)
> + if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
> return NULL;
>
> core_pmu = is_pmu_core(name);
> - if (core_pmu && read_sysfs_core_pmus)
> + if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
> return NULL;
>
> dirfd = perf_pmu__event_source_devices_fd();
> @@ -151,15 +164,27 @@ struct perf_pmu *perf_pmus__find(const char *name)
> /*eager_load=*/false);
> close(dirfd);
>
> - if (!pmu) {
> - /*
> - * Looking up an inidividual PMU failed. This may mean name is
> - * an alias, so read the PMUs from sysfs and try to find again.
> - */
> - pmu_read_sysfs(core_pmu);
> + if (pmu)
> + return pmu;
> +
> + /* Looking up an individual perf event PMU failed, check if a tool PMU should be read. */
> + if (!strncmp(name, "hwmon_", 6))
> + to_read_pmus |= PERF_TOOL_PMU_TYPE_HWMON_MASK;
> + else if (!strcmp(name, "tool"))
> + to_read_pmus |= PERF_TOOL_PMU_TYPE_TOOL_MASK;
> +
> + if (to_read_pmus) {
> + pmu_read_sysfs(to_read_pmus);
> pmu = pmu_find(name);
> + if (pmu)
> + return pmu;
> }
> - return pmu;
> + /* Read all necessary PMUs from sysfs and see if the PMU is found. */
> + to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK;
> + if (!core_pmu)
> + to_read_pmus |= PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
> + pmu_read_sysfs(to_read_pmus);
> + return pmu_find(name);
> }
>
> static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
> @@ -176,11 +201,11 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
> if (pmu)
> return pmu;
>
> - if (read_sysfs_all_pmus)
> + if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
> return NULL;
>
> core_pmu = is_pmu_core(name);
> - if (core_pmu && read_sysfs_core_pmus)
> + if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
> return NULL;
>
> return perf_pmu__lookup(core_pmu ? &core_pmus : &other_pmus, dirfd, name,
> @@ -197,52 +222,60 @@ static int pmus_cmp(void *priv __maybe_unused,
> }
>
> /* Add all pmus in sysfs to pmu list: */
> -static void pmu_read_sysfs(bool core_only)
> +static void pmu_read_sysfs(unsigned int to_read_types)
> {
> - int fd;
> - DIR *dir;
> - struct dirent *dent;
> struct perf_pmu *tool_pmu;
>
> - if (read_sysfs_all_pmus || (core_only && read_sysfs_core_pmus))
> + if ((read_pmu_types & to_read_types) == to_read_types) {
> + /* All requested PMU types have been read. */
> return;
> + }
>
> - fd = perf_pmu__event_source_devices_fd();
> - if (fd < 0)
> - return;
> + if (to_read_types & (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | PERF_TOOL_PMU_TYPE_PE_OTHER_MASK)) {
> + int fd = perf_pmu__event_source_devices_fd();
> + DIR *dir;
> + struct dirent *dent;
> + bool core_only = (to_read_types & PERF_TOOL_PMU_TYPE_PE_OTHER_MASK) == 0;
>
> - dir = fdopendir(fd);
> - if (!dir) {
> - close(fd);
> - return;
> - }
> + if (fd < 0)
> + goto skip_pe_pmus;
>
> - while ((dent = readdir(dir))) {
> - if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
> - continue;
> - if (core_only && !is_pmu_core(dent->d_name))
> - continue;
> - /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
> - perf_pmu__find2(fd, dent->d_name);
> - }
> + dir = fdopendir(fd);
> + if (!dir) {
> + close(fd);
> + goto skip_pe_pmus;
> + }
>
> - closedir(dir);
> - if (list_empty(&core_pmus)) {
> + while ((dent = readdir(dir))) {
> + if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
> + continue;
> + if (core_only && !is_pmu_core(dent->d_name))
> + continue;
> + /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
> + perf_pmu__find2(fd, dent->d_name);
> + }
> +
> + closedir(dir);
> + }
> +skip_pe_pmus:
> + if ((to_read_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK) && list_empty(&core_pmus)) {
> if (!perf_pmu__create_placeholder_core_pmu(&core_pmus))
> pr_err("Failure to set up any core PMUs\n");
> }
> list_sort(NULL, &core_pmus, pmus_cmp);
> - if (!core_only) {
> +
> + if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) != 0 &&
> + (read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) == 0) {
Nit: Maybe
if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) &&
!(read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK))
> tool_pmu = perf_pmus__tool_pmu();
> list_add_tail(&tool_pmu->list, &other_pmus);
> - perf_pmus__read_hwmon_pmus(&other_pmus);
> }
> + if ((to_read_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) != 0 &&
> + (read_pmu_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) == 0)
ditto
Thanks,
Kan
> + perf_pmus__read_hwmon_pmus(&other_pmus);
> +
> list_sort(NULL, &other_pmus, pmus_cmp);
> - if (!list_empty(&core_pmus)) {
> - read_sysfs_core_pmus = true;
> - if (!core_only)
> - read_sysfs_all_pmus = true;
> - }
> +
> + read_pmu_types |= to_read_types;
> }
>
> static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
> @@ -263,12 +296,21 @@ static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
>
> struct perf_pmu *perf_pmus__find_by_type(unsigned int type)
> {
> + unsigned int to_read_pmus;
> struct perf_pmu *pmu = __perf_pmus__find_by_type(type);
>
> - if (pmu || read_sysfs_all_pmus)
> + if (pmu || (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK))
> return pmu;
>
> - pmu_read_sysfs(/*core_only=*/false);
> + if (type >= PERF_PMU_TYPE_PE_START && type <= PERF_PMU_TYPE_PE_END) {
> + to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK |
> + PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
> + } else if (type >= PERF_PMU_TYPE_HWMON_START && type <= PERF_PMU_TYPE_HWMON_END) {
> + to_read_pmus = PERF_TOOL_PMU_TYPE_HWMON_MASK;
> + } else {
> + to_read_pmus = PERF_TOOL_PMU_TYPE_TOOL_MASK;
> + }
> + pmu_read_sysfs(to_read_pmus);
> pmu = __perf_pmus__find_by_type(type);
> return pmu;
> }
> @@ -282,7 +324,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
> bool use_core_pmus = !pmu || pmu->is_core;
>
> if (!pmu) {
> - pmu_read_sysfs(/*core_only=*/false);
> + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
> pmu = list_prepare_entry(pmu, &core_pmus, list);
> }
> if (use_core_pmus) {
> @@ -300,7 +342,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
> struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
> {
> if (!pmu) {
> - pmu_read_sysfs(/*core_only=*/true);
> + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_PE_CORE_MASK);
> return list_first_entry_or_null(&core_pmus, typeof(*pmu), list);
> }
> list_for_each_entry_continue(pmu, &core_pmus, list)
> @@ -316,7 +358,7 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
>
> if (!pmu) {
> - pmu_read_sysfs(/*core_only=*/false);
> + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
> pmu = list_prepare_entry(pmu, &core_pmus, list);
> } else
> last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants
2025-01-23 7:46 ` [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants Ian Rogers
@ 2025-01-30 19:29 ` Liang, Kan
2025-02-01 7:30 ` Ian Rogers
0 siblings, 1 reply; 11+ messages in thread
From: Liang, Kan @ 2025-01-30 19:29 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, James Clark, Ze Gao, Weilin Wang,
Jean-Philippe Romain, Junhao He, Yicong Yang, linux-perf-users,
linux-kernel
On 2025-01-23 2:46 a.m., Ian Rogers wrote:
> Wildcard PMU naming will match a name like pmu_1 to a PMU name like
> pmu_10 but not to a PMU name like pmu_2 as the suffix forms part of
> the match. No suffix matching will match pmu_10 to either pmu_1 or
> pmu_2. Add or rename matching functions on PMU to make it clearer what
> kind of matching is being performed.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/pmu-events/empty-pmu-events.c | 8 +-
> tools/perf/pmu-events/jevents.py | 8 +-
> tools/perf/tests/pmu.c | 85 ++++----
> tools/perf/util/parse-events.c | 2 +-
> tools/perf/util/pmu.c | 252 ++++++++++++++++-------
> tools/perf/util/pmu.h | 5 +-
> 6 files changed, 231 insertions(+), 129 deletions(-)
>
> diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
> index 1c7a2cfa321f..0cb7ba7912e8 100644
> --- a/tools/perf/pmu-events/empty-pmu-events.c
> +++ b/tools/perf/pmu-events/empty-pmu-events.c
> @@ -422,7 +422,7 @@ int pmu_events_table__for_each_event(const struct pmu_events_table *table,
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
> int ret;
>
> - if (pmu && !pmu__name_match(pmu, pmu_name))
> + if (pmu && !perf_pmu__name_wildcard_match(pmu, pmu_name))
> continue;
>
> ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
> @@ -443,7 +443,7 @@ int pmu_events_table__find_event(const struct pmu_events_table *table,
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
> int ret;
>
> - if (!pmu__name_match(pmu, pmu_name))
> + if (!perf_pmu__name_wildcard_match(pmu, pmu_name))
> continue;
>
> ret = pmu_events_table__find_event_pmu(table, table_pmu, name, fn, data);
> @@ -462,7 +462,7 @@ size_t pmu_events_table__num_events(const struct pmu_events_table *table,
> const struct pmu_table_entry *table_pmu = &table->pmus[i];
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
>
> - if (pmu__name_match(pmu, pmu_name))
> + if (perf_pmu__name_wildcard_match(pmu, pmu_name))
> count += table_pmu->num_entries;
> }
> return count;
> @@ -581,7 +581,7 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
> const struct pmu_table_entry *table_pmu = &map->event_table.pmus[i];
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
>
> - if (pmu__name_match(pmu, pmu_name))
> + if (perf_pmu__name_wildcard_match(pmu, pmu_name))
> return &map->event_table;
> }
> return NULL;
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 3e204700b59a..7499a35bfadd 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -945,7 +945,7 @@ int pmu_events_table__for_each_event(const struct pmu_events_table *table,
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
> int ret;
>
> - if (pmu && !pmu__name_match(pmu, pmu_name))
> + if (pmu && !perf_pmu__name_wildcard_match(pmu, pmu_name))
> continue;
>
> ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
> @@ -966,7 +966,7 @@ int pmu_events_table__find_event(const struct pmu_events_table *table,
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
> int ret;
>
> - if (!pmu__name_match(pmu, pmu_name))
> + if (!perf_pmu__name_wildcard_match(pmu, pmu_name))
> continue;
>
> ret = pmu_events_table__find_event_pmu(table, table_pmu, name, fn, data);
> @@ -985,7 +985,7 @@ size_t pmu_events_table__num_events(const struct pmu_events_table *table,
> const struct pmu_table_entry *table_pmu = &table->pmus[i];
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
>
> - if (pmu__name_match(pmu, pmu_name))
> + if (perf_pmu__name_wildcard_match(pmu, pmu_name))
> count += table_pmu->num_entries;
> }
> return count;
> @@ -1104,7 +1104,7 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
> const struct pmu_table_entry *table_pmu = &map->event_table.pmus[i];
> const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
>
> - if (pmu__name_match(pmu, pmu_name))
> + if (perf_pmu__name_wildcard_match(pmu, pmu_name))
> return &map->event_table;
> }
> return NULL;
> diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
> index 6a681e3fb552..4a9f8e090cf4 100644
> --- a/tools/perf/tests/pmu.c
> +++ b/tools/perf/tests/pmu.c
> @@ -452,9 +452,9 @@ static int test__name_cmp(struct test_suite *test __maybe_unused, int subtest __
> }
>
> /**
> - * Test perf_pmu__match() that's used to search for a PMU given a name passed
> + * Test perf_pmu__wildcard_match() that's used to search for a PMU given a name passed
> * on the command line. The name that's passed may also be a filename type glob
> - * match. If the name does not match, perf_pmu__match() attempts to match the
> + * match. If the name does not match, perf_pmu__wildcard_match() attempts to match the
> * alias of the PMU, if provided.
> */
> static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
> @@ -463,41 +463,44 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
> .name = "pmuname",
> };
>
> - TEST_ASSERT_EQUAL("Exact match", perf_pmu__match(&test_pmu, "pmuname"), true);
> - TEST_ASSERT_EQUAL("Longer token", perf_pmu__match(&test_pmu, "longertoken"), false);
> - TEST_ASSERT_EQUAL("Shorter token", perf_pmu__match(&test_pmu, "pmu"), false);
> +#define TEST_PMU_MATCH(msg, to_match, expect) \
> + TEST_ASSERT_EQUAL(msg, perf_pmu__wildcard_match(&test_pmu, to_match), expect)
> +
> + TEST_PMU_MATCH("Exact match", "pmuname", true);
> + TEST_PMU_MATCH("Longer token", "longertoken", false);
> + TEST_PMU_MATCH("Shorter token", "pmu", false);
>
> test_pmu.name = "pmuname_10";
> - TEST_ASSERT_EQUAL("Diff suffix_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
> - TEST_ASSERT_EQUAL("Sub suffix_", perf_pmu__match(&test_pmu, "pmuname_1"), true);
> - TEST_ASSERT_EQUAL("Same suffix_", perf_pmu__match(&test_pmu, "pmuname_10"), true);
> - TEST_ASSERT_EQUAL("No suffix_", perf_pmu__match(&test_pmu, "pmuname"), true);
> - TEST_ASSERT_EQUAL("Underscore_", perf_pmu__match(&test_pmu, "pmuname_"), true);
> - TEST_ASSERT_EQUAL("Substring_", perf_pmu__match(&test_pmu, "pmuna"), false);
> + TEST_PMU_MATCH("Diff suffix_", "pmuname_2", false);
> + TEST_PMU_MATCH("Sub suffix_", "pmuname_1", true);
> + TEST_PMU_MATCH("Same suffix_", "pmuname_10", true);
> + TEST_PMU_MATCH("No suffix_", "pmuname", true);
> + TEST_PMU_MATCH("Underscore_", "pmuname_", true);
> + TEST_PMU_MATCH("Substring_", "pmuna", false);
>
> test_pmu.name = "pmuname_ab23";
> - TEST_ASSERT_EQUAL("Diff suffix hex_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
> - TEST_ASSERT_EQUAL("Sub suffix hex_", perf_pmu__match(&test_pmu, "pmuname_ab"), true);
> - TEST_ASSERT_EQUAL("Same suffix hex_", perf_pmu__match(&test_pmu, "pmuname_ab23"), true);
> - TEST_ASSERT_EQUAL("No suffix hex_", perf_pmu__match(&test_pmu, "pmuname"), true);
> - TEST_ASSERT_EQUAL("Underscore hex_", perf_pmu__match(&test_pmu, "pmuname_"), true);
> - TEST_ASSERT_EQUAL("Substring hex_", perf_pmu__match(&test_pmu, "pmuna"), false);
> + TEST_PMU_MATCH("Diff suffix hex_", "pmuname_2", false);
> + TEST_PMU_MATCH("Sub suffix hex_", "pmuname_ab", true);
> + TEST_PMU_MATCH("Same suffix hex_", "pmuname_ab23", true);
> + TEST_PMU_MATCH("No suffix hex_", "pmuname", true);
> + TEST_PMU_MATCH("Underscore hex_", "pmuname_", true);
> + TEST_PMU_MATCH("Substring hex_", "pmuna", false);
>
> test_pmu.name = "pmuname10";
> - TEST_ASSERT_EQUAL("Diff suffix", perf_pmu__match(&test_pmu, "pmuname2"), false);
> - TEST_ASSERT_EQUAL("Sub suffix", perf_pmu__match(&test_pmu, "pmuname1"), true);
> - TEST_ASSERT_EQUAL("Same suffix", perf_pmu__match(&test_pmu, "pmuname10"), true);
> - TEST_ASSERT_EQUAL("No suffix", perf_pmu__match(&test_pmu, "pmuname"), true);
> - TEST_ASSERT_EQUAL("Underscore", perf_pmu__match(&test_pmu, "pmuname_"), false);
> - TEST_ASSERT_EQUAL("Substring", perf_pmu__match(&test_pmu, "pmuna"), false);
> + TEST_PMU_MATCH("Diff suffix", "pmuname2", false);
> + TEST_PMU_MATCH("Sub suffix", "pmuname1", true);
> + TEST_PMU_MATCH("Same suffix", "pmuname10", true);
> + TEST_PMU_MATCH("No suffix", "pmuname", true);
> + TEST_PMU_MATCH("Underscore", "pmuname_", false);
> + TEST_PMU_MATCH("Substring", "pmuna", false);
>
> test_pmu.name = "pmunameab23";
> - TEST_ASSERT_EQUAL("Diff suffix hex", perf_pmu__match(&test_pmu, "pmuname2"), false);
> - TEST_ASSERT_EQUAL("Sub suffix hex", perf_pmu__match(&test_pmu, "pmunameab"), true);
> - TEST_ASSERT_EQUAL("Same suffix hex", perf_pmu__match(&test_pmu, "pmunameab23"), true);
> - TEST_ASSERT_EQUAL("No suffix hex", perf_pmu__match(&test_pmu, "pmuname"), true);
> - TEST_ASSERT_EQUAL("Underscore hex", perf_pmu__match(&test_pmu, "pmuname_"), false);
> - TEST_ASSERT_EQUAL("Substring hex", perf_pmu__match(&test_pmu, "pmuna"), false);
> + TEST_PMU_MATCH("Diff suffix hex", "pmuname2", false);
> + TEST_PMU_MATCH("Sub suffix hex", "pmunameab", true);
> + TEST_PMU_MATCH("Same suffix hex", "pmunameab23", true);
> + TEST_PMU_MATCH("No suffix hex", "pmuname", true);
> + TEST_PMU_MATCH("Underscore hex", "pmuname_", false);
> + TEST_PMU_MATCH("Substring hex", "pmuna", false);
>
> /*
> * 2 hex chars or less are not considered suffixes so it shouldn't be
> @@ -505,7 +508,7 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
> * false results here than above.
> */
> test_pmu.name = "pmuname_a3";
> - TEST_ASSERT_EQUAL("Diff suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_2"), false);
> + TEST_PMU_MATCH("Diff suffix 2 hex_", "pmuname_2", false);
> /*
> * This one should be false, but because pmuname_a3 ends in 3 which is
> * decimal, it's not possible to determine if it's a short hex suffix or
> @@ -513,19 +516,19 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
> * length of decimal suffix. Run the test anyway and expect the wrong
> * result. And slightly fuzzy matching shouldn't do too much harm.
> */
> - TEST_ASSERT_EQUAL("Sub suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_a"), true);
> - TEST_ASSERT_EQUAL("Same suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname_a3"), true);
> - TEST_ASSERT_EQUAL("No suffix 2 hex_", perf_pmu__match(&test_pmu, "pmuname"), false);
> - TEST_ASSERT_EQUAL("Underscore 2 hex_", perf_pmu__match(&test_pmu, "pmuname_"), false);
> - TEST_ASSERT_EQUAL("Substring 2 hex_", perf_pmu__match(&test_pmu, "pmuna"), false);
> + TEST_PMU_MATCH("Sub suffix 2 hex_", "pmuname_a", true);
> + TEST_PMU_MATCH("Same suffix 2 hex_", "pmuname_a3", true);
> + TEST_PMU_MATCH("No suffix 2 hex_", "pmuname", false);
> + TEST_PMU_MATCH("Underscore 2 hex_", "pmuname_", false);
> + TEST_PMU_MATCH("Substring 2 hex_", "pmuna", false);
>
> test_pmu.name = "pmuname_5";
> - TEST_ASSERT_EQUAL("Glob 1", perf_pmu__match(&test_pmu, "pmu*"), true);
> - TEST_ASSERT_EQUAL("Glob 2", perf_pmu__match(&test_pmu, "nomatch*"), false);
> - TEST_ASSERT_EQUAL("Seq 1", perf_pmu__match(&test_pmu, "pmuname_[12345]"), true);
> - TEST_ASSERT_EQUAL("Seq 2", perf_pmu__match(&test_pmu, "pmuname_[67890]"), false);
> - TEST_ASSERT_EQUAL("? 1", perf_pmu__match(&test_pmu, "pmuname_?"), true);
> - TEST_ASSERT_EQUAL("? 2", perf_pmu__match(&test_pmu, "pmuname_1?"), false);
> + TEST_PMU_MATCH("Glob 1", "pmu*", true);
> + TEST_PMU_MATCH("Glob 2", "nomatch*", false);
> + TEST_PMU_MATCH("Seq 1", "pmuname_[12345]", true);
> + TEST_PMU_MATCH("Seq 2", "pmuname_[67890]", false);
> + TEST_PMU_MATCH("? 1", "pmuname_?", true);
> + TEST_PMU_MATCH("? 2", "pmuname_1?", false);
>
> return TEST_OK;
> }
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 1e23faa364b1..6c36b98875bc 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1660,7 +1660,7 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
> /* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
> while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> if (!parse_events__filter_pmu(parse_state, pmu) &&
> - perf_pmu__match(pmu, event_or_pmu)) {
> + perf_pmu__wildcard_match(pmu, event_or_pmu)) {
> bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
>
> if (!parse_events_add_pmu(parse_state, *listp, pmu,
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 6206c8fe2bf9..4c6f9a1a9461 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -847,21 +847,23 @@ static size_t pmu_deduped_name_len(const struct perf_pmu *pmu, const char *name,
> }
>
> /**
> - * 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}.
> + * perf_pmu__match_wildcard - Does the pmu_name start with tok and is then only
> + * followed by nothing or a suffix? tok may contain
> + * part of a suffix.
> * @pmu_name: The pmu_name with possible suffix.
> - * @tok: The possible match to pmu_name without suffix.
> + * @tok: The wildcard argument to match.
> */
> -static bool perf_pmu__match_ignoring_suffix(const char *pmu_name, const char *tok)
> +static bool perf_pmu__match_wildcard(const char *pmu_name, const char *tok)
> {
> const char *p, *suffix;
> bool has_hex = false;
> + size_t tok_len = strlen(tok);
>
> - if (strncmp(pmu_name, tok, strlen(tok)))
> + /* Check start of pmu_name for equality. */
> + if (strncmp(pmu_name, tok, tok_len))
> return false;
>
> - suffix = p = pmu_name + strlen(tok);
> + suffix = p = pmu_name + tok_len;
> if (*p == 0)
> return true;
>
> @@ -887,60 +889,80 @@ static bool perf_pmu__match_ignoring_suffix(const char *pmu_name, const char *to
> }
>
> /**
> - * pmu_uncore_alias_match - does name match the PMU name?
> - * @pmu_name: the json struct pmu_event name. This may lack a suffix (which
> + * perf_pmu__match_ignoring_suffix_uncore - Does the pmu_name match tok ignoring
> + * any trailing suffix on pmu_name and
> + * tok? 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.
> + */
> +static bool perf_pmu__match_ignoring_suffix_uncore(const char *pmu_name, const char *tok)
> +{
> + size_t pmu_name_len, tok_len;
> +
if (!pmu_name)
return false;
I got a segmentfault when I try the code because of a NULL pmu_name.
Thanks,
Kan
> + /* uncore_ prefixes are ignored. */
> + if (!strncmp(pmu_name, "uncore_", 7))
> + pmu_name += 7;
> + if (!strncmp(tok, "uncore_", 7))
> + tok += 7;
> +
> + pmu_name_len = pmu_name_len_no_suffix(pmu_name);
> + tok_len = pmu_name_len_no_suffix(tok);
> + if (pmu_name_len != tok_len)
> + return false;
> +
> + return strncmp(pmu_name, tok, pmu_name_len) == 0;
> +}
> +
> +
> +/**
> + * perf_pmu__match_wildcard_uncore - does to_match match the PMU's name?
> + * @pmu_name: The pmu->name or pmu->alias to match against.
> + * @to_match: 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)
> +static bool perf_pmu__match_wildcard_uncore(const char *pmu_name, const char *to_match)
> {
> - char *tmp = NULL, *tok, *str;
> - bool res;
> + char *mutable_to_match, *tok, *tmp;
>
> - if (strchr(pmu_name, ',') == NULL)
> - return perf_pmu__match_ignoring_suffix(name, pmu_name);
> -
> - str = strdup(pmu_name);
> - if (!str)
> + if (!pmu_name)
> return false;
>
> - /*
> - * uncore alias may be from different PMU with common prefix
> - */
> - tok = strtok_r(str, ",", &tmp);
> - if (strncmp(pmu_name, tok, strlen(tok))) {
> - res = false;
> - goto out;
> - }
> + /* uncore_ prefixes are ignored. */
> + if (!strncmp(pmu_name, "uncore_", 7))
> + pmu_name += 7;
> + if (!strncmp(to_match, "uncore_", 7))
> + to_match += 7;
>
> - /*
> - * Match more complex aliases where the alias name is a comma-delimited
> - * list of tokens, orderly contained in the matching PMU name.
> - *
> - * Example: For alias "socket,pmuname" and PMU "socketX_pmunameY", we
> - * match "socket" in "socketX_pmunameY" and then "pmuname" in
> - * "pmunameY".
> - */
> - while (1) {
> - char *next_tok = strtok_r(NULL, ",", &tmp);
> + if (strchr(to_match, ',') == NULL)
> + return perf_pmu__match_wildcard(pmu_name, to_match);
>
> - name = strstr(name, tok);
> - if (!name ||
> - (!next_tok && !perf_pmu__match_ignoring_suffix(name, tok))) {
> - res = false;
> - goto out;
> + /* Process comma separated list of PMU name components. */
> + mutable_to_match = strdup(to_match);
> + if (!mutable_to_match)
> + return false;
> +
> + tok = strtok_r(mutable_to_match, ",", &tmp);
> + while (tok) {
> + size_t tok_len = strlen(tok);
> +
> + if (strncmp(pmu_name, tok, tok_len)) {
> + /* Mismatch between part of pmu_name and tok. */
> + free(mutable_to_match);
> + return false;
> }
> - if (!next_tok)
> - break;
> - tok = next_tok;
> - name += strlen(tok);
> + /* Move pmu_name forward over tok and suffix. */
> + pmu_name += tok_len;
> + while (*pmu_name != '\0' && isdigit(*pmu_name))
> + pmu_name++;
> + if (*pmu_name == '_')
> + pmu_name++;
> +
> + tok = strtok_r(NULL, ",", &tmp);
> }
> -
> - res = true;
> -out:
> - free(str);
> - return res;
> + free(mutable_to_match);
> + return *pmu_name == '\0';
> }
>
> bool pmu_uncore_identifier_match(const char *compat, const char *id)
> @@ -1003,11 +1025,19 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe,
> {
> struct perf_pmu *pmu = vdata;
>
> - if (!pe->compat || !pe->pmu)
> + if (!pe->compat || !pe->pmu) {
> + /* No data to match. */
> return 0;
> + }
> +
> + if (!perf_pmu__match_wildcard_uncore(pmu->name, pe->pmu) &&
> + !perf_pmu__match_wildcard_uncore(pmu->alias_name, pe->pmu)) {
> + /* PMU name/alias_name don't match. */
> + return 0;
> + }
>
> - if (pmu_uncore_alias_match(pe->pmu, pmu->name) &&
> - pmu_uncore_identifier_match(pe->compat, pmu->id)) {
> + if (pmu_uncore_identifier_match(pe->compat, pmu->id)) {
> + /* Id matched. */
> perf_pmu__new_alias(pmu,
> pe->name,
> pe->desc,
> @@ -1016,7 +1046,6 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe,
> pe,
> EVENT_SRC_SYS_JSON);
> }
> -
> return 0;
> }
>
> @@ -1974,15 +2003,82 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> return ret;
> }
>
> -bool pmu__name_match(const struct perf_pmu *pmu, const char *pmu_name)
> +static bool perf_pmu___name_match(const struct perf_pmu *pmu, const char *to_match, bool wildcard)
> {
> - return !strcmp(pmu->name, pmu_name) ||
> - (pmu->is_uncore && pmu_uncore_alias_match(pmu_name, pmu->name)) ||
> + const char *names[2] = {
> + pmu->name,
> + pmu->alias_name,
> + };
> + if (pmu->is_core) {
> + for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
> + const char *name = names[i];
> +
> + if (!name)
> + continue;
> +
> + if (!strcmp(name, to_match)) {
> + /* Exact name match. */
> + return true;
> + }
> + }
> + if (!strcmp(to_match, "default_core")) {
> + /*
> + * jevents and tests use default_core as a marker for any core
> + * PMU as the PMU name varies across architectures.
> + */
> + return true;
> + }
> + return false;
> + }
> + if (!pmu->is_uncore) {
> /*
> - * jevents and tests use default_core as a marker for any core
> - * PMU as the PMU name varies across architectures.
> + * PMU isn't core or uncore, some kind of broken CPU mask
> + * situation. Only match exact name.
> */
> - (pmu->is_core && !strcmp(pmu_name, "default_core"));
> + for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
> + const char *name = names[i];
> +
> + if (!name)
> + continue;
> +
> + if (!strcmp(name, to_match)) {
> + /* Exact name match. */
> + return true;
> + }
> + }
> + return false;
> + }
> + for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
> + const char *name = names[i];
> +
> + if (wildcard && perf_pmu__match_wildcard_uncore(name, to_match))
> + return true;
> + if (!wildcard && perf_pmu__match_ignoring_suffix_uncore(name, to_match))
> + return true;
> + }
> + return false;
> +}
> +
> +/**
> + * perf_pmu__name_wildcard_match - Called by the jevents generated code to see
> + * if pmu matches the json to_match string.
> + * @pmu: The pmu whose name/alias to match.
> + * @to_match: The possible match to pmu_name.
> + */
> +bool perf_pmu__name_wildcard_match(const struct perf_pmu *pmu, const char *to_match)
> +{
> + return perf_pmu___name_match(pmu, to_match, /*wildcard=*/true);
> +}
> +
> +/**
> + * perf_pmu__name_no_suffix_match - Does pmu's name match to_match ignoring any
> + * trailing suffix on the pmu_name and/or tok?
> + * @pmu: The pmu whose name/alias to match.
> + * @to_match: The possible match to pmu_name.
> + */
> +bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_match)
> +{
> + return perf_pmu___name_match(pmu, to_match, /*wildcard=*/false);
> }
>
> bool perf_pmu__is_software(const struct perf_pmu *pmu)
> @@ -2229,29 +2325,31 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> name ?: "N/A", buf, config_name, config);
> }
>
> -bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok)
> +bool perf_pmu__wildcard_match(const struct perf_pmu *pmu, const char *wildcard_to_match)
> {
> - const char *name = pmu->name;
> - bool need_fnmatch = strisglob(tok);
> + const char *names[2] = {
> + pmu->name,
> + pmu->alias_name,
> + };
> + bool need_fnmatch = strisglob(wildcard_to_match);
>
> - if (!strncmp(tok, "uncore_", 7))
> - tok += 7;
> - if (!strncmp(name, "uncore_", 7))
> - name += 7;
> + if (!strncmp(wildcard_to_match, "uncore_", 7))
> + wildcard_to_match += 7;
>
> - if (perf_pmu__match_ignoring_suffix(name, tok) ||
> - (need_fnmatch && !fnmatch(tok, name, 0)))
> - return true;
> + for (size_t i = 0; i < ARRAY_SIZE(names); i++) {
> + const char *pmu_name = names[i];
>
> - name = pmu->alias_name;
> - if (!name)
> - return false;
> + if (!pmu_name)
> + continue;
>
> - if (!strncmp(name, "uncore_", 7))
> - name += 7;
> + if (!strncmp(pmu_name, "uncore_", 7))
> + pmu_name += 7;
>
> - return perf_pmu__match_ignoring_suffix(name, tok) ||
> - (need_fnmatch && !fnmatch(tok, name, 0));
> + if (perf_pmu__match_wildcard(pmu_name, wildcard_to_match) ||
> + (need_fnmatch && !fnmatch(wildcard_to_match, pmu_name, 0)))
> + return true;
> + }
> + return false;
> }
>
> int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size)
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index edd36c20aedc..f5306428c03f 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -240,7 +240,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name);
> size_t perf_pmu__num_events(struct perf_pmu *pmu);
> int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> void *state, pmu_event_callback cb);
> -bool pmu__name_match(const struct perf_pmu *pmu, const char *pmu_name);
> +bool perf_pmu__name_wildcard_match(const struct perf_pmu *pmu, const char *to_match);
> +bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_match);
>
> /**
> * perf_pmu_is_software - is the PMU a software PMU as in it uses the
> @@ -275,7 +276,7 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> const char *config_name);
> void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
>
> -bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok);
> +bool perf_pmu__wildcard_match(const struct perf_pmu *pmu, const char *wildcard_to_match);
>
> int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size);
> int perf_pmu__pathname_scnprintf(char *buf, size_t size,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs
2025-01-30 19:24 ` Liang, Kan
@ 2025-02-01 7:26 ` Ian Rogers
0 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-02-01 7:26 UTC (permalink / raw)
To: Liang, Kan
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, James Clark, Ze Gao, Weilin Wang,
Jean-Philippe Romain, Junhao He, Yicong Yang, linux-perf-users,
linux-kernel
On Thu, Jan 30, 2025 at 11:24 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2025-01-23 2:46 a.m., Ian Rogers wrote:
> > Rather than scanning core or all PMUs, allow pmu_read_sysfs to read
> > some combination of core, other, hwmon and tool PMUs. The PMUs that
> > should be read and are already read are held as bitmaps. It is known
> > that a "hwmon_" prefix is necessary for a hwmon PMU's name, similarly
> > with "tool", so only scan those PMUs in situations the PMU name or the
> > PMU's type number make sense to.
> >
> > The number of openat system calls reduces from 276 to 98 for a hwmon
> > event. The number of openats for regular perf events isn't changed.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/pmu.h | 2 +
> > tools/perf/util/pmus.c | 144 ++++++++++++++++++++++++++---------------
> > 2 files changed, 95 insertions(+), 51 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> > index dbed6c243a5e..edd36c20aedc 100644
> > --- a/tools/perf/util/pmu.h
> > +++ b/tools/perf/util/pmu.h
> > @@ -37,6 +37,8 @@ struct perf_pmu_caps {
> > };
> >
> > enum {
> > + PERF_PMU_TYPE_PE_START = 0,
> > + PERF_PMU_TYPE_PE_END = 0xFFFEFFFF,
> > PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
> > PERF_PMU_TYPE_HWMON_END = 0xFFFFFFFD,
> > PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
> > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> > index b493da0d22ef..3e3ffafcad71 100644
> > --- a/tools/perf/util/pmus.c
> > +++ b/tools/perf/util/pmus.c
> > @@ -37,10 +37,23 @@
> > */
> > static LIST_HEAD(core_pmus);
> > static LIST_HEAD(other_pmus);
> > -static bool read_sysfs_core_pmus;
> > -static bool read_sysfs_all_pmus;
> > +enum perf_tool_pmu_type {
> > + PERF_TOOL_PMU_TYPE_PE_CORE,
> > +#define PERF_TOOL_PMU_TYPE_PE_CORE_MASK (1 << PERF_TOOL_PMU_TYPE_PE_CORE)
> > + PERF_TOOL_PMU_TYPE_PE_OTHER,
>
> What is the PE short for?
Perf event, rather than type numbers being used for things like the fake PMU.
> The terminology "other" confused me at first glance.
Agreed. It corresponds to the other list immediately above this code.
> I think the PE_OTHER means the PMUs not core, hwmon or tool.
> There is a "other_pmus" nearby, which means the PMUs not core, but
> include hwmon and tool.
>
> Please add some comments to explain the scope of the TYPE_PE_OTHER.
This is already this immediately above:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=perf-tools-next#n31
```
* other_pmus: All other PMUs which are not part of core_pmus list. It doesn't
* matter whether PMU is present per SMT-thread or outside of the
* core in the hw. For e.g., an instance of AMD ibs_fetch// and
* ibs_op// PMUs is present in each hw SMT thread, however they
* are captured under other_pmus. PMUs belonging to other_pmus
* must have pmu->is_core=0 but pmu->is_uncore could be 0 or 1.
```
> > +#define PERF_TOOL_PMU_TYPE_PE_OTHER_MASK (1 << PERF_TOOL_PMU_TYPE_PE_OTHER)
> > + PERF_TOOL_PMU_TYPE_TOOL,
> > +#define PERF_TOOL_PMU_TYPE_TOOL_MASK (1 << PERF_TOOL_PMU_TYPE_TOOL)
> > + PERF_TOOL_PMU_TYPE_HWMON,
> > +#define PERF_TOOL_PMU_TYPE_HWMON_MASK (1 << PERF_TOOL_PMU_TYPE_HWMON)
> > +#define PERF_TOOL_PMU_TYPE_ALL_MASK (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | \
> > + PERF_TOOL_PMU_TYPE_PE_OTHER_MASK | \
> > + PERF_TOOL_PMU_TYPE_TOOL_MASK | \
> > + PERF_TOOL_PMU_TYPE_HWMON_MASK)
> > +};
>
> Nit: Maybe it's better to split the enum perf_tool_pmu_type and #define.
> It's a little bit hard for me to locate how many types there are at
> first glance.
Ok, will do in v3.
> > +static unsigned int read_pmu_types;
> >
> > -static void pmu_read_sysfs(bool core_only);
> > +static void pmu_read_sysfs(unsigned int to_read_pmus);
> >
> > size_t pmu_name_len_no_suffix(const char *str)
> > {
> > @@ -102,8 +115,7 @@ void perf_pmus__destroy(void)
> >
> > perf_pmu__delete(pmu);
> > }
> > - read_sysfs_core_pmus = false;
> > - read_sysfs_all_pmus = false;
> > + read_pmu_types = 0;
> > }
> >
> > static struct perf_pmu *pmu_find(const char *name)
> > @@ -129,6 +141,7 @@ struct perf_pmu *perf_pmus__find(const char *name)
> > struct perf_pmu *pmu;
> > int dirfd;
> > bool core_pmu;
> > + unsigned int to_read_pmus = 0;
> >
> > /*
> > * Once PMU is loaded it stays in the list,
> > @@ -139,11 +152,11 @@ struct perf_pmu *perf_pmus__find(const char *name)
> > if (pmu)
> > return pmu;
> >
> > - if (read_sysfs_all_pmus)
> > + if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
> > return NULL;
> >
> > core_pmu = is_pmu_core(name);
> > - if (core_pmu && read_sysfs_core_pmus)
> > + if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
> > return NULL;
> >
> > dirfd = perf_pmu__event_source_devices_fd();
> > @@ -151,15 +164,27 @@ struct perf_pmu *perf_pmus__find(const char *name)
> > /*eager_load=*/false);
> > close(dirfd);
> >
> > - if (!pmu) {
> > - /*
> > - * Looking up an inidividual PMU failed. This may mean name is
> > - * an alias, so read the PMUs from sysfs and try to find again.
> > - */
> > - pmu_read_sysfs(core_pmu);
> > + if (pmu)
> > + return pmu;
> > +
> > + /* Looking up an individual perf event PMU failed, check if a tool PMU should be read. */
> > + if (!strncmp(name, "hwmon_", 6))
> > + to_read_pmus |= PERF_TOOL_PMU_TYPE_HWMON_MASK;
> > + else if (!strcmp(name, "tool"))
> > + to_read_pmus |= PERF_TOOL_PMU_TYPE_TOOL_MASK;
> > +
> > + if (to_read_pmus) {
> > + pmu_read_sysfs(to_read_pmus);
> > pmu = pmu_find(name);
> > + if (pmu)
> > + return pmu;
> > }
> > - return pmu;
> > + /* Read all necessary PMUs from sysfs and see if the PMU is found. */
> > + to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK;
> > + if (!core_pmu)
> > + to_read_pmus |= PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
> > + pmu_read_sysfs(to_read_pmus);
> > + return pmu_find(name);
> > }
> >
> > static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
> > @@ -176,11 +201,11 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
> > if (pmu)
> > return pmu;
> >
> > - if (read_sysfs_all_pmus)
> > + if (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK)
> > return NULL;
> >
> > core_pmu = is_pmu_core(name);
> > - if (core_pmu && read_sysfs_core_pmus)
> > + if (core_pmu && (read_pmu_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK))
> > return NULL;
> >
> > return perf_pmu__lookup(core_pmu ? &core_pmus : &other_pmus, dirfd, name,
> > @@ -197,52 +222,60 @@ static int pmus_cmp(void *priv __maybe_unused,
> > }
> >
> > /* Add all pmus in sysfs to pmu list: */
> > -static void pmu_read_sysfs(bool core_only)
> > +static void pmu_read_sysfs(unsigned int to_read_types)
> > {
> > - int fd;
> > - DIR *dir;
> > - struct dirent *dent;
> > struct perf_pmu *tool_pmu;
> >
> > - if (read_sysfs_all_pmus || (core_only && read_sysfs_core_pmus))
> > + if ((read_pmu_types & to_read_types) == to_read_types) {
> > + /* All requested PMU types have been read. */
> > return;
> > + }
> >
> > - fd = perf_pmu__event_source_devices_fd();
> > - if (fd < 0)
> > - return;
> > + if (to_read_types & (PERF_TOOL_PMU_TYPE_PE_CORE_MASK | PERF_TOOL_PMU_TYPE_PE_OTHER_MASK)) {
> > + int fd = perf_pmu__event_source_devices_fd();
> > + DIR *dir;
> > + struct dirent *dent;
> > + bool core_only = (to_read_types & PERF_TOOL_PMU_TYPE_PE_OTHER_MASK) == 0;
> >
> > - dir = fdopendir(fd);
> > - if (!dir) {
> > - close(fd);
> > - return;
> > - }
> > + if (fd < 0)
> > + goto skip_pe_pmus;
> >
> > - while ((dent = readdir(dir))) {
> > - if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
> > - continue;
> > - if (core_only && !is_pmu_core(dent->d_name))
> > - continue;
> > - /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
> > - perf_pmu__find2(fd, dent->d_name);
> > - }
> > + dir = fdopendir(fd);
> > + if (!dir) {
> > + close(fd);
> > + goto skip_pe_pmus;
> > + }
> >
> > - closedir(dir);
> > - if (list_empty(&core_pmus)) {
> > + while ((dent = readdir(dir))) {
> > + if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
> > + continue;
> > + if (core_only && !is_pmu_core(dent->d_name))
> > + continue;
> > + /* add to static LIST_HEAD(core_pmus) or LIST_HEAD(other_pmus): */
> > + perf_pmu__find2(fd, dent->d_name);
> > + }
> > +
> > + closedir(dir);
> > + }
> > +skip_pe_pmus:
> > + if ((to_read_types & PERF_TOOL_PMU_TYPE_PE_CORE_MASK) && list_empty(&core_pmus)) {
> > if (!perf_pmu__create_placeholder_core_pmu(&core_pmus))
> > pr_err("Failure to set up any core PMUs\n");
> > }
> > list_sort(NULL, &core_pmus, pmus_cmp);
> > - if (!core_only) {
> > +
> > + if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) != 0 &&
> > + (read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) == 0) {
>
> Nit: Maybe
>
> if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) &&
> !(read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK))
Thanks. I'm not convinced on the readability, which was why I was
being a bit more verbose in what I wrote. I worrited the & reads like
an && and the ! is hard to miss.
Thanks,
Ian
> > tool_pmu = perf_pmus__tool_pmu();
> > list_add_tail(&tool_pmu->list, &other_pmus);
> > - perf_pmus__read_hwmon_pmus(&other_pmus);
> > }
> > + if ((to_read_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) != 0 &&
> > + (read_pmu_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) == 0)
>
> ditto
>
> Thanks,
> Kan
> > + perf_pmus__read_hwmon_pmus(&other_pmus);
> > +
> > list_sort(NULL, &other_pmus, pmus_cmp);
> > - if (!list_empty(&core_pmus)) {
> > - read_sysfs_core_pmus = true;
> > - if (!core_only)
> > - read_sysfs_all_pmus = true;
> > - }
> > +
> > + read_pmu_types |= to_read_types;
> > }
> >
> > static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
> > @@ -263,12 +296,21 @@ static struct perf_pmu *__perf_pmus__find_by_type(unsigned int type)
> >
> > struct perf_pmu *perf_pmus__find_by_type(unsigned int type)
> > {
> > + unsigned int to_read_pmus;
> > struct perf_pmu *pmu = __perf_pmus__find_by_type(type);
> >
> > - if (pmu || read_sysfs_all_pmus)
> > + if (pmu || (read_pmu_types == PERF_TOOL_PMU_TYPE_ALL_MASK))
> > return pmu;
> >
> > - pmu_read_sysfs(/*core_only=*/false);
> > + if (type >= PERF_PMU_TYPE_PE_START && type <= PERF_PMU_TYPE_PE_END) {
> > + to_read_pmus = PERF_TOOL_PMU_TYPE_PE_CORE_MASK |
> > + PERF_TOOL_PMU_TYPE_PE_OTHER_MASK;
> > + } else if (type >= PERF_PMU_TYPE_HWMON_START && type <= PERF_PMU_TYPE_HWMON_END) {
> > + to_read_pmus = PERF_TOOL_PMU_TYPE_HWMON_MASK;
> > + } else {
> > + to_read_pmus = PERF_TOOL_PMU_TYPE_TOOL_MASK;
> > + }
> > + pmu_read_sysfs(to_read_pmus);
> > pmu = __perf_pmus__find_by_type(type);
> > return pmu;
> > }
> > @@ -282,7 +324,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
> > bool use_core_pmus = !pmu || pmu->is_core;
> >
> > if (!pmu) {
> > - pmu_read_sysfs(/*core_only=*/false);
> > + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
> > pmu = list_prepare_entry(pmu, &core_pmus, list);
> > }
> > if (use_core_pmus) {
> > @@ -300,7 +342,7 @@ struct perf_pmu *perf_pmus__scan(struct perf_pmu *pmu)
> > struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
> > {
> > if (!pmu) {
> > - pmu_read_sysfs(/*core_only=*/true);
> > + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_PE_CORE_MASK);
> > return list_first_entry_or_null(&core_pmus, typeof(*pmu), list);
> > }
> > list_for_each_entry_continue(pmu, &core_pmus, list)
> > @@ -316,7 +358,7 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> > const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
> >
> > if (!pmu) {
> > - pmu_read_sysfs(/*core_only=*/false);
> > + pmu_read_sysfs(PERF_TOOL_PMU_TYPE_ALL_MASK);
> > pmu = list_prepare_entry(pmu, &core_pmus, list);
> > } else
> > last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants
2025-01-30 19:29 ` Liang, Kan
@ 2025-02-01 7:30 ` Ian Rogers
0 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2025-02-01 7:30 UTC (permalink / raw)
To: Liang, Kan
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, James Clark, Ze Gao, Weilin Wang,
Jean-Philippe Romain, Junhao He, Yicong Yang, linux-perf-users,
linux-kernel
On Thu, Jan 30, 2025 at 11:29 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> On 2025-01-23 2:46 a.m., Ian Rogers wrote:
[ snip ]
> > /**
> > - * pmu_uncore_alias_match - does name match the PMU name?
> > - * @pmu_name: the json struct pmu_event name. This may lack a suffix (which
> > + * perf_pmu__match_ignoring_suffix_uncore - Does the pmu_name match tok ignoring
> > + * any trailing suffix on pmu_name and
> > + * tok? 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.
> > + */
> > +static bool perf_pmu__match_ignoring_suffix_uncore(const char *pmu_name, const char *tok)
> > +{
> > + size_t pmu_name_len, tok_len;
> > +
>
> if (!pmu_name)
> return false;
>
> I got a segmentfault when I try the code because of a NULL pmu_name.
Thanks for testing! I'll add the extra check. I think it may be
redundant with other event parsing changes that aren't merged in the
main tree.
Ian
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-02-01 7:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-23 7:46 [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
2025-01-23 7:46 ` [PATCH v2 1/5] perf evsel: Reduce scanning core PMUs in is_hybrid Ian Rogers
2025-01-23 7:46 ` [PATCH v2 2/5] perf pmus: Restructure pmu_read_sysfs to scan fewer PMUs Ian Rogers
2025-01-30 19:24 ` Liang, Kan
2025-02-01 7:26 ` Ian Rogers
2025-01-23 7:46 ` [PATCH v2 3/5] perf pmu: Rename name matching for no suffix or wildcard variants Ian Rogers
2025-01-30 19:29 ` Liang, Kan
2025-02-01 7:30 ` Ian Rogers
2025-01-23 7:46 ` [PATCH v2 4/5] perf stat: Don't merge counters purely on name Ian Rogers
2025-01-23 7:46 ` [PATCH v2 5/5] perf stat: Changes to event name uniquification Ian Rogers
2025-01-30 4:29 ` [PATCH v2 0/5] perf hwmon related improvements Ian Rogers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox