AlderLake uses a hybrid architecture utilizing Golden Cove cores
(core cpu) and Gracemont cores (atom cpu). Each cpu has dedicated
event list. Some events are available on core cpu, some events
are available on atom cpu and some events can be available on both.
Kernel exports new pmus "cpu_core" and "cpu_atom" through sysfs:
/sys/devices/cpu_core
/sys/devices/cpu_atom
cat /sys/devices/cpu_core/cpus
0-15
cat /sys/devices/cpu_atom/cpus
16-23
In this example, core cpus are 0-15 and atom cpus are 16-23.
To enable a core only event or atom only event:
cpu_core/<event name>/
or
cpu_atom/<event name>/
Count the 'cycles' event on core cpus.
# perf stat -e cpu_core/cycles/ -a -- sleep 1
Performance counter stats for 'system wide':
12,853,951,349 cpu_core/cycles/
1.002581249 seconds time elapsed
If one event is available on both atom cpu and core cpu, two events
are created automatically.
# perf stat -e cycles -a -- sleep 1
Performance counter stats for 'system wide':
12,856,467,438 cpu_core/cycles/
6,404,634,785 cpu_atom/cycles/
1.002453013 seconds time elapsed
Group is supported if the events are from same pmu, otherwise a warning
is displayed and disable grouping automatically.
# perf stat -e '{cpu_core/cycles/,cpu_core/instructions/}' -a -- sleep 1
Performance counter stats for 'system wide':
12,863,866,968 cpu_core/cycles/
554,795,017 cpu_core/instructions/
1.002616117 seconds time elapsed
# perf stat -e '{cpu_core/cycles/,cpu_atom/instructions/}' -a -- sleep 1
WARNING: events in group from different hybrid PMUs!
WARNING: grouped events cpus do not match, disabling group:
anon group { cpu_core/cycles/, cpu_atom/instructions/ }
Performance counter stats for 'system wide':
6,283,970 cpu_core/cycles/
765,635 cpu_atom/instructions/
1.003959036 seconds time elapsed
Note that, since the whole patchset for AlderLake hybrid support is very
large (40+ patches). For simplicity, it's splitted into several patch
series.
The patch series 1 only supports the basic functionality. The advanced
supports for perf-c2c/perf-mem/topdown/metrics/topology header and others
will be added in follow-up patch series.
The perf tool codes can also be found at:
https://github.com/yaoj/perf.git
v3:
---
- Drop 'perf evlist: Hybrid event uses its own cpus'. This patch is wide
and actually it's not very necessary. The current perf framework has
processed the cpus for evsel well even for hybrid evsel. So this patch can
be dropped.
- Drop 'perf evsel: Adjust hybrid event and global event mixed group'.
The patch is a bit tricky and hard to understand. In v3, we will disable
grouping when the group members are from different PMUs. So this patch
would be not necessary.
- Create parse-events-hybrid.c/parse-events-hybrid.h and evlist-hybrid.c/evlist-hybrid.h.
Move hybrid related codes to these files.
- Create a new patch 'perf pmu: Support 'cycles' and 'branches' inside hybrid PMU' to
support 'cycles' and 'branches' inside PMU.
- Create a new patch 'perf record: Uniquify hybrid event name' to tell user the
pmu which the event belongs to for perf-record.
- If group members are from different hybrid PMUs, shows warning and disable
grouping.
- Other refining and refactoring.
v2:
---
- Drop kernel patches (Kan posted the series "Add Alder Lake support for perf (kernel)" separately).
- Drop the patches for perf-c2c/perf-mem/topdown/metrics/topology header supports,
which will be added in series 2 or series 3.
- Simplify the arguments of __perf_pmu__new_alias() by passing
the 'struct pme_event' pointer.
- Check sysfs validity before access.
- Use pmu style event name, such as "cpu_core/cycles/".
- Move command output two chars to the right.
- Move pmu hybrid functions to new created pmu-hybrid.c/pmu-hybrid.h.
This is to pass the perf test python case.
Jin Yao (27):
tools headers uapi: Update tools's copy of linux/perf_event.h
perf jevents: Support unit value "cpu_core" and "cpu_atom"
perf pmu: Simplify arguments of __perf_pmu__new_alias
perf pmu: Save pmu name
perf pmu: Save detected hybrid pmus to a global pmu list
perf pmu: Add hybrid helper functions
perf stat: Uniquify hybrid event name
perf parse-events: Create two hybrid hardware events
perf parse-events: Create two hybrid cache events
perf parse-events: Create two hybrid raw events
perf pmu: Support 'cycles' and 'branches' inside hybrid PMU
perf parse-events: Support no alias assigned event inside hybrid PMU
perf record: Create two hybrid 'cycles' events by default
perf stat: Add default hybrid events
perf stat: Filter out unmatched aggregation for hybrid event
perf stat: Warn group events from different hybrid PMU
perf script: Support PERF_TYPE_HARDWARE_PMU and PERF_TYPE_HW_CACHE_PMU
perf record: Uniquify hybrid event name
perf tests: Add hybrid cases for 'Parse event definition strings' test
perf tests: Add hybrid cases for 'Roundtrip evsel->name' test
perf tests: Skip 'Setup struct perf_event_attr' test for hybrid
perf tests: Support 'Track with sched_switch' test for hybrid
perf tests: Support 'Parse and process metrics' test for hybrid
perf tests: Support 'Session topology' test for hybrid
perf tests: Support 'Convert perf time to TSC' test for hybrid
perf tests: Skip 'perf stat metrics (shadow stat) test' for hybrid
perf Documentation: Document intel-hybrid support
tools/include/uapi/linux/perf_event.h | 26 +++
tools/perf/Documentation/intel-hybrid.txt | 228 +++++++++++++++++++++
tools/perf/Documentation/perf-record.txt | 1 +
tools/perf/Documentation/perf-stat.txt | 2 +
tools/perf/builtin-record.c | 47 ++++-
tools/perf/builtin-script.c | 24 +++
tools/perf/builtin-stat.c | 29 +++
tools/perf/pmu-events/jevents.c | 2 +
tools/perf/tests/attr.c | 4 +
tools/perf/tests/evsel-roundtrip-name.c | 19 +-
tools/perf/tests/parse-events.c | 170 +++++++++++++++
tools/perf/tests/parse-metric.c | 10 +-
tools/perf/tests/perf-time-to-tsc.c | 16 ++
tools/perf/tests/shell/stat+shadow_stat.sh | 3 +
tools/perf/tests/switch-tracking.c | 10 +-
tools/perf/tests/topology.c | 10 +-
tools/perf/util/Build | 3 +
tools/perf/util/evlist-hybrid.c | 88 ++++++++
tools/perf/util/evlist-hybrid.h | 14 ++
tools/perf/util/evlist.c | 5 +-
tools/perf/util/evsel.c | 12 +-
tools/perf/util/evsel.h | 3 +-
tools/perf/util/parse-events-hybrid.c | 174 ++++++++++++++++
tools/perf/util/parse-events-hybrid.h | 23 +++
tools/perf/util/parse-events.c | 104 +++++++++-
tools/perf/util/parse-events.h | 9 +-
tools/perf/util/parse-events.y | 9 +-
tools/perf/util/pmu-hybrid.c | 75 +++++++
tools/perf/util/pmu-hybrid.h | 22 ++
tools/perf/util/pmu.c | 100 +++++++--
tools/perf/util/pmu.h | 9 +-
tools/perf/util/python-ext-sources | 2 +
tools/perf/util/stat-display.c | 33 ++-
33 files changed, 1234 insertions(+), 52 deletions(-)
create mode 100644 tools/perf/Documentation/intel-hybrid.txt
create mode 100644 tools/perf/util/evlist-hybrid.c
create mode 100644 tools/perf/util/evlist-hybrid.h
create mode 100644 tools/perf/util/parse-events-hybrid.c
create mode 100644 tools/perf/util/parse-events-hybrid.h
create mode 100644 tools/perf/util/pmu-hybrid.c
create mode 100644 tools/perf/util/pmu-hybrid.h
--
2.17.1
To get the changes in:
Liang Kan's patch
("perf: Introduce PERF_TYPE_HARDWARE_PMU and PERF_TYPE_HW_CACHE_PMU")
Kan's patch is in review at the moment, but the following perf tool
patches need this interface for hybrid support.
This patch can be removed after Kan's patch is upstreamed.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No change.
tools/include/uapi/linux/perf_event.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index ad15e40d7f5d..c0a511eea498 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -33,6 +33,8 @@ enum perf_type_id {
PERF_TYPE_HW_CACHE = 3,
PERF_TYPE_RAW = 4,
PERF_TYPE_BREAKPOINT = 5,
+ PERF_TYPE_HARDWARE_PMU = 6,
+ PERF_TYPE_HW_CACHE_PMU = 7,
PERF_TYPE_MAX, /* non-ABI */
};
@@ -94,6 +96,30 @@ enum perf_hw_cache_op_result_id {
PERF_COUNT_HW_CACHE_RESULT_MAX, /* non-ABI */
};
+/*
+ * attr.config layout for type PERF_TYPE_HARDWARE* and PERF_TYPE_HW_CACHE*
+ * PERF_TYPE_HARDWARE: 0xAA
+ * AA: hardware event ID
+ * PERF_TYPE_HW_CACHE: 0xCCBBAA
+ * AA: hardware cache ID
+ * BB: hardware cache op ID
+ * CC: hardware cache op result ID
+ * PERF_TYPE_HARDWARE_PMU: 0xDD000000AA
+ * AA: hardware event ID
+ * DD: PMU type ID
+ * PERF_TYPE_HW_CACHE_PMU: 0xDD00CCBBAA
+ * AA: hardware cache ID
+ * BB: hardware cache op ID
+ * CC: hardware cache op result ID
+ * DD: PMU type ID
+ */
+#define PERF_HW_CACHE_ID_SHIFT 0
+#define PERF_HW_CACHE_OP_ID_SHIFT 8
+#define PERF_HW_CACHE_OP_RESULT_ID_SHIFT 16
+#define PERF_HW_CACHE_EVENT_MASK 0xffffff
+
+#define PERF_PMU_TYPE_SHIFT 32
+
/*
* Special "software" events provided by the kernel, even if the hardware
* does not support performance events. These events measure various
--
2.17.1
For some Intel platforms, such as Alderlake, which is a hybrid platform
and it consists of atom cpu and core cpu. Each cpu has dedicated event
list. Part of events are available on core cpu, part of events are
available on atom cpu.
The kernel exports new cpu pmus: cpu_core and cpu_atom. The event in
json is added with a new field "Unit" to indicate which pmu the event
is available on.
For example, one event in cache.json,
{
"BriefDescription": "Counts the number of load ops retired that",
"CollectPEBSRecord": "2",
"Counter": "0,1,2,3",
"EventCode": "0xd2",
"EventName": "MEM_LOAD_UOPS_RETIRED_MISC.MMIO",
"PEBScounters": "0,1,2,3",
"SampleAfterValue": "1000003",
"UMask": "0x80",
"Unit": "cpu_atom"
},
The unit "cpu_atom" indicates this event is only availabe on "cpu_atom".
In generated pmu-events.c, we can see:
{
.name = "mem_load_uops_retired_misc.mmio",
.event = "period=1000003,umask=0x80,event=0xd2",
.desc = "Counts the number of load ops retired that. Unit: cpu_atom ",
.topic = "cache",
.pmu = "cpu_atom",
},
But if without this patch, the "uncore_" prefix is added before "cpu_atom",
such as:
.pmu = "uncore_cpu_atom"
That would be a wrong pmu.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No change.
tools/perf/pmu-events/jevents.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 33aa3c885eaf..ed4f0bd72e5a 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -285,6 +285,8 @@ static struct map {
{ "imx8_ddr", "imx8_ddr" },
{ "L3PMC", "amd_l3" },
{ "DFPMC", "amd_df" },
+ { "cpu_core", "cpu_core" },
+ { "cpu_atom", "cpu_atom" },
{}
};
--
2.17.1
Simplify the arguments of __perf_pmu__new_alias() by passing
the whole 'struct pme_event' pointer.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No change.
tools/perf/util/pmu.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 88da5cf6aee8..9ed9a6a8b2d2 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -306,18 +306,25 @@ static bool perf_pmu_merge_alias(struct perf_pmu_alias *newalias,
}
static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
- char *desc, char *val,
- char *long_desc, char *topic,
- char *unit, char *perpkg,
- char *metric_expr,
- char *metric_name,
- char *deprecated)
+ char *desc, char *val, struct pmu_event *pe)
{
struct parse_events_term *term;
struct perf_pmu_alias *alias;
int ret;
int num;
char newval[256];
+ char *long_desc = NULL, *topic = NULL, *unit = NULL, *perpkg = NULL,
+ *metric_expr = NULL, *metric_name = NULL, *deprecated = NULL;
+
+ if (pe) {
+ long_desc = (char *)pe->long_desc;
+ topic = (char *)pe->topic;
+ unit = (char *)pe->unit;
+ perpkg = (char *)pe->perpkg;
+ metric_expr = (char *)pe->metric_expr;
+ metric_name = (char *)pe->metric_name;
+ deprecated = (char *)pe->deprecated;
+ }
alias = malloc(sizeof(*alias));
if (!alias)
@@ -406,8 +413,7 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
/* Remove trailing newline from sysfs file */
strim(buf);
- return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL);
+ return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL);
}
static inline bool pmu_alias_info_file(char *name)
@@ -793,11 +799,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
/* need type casts to override 'const' */
__perf_pmu__new_alias(head, NULL, (char *)pe->name,
(char *)pe->desc, (char *)pe->event,
- (char *)pe->long_desc, (char *)pe->topic,
- (char *)pe->unit, (char *)pe->perpkg,
- (char *)pe->metric_expr,
- (char *)pe->metric_name,
- (char *)pe->deprecated);
+ pe);
}
}
@@ -864,13 +866,7 @@ static int pmu_add_sys_aliases_iter_fn(struct pmu_event *pe, void *data)
(char *)pe->name,
(char *)pe->desc,
(char *)pe->event,
- (char *)pe->long_desc,
- (char *)pe->topic,
- (char *)pe->unit,
- (char *)pe->perpkg,
- (char *)pe->metric_expr,
- (char *)pe->metric_name,
- (char *)pe->deprecated);
+ pe);
}
return 0;
--
2.17.1
On hybrid platform, one event is available on one pmu
(such as, available on cpu_core or on cpu_atom).
This patch saves the pmu name to the pmu field of struct perf_pmu_alias.
Then next we can know the pmu which the event can be enabled on.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Change pmu to pmu_name in struct perf_pmu_alias.
tools/perf/util/pmu.c | 10 +++++++++-
tools/perf/util/pmu.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9ed9a6a8b2d2..10709ec1cc3e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -283,6 +283,7 @@ void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
zfree(&newalias->str);
zfree(&newalias->metric_expr);
zfree(&newalias->metric_name);
+ zfree(&newalias->pmu_name);
parse_events_terms__purge(&newalias->terms);
free(newalias);
}
@@ -297,6 +298,10 @@ static bool perf_pmu_merge_alias(struct perf_pmu_alias *newalias,
list_for_each_entry(a, alist, list) {
if (!strcasecmp(newalias->name, a->name)) {
+ if (newalias->pmu_name && a->pmu_name &&
+ !strcasecmp(newalias->pmu_name, a->pmu_name)) {
+ continue;
+ }
perf_pmu_update_alias(a, newalias);
perf_pmu_free_alias(newalias);
return true;
@@ -314,7 +319,8 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
int num;
char newval[256];
char *long_desc = NULL, *topic = NULL, *unit = NULL, *perpkg = NULL,
- *metric_expr = NULL, *metric_name = NULL, *deprecated = NULL;
+ *metric_expr = NULL, *metric_name = NULL, *deprecated = NULL,
+ *pmu_name = NULL;
if (pe) {
long_desc = (char *)pe->long_desc;
@@ -324,6 +330,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
metric_expr = (char *)pe->metric_expr;
metric_name = (char *)pe->metric_name;
deprecated = (char *)pe->deprecated;
+ pmu_name = (char *)pe->pmu;
}
alias = malloc(sizeof(*alias));
@@ -389,6 +396,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
}
alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
alias->str = strdup(newval);
+ alias->pmu_name = pmu_name ? strdup(pmu_name) : NULL;
if (deprecated)
alias->deprecated = true;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 8164388478c6..8493b1719e10 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -72,6 +72,7 @@ struct perf_pmu_alias {
bool deprecated;
char *metric_expr;
char *metric_name;
+ char *pmu_name;
};
struct perf_pmu *perf_pmu__find(const char *name);
--
2.17.1
We identify the cpu_core pmu and cpu_atom pmu by explicitly
checking following files:
For cpu_core, checks:
"/sys/bus/event_source/devices/cpu_core/cpus"
For cpu_atom, checks:
"/sys/bus/event_source/devices/cpu_atom/cpus"
If the 'cpus' file exists, the pmu exists.
But in order not to hardcode the "cpu_core" and "cpu_atom",
and make the code in a generic way. So if the path
"/sys/bus/event_source/devices/cpu_xxx/cpus" exists, the hybrid
pmu exists. All the detected hybrid pmus are linked to a
global list 'perf_pmu__hybrid_pmus' and then next we just need
to iterate the list to get all hybrid pmu by using
perf_pmu__for_each_hybrid_pmu.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/util/Build | 1 +
tools/perf/util/pmu-hybrid.c | 35 +++++++++++++++++++++++++++++++++++
tools/perf/util/pmu-hybrid.h | 18 ++++++++++++++++++
tools/perf/util/pmu.c | 9 ++++++++-
tools/perf/util/pmu.h | 4 ++++
5 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/util/pmu-hybrid.c
create mode 100644 tools/perf/util/pmu-hybrid.h
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index e3e12f9d4733..37a8a63c7195 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -69,6 +69,7 @@ perf-y += parse-events-bison.o
perf-y += pmu.o
perf-y += pmu-flex.o
perf-y += pmu-bison.o
+perf-y += pmu-hybrid.o
perf-y += trace-event-read.o
perf-y += trace-event-info.o
perf-y += trace-event-scripting.o
diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c
new file mode 100644
index 000000000000..7316bf46e54b
--- /dev/null
+++ b/tools/perf/util/pmu-hybrid.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/list.h>
+#include <linux/compiler.h>
+#include <linux/string.h>
+#include <linux/zalloc.h>
+#include <sys/types.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <locale.h>
+#include <api/fs/fs.h>
+#include "fncache.h"
+#include "pmu-hybrid.h"
+
+LIST_HEAD(perf_pmu__hybrid_pmus);
+
+bool perf_pmu__hybrid_mounted(const char *name)
+{
+ char path[PATH_MAX];
+ const char *sysfs;
+
+ if (strncmp(name, "cpu_", 4))
+ return false;
+
+ sysfs = sysfs__mountpoint();
+ if (!sysfs)
+ return false;
+
+ snprintf(path, PATH_MAX, CPUS_TEMPLATE_CPU, sysfs, name);
+ return file_available(path);
+}
diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h
new file mode 100644
index 000000000000..35bed3714438
--- /dev/null
+++ b/tools/perf/util/pmu-hybrid.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PMU_HYBRID_H
+#define __PMU_HYBRID_H
+
+#include <linux/perf_event.h>
+#include <linux/compiler.h>
+#include <linux/list.h>
+#include <stdbool.h>
+#include "pmu.h"
+
+extern struct list_head perf_pmu__hybrid_pmus;
+
+#define perf_pmu__for_each_hybrid_pmu(pmu) \
+ list_for_each_entry(pmu, &perf_pmu__hybrid_pmus, hybrid_list)
+
+bool perf_pmu__hybrid_mounted(const char *name);
+
+#endif /* __PMU_HYBRID_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 10709ec1cc3e..35e9660c3904 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -25,6 +25,7 @@
#include "string2.h"
#include "strbuf.h"
#include "fncache.h"
+#include "pmu-hybrid.h"
struct perf_pmu perf_pmu__fake;
@@ -613,7 +614,6 @@ static struct perf_cpu_map *__pmu_cpumask(const char *path)
*/
#define SYS_TEMPLATE_ID "./bus/event_source/devices/%s/identifier"
#define CPUS_TEMPLATE_UNCORE "%s/bus/event_source/devices/%s/cpumask"
-#define CPUS_TEMPLATE_CPU "%s/bus/event_source/devices/%s/cpus"
static struct perf_cpu_map *pmu_cpumask(const char *name)
{
@@ -645,6 +645,9 @@ static bool pmu_is_uncore(const char *name)
char path[PATH_MAX];
const char *sysfs;
+ if (perf_pmu__hybrid_mounted(name))
+ return false;
+
sysfs = sysfs__mountpoint();
snprintf(path, PATH_MAX, CPUS_TEMPLATE_UNCORE, sysfs, name);
return file_available(path);
@@ -946,6 +949,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
pmu->is_uncore = pmu_is_uncore(name);
if (pmu->is_uncore)
pmu->id = pmu_id(name);
+ pmu->is_hybrid = perf_pmu__hybrid_mounted(name);
pmu->max_precise = pmu_max_precise(name);
pmu_add_cpu_aliases(&aliases, pmu);
pmu_add_sys_aliases(&aliases, pmu);
@@ -957,6 +961,9 @@ static struct perf_pmu *pmu_lookup(const char *name)
list_splice(&aliases, &pmu->aliases);
list_add_tail(&pmu->list, &pmus);
+ if (pmu->is_hybrid)
+ list_add_tail(&pmu->hybrid_list, &perf_pmu__hybrid_pmus);
+
pmu->default_config = perf_pmu__get_default_config(pmu);
return pmu;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 8493b1719e10..29289e7c2649 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -5,6 +5,7 @@
#include <linux/bitmap.h>
#include <linux/compiler.h>
#include <linux/perf_event.h>
+#include <linux/list.h>
#include <stdbool.h>
#include "parse-events.h"
#include "pmu-events/pmu-events.h"
@@ -19,6 +20,7 @@ enum {
#define PERF_PMU_FORMAT_BITS 64
#define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
+#define CPUS_TEMPLATE_CPU "%s/bus/event_source/devices/%s/cpus"
struct perf_event_attr;
@@ -34,6 +36,7 @@ struct perf_pmu {
__u32 type;
bool selectable;
bool is_uncore;
+ bool is_hybrid;
bool auxtrace;
int max_precise;
struct perf_event_attr *default_config;
@@ -42,6 +45,7 @@ struct perf_pmu {
struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
struct list_head caps; /* HEAD struct perf_pmu_caps -> list */
struct list_head list; /* ELEM */
+ struct list_head hybrid_list;
};
extern struct perf_pmu perf_pmu__fake;
--
2.17.1
It would be useful to tell user the pmu which the event belongs to.
perf-stat has supported '--no-merge' option and it can print the pmu
name after the event name, such as:
"cycles [cpu_core]"
Now this option is enabled by default for hybrid platform but change
the format to:
"cpu_core/cycles/"
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/builtin-stat.c | 4 ++++
tools/perf/util/stat-display.c | 13 +++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 4bb48c6b6698..7b2dfe21c5a8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -68,6 +68,7 @@
#include "util/affinity.h"
#include "util/pfm.h"
#include "util/bpf_counter.h"
+#include "util/pmu-hybrid.h"
#include "asm/bug.h"
#include <linux/time64.h>
@@ -2371,6 +2372,9 @@ int cmd_stat(int argc, const char **argv)
evlist__check_cpu_maps(evsel_list);
+ if (perf_pmu__has_hybrid())
+ stat_config.no_merge = true;
+
/*
* Initialize thread_map with comm names,
* so we could print it out on output.
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 7f09cdaf5b60..161826938a00 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -17,6 +17,7 @@
#include "cgroup.h"
#include <api/fs/fs.h>
#include "util.h"
+#include "pmu-hybrid.h"
#define CNTR_NOT_SUPPORTED "<not supported>"
#define CNTR_NOT_COUNTED "<not counted>"
@@ -526,6 +527,7 @@ static void uniquify_event_name(struct evsel *counter)
{
char *new_name;
char *config;
+ int ret;
if (counter->uniquified_name ||
!counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
@@ -540,8 +542,15 @@ static void uniquify_event_name(struct evsel *counter)
counter->name = new_name;
}
} else {
- if (asprintf(&new_name,
- "%s [%s]", counter->name, counter->pmu_name) > 0) {
+ if (perf_pmu__has_hybrid()) {
+ ret = asprintf(&new_name, "%s/%s/",
+ counter->pmu_name, counter->name);
+ } else {
+ ret = asprintf(&new_name, "%s [%s]",
+ counter->name, counter->pmu_name);
+ }
+
+ if (ret) {
free(counter->name);
counter->name = new_name;
}
--
2.17.1
On hybrid platform, user may want to enable the hardware event
only on one PMU. So following syntax is supported:
cpu_core/<hardware event>/
cpu_atom/<hardware event>/
# perf stat -e cpu_core/cpu-cycles/ -a -- sleep 1
Performance counter stats for 'system wide':
6,049,336 cpu_core/cpu-cycles/
1.003577042 seconds time elapsed
It enables the event 'cpu-cycles' only on cpu_core pmu.
But for 'cycles' and 'branches', the syntax doesn't work.
Before:
# perf stat -e cpu_core/cycles/ -a -- sleep 1
event syntax error: 'cpu_core/cycles/'
\___ unknown term 'cycles' for pmu 'cpu_core'
# perf stat -e cpu_core/branches/ -a -- sleep 1
event syntax error: 'cpu_core/branches/'
\___ unknown term 'branches' for pmu 'cpu_core'
For 'cpu-cycles', why it works is because the event is defined in
/sys/devices/cpu_core/events/. It's added as alias by
pmu_add_sys_aliases and it's treated as 'event' when parsing the
term->config.
We use a similar idea, create a pme_hybrid_fixup table for
'cycles' and 'branches' and add them as aliases.
After:
# perf stat -e cpu_core/cycles/ -a -- sleep 1
Performance counter stats for 'system wide':
5,769,631 cpu_core/cycles/
1.003833235 seconds time elapsed
# perf stat -e cpu_core/branches/ -a -- sleep 1
Performance counter stats for 'system wide':
490,951 cpu_core/branches/
1.003693946 seconds time elapsed
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- New patch in v3.
tools/perf/util/pmu.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index beff29981101..72e5ae5e868e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -916,6 +916,35 @@ static int pmu_max_precise(const char *name)
return max_precise;
}
+static void perf_pmu__add_hybrid_aliases(struct list_head *head)
+{
+ static struct pmu_event pme_hybrid_fixup[] = {
+ {
+ .name = "cycles",
+ .event = "event=0x3c",
+ },
+ {
+ .name = "branches",
+ .event = "event=0xc4",
+ },
+ {
+ .name = 0,
+ .event = 0,
+ },
+ };
+ int i = 0;
+
+ while (1) {
+ struct pmu_event *pe = &pme_hybrid_fixup[i++];
+
+ if (!pe->name)
+ break;
+
+ __perf_pmu__new_alias(head, NULL, (char *)pe->name, NULL,
+ (char *)pe->event, NULL);
+ }
+}
+
static struct perf_pmu *pmu_lookup(const char *name)
{
struct perf_pmu *pmu;
@@ -955,6 +984,9 @@ static struct perf_pmu *pmu_lookup(const char *name)
pmu_add_cpu_aliases(&aliases, pmu);
pmu_add_sys_aliases(&aliases, pmu);
+ if (pmu->is_hybrid)
+ perf_pmu__add_hybrid_aliases(&aliases);
+
INIT_LIST_HEAD(&pmu->format);
INIT_LIST_HEAD(&pmu->aliases);
INIT_LIST_HEAD(&pmu->caps);
--
2.17.1
For cache events, they have pre-defined configs. The kernel needs
to know where the cache event comes from (e.g. from cpu_core pmu
or from cpu_atom pmu). But the perf type 'PERF_TYPE_HW_CACHE'
can't carry pmu information.
So the kernel introduces a new type 'PERF_TYPE_HW_CACHE_PMU'.
The new attr.config layout for PERF_TYPE_HW_CACHE_PMU is
0xDD00CCBBAA
AA: hardware cache ID
BB: hardware cache op ID
CC: hardware cache op result ID
DD: PMU type ID
Similar as hardware event, PMU type ID is retrieved from sysfs.
When enabling a hybrid cache event without specified pmu, such as,
'perf stat -e L1-dcache-loads -a', two events are created
automatically. One is for atom, the other is for core.
# ./perf stat -e L1-dcache-loads -vv -a -- sleep 1
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 3
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 4
------------------------------------------------------------
......
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 15 group_fd -1 flags 0x8 = 19
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 16 group_fd -1 flags 0x8 = 20
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 17 group_fd -1 flags 0x8 = 21
------------------------------------------------------------
......
------------------------------------------------------------
perf_event_attr:
type 7
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 23 group_fd -1 flags 0x8 = 27
L1-dcache-loads: 0: 12071 1001549888 1001549888
L1-dcache-loads: 1: 12704 1001602818 1001602818
L1-dcache-loads: 2: 21560 1001687258 1001687258
L1-dcache-loads: 3: 8104 1001727825 1001727825
L1-dcache-loads: 4: 7855 1001738995 1001738995
L1-dcache-loads: 5: 7714 1001766241 1001766241
L1-dcache-loads: 6: 7704 1001790333 1001790333
L1-dcache-loads: 7: 58026 1001826950 1001826950
L1-dcache-loads: 8: 107639 1001641286 1001641286
L1-dcache-loads: 9: 67013 1001682357 1001682357
L1-dcache-loads: 10: 17137 1001700068 1001700068
L1-dcache-loads: 11: 86449 1001696907 1001696907
L1-dcache-loads: 12: 7723 1001716599 1001716599
L1-dcache-loads: 13: 10211 1001743285 1001743285
L1-dcache-loads: 14: 13023 1001765343 1001765343
L1-dcache-loads: 15: 8991 1001788673 1001788673
L1-dcache-loads: 0: 240163 1001800830 1001800830
L1-dcache-loads: 1: 7454 1001773983 1001773983
L1-dcache-loads: 2: 32323 1001686339 1001686339
L1-dcache-loads: 3: 11039 1001732430 1001732430
L1-dcache-loads: 4: 52867 1001753753 1001753753
L1-dcache-loads: 5: 7481 1001756879 1001756879
L1-dcache-loads: 6: 7471 1001814616 1001814616
L1-dcache-loads: 7: 29627 1001815092 1001815092
L1-dcache-loads: 453924 16027424826 16027424826
L1-dcache-loads: 388425 8014133922 8014133922
Performance counter stats for 'system wide':
453,924 cpu_core/L1-dcache-loads/
388,425 cpu_atom/L1-dcache-loads/
1.003644499 seconds time elapsed
type 7 is PERF_TYPE_HW_CACHE_PMU.
0x4 in 0x400000000 indicates the cpu_core pmu.
0xa in 0xa00000000 indicates the cpu_atom pmu.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Hybrid cache event creation is moved to parse-events-hybrid.c.
tools/perf/util/parse-events-hybrid.c | 23 +++++++++++++++++++++++
tools/perf/util/parse-events-hybrid.h | 5 +++++
tools/perf/util/parse-events.c | 8 ++++++++
3 files changed, 36 insertions(+)
diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
index bd48563596e0..ff2909bfbf86 100644
--- a/tools/perf/util/parse-events-hybrid.c
+++ b/tools/perf/util/parse-events-hybrid.c
@@ -95,3 +95,26 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
return -1;
}
+
+int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
+ struct perf_event_attr *attr, char *name,
+ struct list_head *config_terms,
+ bool *hybrid)
+{
+ struct perf_pmu *pmu;
+ int ret;
+
+ *hybrid = false;
+ if (!perf_pmu__has_hybrid())
+ return 0;
+
+ *hybrid = true;
+ perf_pmu__for_each_hybrid_pmu(pmu) {
+ ret = create_event_hybrid(PERF_TYPE_HW_CACHE_PMU, idx, list,
+ attr, name, config_terms, pmu);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h
index d81a76978480..9ad33cd0cef4 100644
--- a/tools/perf/util/parse-events-hybrid.h
+++ b/tools/perf/util/parse-events-hybrid.h
@@ -14,4 +14,9 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
char *name, struct list_head *config_terms,
bool *hybrid);
+int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
+ struct perf_event_attr *attr, char *name,
+ struct list_head *config_terms,
+ bool *hybrid);
+
#endif /* __PERF_PARSE_EVENTS_HYBRID_H */
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1bbd0ba92ba7..3692fa3c964a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -458,6 +458,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
int cache_type = -1, cache_op = -1, cache_result = -1;
char *op_result[2] = { op_result1, op_result2 };
int i, n;
+ bool hybrid;
/*
* No fallback - if we cannot get a clear cache type
@@ -517,6 +518,13 @@ int parse_events_add_cache(struct list_head *list, int *idx,
if (get_config_terms(head_config, &config_terms))
return -ENOMEM;
}
+
+ i = parse_events__add_cache_hybrid(list, idx, &attr,
+ config_name ? : name, &config_terms,
+ &hybrid);
+ if (hybrid)
+ return i;
+
return add_event(list, idx, &attr, config_name ? : name, &config_terms);
}
--
2.17.1
For hardware events, they have pre-defined configs. The kernel
needs to know where the event comes from (e.g. from cpu_core pmu
or from cpu_atom pmu). But the perf type 'PERF_TYPE_HARDWARE'
can't carry pmu information.
So the kernel introduces a new type 'PERF_TYPE_HARDWARE_PMU'.
The new attr.config layout for PERF_TYPE_HARDWARE_PMU is:
0xDD000000AA
AA: original hardware event ID
DD: PMU type ID
PMU type ID is retrieved from sysfs. For example,
cat /sys/devices/cpu_atom/type
10
cat /sys/devices/cpu_core/type
4
When enabling a hybrid hardware event without specified pmu, such as,
'perf stat -e cycles -a', two events are created automatically. One
is for atom, the other is for core.
# ./perf stat -e cycles -a -vv -- sleep 1
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 3
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 4
------------------------------------------------------------
......
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0x400000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 15 group_fd -1 flags 0x8 = 19
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 16 group_fd -1 flags 0x8 = 20
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 17 group_fd -1 flags 0x8 = 21
------------------------------------------------------------
......
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 22 group_fd -1 flags 0x8 = 26
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0xa00000000
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 23 group_fd -1 flags 0x8 = 27
cycles: 0: 402953 1001514714 1001514714
cycles: 1: 137456 1001554851 1001554851
cycles: 2: 130449 1001575948 1001575948
cycles: 3: 61370 1001600256 1001600256
cycles: 4: 100084 1001614135 1001614135
cycles: 5: 62549 1001646802 1001646802
cycles: 6: 453760 1001703487 1001703487
cycles: 7: 66527 1001739138 1001739138
cycles: 8: 80526 1001823867 1001823867
cycles: 9: 74942 1001863884 1001863884
cycles: 10: 322356 1001952832 1001952832
cycles: 11: 1681751 1001846058 1001846058
cycles: 12: 97608 1001874578 1001874578
cycles: 13: 62060 1001899200 1001899200
cycles: 14: 546496 1001920608 1001920608
cycles: 15: 65631 1001939206 1001939206
cycles: 0: 59901 1002047096 1002047096
cycles: 1: 57304 1001995373 1001995373
cycles: 2: 781291 1002027732 1002027732
cycles: 3: 99656 1002058466 1002058466
cycles: 4: 95071 1002092749 1002092749
cycles: 5: 346827 1002142979 1002142979
cycles: 6: 183967 1002183046 1002183046
cycles: 7: 391779 1002218286 1002218286
cycles: 4346518 16028069564 16028069564
cycles: 2015796 8016765727 8016765727
Performance counter stats for 'system wide':
4,346,518 cpu_core/cycles/
2,015,796 cpu_atom/cycles/
1.003685897 seconds time elapsed
type 6 is PERF_TYPE_HARDWARE_PMU.
0x4 in 0x400000000 indicates the cpu_core pmu.
0xa in 0xa00000000 indicates the cpu_atom pmu.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Create new parse-events-hybrid.c/parse-events-hybrid.h
- Refine the code
tools/perf/util/Build | 1 +
tools/perf/util/parse-events-hybrid.c | 97 +++++++++++++++++++++++++++
tools/perf/util/parse-events-hybrid.h | 17 +++++
tools/perf/util/parse-events.c | 18 +++++
tools/perf/util/parse-events.h | 5 ++
5 files changed, 138 insertions(+)
create mode 100644 tools/perf/util/parse-events-hybrid.c
create mode 100644 tools/perf/util/parse-events-hybrid.h
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 37a8a63c7195..00c9fb064ba6 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -23,6 +23,7 @@ perf-y += llvm-utils.o
perf-y += mmap.o
perf-y += memswap.o
perf-y += parse-events.o
+perf-y += parse-events-hybrid.o
perf-y += perf_regs.o
perf-y += path.o
perf-y += print_binary.o
diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
new file mode 100644
index 000000000000..bd48563596e0
--- /dev/null
+++ b/tools/perf/util/parse-events-hybrid.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/err.h>
+#include <linux/zalloc.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/param.h>
+#include "evlist.h"
+#include "evsel.h"
+#include "parse-events.h"
+#include "parse-events-hybrid.h"
+#include "debug.h"
+#include "pmu.h"
+#include "pmu-hybrid.h"
+#include "perf.h"
+
+static void config_hybrid_attr(struct perf_event_attr *attr,
+ int type, int pmu_type)
+{
+ /*
+ * attr.config layout:
+ * PERF_TYPE_HARDWARE_PMU: 0xDD000000AA
+ * AA: hardware event ID
+ * DD: PMU type ID
+ * PERF_TYPE_HW_CACHE_PMU: 0xDD00CCBBAA
+ * AA: hardware cache ID
+ * BB: hardware cache op ID
+ * CC: hardware cache op result ID
+ * DD: PMU type ID
+ */
+ attr->type = type;
+ attr->config = attr->config | ((__u64)pmu_type << PERF_PMU_TYPE_SHIFT);
+}
+
+static int create_event_hybrid(__u32 config_type, int *idx,
+ struct list_head *list,
+ struct perf_event_attr *attr, char *name,
+ struct list_head *config_terms,
+ struct perf_pmu *pmu)
+{
+ struct evsel *evsel;
+ __u32 type = attr->type;
+ __u64 config = attr->config;
+
+ config_hybrid_attr(attr, config_type, pmu->type);
+ evsel = parse_events__add_event_hybrid(list, idx, attr, name,
+ pmu, config_terms);
+ if (evsel)
+ evsel->pmu_name = strdup(pmu->name);
+ else
+ return -ENOMEM;
+
+ attr->type = type;
+ attr->config = config;
+ return 0;
+}
+
+static int add_hw_hybrid(struct parse_events_state *parse_state,
+ struct list_head *list, struct perf_event_attr *attr,
+ char *name, struct list_head *config_terms)
+{
+ struct perf_pmu *pmu;
+ int ret;
+
+ perf_pmu__for_each_hybrid_pmu(pmu) {
+ ret = create_event_hybrid(PERF_TYPE_HARDWARE_PMU,
+ &parse_state->idx, list, attr, name,
+ config_terms, pmu);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
+ struct list_head *list,
+ struct perf_event_attr *attr,
+ char *name, struct list_head *config_terms,
+ bool *hybrid)
+{
+ *hybrid = false;
+ if (attr->type == PERF_TYPE_SOFTWARE)
+ return 0;
+
+ if (!perf_pmu__has_hybrid())
+ return 0;
+
+ *hybrid = true;
+ if (attr->type != PERF_TYPE_RAW) {
+ return add_hw_hybrid(parse_state, list, attr, name,
+ config_terms);
+ }
+
+ return -1;
+}
diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h
new file mode 100644
index 000000000000..d81a76978480
--- /dev/null
+++ b/tools/perf/util/parse-events-hybrid.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_PARSE_EVENTS_HYBRID_H
+#define __PERF_PARSE_EVENTS_HYBRID_H
+
+#include <linux/list.h>
+#include <stdbool.h>
+#include <linux/types.h>
+#include <linux/perf_event.h>
+#include <string.h>
+
+int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
+ struct list_head *list,
+ struct perf_event_attr *attr,
+ char *name, struct list_head *config_terms,
+ bool *hybrid);
+
+#endif /* __PERF_PARSE_EVENTS_HYBRID_H */
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9ecb45bea948..1bbd0ba92ba7 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -37,6 +37,7 @@
#include "util/evsel_config.h"
#include "util/event.h"
#include "util/pfm.h"
+#include "util/parse-events-hybrid.h"
#include "perf.h"
#define MAX_NAME_LEN 100
@@ -1416,6 +1417,8 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
{
struct perf_event_attr attr;
LIST_HEAD(config_terms);
+ bool hybrid;
+ int ret;
memset(&attr, 0, sizeof(attr));
attr.type = type;
@@ -1430,6 +1433,12 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
return -ENOMEM;
}
+ ret = parse_events__add_numeric_hybrid(parse_state, list, &attr,
+ get_config_name(head_config),
+ &config_terms, &hybrid);
+ if (hybrid)
+ return ret;
+
return add_event(list, &parse_state->idx, &attr,
get_config_name(head_config), &config_terms);
}
@@ -3182,3 +3191,12 @@ char *parse_events_formats_error_string(char *additional_terms)
fail:
return NULL;
}
+
+struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx,
+ struct perf_event_attr *attr,
+ char *name, struct perf_pmu *pmu,
+ struct list_head *config_terms)
+{
+ return __add_event(list, idx, attr, true, name, pmu,
+ config_terms, false, NULL);
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index e80c9b74f2f2..c4f2f96304ce 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -263,4 +263,9 @@ static inline bool is_sdt_event(char *str __maybe_unused)
int perf_pmu__test_parse_init(void);
+struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx,
+ struct perf_event_attr *attr,
+ char *name, struct perf_pmu *pmu,
+ struct list_head *config_terms);
+
#endif /* __PERF_PARSE_EVENTS_H */
--
2.17.1
On hybrid platform, similar to hardware event, user may want
to enable other events only on one PMU. So following syntax
should be supported:
cpu_core/<event>/
cpu_atom/<event>/
But the syntax doesn't work for some events, such as cache
event.
Before:
# perf stat -e cpu_core/LLC-loads/ -a -- sleep 1
event syntax error: 'cpu_core/LLC-loads/'
\___ unknown term 'LLC-loads' for pmu 'cpu_core'
Cache events are much more complex than hardware events, so
we can't create aliases for them. We use another solution.
For example, if we use "cpu_core/LLC-loads/", in parse_events_add_pmu(),
term->config is "LLC-loads".
We create a new "parse_events_state" with the pmu_name and use
parse_events__scanner to scan the term->config ("LLC-loads" in
this example). The parse_events_add_cache() would be called during
parsing. The parse_state->pmu_name is used to identify the pmu
where the event should be enabled on.
After:
# ./perf stat -e cpu_core/LLC-loads/ -a -- sleep 1
Performance counter stats for 'system wide':
24,593 cpu_core/LLC-loads/
1.003911601 seconds time elapsed
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Rename the patch:
'perf parse-events: Support hardware events inside PMU' -->
'perf parse-events: Support no alias assigned event inside hybrid PMU'
- Major code is moved to parse-events-hybrid.c.
- Refine the code.
tools/perf/util/parse-events-hybrid.c | 18 +++++-
tools/perf/util/parse-events-hybrid.h | 3 +-
tools/perf/util/parse-events.c | 80 +++++++++++++++++++++++++--
tools/perf/util/parse-events.h | 4 +-
tools/perf/util/parse-events.y | 9 ++-
tools/perf/util/pmu.c | 4 +-
tools/perf/util/pmu.h | 2 +-
7 files changed, 108 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
index 8a630cbab8f3..5bf176b55573 100644
--- a/tools/perf/util/parse-events-hybrid.c
+++ b/tools/perf/util/parse-events-hybrid.c
@@ -64,6 +64,11 @@ static int add_hw_hybrid(struct parse_events_state *parse_state,
int ret;
perf_pmu__for_each_hybrid_pmu(pmu) {
+ if (parse_state->pmu_name &&
+ strcmp(parse_state->pmu_name, pmu->name)) {
+ continue;
+ }
+
ret = create_event_hybrid(PERF_TYPE_HARDWARE_PMU,
&parse_state->idx, list, attr, name,
config_terms, pmu);
@@ -100,6 +105,11 @@ static int add_raw_hybrid(struct parse_events_state *parse_state,
int ret;
perf_pmu__for_each_hybrid_pmu(pmu) {
+ if (parse_state->pmu_name &&
+ strcmp(parse_state->pmu_name, pmu->name)) {
+ continue;
+ }
+
ret = create_raw_event_hybrid(&parse_state->idx, list, attr,
name, config_terms, pmu);
if (ret)
@@ -137,7 +147,8 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
struct perf_event_attr *attr, char *name,
struct list_head *config_terms,
- bool *hybrid)
+ bool *hybrid,
+ struct parse_events_state *parse_state)
{
struct perf_pmu *pmu;
int ret;
@@ -148,6 +159,11 @@ int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
*hybrid = true;
perf_pmu__for_each_hybrid_pmu(pmu) {
+ if (parse_state->pmu_name &&
+ strcmp(parse_state->pmu_name, pmu->name)) {
+ continue;
+ }
+
ret = create_event_hybrid(PERF_TYPE_HW_CACHE_PMU, idx, list,
attr, name, config_terms, pmu);
if (ret)
diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h
index 9ad33cd0cef4..f33bd67aa851 100644
--- a/tools/perf/util/parse-events-hybrid.h
+++ b/tools/perf/util/parse-events-hybrid.h
@@ -17,6 +17,7 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
int parse_events__add_cache_hybrid(struct list_head *list, int *idx,
struct perf_event_attr *attr, char *name,
struct list_head *config_terms,
- bool *hybrid);
+ bool *hybrid,
+ struct parse_events_state *parse_state);
#endif /* __PERF_PARSE_EVENTS_HYBRID_H */
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3692fa3c964a..020c785be742 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -38,6 +38,7 @@
#include "util/event.h"
#include "util/pfm.h"
#include "util/parse-events-hybrid.h"
+#include "util/pmu-hybrid.h"
#include "perf.h"
#define MAX_NAME_LEN 100
@@ -48,6 +49,9 @@ extern int parse_events_debug;
int parse_events_parse(void *parse_state, void *scanner);
static int get_config_terms(struct list_head *head_config,
struct list_head *head_terms __maybe_unused);
+static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
+ const char *str, char *pmu_name,
+ bool *found, struct list_head *list);
static struct perf_pmu_event_symbol *perf_pmu_events_list;
/*
@@ -450,7 +454,8 @@ static int config_attr(struct perf_event_attr *attr,
int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2,
struct parse_events_error *err,
- struct list_head *head_config)
+ struct list_head *head_config,
+ struct parse_events_state *parse_state)
{
struct perf_event_attr attr;
LIST_HEAD(config_terms);
@@ -521,7 +526,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
i = parse_events__add_cache_hybrid(list, idx, &attr,
config_name ? : name, &config_terms,
- &hybrid);
+ &hybrid, parse_state);
if (hybrid)
return i;
@@ -1481,7 +1486,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
struct perf_pmu *pmu;
struct evsel *evsel;
struct parse_events_error *err = parse_state->error;
- bool use_uncore_alias;
+ bool use_uncore_alias, found = false;
LIST_HEAD(config_terms);
if (verbose > 1) {
@@ -1530,8 +1535,28 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
}
}
- if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, &info))
+ if (!parse_state->fake_pmu &&
+ perf_pmu__check_alias(pmu, head_config, &info, &found)) {
return -EINVAL;
+ }
+
+ if (!parse_state->fake_pmu && head_config && !found &&
+ perf_pmu__is_hybrid(name)) {
+ struct parse_events_term *term;
+ int ret;
+
+ list_for_each_entry(term, head_config, list) {
+ if (!term->config)
+ continue;
+
+ ret = parse_events__with_hybrid_pmu(parse_state,
+ term->config,
+ name, &found,
+ list);
+ if (found)
+ return ret;
+ }
+ }
if (verbose > 1) {
fprintf(stderr, "After aliases, add event pmu '%s' with '",
@@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
struct perf_pmu *pmu = NULL;
int ok = 0;
+ if (parse_state->pmu_name) {
+ list = malloc(sizeof(struct list_head));
+ if (!list)
+ return -1;
+ INIT_LIST_HEAD(list);
+ *listp = list;
+ return 0;
+ }
+
*listp = NULL;
/* Add it for all PMUs that support the alias */
list = malloc(sizeof(struct list_head));
@@ -2176,6 +2210,44 @@ int parse_events_terms(struct list_head *terms, const char *str)
return ret;
}
+static int list_entries_nr(struct list_head *list)
+{
+ struct list_head *pos;
+ int n = 0;
+
+ list_for_each(pos, list)
+ n++;
+
+ return n;
+}
+
+static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
+ const char *str, char *pmu_name,
+ bool *found, struct list_head *list)
+{
+ struct parse_events_state ps = {
+ .list = LIST_HEAD_INIT(ps.list),
+ .stoken = PE_START_EVENTS,
+ .pmu_name = pmu_name,
+ .idx = parse_state->idx,
+ };
+ int ret;
+
+ *found = false;
+ ret = parse_events__scanner(str, &ps);
+ perf_pmu__parse_cleanup();
+
+ if (!ret) {
+ if (!list_empty(&ps.list)) {
+ *found = true;
+ list_splice(&ps.list, list);
+ parse_state->idx = list_entries_nr(list);
+ }
+ }
+
+ return ret;
+}
+
int __parse_events(struct evlist *evlist, const char *str,
struct parse_events_error *err, struct perf_pmu *fake_pmu)
{
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index c4f2f96304ce..f9d8e8e41c38 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -138,6 +138,7 @@ struct parse_events_state {
struct list_head *terms;
int stoken;
struct perf_pmu *fake_pmu;
+ char *pmu_name;
};
void parse_events__handle_error(struct parse_events_error *err, int idx,
@@ -188,7 +189,8 @@ int parse_events_add_tool(struct parse_events_state *parse_state,
int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2,
struct parse_events_error *error,
- struct list_head *head_config);
+ struct list_head *head_config,
+ struct parse_events_state *parse_state);
int parse_events_add_breakpoint(struct list_head *list, int *idx,
u64 addr, char *type, u64 len);
int parse_events_add_pmu(struct parse_events_state *parse_state,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index d57ac86ce7ca..aba12a4d488e 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -454,7 +454,8 @@ PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT opt_e
list = alloc_list();
ABORT_ON(!list);
- err = parse_events_add_cache(list, &parse_state->idx, $1, $3, $5, error, $6);
+ err = parse_events_add_cache(list, &parse_state->idx, $1, $3, $5, error, $6,
+ parse_state);
parse_events_terms__delete($6);
free($1);
free($3);
@@ -475,7 +476,8 @@ PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT opt_event_config
list = alloc_list();
ABORT_ON(!list);
- err = parse_events_add_cache(list, &parse_state->idx, $1, $3, NULL, error, $4);
+ err = parse_events_add_cache(list, &parse_state->idx, $1, $3, NULL, error, $4,
+ parse_state);
parse_events_terms__delete($4);
free($1);
free($3);
@@ -495,7 +497,8 @@ PE_NAME_CACHE_TYPE opt_event_config
list = alloc_list();
ABORT_ON(!list);
- err = parse_events_add_cache(list, &parse_state->idx, $1, NULL, NULL, error, $2);
+ err = parse_events_add_cache(list, &parse_state->idx, $1, NULL, NULL, error, $2,
+ parse_state);
parse_events_terms__delete($2);
free($1);
if (err) {
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 72e5ae5e868e..7167ae88e0db 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1412,7 +1412,7 @@ static int check_info_data(struct perf_pmu_alias *alias,
* defined for the alias
*/
int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
- struct perf_pmu_info *info)
+ struct perf_pmu_info *info, bool *found)
{
struct parse_events_term *term, *h;
struct perf_pmu_alias *alias;
@@ -1430,6 +1430,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
info->metric_expr = NULL;
info->metric_name = NULL;
+ *found = false;
list_for_each_entry_safe(term, h, head_terms, list) {
alias = pmu_find_alias(pmu, term);
if (!alias)
@@ -1438,6 +1439,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
if (ret)
return ret;
+ *found = true;
ret = check_info_data(alias, info);
if (ret)
return ret;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 5ed2abab7fe0..381a2e94fbba 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -91,7 +91,7 @@ int perf_pmu__config_terms(const char *pmu_name, struct list_head *formats,
__u64 perf_pmu__format_bits(struct list_head *formats, const char *name);
int perf_pmu__format_type(struct list_head *formats, const char *name);
int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
- struct perf_pmu_info *info);
+ struct perf_pmu_info *info, bool *found);
struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
struct list_head *head_terms);
void perf_pmu_error(struct list_head *list, char *name, char const *msg);
--
2.17.1
On hybrid platform, same raw event is possible to be available on
both cpu_core pmu and cpu_atom pmu. So it's supported to create
two raw events for one event encoding.
# ./perf stat -e r3c -a -vv -- sleep 1
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 4
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 3
------------------------------------------------------------
perf_event_attr:
type 4
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 4
------------------------------------------------------------
...
------------------------------------------------------------
perf_event_attr:
type 4
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 15 group_fd -1 flags 0x8 = 19
------------------------------------------------------------
perf_event_attr:
type 10
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 16 group_fd -1 flags 0x8 = 20
------------------------------------------------------------
perf_event_attr:
type 10
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 17 group_fd -1 flags 0x8 = 21
------------------------------------------------------------
...
------------------------------------------------------------
perf_event_attr:
type 10
size 120
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 23 group_fd -1 flags 0x8 = 27
r3c: 0: 295983 1001500321 1001500321
r3c: 1: 66453 1001536385 1001536385
r3c: 2: 156191 1001559687 1001559687
r3c: 3: 177709 1001590937 1001590937
r3c: 4: 110819 1001674167 1001674167
r3c: 5: 368516 1001693927 1001693927
r3c: 6: 185745 1001782896 1001782896
r3c: 7: 735332 1001567982 1001567982
r3c: 8: 1783115 1001595406 1001595406
r3c: 9: 90088 1001606000 1001606000
r3c: 10: 409253 1001697624 1001697624
r3c: 11: 2942563 1001721371 1001721371
r3c: 12: 100495 1001740753 1001740753
r3c: 13: 62970 1001769269 1001769269
r3c: 14: 201978 1001799810 1001799810
r3c: 15: 1401633 1001829314 1001829314
r3c: 0: 328088 1001962968 1001962968
r3c: 1: 257037 1001990202 1001990202
r3c: 2: 168703 1002065267 1002065267
r3c: 3: 187959 1002115120 1002115120
r3c: 4: 126987 1002151672 1002151672
r3c: 5: 79831 1002202614 1002202614
r3c: 6: 72101 1002214094 1002214094
r3c: 7: 442006 1002248710 1002248710
r3c: 9088843 16026665849 16026665849
r3c: 1662712 8016950647 8016950647
Performance counter stats for 'system wide':
9,088,843 cpu_core/r3c/
1,662,712 cpu_atom/r3c/
1.003722876 seconds time elapsed
type 4 is cpu_core pmu type.
type 10 is cpu_atom pmu type.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Raw event creation is moved to parse-events-hybrid.c.
tools/perf/util/parse-events-hybrid.c | 38 +++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
index ff2909bfbf86..8a630cbab8f3 100644
--- a/tools/perf/util/parse-events-hybrid.c
+++ b/tools/perf/util/parse-events-hybrid.c
@@ -74,6 +74,41 @@ static int add_hw_hybrid(struct parse_events_state *parse_state,
return 0;
}
+static int create_raw_event_hybrid(int *idx, struct list_head *list,
+ struct perf_event_attr *attr, char *name,
+ struct list_head *config_terms,
+ struct perf_pmu *pmu)
+{
+ struct evsel *evsel;
+
+ attr->type = pmu->type;
+ evsel = parse_events__add_event_hybrid(list, idx, attr, name,
+ pmu, config_terms);
+ if (evsel)
+ evsel->pmu_name = strdup(pmu->name);
+ else
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int add_raw_hybrid(struct parse_events_state *parse_state,
+ struct list_head *list, struct perf_event_attr *attr,
+ char *name, struct list_head *config_terms)
+{
+ struct perf_pmu *pmu;
+ int ret;
+
+ perf_pmu__for_each_hybrid_pmu(pmu) {
+ ret = create_raw_event_hybrid(&parse_state->idx, list, attr,
+ name, config_terms, pmu);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
struct list_head *list,
struct perf_event_attr *attr,
@@ -91,6 +126,9 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
if (attr->type != PERF_TYPE_RAW) {
return add_hw_hybrid(parse_state, list, attr, name,
config_terms);
+ } else {
+ return add_raw_hybrid(parse_state, list, attr, name,
+ config_terms);
}
return -1;
--
2.17.1
When evlist is empty, for example no '-e' specified in perf record,
one default 'cycles' event is added to evlist.
While on hybrid platform, it needs to create two default 'cycles'
events. One is for cpu_core, the other is for cpu_atom.
This patch actually calls evsel__new_cycles() two times to create
two 'cycles' events.
# ./perf record -vv -a -- sleep 1
...
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0x400000000
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|ID|CPU|PERIOD
read_format ID
disabled 1
inherit 1
freq 1
precise_ip 3
sample_id_all 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 5
sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 6
sys_perf_event_open: pid -1 cpu 2 group_fd -1 flags 0x8 = 7
sys_perf_event_open: pid -1 cpu 3 group_fd -1 flags 0x8 = 9
sys_perf_event_open: pid -1 cpu 4 group_fd -1 flags 0x8 = 10
sys_perf_event_open: pid -1 cpu 5 group_fd -1 flags 0x8 = 11
sys_perf_event_open: pid -1 cpu 6 group_fd -1 flags 0x8 = 12
sys_perf_event_open: pid -1 cpu 7 group_fd -1 flags 0x8 = 13
sys_perf_event_open: pid -1 cpu 8 group_fd -1 flags 0x8 = 14
sys_perf_event_open: pid -1 cpu 9 group_fd -1 flags 0x8 = 15
sys_perf_event_open: pid -1 cpu 10 group_fd -1 flags 0x8 = 16
sys_perf_event_open: pid -1 cpu 11 group_fd -1 flags 0x8 = 17
sys_perf_event_open: pid -1 cpu 12 group_fd -1 flags 0x8 = 18
sys_perf_event_open: pid -1 cpu 13 group_fd -1 flags 0x8 = 19
sys_perf_event_open: pid -1 cpu 14 group_fd -1 flags 0x8 = 20
sys_perf_event_open: pid -1 cpu 15 group_fd -1 flags 0x8 = 21
------------------------------------------------------------
perf_event_attr:
type 6
size 120
config 0xa00000000
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|ID|CPU|PERIOD
read_format ID
disabled 1
inherit 1
freq 1
precise_ip 3
sample_id_all 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid -1 cpu 16 group_fd -1 flags 0x8 = 22
sys_perf_event_open: pid -1 cpu 17 group_fd -1 flags 0x8 = 23
sys_perf_event_open: pid -1 cpu 18 group_fd -1 flags 0x8 = 24
sys_perf_event_open: pid -1 cpu 19 group_fd -1 flags 0x8 = 25
sys_perf_event_open: pid -1 cpu 20 group_fd -1 flags 0x8 = 26
sys_perf_event_open: pid -1 cpu 21 group_fd -1 flags 0x8 = 27
sys_perf_event_open: pid -1 cpu 22 group_fd -1 flags 0x8 = 28
sys_perf_event_open: pid -1 cpu 23 group_fd -1 flags 0x8 = 29
------------------------------------------------------------
We have to create evlist-hybrid.c otherwise due to the symbol
dependency the perf test python would be failed.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Move the major code to new created evlist-hybrid.c.
tools/perf/builtin-record.c | 19 +++++++++++----
tools/perf/util/Build | 1 +
tools/perf/util/evlist-hybrid.c | 41 +++++++++++++++++++++++++++++++++
tools/perf/util/evlist-hybrid.h | 12 ++++++++++
tools/perf/util/evlist.c | 5 +++-
tools/perf/util/evsel.c | 6 ++---
tools/perf/util/evsel.h | 2 +-
7 files changed, 77 insertions(+), 9 deletions(-)
create mode 100644 tools/perf/util/evlist-hybrid.c
create mode 100644 tools/perf/util/evlist-hybrid.h
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 35465d1db6dd..1b44b1a8636e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -47,6 +47,8 @@
#include "util/util.h"
#include "util/pfm.h"
#include "util/clockid.h"
+#include "util/pmu-hybrid.h"
+#include "util/evlist-hybrid.h"
#include "asm/bug.h"
#include "perf.h"
@@ -2786,10 +2788,19 @@ int cmd_record(int argc, const char **argv)
if (record.opts.overwrite)
record.opts.tail_synthesize = true;
- if (rec->evlist->core.nr_entries == 0 &&
- __evlist__add_default(rec->evlist, !record.opts.no_samples) < 0) {
- pr_err("Not enough memory for event selector list\n");
- goto out;
+ if (rec->evlist->core.nr_entries == 0) {
+ if (perf_pmu__has_hybrid()) {
+ err = evlist__add_default_hybrid(rec->evlist,
+ !record.opts.no_samples);
+ } else {
+ err = __evlist__add_default(rec->evlist,
+ !record.opts.no_samples);
+ }
+
+ if (err < 0) {
+ pr_err("Not enough memory for event selector list\n");
+ goto out;
+ }
}
if (rec->opts.target.tid && !rec->opts.no_inherit_set)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 00c9fb064ba6..5ac116c895bf 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -10,6 +10,7 @@ perf-y += db-export.o
perf-y += env.o
perf-y += event.o
perf-y += evlist.o
+perf-y += evlist-hybrid.o
perf-y += sideband_evlist.o
perf-y += evsel.o
perf-y += evsel_fprintf.o
diff --git a/tools/perf/util/evlist-hybrid.c b/tools/perf/util/evlist-hybrid.c
new file mode 100644
index 000000000000..185f60ec4351
--- /dev/null
+++ b/tools/perf/util/evlist-hybrid.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
+#include <inttypes.h>
+#include "cpumap.h"
+#include "evlist.h"
+#include "evsel.h"
+#include "../perf.h"
+#include "util/pmu-hybrid.h"
+#include "util/evlist-hybrid.h"
+#include <unistd.h>
+#include <stdlib.h>
+#include <linux/err.h>
+#include <linux/string.h>
+#include <perf/evlist.h>
+#include <perf/evsel.h>
+#include <perf/cpumap.h>
+
+int evlist__add_default_hybrid(struct evlist *evlist, bool precise)
+{
+ struct evsel *evsel;
+ struct perf_pmu *pmu;
+ __u64 config;
+ struct perf_cpu_map *cpus;
+
+ perf_pmu__for_each_hybrid_pmu(pmu) {
+ config = PERF_COUNT_HW_CPU_CYCLES |
+ ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT);
+ evsel = evsel__new_cycles(precise, PERF_TYPE_HARDWARE_PMU,
+ config);
+ if (!evsel)
+ return -ENOMEM;
+
+ cpus = perf_cpu_map__get(pmu->cpus);
+ evsel->core.cpus = cpus;
+ evsel->core.own_cpus = perf_cpu_map__get(cpus);
+ evsel->pmu_name = strdup(pmu->name);
+ evlist__add(evlist, evsel);
+ }
+
+ return 0;
+}
diff --git a/tools/perf/util/evlist-hybrid.h b/tools/perf/util/evlist-hybrid.h
new file mode 100644
index 000000000000..e25861649d8f
--- /dev/null
+++ b/tools/perf/util/evlist-hybrid.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_EVLIST_HYBRID_H
+#define __PERF_EVLIST_HYBRID_H
+
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include "evlist.h"
+#include <unistd.h>
+
+int evlist__add_default_hybrid(struct evlist *evlist, bool precise);
+
+#endif /* __PERF_EVLIST_HYBRID_H */
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index f1c79ecf8107..013f8ed5d0f5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -25,6 +25,7 @@
#include "util/string2.h"
#include "util/perf_api_probe.h"
#include "util/evsel_fprintf.h"
+#include "util/evlist-hybrid.h"
#include <signal.h>
#include <unistd.h>
#include <sched.h>
@@ -247,8 +248,10 @@ void evlist__set_leader(struct evlist *evlist)
int __evlist__add_default(struct evlist *evlist, bool precise)
{
- struct evsel *evsel = evsel__new_cycles(precise);
+ struct evsel *evsel;
+ evsel = evsel__new_cycles(precise, PERF_TYPE_HARDWARE,
+ PERF_COUNT_HW_CPU_CYCLES);
if (evsel == NULL)
return -ENOMEM;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2d2614eeaa20..0ba4daa09453 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -295,11 +295,11 @@ static bool perf_event_can_profile_kernel(void)
return perf_event_paranoid_check(1);
}
-struct evsel *evsel__new_cycles(bool precise)
+struct evsel *evsel__new_cycles(bool precise, __u32 type, __u64 config)
{
struct perf_event_attr attr = {
- .type = PERF_TYPE_HARDWARE,
- .config = PERF_COUNT_HW_CPU_CYCLES,
+ .type = type,
+ .config = config,
.exclude_kernel = !perf_event_can_profile_kernel(),
};
struct evsel *evsel;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index dd4f56f9cfdf..e7dc3448ab2f 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -220,7 +220,7 @@ static inline struct evsel *evsel__newtp(const char *sys, const char *name)
return evsel__newtp_idx(sys, name, 0);
}
-struct evsel *evsel__new_cycles(bool precise);
+struct evsel *evsel__new_cycles(bool precise, __u32 type, __u64 config);
struct tep_event *event_format__new(const char *sys, const char *name);
--
2.17.1
Previously if '-e' is not specified in perf stat, some software events
and hardware events are added to evlist by default.
Before:
# ./perf stat -a -- sleep 1
Performance counter stats for 'system wide':
24,044.40 msec cpu-clock # 23.946 CPUs utilized
99 context-switches # 4.117 /sec
24 cpu-migrations # 0.998 /sec
3 page-faults # 0.125 /sec
7,000,244 cycles # 0.000 GHz
2,955,024 instructions # 0.42 insn per cycle
608,941 branches # 25.326 K/sec
31,991 branch-misses # 5.25% of all branches
1.004106859 seconds time elapsed
Among the events, cycles, instructions, branches and branch-misses
are hardware events.
One hybrid platform, two hardware events are created for one
hardware event.
cpu_core/cycles/,
cpu_atom/cycles/,
cpu_core/instructions/,
cpu_atom/instructions/,
cpu_core/branches/,
cpu_atom/branches/,
cpu_core/branch-misses/,
cpu_atom/branch-misses/
These events would be added to evlist on hybrid platform.
Since parse_events() has been supported to create two hardware events
for one event on hybrid platform, so we just use parse_events(evlist,
"cycles,instructions,branches,branch-misses") to create the default
events and add them to evlist.
After:
# ./perf stat -a -- sleep 1
Performance counter stats for 'system wide':
24,048.60 msec task-clock # 23.947 CPUs utilized
438 context-switches # 18.213 /sec
24 cpu-migrations # 0.998 /sec
6 page-faults # 0.249 /sec
24,813,157 cpu_core/cycles/ # 1.032 M/sec
8,072,687 cpu_atom/cycles/ # 335.682 K/sec
20,731,286 cpu_core/instructions/ # 862.058 K/sec
3,737,203 cpu_atom/instructions/ # 155.402 K/sec
2,620,924 cpu_core/branches/ # 108.984 K/sec
381,186 cpu_atom/branches/ # 15.851 K/sec
93,248 cpu_core/branch-misses/ # 3.877 K/sec
36,515 cpu_atom/branch-misses/ # 1.518 K/sec
1.004235472 seconds time elapsed
We can see two events are created for one hardware event.
One TODO is, the shadow stats looks a bit different, now it's just
'M/sec'.
The perf_stat__update_shadow_stats and perf_stat__print_shadow_stats
need to be improved in future if we want to get the original shadow
stats.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/builtin-stat.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7b2dfe21c5a8..33fda8f55f66 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1140,6 +1140,13 @@ static int parse_stat_cgroups(const struct option *opt,
return parse_cgroups(opt, str, unset);
}
+static int add_default_hybrid_events(struct evlist *evlist)
+{
+ struct parse_events_error err;
+
+ return parse_events(evlist, "cycles,instructions,branches,branch-misses", &err);
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -1619,6 +1626,12 @@ static int add_default_attributes(void)
{ .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS },
{ .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_MISSES },
+};
+ struct perf_event_attr default_sw_attrs[] = {
+ { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK },
+ { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CONTEXT_SWITCHES },
+ { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_MIGRATIONS },
+ { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_PAGE_FAULTS },
};
/*
@@ -1856,6 +1869,14 @@ static int add_default_attributes(void)
}
if (!evsel_list->core.nr_entries) {
+ if (perf_pmu__has_hybrid()) {
+ if (evlist__add_default_attrs(evsel_list,
+ default_sw_attrs) < 0) {
+ return -1;
+ }
+ return add_default_hybrid_events(evsel_list);
+ }
+
if (target__has_cpu(&target))
default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
--
2.17.1
For a hybrid system, the perf subsystem doesn't know which PMU the
events belong to. So the PMU aware version PERF_TYPE_HARDWARE_PMU and
PERF_TYPE_HW_CACHE_PMU are introduced.
Now define the new output[] entries for these two types.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No change.
tools/perf/builtin-script.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1280cbfad4db..627ec640d2e6 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -275,6 +275,30 @@ static struct {
.invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
},
+ [PERF_TYPE_HARDWARE_PMU] = {
+ .user_set = false,
+
+ .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
+ PERF_OUTPUT_SYM | PERF_OUTPUT_SYMOFFSET |
+ PERF_OUTPUT_DSO | PERF_OUTPUT_PERIOD,
+
+ .invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
+ },
+
+ [PERF_TYPE_HW_CACHE_PMU] = {
+ .user_set = false,
+
+ .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
+ PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
+ PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
+ PERF_OUTPUT_SYM | PERF_OUTPUT_SYMOFFSET |
+ PERF_OUTPUT_DSO | PERF_OUTPUT_PERIOD,
+
+ .invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
+ },
+
[OUTPUT_TYPE_SYNTH] = {
.user_set = false,
--
2.17.1
If a group has events which are from different hybrid PMUs,
shows a warning:
"WARNING: events in group from different hybrid PMUs!"
This is to remind the user not to put the core event and atom
event into one group.
Next, just disable grouping.
# perf stat -e "{cpu_core/cycles/,cpu_atom/cycles/}" -a -- sleep 1
WARNING: events in group from different hybrid PMUs!
WARNING: grouped events cpus do not match, disabling group:
anon group { cpu_core/cycles/, cpu_atom/cycles/ }
Performance counter stats for 'system wide':
5,438,125 cpu_core/cycles/
3,914,586 cpu_atom/cycles/
1.004250966 seconds time elapsed
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Change the processing logic. In v2, it just reported the warning
and returned error. But in v3, we also disable grouping.
tools/perf/builtin-stat.c | 4 +++
tools/perf/util/evlist-hybrid.c | 47 ++++++++++++++++++++++++++++++
tools/perf/util/evlist-hybrid.h | 2 ++
tools/perf/util/evsel.c | 6 ++++
tools/perf/util/evsel.h | 1 +
tools/perf/util/python-ext-sources | 2 ++
6 files changed, 62 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 33fda8f55f66..6f70a9f1971e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -48,6 +48,7 @@
#include "util/pmu.h"
#include "util/event.h"
#include "util/evlist.h"
+#include "util/evlist-hybrid.h"
#include "util/evsel.h"
#include "util/debug.h"
#include "util/color.h"
@@ -240,6 +241,9 @@ static void evlist__check_cpu_maps(struct evlist *evlist)
struct evsel *evsel, *pos, *leader;
char buf[1024];
+ if (evlist__has_hybrid(evlist))
+ evlist__warn_hybrid_group(evlist);
+
evlist__for_each_entry(evlist, evsel) {
leader = evsel->leader;
diff --git a/tools/perf/util/evlist-hybrid.c b/tools/perf/util/evlist-hybrid.c
index 185f60ec4351..39f520372447 100644
--- a/tools/perf/util/evlist-hybrid.c
+++ b/tools/perf/util/evlist-hybrid.c
@@ -7,6 +7,7 @@
#include "../perf.h"
#include "util/pmu-hybrid.h"
#include "util/evlist-hybrid.h"
+#include "debug.h"
#include <unistd.h>
#include <stdlib.h>
#include <linux/err.h>
@@ -39,3 +40,49 @@ int evlist__add_default_hybrid(struct evlist *evlist, bool precise)
return 0;
}
+
+static bool group_hybrid_conflict(struct evsel *leader)
+{
+ struct evsel *pos, *prev = NULL;
+
+ for_each_group_evsel(pos, leader) {
+ if (!evsel__is_hybrid(pos))
+ continue;
+
+ if (prev && strcmp(prev->pmu_name, pos->pmu_name))
+ return true;
+
+ prev = pos;
+ }
+
+ return false;
+}
+
+void evlist__warn_hybrid_group(struct evlist *evlist)
+{
+ struct evsel *evsel;
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (evsel__is_group_leader(evsel) &&
+ evsel->core.nr_members > 1 &&
+ group_hybrid_conflict(evsel)) {
+ pr_warning("WARNING: events in group from "
+ "different hybrid PMUs!\n");
+ return;
+ }
+ }
+}
+
+bool evlist__has_hybrid(struct evlist *evlist)
+{
+ struct evsel *evsel;
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (evsel->pmu_name &&
+ perf_pmu__is_hybrid(evsel->pmu_name)) {
+ return true;
+ }
+ }
+
+ return false;
+}
diff --git a/tools/perf/util/evlist-hybrid.h b/tools/perf/util/evlist-hybrid.h
index e25861649d8f..19f74b4c340a 100644
--- a/tools/perf/util/evlist-hybrid.h
+++ b/tools/perf/util/evlist-hybrid.h
@@ -8,5 +8,7 @@
#include <unistd.h>
int evlist__add_default_hybrid(struct evlist *evlist, bool precise);
+void evlist__warn_hybrid_group(struct evlist *evlist);
+bool evlist__has_hybrid(struct evlist *evlist);
#endif /* __PERF_EVLIST_HYBRID_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0ba4daa09453..0f64a32ea9c5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -47,6 +47,7 @@
#include "memswap.h"
#include "util.h"
#include "hashmap.h"
+#include "pmu-hybrid.h"
#include "../perf-sys.h"
#include "util/parse-branch-options.h"
#include <internal/xyarray.h>
@@ -2797,3 +2798,8 @@ void evsel__zero_per_pkg(struct evsel *evsel)
hashmap__clear(evsel->per_pkg_mask);
}
}
+
+bool evsel__is_hybrid(struct evsel *evsel)
+{
+ return evsel->pmu_name && perf_pmu__is_hybrid(evsel->pmu_name);
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index e7dc3448ab2f..e56826bbb628 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -453,4 +453,5 @@ struct perf_env *evsel__env(struct evsel *evsel);
int evsel__store_ids(struct evsel *evsel, struct evlist *evlist);
void evsel__zero_per_pkg(struct evsel *evsel);
+bool evsel__is_hybrid(struct evsel *evsel);
#endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index 845dd46e3c61..d7c976671e3a 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -37,3 +37,5 @@ util/units.c
util/affinity.c
util/rwsem.c
util/hashmap.c
+util/pmu-hybrid.c
+util/fncache.c
--
2.17.1
For perf-record, it would be useful to tell user the pmu which the
event belongs to.
For example,
# perf record -a -- sleep 1
# perf report
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 106 of event 'cpu_core/cycles/'
# Event count (approx.): 22043448
#
# Overhead Command Shared Object Symbol
# ........ ............ ....................... ............................
#
...
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- New patch in v3.
tools/perf/builtin-record.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 1b44b1a8636e..74cc9ffbd9ef 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1605,6 +1605,32 @@ static void hit_auxtrace_snapshot_trigger(struct record *rec)
}
}
+static void record__uniquify_name(struct record *rec)
+{
+ struct evsel *pos;
+ struct evlist *evlist = rec->evlist;
+ char *new_name;
+ int ret;
+
+ if (!perf_pmu__has_hybrid())
+ return;
+
+ evlist__for_each_entry(evlist, pos) {
+ if (!evsel__is_hybrid(pos))
+ continue;
+
+ if (strchr(pos->name, '/'))
+ continue;
+
+ ret = asprintf(&new_name, "%s/%s/",
+ pos->pmu_name, pos->name);
+ if (ret) {
+ free(pos->name);
+ pos->name = new_name;
+ }
+ }
+}
+
static int __cmd_record(struct record *rec, int argc, const char **argv)
{
int err;
@@ -1709,6 +1735,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
if (data->is_pipe && rec->evlist->core.nr_entries == 1)
rec->opts.sample_id = true;
+ record__uniquify_name(rec);
+
if (record__open(rec) != 0) {
err = -1;
goto out_child;
--
2.17.1
The functions perf_pmu__is_hybrid and perf_pmu__find_hybrid_pmu
can be used to identify the hybrid platform and return the found
hybrid cpu pmu. All the detected hybrid pmus have been saved in
'perf_pmu__hybrid_pmus' list. So we just need to search this list.
perf_pmu__hybrid_type_to_pmu converts the user specified string
to hybrid pmu name. This is used to support the '--cputype' option
in next patches.
perf_pmu__has_hybrid checks the existing of hybrid pmu. Note that,
we have to define it in pmu.c (try to make pmu-hybrid.c no more
symbol dependency), otherwise perf test python would be failed.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Move perf_pmu__has_hybrid from pmu-hybrid.c to pmu.c. We have to
add pmu-hybrid.c to python-ext-sources to solve symbol dependency
issue found in perf test python. For perf_pmu__has_hybrid, it calls
perf_pmu__scan, which is defined in pmu.c. It's very hard to add
pmu.c to python-ext-sources, too much symbol dependency here.
tools/perf/util/pmu-hybrid.c | 40 ++++++++++++++++++++++++++++++++++++
tools/perf/util/pmu-hybrid.h | 4 ++++
tools/perf/util/pmu.c | 11 ++++++++++
tools/perf/util/pmu.h | 2 ++
4 files changed, 57 insertions(+)
diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c
index 7316bf46e54b..86ba84d9469c 100644
--- a/tools/perf/util/pmu-hybrid.c
+++ b/tools/perf/util/pmu-hybrid.c
@@ -33,3 +33,43 @@ bool perf_pmu__hybrid_mounted(const char *name)
snprintf(path, PATH_MAX, CPUS_TEMPLATE_CPU, sysfs, name);
return file_available(path);
}
+
+struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name)
+{
+ struct perf_pmu *pmu;
+
+ if (!name)
+ return NULL;
+
+ perf_pmu__for_each_hybrid_pmu(pmu) {
+ if (!strcmp(name, pmu->name))
+ return pmu;
+ }
+
+ return NULL;
+}
+
+bool perf_pmu__is_hybrid(const char *name)
+{
+ return perf_pmu__find_hybrid_pmu(name) != NULL;
+}
+
+char *perf_pmu__hybrid_type_to_pmu(const char *type)
+{
+ char *pmu_name = NULL;
+
+ if (asprintf(&pmu_name, "cpu_%s", type) < 0)
+ return NULL;
+
+ if (perf_pmu__is_hybrid(pmu_name))
+ return pmu_name;
+
+ /*
+ * pmu may be not scanned, check the sysfs.
+ */
+ if (perf_pmu__hybrid_mounted(pmu_name))
+ return pmu_name;
+
+ free(pmu_name);
+ return NULL;
+}
diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h
index 35bed3714438..d0fa7bc50a76 100644
--- a/tools/perf/util/pmu-hybrid.h
+++ b/tools/perf/util/pmu-hybrid.h
@@ -15,4 +15,8 @@ extern struct list_head perf_pmu__hybrid_pmus;
bool perf_pmu__hybrid_mounted(const char *name);
+struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name);
+bool perf_pmu__is_hybrid(const char *name);
+char *perf_pmu__hybrid_type_to_pmu(const char *type);
+
#endif /* __PMU_HYBRID_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 35e9660c3904..beff29981101 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -40,6 +40,7 @@ int perf_pmu_parse(struct list_head *list, char *name);
extern FILE *perf_pmu_in;
static LIST_HEAD(pmus);
+static bool hybrid_scanned;
/*
* Parse & process all the sysfs attributes located under
@@ -1823,3 +1824,13 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
return nr_caps;
}
+
+bool perf_pmu__has_hybrid(void)
+{
+ if (!hybrid_scanned) {
+ hybrid_scanned = true;
+ perf_pmu__scan(NULL);
+ }
+
+ return !list_empty(&perf_pmu__hybrid_pmus);
+}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 29289e7c2649..5ed2abab7fe0 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -128,4 +128,6 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
int perf_pmu__caps_parse(struct perf_pmu *pmu);
+bool perf_pmu__has_hybrid(void);
+
#endif /* __PMU_H */
--
2.17.1
Since for "cycles:u' on hybrid platform, it creates two "cycles".
So the number of events in evlist is not expected in next test
steps. Now we just use one event "cpu_core/cycles:u/" for hybrid.
# ./perf test 35
35: Track with sched_switch : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/switch-tracking.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 3ebaa758df77..3a12176f8c46 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -18,6 +18,7 @@
#include "record.h"
#include "tests.h"
#include "util/mmap.h"
+#include "pmu.h"
static int spin_sleep(void)
{
@@ -340,6 +341,10 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
struct evsel *switch_evsel, *tracking_evsel;
const char *comm;
int err = -1;
+ bool hybrid = false;
+
+ if (perf_pmu__has_hybrid())
+ hybrid = true;
threads = thread_map__new(-1, getpid(), UINT_MAX);
if (!threads) {
@@ -371,7 +376,10 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
cpu_clocks_evsel = evlist__last(evlist);
/* Second event */
- err = parse_events(evlist, "cycles:u", NULL);
+ if (!hybrid)
+ err = parse_events(evlist, "cycles:u", NULL);
+ else
+ err = parse_events(evlist, "cpu_core/cycles/u", NULL);
if (err) {
pr_debug("Failed to parse event cycles:u\n");
goto out_err;
--
2.17.1
Currently we don't support shadow stat for hybrid.
root@ssp-pwrt-002:~# ./perf stat -e cycles,instructions -a -- sleep 1
Performance counter stats for 'system wide':
12,883,109,591 cpu_core/cycles/
6,405,163,221 cpu_atom/cycles/
555,553,778 cpu_core/instructions/
841,158,734 cpu_atom/instructions/
1.002644773 seconds time elapsed
Now there is no shadow stat 'insn per cycle' reported. We will support
it later and now just skip the 'perf stat metrics (shadow stat) test'.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/shell/stat+shadow_stat.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
index ebebd3596cf9..e6e35fc6c882 100755
--- a/tools/perf/tests/shell/stat+shadow_stat.sh
+++ b/tools/perf/tests/shell/stat+shadow_stat.sh
@@ -7,6 +7,9 @@ set -e
# skip if system-wide mode is forbidden
perf stat -a true > /dev/null 2>&1 || exit 2
+# skip if on hybrid platform
+perf stat -a -e cycles sleep 1 2>&1 | grep -e cpu_core && exit 2
+
test_global_aggr()
{
perf stat -a --no-big-num -e cycles,instructions sleep 1 2>&1 | \
--
2.17.1
Add some words and examples to help understanding of
Intel hybrid perf support.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/Documentation/intel-hybrid.txt | 228 ++++++++++++++++++++++
tools/perf/Documentation/perf-record.txt | 1 +
tools/perf/Documentation/perf-stat.txt | 2 +
3 files changed, 231 insertions(+)
create mode 100644 tools/perf/Documentation/intel-hybrid.txt
diff --git a/tools/perf/Documentation/intel-hybrid.txt b/tools/perf/Documentation/intel-hybrid.txt
new file mode 100644
index 000000000000..784f598dd36f
--- /dev/null
+++ b/tools/perf/Documentation/intel-hybrid.txt
@@ -0,0 +1,228 @@
+Intel hybrid support
+--------------------
+Support for Intel hybrid events within perf tools.
+
+For some Intel platforms, such as AlderLake, which is hybrid platform and
+it consists of atom cpu and core cpu. Each cpu has dedicated event list.
+Part of events are available on core cpu, part of events are available
+on atom cpu and even part of events are available on both.
+
+Kernel exports two new cpu pmus via sysfs:
+/sys/devices/cpu_core
+/sys/devices/cpu_atom
+
+The 'cpus' files are created under the directories. For example,
+
+cat /sys/devices/cpu_core/cpus
+0-15
+
+cat /sys/devices/cpu_atom/cpus
+16-23
+
+It indicates cpu0-cpu15 are core cpus and cpu16-cpu23 are atom cpus.
+
+Quickstart
+
+List hybrid event
+-----------------
+
+As before, use perf-list to list the symbolic event.
+
+perf list
+
+inst_retired.any
+ [Fixed Counter: Counts the number of instructions retired. Unit: cpu_atom]
+inst_retired.any
+ [Number of instructions retired. Fixed Counter - architectural event. Unit: cpu_core]
+
+The 'Unit: xxx' is added to brief description to indicate which pmu
+the event is belong to. Same event name but with different pmu can
+be supported.
+
+Enable hybrid event with a specific pmu
+---------------------------------------
+
+To enable a core only event or atom only event, following syntax is supported:
+
+ cpu_core/<event name>/
+or
+ cpu_atom/<event name>/
+
+For example, count the 'cycles' event on core cpus.
+
+ perf stat -e cpu_core/cycles/
+
+Create two events for one hardware event automatically
+------------------------------------------------------
+
+When creating one event and the event is available on both atom and core,
+two events are created automatically. One is for atom, the other is for
+core. Most of hardware events and cache events are available on both
+cpu_core and cpu_atom.
+
+For hardware events, they have pre-defined configs (e.g. 0 for cycles).
+But on hybrid platform, kernel needs to know where the event comes from
+(from atom or from core). The original perf event type PERF_TYPE_HARDWARE
+can't carry pmu information. So a new type PERF_TYPE_HARDWARE_PMU is
+introduced.
+
+The new attr.config layout for PERF_TYPE_HARDWARE_PMU:
+
+0xDD000000AA
+AA: original hardware event ID
+DD: PMU type ID
+
+Cache event is similar. A new type PERF_TYPE_HW_CACHE_PMU is introduced.
+
+The new attr.config layout for PERF_TYPE_HW_CACHE_PMU:
+
+0xDD00CCBBAA
+AA: original hardware cache ID
+BB: original hardware cache op ID
+CC: original hardware cache op result ID
+DD: PMU type ID
+
+PMU type ID is retrieved from sysfs
+
+cat /sys/devices/cpu_atom/type
+10
+
+cat /sys/devices/cpu_core/type
+4
+
+When enabling a hardware event without specified pmu, such as,
+perf stat -e cycles -a (use system-wide in this example), two events
+are created automatically.
+
+ ------------------------------------------------------------
+ perf_event_attr:
+ type 6
+ size 120
+ config 0x400000000
+ sample_type IDENTIFIER
+ read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
+ disabled 1
+ inherit 1
+ exclude_guest 1
+ ------------------------------------------------------------
+
+and
+
+ ------------------------------------------------------------
+ perf_event_attr:
+ type 6
+ size 120
+ config 0xa00000000
+ sample_type IDENTIFIER
+ read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
+ disabled 1
+ inherit 1
+ exclude_guest 1
+ ------------------------------------------------------------
+
+type 6 is PERF_TYPE_HARDWARE_PMU.
+0x4 in 0x400000000 indicates it's cpu_core pmu.
+0xa in 0xa00000000 indicates it's cpu_atom pmu (atom pmu type id is random).
+
+The kernel creates 'cycles' (0x400000000) on cpu0-cpu15 (core cpus),
+and create 'cycles' (0xa00000000) on cpu16-cpu23 (atom cpus).
+
+For perf-stat result, it displays two events:
+
+ Performance counter stats for 'system wide':
+
+ 12,869,720,529 cpu_core/cycles/
+ 6,405,459,328 cpu_atom/cycles/
+
+The first 'cycles' is core event, the second 'cycles' is atom event.
+
+Thread mode example:
+--------------------
+
+perf-stat reports the scaled counts for hybrid event and with a percentage
+displayed. The percentage is the event's running time/enabling time.
+
+One example, 'triad_loop' runs on cpu16 (atom core), while we can see the
+scaled value for core cycles is 160,444,092 and the percentage is 0.47%.
+
+perf stat -e cycles -- taskset -c 16 ./triad_loop
+
+As previous, two events are created.
+
+------------------------------------------------------------
+perf_event_attr:
+ type 6
+ size 120
+ config 0x400000000
+ sample_type IDENTIFIER
+ read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
+ disabled 1
+ inherit 1
+ enable_on_exec 1
+ exclude_guest 1
+------------------------------------------------------------
+
+and
+
+------------------------------------------------------------
+perf_event_attr:
+ type 6
+ size 120
+ config 0xa00000000
+ sample_type IDENTIFIER
+ read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
+ disabled 1
+ inherit 1
+ enable_on_exec 1
+ exclude_guest 1
+------------------------------------------------------------
+
+cycles: 0: 784136 339210144 1013444
+cycles: 0: 601988862 339210144 338196700
+cycles: 262458394 339210144 1013444
+cycles: 603792788 339210144 338196700
+
+ Performance counter stats for 'taskset -c 16 ./triad_loop':
+
+ 262,458,394 cpu_core/cycles/ (0.30%)
+ 603,792,788 cpu_atom/cycles/ (99.70%)
+
+ 0.340467913 seconds time elapsed
+
+perf-record:
+------------
+
+If there is no '-e' specified in perf record, on hybrid platform,
+it creates two default 'cycles' and adds them to event list. One
+is for core, the other is for atom.
+
+perf-stat:
+----------
+
+If there is no '-e' specified in perf stat, on hybrid platform,
+besides of software events, following events are created and
+added to event list in order.
+
+cpu_core/cycles/,
+cpu_atom/cycles/,
+cpu_core/instructions/,
+cpu_atom/instructions/,
+cpu_core/branches/,
+cpu_atom/branches/,
+cpu_core/branch-misses/,
+cpu_atom/branch-misses/
+
+Of course, both perf-stat and perf-record support to enable
+hybrid event with a specific pmu.
+
+e.g.
+perf stat -e cpu_core/cycles/
+perf stat -e cpu_atom/cycles/
+perf stat -e cpu_core/r1a/
+perf stat -e cpu_atom/L1-icache-loads/
+perf stat -e cpu_core/cycles/,cpu_atom/instructions/
+perf stat -e '{cpu_core/cycles/,cpu_core/instructions/}'
+
+But '{cpu_core/cycles/,cpu_atom/instructions/}' will return
+"<not supported>" for 'instructions', because the pmus in
+group are not matched (cpu_core vs. cpu_atom).
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index f3161c9673e9..d71bac847936 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -695,6 +695,7 @@ measurements:
wait -n ${perf_pid}
exit $?
+include::intel-hybrid.txt[]
SEE ALSO
--------
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 744211fa8c18..13ec4165004b 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -541,6 +541,8 @@ The fields are in this order:
Additional metrics may be printed with all earlier fields being empty.
+include::intel-hybrid.txt[]
+
SEE ALSO
--------
linkperf:perf-top[1], linkperf:perf-list[1]
--
2.17.1
perf-stat has supported some aggregation modes, such as --per-core,
--per-socket and etc. While for hybrid event, it may only available
on part of cpus. So for --per-core, we need to filter out the
unavailable cores, for --per-socket, filter out the unavailable
sockets, and so on.
Before:
# perf stat --per-core -e cpu_core/cycles/ -a -- sleep 1
Performance counter stats for 'system wide':
S0-D0-C0 2 479,530 cpu_core/cycles/
S0-D0-C4 2 175,007 cpu_core/cycles/
S0-D0-C8 2 166,240 cpu_core/cycles/
S0-D0-C12 2 704,673 cpu_core/cycles/
S0-D0-C16 2 865,835 cpu_core/cycles/
S0-D0-C20 2 2,958,461 cpu_core/cycles/
S0-D0-C24 2 163,988 cpu_core/cycles/
S0-D0-C28 2 164,729 cpu_core/cycles/
S0-D0-C32 0 <not counted> cpu_core/cycles/
S0-D0-C33 0 <not counted> cpu_core/cycles/
S0-D0-C34 0 <not counted> cpu_core/cycles/
S0-D0-C35 0 <not counted> cpu_core/cycles/
S0-D0-C36 0 <not counted> cpu_core/cycles/
S0-D0-C37 0 <not counted> cpu_core/cycles/
S0-D0-C38 0 <not counted> cpu_core/cycles/
S0-D0-C39 0 <not counted> cpu_core/cycles/
1.003597211 seconds time elapsed
After:
# perf stat --per-core -e cpu_core/cycles/ -a -- sleep 1
Performance counter stats for 'system wide':
S0-D0-C0 2 210,428 cpu_core/cycles/
S0-D0-C4 2 444,830 cpu_core/cycles/
S0-D0-C8 2 435,241 cpu_core/cycles/
S0-D0-C12 2 423,976 cpu_core/cycles/
S0-D0-C16 2 859,350 cpu_core/cycles/
S0-D0-C20 2 1,559,589 cpu_core/cycles/
S0-D0-C24 2 163,924 cpu_core/cycles/
S0-D0-C28 2 376,610 cpu_core/cycles/
1.003621290 seconds time elapsed
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/util/stat-display.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 161826938a00..b7ce3c4ae5a8 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -635,6 +635,20 @@ static void aggr_cb(struct perf_stat_config *config,
}
}
+static bool aggr_id_hybrid_matched(struct perf_stat_config *config,
+ struct evsel *counter, struct aggr_cpu_id id)
+{
+ struct aggr_cpu_id s;
+
+ for (int i = 0; i < evsel__nr_cpus(counter); i++) {
+ s = config->aggr_get_id(config, evsel__cpus(counter), i);
+ if (cpu_map__compare_aggr_cpu_id(s, id))
+ return true;
+ }
+
+ return false;
+}
+
static void print_counter_aggrdata(struct perf_stat_config *config,
struct evsel *counter, int s,
char *prefix, bool metric_only,
@@ -648,6 +662,12 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
double uval;
ad.id = id = config->aggr_map->map[s];
+
+ if (perf_pmu__has_hybrid() &&
+ !aggr_id_hybrid_matched(config, counter, id)) {
+ return;
+ }
+
ad.val = ad.ena = ad.run = 0;
ad.nr = 0;
if (!collect_data(config, counter, aggr_cb, &ad))
--
2.17.1
Add basic hybrid test cases for 'Parse event definition strings' test.
# ./perf test 6
6: Parse event definition strings : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- Use PERF_TYPE_RAW for cpu_core/cycles/
tools/perf/tests/parse-events.c | 170 ++++++++++++++++++++++++++++++++
1 file changed, 170 insertions(+)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 026c54743311..1cd1d2778172 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1512,6 +1512,123 @@ static int test__all_tracepoints(struct evlist *evlist)
return test__checkevent_tracepoint_multi(evlist);
}
+static int test__hybrid_hw_event_with_pmu(struct evlist *evlist)
+{
+ struct evsel *evsel = evlist__first(evlist);
+
+ TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
+ return 0;
+}
+
+static int test__hybrid_hw_event(struct evlist *evlist)
+{
+ struct evsel *evsel1 = evlist__first(evlist);
+ struct evsel *evsel2 = evlist__last(evlist);
+
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE_PMU == evsel1->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x400000000 == evsel1->core.attr.config);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_HARDWARE_PMU == evsel2->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0xa00000000 == evsel2->core.attr.config);
+ return 0;
+}
+
+static int test__hybrid_hw_group_event(struct evlist *evlist)
+{
+ struct evsel *evsel, *leader;
+
+ evsel = leader = evlist__first(evlist);
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+
+ evsel = evsel__next(evsel);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0xc0 == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+ return 0;
+}
+
+static int test__hybrid_sw_hw_group_event(struct evlist *evlist)
+{
+ struct evsel *evsel, *leader;
+
+ evsel = leader = evlist__first(evlist);
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_SOFTWARE == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+
+ evsel = evsel__next(evsel);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+ return 0;
+}
+
+static int test__hybrid_hw_sw_group_event(struct evlist *evlist)
+{
+ struct evsel *evsel, *leader;
+
+ evsel = leader = evlist__first(evlist);
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+
+ evsel = evsel__next(evsel);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_SOFTWARE == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+ return 0;
+}
+
+static int test__hybrid_group_modifier1(struct evlist *evlist)
+{
+ struct evsel *evsel, *leader;
+
+ evsel = leader = evlist__first(evlist);
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+ TEST_ASSERT_VAL("wrong exclude_user", evsel->core.attr.exclude_user);
+ TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
+
+ evsel = evsel__next(evsel);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0xc0 == evsel->core.attr.config);
+ TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+ TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
+ TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
+ return 0;
+}
+
+static int test__hybrid_raw1(struct evlist *evlist)
+{
+ struct evsel *evsel = evlist__first(evlist);
+
+ TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x1a == evsel->core.attr.config);
+
+ /* The type of second event is randome value */
+ evsel = evsel__next(evsel);
+ TEST_ASSERT_VAL("wrong config", 0x1a == evsel->core.attr.config);
+ return 0;
+}
+
+static int test__hybrid_raw2(struct evlist *evlist)
+{
+ struct evsel *evsel = evlist__first(evlist);
+
+ TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries);
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
+ TEST_ASSERT_VAL("wrong config", 0x1a == evsel->core.attr.config);
+ return 0;
+}
+
struct evlist_test {
const char *name;
__u32 type;
@@ -1868,6 +1985,54 @@ static struct terms_test test__terms[] = {
},
};
+static struct evlist_test test__hybrid_events[] = {
+ {
+ .name = "cpu_core/cpu-cycles/",
+ .check = test__hybrid_hw_event_with_pmu,
+ .id = 0,
+ },
+ {
+ .name = "cycles",
+ .check = test__hybrid_hw_event,
+ .id = 1,
+ },
+ {
+ .name = "{cpu_core/cycles/,cpu_core/instructions/}",
+ .check = test__hybrid_hw_group_event,
+ .id = 2,
+ },
+ {
+ .name = "{cpu-clock,cpu_core/cycles/}",
+ .check = test__hybrid_sw_hw_group_event,
+ .id = 3,
+ },
+ {
+ .name = "{cpu_core/cycles/,cpu-clock}",
+ .check = test__hybrid_hw_sw_group_event,
+ .id = 4,
+ },
+ {
+ .name = "{cpu_core/cycles/k,cpu_core/instructions/u}",
+ .check = test__hybrid_group_modifier1,
+ .id = 5,
+ },
+ {
+ .name = "r1a",
+ .check = test__hybrid_raw1,
+ .id = 6,
+ },
+ {
+ .name = "cpu_core/r1a/",
+ .check = test__hybrid_raw2,
+ .id = 7,
+ },
+ {
+ .name = "cpu_core/config=10,config1,config2=3,period=1000/u",
+ .check = test__checkevent_pmu,
+ .id = 0,
+ },
+};
+
static int test_event(struct evlist_test *e)
{
struct parse_events_error err;
@@ -2035,6 +2200,11 @@ do { \
ret2 = ret1; \
} while (0)
+ if (perf_pmu__has_hybrid()) {
+ TEST_EVENTS(test__hybrid_events);
+ return ret2;
+ }
+
TEST_EVENTS(test__events);
if (test_pmu())
--
2.17.1
Since for one hw event, two hybrid events are created.
For example,
evsel->idx evsel__name(evsel)
0 cycles
1 cycles
2 instructions
3 instructions
...
So for comparing the evsel name on hybrid, the evsel->idx
needs to be divided by 2.
# ./perf test 14
14: Roundtrip evsel->name : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/evsel-roundtrip-name.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index f7f3e5b4c180..b74cf80d1f10 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -4,6 +4,7 @@
#include "parse-events.h"
#include "tests.h"
#include "debug.h"
+#include "pmu.h"
#include <errno.h>
#include <linux/kernel.h>
@@ -62,7 +63,8 @@ static int perf_evsel__roundtrip_cache_name_test(void)
return ret;
}
-static int __perf_evsel__name_array_test(const char *names[], int nr_names)
+static int __perf_evsel__name_array_test(const char *names[], int nr_names,
+ int distance)
{
int i, err;
struct evsel *evsel;
@@ -82,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names)
err = 0;
evlist__for_each_entry(evlist, evsel) {
- if (strcmp(evsel__name(evsel), names[evsel->idx])) {
+ if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
--err;
- pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx]);
+ pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
}
}
@@ -93,18 +95,21 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names)
return err;
}
-#define perf_evsel__name_array_test(names) \
- __perf_evsel__name_array_test(names, ARRAY_SIZE(names))
+#define perf_evsel__name_array_test(names, distance) \
+ __perf_evsel__name_array_test(names, ARRAY_SIZE(names), distance)
int test__perf_evsel__roundtrip_name_test(struct test *test __maybe_unused, int subtest __maybe_unused)
{
int err = 0, ret = 0;
- err = perf_evsel__name_array_test(evsel__hw_names);
+ if (perf_pmu__has_hybrid())
+ return perf_evsel__name_array_test(evsel__hw_names, 2);
+
+ err = perf_evsel__name_array_test(evsel__hw_names, 1);
if (err)
ret = err;
- err = __perf_evsel__name_array_test(evsel__sw_names, PERF_COUNT_SW_DUMMY + 1);
+ err = __perf_evsel__name_array_test(evsel__sw_names, PERF_COUNT_SW_DUMMY + 1, 1);
if (err)
ret = err;
--
2.17.1
For hybrid, kernel introduces new perf type PERF_TYPE_HARDWARE_PMU (6)
and it's assigned to hybrid hardware events.
# ./perf test 17 -vvv
...
compare
matching [event:base-stat]
to [event-6-17179869184-4]
[cpu] * 0
[flags] 0|8 8
[type] 0 6
->FAIL
match: [event:base-stat] matches []
event:base-stat does not match, but is optional
matched
compare
matching [event-6-17179869184-4]
to [event:base-stat]
[cpu] 0 *
[flags] 8 0|8
[type] 6 0
->FAIL
match: [event-6-17179869184-4] matches []
expected type=6, got 0
expected config=17179869184, got 0
FAILED './tests/attr/test-stat-C0' - match failure
The type matching is failed because one type is 0 but the
type of hybrid hardware event is 6. We temporarily skip this
test case and TODO in future.
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/attr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
index dd39ce9b0277..b37c35fb5a46 100644
--- a/tools/perf/tests/attr.c
+++ b/tools/perf/tests/attr.c
@@ -34,6 +34,7 @@
#include "event.h"
#include "util.h"
#include "tests.h"
+#include "pmu.h"
#define ENV "PERF_TEST_ATTR"
@@ -184,6 +185,9 @@ int test__attr(struct test *test __maybe_unused, int subtest __maybe_unused)
char path_dir[PATH_MAX];
char *exec_path;
+ if (perf_pmu__has_hybrid())
+ return 0;
+
/* First try development tree tests. */
if (!lstat("./tests", &st))
return run_dir("./tests", "./perf");
--
2.17.1
Force to create one event "cpu_core/cycles/" by default,
otherwise in evlist__valid_sample_type, the checking of
'if (evlist->core.nr_entries == 1)' would be failed.
# ./perf test 41
41: Session topology : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/topology.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
index 050489807a47..30b4acb08d35 100644
--- a/tools/perf/tests/topology.c
+++ b/tools/perf/tests/topology.c
@@ -8,6 +8,7 @@
#include "session.h"
#include "evlist.h"
#include "debug.h"
+#include "pmu.h"
#include <linux/err.h>
#define TEMPL "/tmp/perf-test-XXXXXX"
@@ -40,7 +41,14 @@ static int session_write_header(char *path)
session = perf_session__new(&data, false, NULL);
TEST_ASSERT_VAL("can't get session", !IS_ERR(session));
- session->evlist = evlist__new_default();
+ if (!perf_pmu__has_hybrid()) {
+ session->evlist = evlist__new_default();
+ } else {
+ struct parse_events_error err;
+
+ session->evlist = evlist__new();
+ parse_events(session->evlist, "cpu_core/cycles/", &err);
+ }
TEST_ASSERT_VAL("can't get evlist", session->evlist);
perf_header__set_feat(&session->header, HEADER_CPU_TOPOLOGY);
--
2.17.1
Some events are not supported. Only pick up some cases for hybrid.
# ./perf test 67
67: Parse and process metrics : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/parse-metric.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 4968c4106254..24e5ddff515e 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -11,6 +11,7 @@
#include "debug.h"
#include "expr.h"
#include "stat.h"
+#include "pmu.h"
static struct pmu_event pme_test[] = {
{
@@ -370,12 +371,17 @@ static int test_metric_group(void)
int test__parse_metric(struct test *test __maybe_unused, int subtest __maybe_unused)
{
+ perf_pmu__scan(NULL);
+
TEST_ASSERT_VAL("IPC failed", test_ipc() == 0);
TEST_ASSERT_VAL("frontend failed", test_frontend() == 0);
- TEST_ASSERT_VAL("cache_miss_cycles failed", test_cache_miss_cycles() == 0);
TEST_ASSERT_VAL("DCache_L2 failed", test_dcache_l2() == 0);
TEST_ASSERT_VAL("recursion fail failed", test_recursion_fail() == 0);
- TEST_ASSERT_VAL("test metric group", test_metric_group() == 0);
TEST_ASSERT_VAL("Memory bandwidth", test_memory_bandwidth() == 0);
+
+ if (!perf_pmu__has_hybrid()) {
+ TEST_ASSERT_VAL("cache_miss_cycles failed", test_cache_miss_cycles() == 0);
+ TEST_ASSERT_VAL("test metric group", test_metric_group() == 0);
+ }
return 0;
}
--
2.17.1
Since for "cycles:u' on hybrid platform, it creates two "cycles".
So the second evsel in evlist also needs initialization.
With this patch,
# ./perf test 71
71: Convert perf time to TSC : Ok
Signed-off-by: Jin Yao <[email protected]>
---
v3:
- No functional change.
tools/perf/tests/perf-time-to-tsc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index 680c3cffb128..72f268c6cc5d 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -20,6 +20,7 @@
#include "tsc.h"
#include "mmap.h"
#include "tests.h"
+#include "pmu.h"
#define CHECK__(x) { \
while ((x) < 0) { \
@@ -66,6 +67,10 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
u64 test_tsc, comm1_tsc, comm2_tsc;
u64 test_time, comm1_time = 0, comm2_time = 0;
struct mmap *md;
+ bool hybrid = false;
+
+ if (perf_pmu__has_hybrid())
+ hybrid = true;
threads = thread_map__new(-1, getpid(), UINT_MAX);
CHECK_NOT_NULL__(threads);
@@ -88,6 +93,17 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
evsel->core.attr.disabled = 1;
evsel->core.attr.enable_on_exec = 0;
+ /*
+ * For hybrid "cycles:u", it creates two events.
+ * Init the second evsel here.
+ */
+ if (hybrid) {
+ evsel = evsel__next(evsel);
+ evsel->core.attr.comm = 1;
+ evsel->core.attr.disabled = 1;
+ evsel->core.attr.enable_on_exec = 0;
+ }
+
CHECK__(evlist__open(evlist));
CHECK__(evlist__mmap(evlist, UINT_MAX));
--
2.17.1
On Mon, Mar 29, 2021 at 03:00:28PM +0800, Jin Yao wrote:
SNIP
> index 1bbd0ba92ba7..3692fa3c964a 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -458,6 +458,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
> int cache_type = -1, cache_op = -1, cache_result = -1;
> char *op_result[2] = { op_result1, op_result2 };
> int i, n;
> + bool hybrid;
>
> /*
> * No fallback - if we cannot get a clear cache type
> @@ -517,6 +518,13 @@ int parse_events_add_cache(struct list_head *list, int *idx,
> if (get_config_terms(head_config, &config_terms))
> return -ENOMEM;
> }
> +
> + i = parse_events__add_cache_hybrid(list, idx, &attr,
> + config_name ? : name, &config_terms,
> + &hybrid);
> + if (hybrid)
> + return i;
please define 'ret' for the return value, i is confusing
thanks,
jirka
> +
> return add_event(list, idx, &attr, config_name ? : name, &config_terms);
> }
>
> --
> 2.17.1
>
On Mon, Mar 29, 2021 at 03:00:30PM +0800, Jin Yao wrote:
> On hybrid platform, user may want to enable the hardware event
> only on one PMU. So following syntax is supported:
>
> cpu_core/<hardware event>/
> cpu_atom/<hardware event>/
>
> # perf stat -e cpu_core/cpu-cycles/ -a -- sleep 1
>
> Performance counter stats for 'system wide':
>
> 6,049,336 cpu_core/cpu-cycles/
>
> 1.003577042 seconds time elapsed
>
> It enables the event 'cpu-cycles' only on cpu_core pmu.
>
> But for 'cycles' and 'branches', the syntax doesn't work.
because the alias is not there.. but there's:
cpu/cpu-cycles/
cpu/branch-instructions/
doing the same thing.. what's wrong with that?
I have a feeling we discussed this in the previous
version.. did I give up? ;-)
SNIP
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index beff29981101..72e5ae5e868e 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -916,6 +916,35 @@ static int pmu_max_precise(const char *name)
> return max_precise;
> }
>
> +static void perf_pmu__add_hybrid_aliases(struct list_head *head)
> +{
> + static struct pmu_event pme_hybrid_fixup[] = {
> + {
> + .name = "cycles",
> + .event = "event=0x3c",
> + },
> + {
> + .name = "branches",
> + .event = "event=0xc4",
> + },
> + {
> + .name = 0,
> + .event = 0,
> + },
if you really need to access these 2 events with special name,
why not add it through the json.. let's not have yet another
place that defines aliases ... also this should be model specific
no?
jirka
On Mon, Mar 29, 2021 at 03:00:31PM +0800, Jin Yao wrote:
SNIP
> + struct parse_events_state *parse_state)
> {
> struct perf_event_attr attr;
> LIST_HEAD(config_terms);
> @@ -521,7 +526,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
>
> i = parse_events__add_cache_hybrid(list, idx, &attr,
> config_name ? : name, &config_terms,
> - &hybrid);
> + &hybrid, parse_state);
> if (hybrid)
> return i;
>
> @@ -1481,7 +1486,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> struct perf_pmu *pmu;
> struct evsel *evsel;
> struct parse_events_error *err = parse_state->error;
> - bool use_uncore_alias;
> + bool use_uncore_alias, found = false;
> LIST_HEAD(config_terms);
>
> if (verbose > 1) {
> @@ -1530,8 +1535,28 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> }
> }
>
> - if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, &info))
> + if (!parse_state->fake_pmu &&
> + perf_pmu__check_alias(pmu, head_config, &info, &found)) {
> return -EINVAL;
> + }
> +
ok, let's not polute surronding functions and make strict check
on what we want in here.. we are after following events:
cpu_xxx/L1-dcache/
cpu_xxx/l1-d|/
...
right?
so we are after events with single term in head_config that has name in:
L1-dcache|l1-d|l1d|L1-data |
L1-icache|l1-i|l1i|L1-instruction |
LLC|L2 |
dTLB|d-tlb|Data-TLB |
iTLB|i-tlb|Instruction-TLB |
branch|branches|bpu|btb|bpc |
node
I think that with such direct check the code will be more straight
forward, also let's move it to parse-events-hybrid
> + if (!parse_state->fake_pmu && head_config && !found &&
> + perf_pmu__is_hybrid(name)) {
> + struct parse_events_term *term;
> + int ret;
> +
> + list_for_each_entry(term, head_config, list) {
> + if (!term->config)
> + continue;
> +
> + ret = parse_events__with_hybrid_pmu(parse_state,
> + term->config,
> + name, &found,
> + list);
do we need to call the parsing again? could we just call
parse_events__add_cache_hybrid?
jirka
> + if (found)
> + return ret;
> + }
> + }
>
> if (verbose > 1) {
> fprintf(stderr, "After aliases, add event pmu '%s' with '",
> @@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> struct perf_pmu *pmu = NULL;
> int ok = 0;
>
SNIP
On Mon, Mar 29, 2021 at 03:00:29PM +0800, Jin Yao wrote:
SNIP
> + name, config_terms, pmu);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
> struct list_head *list,
> struct perf_event_attr *attr,
> @@ -91,6 +126,9 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
> if (attr->type != PERF_TYPE_RAW) {
> return add_hw_hybrid(parse_state, list, attr, name,
> config_terms);
> + } else {
> + return add_raw_hybrid(parse_state, list, attr, name,
> + config_terms);
> }
>
> return -1;
no need for the return -1
jirka
> --
> 2.17.1
>
Hi Jiri,
On 4/9/2021 9:48 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:28PM +0800, Jin Yao wrote:
>
> SNIP
>
>> index 1bbd0ba92ba7..3692fa3c964a 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -458,6 +458,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
>> int cache_type = -1, cache_op = -1, cache_result = -1;
>> char *op_result[2] = { op_result1, op_result2 };
>> int i, n;
>> + bool hybrid;
>>
>> /*
>> * No fallback - if we cannot get a clear cache type
>> @@ -517,6 +518,13 @@ int parse_events_add_cache(struct list_head *list, int *idx,
>> if (get_config_terms(head_config, &config_terms))
>> return -ENOMEM;
>> }
>> +
>> + i = parse_events__add_cache_hybrid(list, idx, &attr,
>> + config_name ? : name, &config_terms,
>> + &hybrid);
>> + if (hybrid)
>> + return i;
>
> please define 'ret' for the return value, i is confusing
>
> thanks,
> jirka
>
Previously I wanted to save a 'ret' variable, but yes it's confusing. I will define 'ret' in next
version.
Thanks
Jin Yao
>> +
>> return add_event(list, idx, &attr, config_name ? : name, &config_terms);
>> }
>>
>> --
>> 2.17.1
>>
>
Hi Jiri,
On 4/9/2021 9:49 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:29PM +0800, Jin Yao wrote:
>
> SNIP
>
>> + name, config_terms, pmu);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
>> struct list_head *list,
>> struct perf_event_attr *attr,
>> @@ -91,6 +126,9 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state,
>> if (attr->type != PERF_TYPE_RAW) {
>> return add_hw_hybrid(parse_state, list, attr, name,
>> config_terms);
>> + } else {
>> + return add_raw_hybrid(parse_state, list, attr, name,
>> + config_terms);
>> }
>>
>> return -1;
>
> no need for the return -1
>
> jirka
>
Yes, no need return -1 here.
if (attr->type != PERF_TYPE_RAW) {
return add_hw_hybrid(parse_state, list, attr, name,
config_terms);
}
return add_raw_hybrid(parse_state, list, attr, name,
config_terms);
Thanks
Jin Yao
>> --
>> 2.17.1
>>
>
Hi Jiri,
On 4/9/2021 9:48 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:30PM +0800, Jin Yao wrote:
>> On hybrid platform, user may want to enable the hardware event
>> only on one PMU. So following syntax is supported:
>>
>> cpu_core/<hardware event>/
>> cpu_atom/<hardware event>/
>>
>> # perf stat -e cpu_core/cpu-cycles/ -a -- sleep 1
>>
>> Performance counter stats for 'system wide':
>>
>> 6,049,336 cpu_core/cpu-cycles/
>>
>> 1.003577042 seconds time elapsed
>>
>> It enables the event 'cpu-cycles' only on cpu_core pmu.
>>
>> But for 'cycles' and 'branches', the syntax doesn't work.
>
> because the alias is not there.. but there's:
> cpu/cpu-cycles/
> cpu/branch-instructions/
>
> doing the same thing.. what's wrong with that?
>
> I have a feeling we discussed this in the previous
> version.. did I give up? ;-)
>
Yes, we discussed this in previous threads. :)
Now I'm fine to keep the original behavior. Because the syntax 'cpu/cycles/' and 'cpu/branches/' are
not supported by current perf.
> SNIP
>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index beff29981101..72e5ae5e868e 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -916,6 +916,35 @@ static int pmu_max_precise(const char *name)
>> return max_precise;
>> }
>>
>> +static void perf_pmu__add_hybrid_aliases(struct list_head *head)
>> +{
>> + static struct pmu_event pme_hybrid_fixup[] = {
>> + {
>> + .name = "cycles",
>> + .event = "event=0x3c",
>> + },
>> + {
>> + .name = "branches",
>> + .event = "event=0xc4",
>> + },
>> + {
>> + .name = 0,
>> + .event = 0,
>> + },
>
> if you really need to access these 2 events with special name,
> why not add it through the json.. let's not have yet another
> place that defines aliases ... also this should be model specific
> no?
>
Yes, defining in json is a good idea if we really need to support 'cpu/cycles/' and 'cpu/branches/'.
Anyway, I will drop this patch in next version.
Thanks
Jin Yao
> jirka
>
Hi Jiri,
On 4/9/2021 9:47 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:31PM +0800, Jin Yao wrote:
>
> SNIP
>
>> + struct parse_events_state *parse_state)
>> {
>> struct perf_event_attr attr;
>> LIST_HEAD(config_terms);
>> @@ -521,7 +526,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
>>
>> i = parse_events__add_cache_hybrid(list, idx, &attr,
>> config_name ? : name, &config_terms,
>> - &hybrid);
>> + &hybrid, parse_state);
>> if (hybrid)
>> return i;
>>
>> @@ -1481,7 +1486,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>> struct perf_pmu *pmu;
>> struct evsel *evsel;
>> struct parse_events_error *err = parse_state->error;
>> - bool use_uncore_alias;
>> + bool use_uncore_alias, found = false;
>> LIST_HEAD(config_terms);
>>
>> if (verbose > 1) {
>> @@ -1530,8 +1535,28 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>> }
>> }
>>
>> - if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, head_config, &info))
>> + if (!parse_state->fake_pmu &&
>> + perf_pmu__check_alias(pmu, head_config, &info, &found)) {
>> return -EINVAL;
>> + }
>> +
>
> ok, let's not polute surronding functions and make strict check
> on what we want in here.. we are after following events:
>
> cpu_xxx/L1-dcache/
> cpu_xxx/l1-d|/
> ...
> right?
>
Yes, we only focus on the cache events now.
> so we are after events with single term in head_config that has name in:
>
> L1-dcache|l1-d|l1d|L1-data |
> L1-icache|l1-i|l1i|L1-instruction |
> LLC|L2 |
> dTLB|d-tlb|Data-TLB |
> iTLB|i-tlb|Instruction-TLB |
> branch|branches|bpu|btb|bpc |
> node
>
> I think that with such direct check the code will be more straight
> forward, also let's move it to parse-events-hybrid
>
Do you suggest we just use string comparison for doing the direct check?
e.g.
if (strstr(term->config, "L1-dcache"))
...
Of course, we can define a string array first and use a loop for string comparison.
>> + if (!parse_state->fake_pmu && head_config && !found &&
>> + perf_pmu__is_hybrid(name)) {
>> + struct parse_events_term *term;
>> + int ret;
>> +
>> + list_for_each_entry(term, head_config, list) {
>> + if (!term->config)
>> + continue;
>> +
>> + ret = parse_events__with_hybrid_pmu(parse_state,
>> + term->config,
>> + name, &found,
>> + list);
>
> do we need to call the parsing again? could we just call
> parse_events__add_cache_hybrid?
>
> jirka
>
>
If we do the direct check for cache events, I think we don't need the parsing again.
As I mentioned above, we need to define a string array and compare with term->config one by one.
I'm OK for this solution. :)
Thanks
Jin Yao
>> + if (found)
>> + return ret;
>> + }
>> + }
>>
>> if (verbose > 1) {
>> fprintf(stderr, "After aliases, add event pmu '%s' with '",
>> @@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>> struct perf_pmu *pmu = NULL;
>> int ok = 0;
>>
>
> SNIP
>
On Mon, Apr 12, 2021 at 10:51:14AM +0800, Jin, Yao wrote:
SNIP
>
> Do you suggest we just use string comparison for doing the direct check?
>
> e.g.
>
> if (strstr(term->config, "L1-dcache"))
> ...
>
> Of course, we can define a string array first and use a loop for string comparison.
>
> > > + if (!parse_state->fake_pmu && head_config && !found &&
> > > + perf_pmu__is_hybrid(name)) {
> > > + struct parse_events_term *term;
> > > + int ret;
> > > +
> > > + list_for_each_entry(term, head_config, list) {
> > > + if (!term->config)
> > > + continue;
> > > +
> > > + ret = parse_events__with_hybrid_pmu(parse_state,
> > > + term->config,
> > > + name, &found,
> > > + list);
> >
> > do we need to call the parsing again? could we just call
> > parse_events__add_cache_hybrid?
> >
> > jirka
> >
> >
>
> If we do the direct check for cache events, I think we don't need the parsing again.
>
> As I mentioned above, we need to define a string array and compare with term->config one by one.
>
maybe another way is to find a way to run just the lexer (without
parser) and check that it returns PE_NAME_CACHE_OP_RESULT
jirka
On Mon, Mar 29, 2021 at 03:00:31PM +0800, Jin Yao wrote:
SNIP
> ---
> v3:
> - Rename the patch:
> 'perf parse-events: Support hardware events inside PMU' -->
> 'perf parse-events: Support no alias assigned event inside hybrid PMU'
>
> - Major code is moved to parse-events-hybrid.c.
> - Refine the code.
>
> tools/perf/util/parse-events-hybrid.c | 18 +++++-
> tools/perf/util/parse-events-hybrid.h | 3 +-
> tools/perf/util/parse-events.c | 80 +++++++++++++++++++++++++--
> tools/perf/util/parse-events.h | 4 +-
> tools/perf/util/parse-events.y | 9 ++-
> tools/perf/util/pmu.c | 4 +-
> tools/perf/util/pmu.h | 2 +-
> 7 files changed, 108 insertions(+), 12 deletions(-)
please move the support to pass pmu_name and filter
on it within hybrid code in to separate patch
>
> diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
> index 8a630cbab8f3..5bf176b55573 100644
> --- a/tools/perf/util/parse-events-hybrid.c
> +++ b/tools/perf/util/parse-events-hybrid.c
> @@ -64,6 +64,11 @@ static int add_hw_hybrid(struct parse_events_state *parse_state,
> int ret;
>
> perf_pmu__for_each_hybrid_pmu(pmu) {
> + if (parse_state->pmu_name &&
> + strcmp(parse_state->pmu_name, pmu->name)) {
> + continue;
please add this check to separate function
if (pmu_cmp(parse_stat))
continue;
SNIP
> + if (!parse_state->fake_pmu && head_config && !found &&
> + perf_pmu__is_hybrid(name)) {
> + struct parse_events_term *term;
> + int ret;
> +
> + list_for_each_entry(term, head_config, list) {
> + if (!term->config)
> + continue;
> +
> + ret = parse_events__with_hybrid_pmu(parse_state,
> + term->config,
> + name, &found,
> + list);
> + if (found)
> + return ret;
what if there are more terms in head_config?
should we make sure there's just one term and fail if there's more?
also we already know the perf_pmu__is_hybrid(name) is true,
so can't we just call:
return parse_events__with_hybrid_pmu(....)
> + }
> + }
>
> if (verbose > 1) {
> fprintf(stderr, "After aliases, add event pmu '%s' with '",
> @@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> struct perf_pmu *pmu = NULL;
> int ok = 0;
>
> + if (parse_state->pmu_name) {
> + list = malloc(sizeof(struct list_head));
> + if (!list)
> + return -1;
> + INIT_LIST_HEAD(list);
> + *listp = list;
> + return 0;
> + }
hum, why is this needed?
> +
> *listp = NULL;
> /* Add it for all PMUs that support the alias */
> list = malloc(sizeof(struct list_head));
> @@ -2176,6 +2210,44 @@ int parse_events_terms(struct list_head *terms, const char *str)
> return ret;
> }
>
> +static int list_entries_nr(struct list_head *list)
> +{
> + struct list_head *pos;
> + int n = 0;
> +
> + list_for_each(pos, list)
> + n++;
> +
> + return n;
> +}
> +
> +static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
> + const char *str, char *pmu_name,
> + bool *found, struct list_head *list)
> +{
> + struct parse_events_state ps = {
> + .list = LIST_HEAD_INIT(ps.list),
> + .stoken = PE_START_EVENTS,
> + .pmu_name = pmu_name,
> + .idx = parse_state->idx,
> + };
could we add this pmu_name directly to __parse_events?
it duplicates the code plus there are some extra checks
you don't do in here and which might be needed, like
last->cmdline_group_boundary setup
> + int ret;
> +
> + *found = false;
> + ret = parse_events__scanner(str, &ps);
> + perf_pmu__parse_cleanup();
> +
> + if (!ret) {
> + if (!list_empty(&ps.list)) {
> + *found = true;
> + list_splice(&ps.list, list);
> + parse_state->idx = list_entries_nr(list);
could you just use ps.idx instead of list_entries_nr ?
> + }
> + }
> +
> + return ret;
> +}
> +
> int __parse_events(struct evlist *evlist, const char *str,
> struct parse_events_error *err, struct perf_pmu *fake_pmu)
> {
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index c4f2f96304ce..f9d8e8e41c38 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -138,6 +138,7 @@ struct parse_events_state {
> struct list_head *terms;
> int stoken;
> struct perf_pmu *fake_pmu;
> + char *pmu_name;
so it's hybrid specific, we should name it like hybrid_pmu_name or such
thanks,
jirka
Hi Jiri,
On 4/15/2021 7:03 PM, Jiri Olsa wrote:
> On Mon, Mar 29, 2021 at 03:00:31PM +0800, Jin Yao wrote:
>
> SNIP
>
>> ---
>> v3:
>> - Rename the patch:
>> 'perf parse-events: Support hardware events inside PMU' -->
>> 'perf parse-events: Support no alias assigned event inside hybrid PMU'
>>
>> - Major code is moved to parse-events-hybrid.c.
>> - Refine the code.
>>
>> tools/perf/util/parse-events-hybrid.c | 18 +++++-
>> tools/perf/util/parse-events-hybrid.h | 3 +-
>> tools/perf/util/parse-events.c | 80 +++++++++++++++++++++++++--
>> tools/perf/util/parse-events.h | 4 +-
>> tools/perf/util/parse-events.y | 9 ++-
>> tools/perf/util/pmu.c | 4 +-
>> tools/perf/util/pmu.h | 2 +-
>> 7 files changed, 108 insertions(+), 12 deletions(-)
>
> please move the support to pass pmu_name and filter
> on it within hybrid code in to separate patch
>
OK.
>>
>> diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c
>> index 8a630cbab8f3..5bf176b55573 100644
>> --- a/tools/perf/util/parse-events-hybrid.c
>> +++ b/tools/perf/util/parse-events-hybrid.c
>> @@ -64,6 +64,11 @@ static int add_hw_hybrid(struct parse_events_state *parse_state,
>> int ret;
>>
>> perf_pmu__for_each_hybrid_pmu(pmu) {
>> + if (parse_state->pmu_name &&
>> + strcmp(parse_state->pmu_name, pmu->name)) {
>> + continue;
>
> please add this check to separate function
>
> if (pmu_cmp(parse_stat))
> continue;
>
OK.
> SNIP
>
>> + if (!parse_state->fake_pmu && head_config && !found &&
>> + perf_pmu__is_hybrid(name)) {
>> + struct parse_events_term *term;
>> + int ret;
>> +
>> + list_for_each_entry(term, head_config, list) {
>> + if (!term->config)
>> + continue;
>> +
>> + ret = parse_events__with_hybrid_pmu(parse_state,
>> + term->config,
>> + name, &found,
>> + list);
>> + if (found)
>> + return ret;
>
> what if there are more terms in head_config?
> should we make sure there's just one term and fail if there's more?
>
Yes, it should have only one term in head_config.
Now I change the code to:
+ if (!parse_state->fake_pmu && head_config && !found &&
+ perf_pmu__is_hybrid(name)) {
+ struct parse_events_term *term;
+
+ term = list_first_entry(head_config, struct parse_events_term,
+ list);
+ if (term->config) {
+ return parse_events__with_hybrid_pmu(parse_state,
+ term->config,
+ name, list);
+ }
+ }
> also we already know the perf_pmu__is_hybrid(name) is true,
> so can't we just call:
>
> return parse_events__with_hybrid_pmu(....)
>
>
Yes, we can direct return parse_events__with_hybrid_pmu().
>> + }
>> + }
>>
>> if (verbose > 1) {
>> fprintf(stderr, "After aliases, add event pmu '%s' with '",
>> @@ -1605,6 +1630,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>> struct perf_pmu *pmu = NULL;
>> int ok = 0;
>>
>> + if (parse_state->pmu_name) {
>> + list = malloc(sizeof(struct list_head));
>> + if (!list)
>> + return -1;
>> + INIT_LIST_HEAD(list);
>> + *listp = list;
>> + return 0;
>> + }
>
> hum, why is this needed?
>
Hmm, it's not necessary in new code, sorry about that.
>> +
>> *listp = NULL;
>> /* Add it for all PMUs that support the alias */
>> list = malloc(sizeof(struct list_head));
>> @@ -2176,6 +2210,44 @@ int parse_events_terms(struct list_head *terms, const char *str)
>> return ret;
>> }
>>
>> +static int list_entries_nr(struct list_head *list)
>> +{
>> + struct list_head *pos;
>> + int n = 0;
>> +
>> + list_for_each(pos, list)
>> + n++;
>> +
>> + return n;
>> +}
>> +
>> +static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
>> + const char *str, char *pmu_name,
>> + bool *found, struct list_head *list)
>> +{
>> + struct parse_events_state ps = {
>> + .list = LIST_HEAD_INIT(ps.list),
>> + .stoken = PE_START_EVENTS,
>> + .pmu_name = pmu_name,
>> + .idx = parse_state->idx,
>> + };
>
> could we add this pmu_name directly to __parse_events?
>
Do you suggest we directly call __parse_events()?
int __parse_events(struct evlist *evlist, const char *str,
struct parse_events_error *err, struct perf_pmu *fake_pmu)
struct parse_events_state parse_state = {
.list = LIST_HEAD_INIT(parse_state.list),
.idx = evlist->core.nr_entries,
.error = err,
.evlist = evlist,
.stoken = PE_START_EVENTS,
.fake_pmu = fake_pmu,
};
But for parse_events__with_hybrid_pmu, we don't have valid evlist. So if we switch to
__parse_events, evlist processing may be a problem.
So could we still keep current parse_events__with_hybrid_pmu()?
> it duplicates the code plus there are some extra checks
> you don't do in here and which might be needed, like
> last->cmdline_group_boundary setup
>
>> + int ret;
>> +
>> + *found = false;
>> + ret = parse_events__scanner(str, &ps);
>> + perf_pmu__parse_cleanup();
>> +
>> + if (!ret) {
>> + if (!list_empty(&ps.list)) {
>> + *found = true;
>> + list_splice(&ps.list, list);
>> + parse_state->idx = list_entries_nr(list);
>
> could you just use ps.idx instead of list_entries_nr ?
>
Yes, the code will be changed to:
+
+ if (!ret) {
+ if (!list_empty(&ps.list)) {
+ list_splice(&ps.list, list);
+ parse_state->idx = ps.idx;
+ }
+ }
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> int __parse_events(struct evlist *evlist, const char *str,
>> struct parse_events_error *err, struct perf_pmu *fake_pmu)
>> {
>> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
>> index c4f2f96304ce..f9d8e8e41c38 100644
>> --- a/tools/perf/util/parse-events.h
>> +++ b/tools/perf/util/parse-events.h
>> @@ -138,6 +138,7 @@ struct parse_events_state {
>> struct list_head *terms;
>> int stoken;
>> struct perf_pmu *fake_pmu;
>> + char *pmu_name;
>
> so it's hybrid specific, we should name it like hybrid_pmu_name or such
>
OK, I will use hybrid_pmu_name in next version.
Thanks
Jin Yao
> thanks,
> jirka
>
On Thu, Apr 15, 2021 at 09:36:16PM +0800, Jin, Yao wrote:
SNIP
> > > + int n = 0;
> > > +
> > > + list_for_each(pos, list)
> > > + n++;
> > > +
> > > + return n;
> > > +}
> > > +
> > > +static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
> > > + const char *str, char *pmu_name,
> > > + bool *found, struct list_head *list)
> > > +{
> > > + struct parse_events_state ps = {
> > > + .list = LIST_HEAD_INIT(ps.list),
> > > + .stoken = PE_START_EVENTS,
> > > + .pmu_name = pmu_name,
> > > + .idx = parse_state->idx,
> > > + };
> >
> > could we add this pmu_name directly to __parse_events?
> >
>
> Do you suggest we directly call __parse_events()?
>
> int __parse_events(struct evlist *evlist, const char *str,
> struct parse_events_error *err, struct perf_pmu *fake_pmu)
>
> struct parse_events_state parse_state = {
> .list = LIST_HEAD_INIT(parse_state.list),
> .idx = evlist->core.nr_entries,
> .error = err,
> .evlist = evlist,
> .stoken = PE_START_EVENTS,
> .fake_pmu = fake_pmu,
> };
>
> But for parse_events__with_hybrid_pmu, we don't have valid evlist. So if we
> switch to __parse_events, evlist processing may be a problem.
you should use parse_state->evlist no? but we can chec/make this
change in next itaration.. it's already lot of changes
jirka
Hi Jiri,
On 4/15/2021 10:11 PM, Jiri Olsa wrote:
> On Thu, Apr 15, 2021 at 09:36:16PM +0800, Jin, Yao wrote:
>
> SNIP
>
>>>> + int n = 0;
>>>> +
>>>> + list_for_each(pos, list)
>>>> + n++;
>>>> +
>>>> + return n;
>>>> +}
>>>> +
>>>> +static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
>>>> + const char *str, char *pmu_name,
>>>> + bool *found, struct list_head *list)
>>>> +{
>>>> + struct parse_events_state ps = {
>>>> + .list = LIST_HEAD_INIT(ps.list),
>>>> + .stoken = PE_START_EVENTS,
>>>> + .pmu_name = pmu_name,
>>>> + .idx = parse_state->idx,
>>>> + };
>>>
>>> could we add this pmu_name directly to __parse_events?
>>>
>>
>> Do you suggest we directly call __parse_events()?
>>
>> int __parse_events(struct evlist *evlist, const char *str,
>> struct parse_events_error *err, struct perf_pmu *fake_pmu)
>>
>> struct parse_events_state parse_state = {
>> .list = LIST_HEAD_INIT(parse_state.list),
>> .idx = evlist->core.nr_entries,
>> .error = err,
>> .evlist = evlist,
>> .stoken = PE_START_EVENTS,
>> .fake_pmu = fake_pmu,
>> };
>>
>> But for parse_events__with_hybrid_pmu, we don't have valid evlist. So if we
>> switch to __parse_events, evlist processing may be a problem.
>
> you should use parse_state->evlist no? but we can chec/make this
> change in next itaration.. it's already lot of changes
>
> jirka
>
With my current code,
static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
const char *str, char *pmu_name,
struct list_head *list)
{
struct parse_events_state ps = {
.list = LIST_HEAD_INIT(ps.list),
.stoken = PE_START_EVENTS,
.pmu_name = pmu_name,
.idx = parse_state->idx,
};
int ret;
ret = parse_events__scanner(str, &ps);
perf_pmu__parse_cleanup();
if (!ret) {
if (!list_empty(&ps.list)) {
list_splice(&ps.list, list);
parse_state->idx = ps.idx;
}
}
return ret;
}
The new created evsels are added to the tail of list (ps.list) and ps.list is joined to the list
(the parameter 'list').
If we want to reuse the __parse_events(), we may need to:
struct evlist *evlist = evlist__new();
__parse_events(evlist, str, NULL, NULL);
Add the evsels in evlist to the tail of list (the parameter 'list')
evlist__delete(evlist);
Is my understanding correct?
Yes, we have to change the interface of __parse_events() by adding a new parameter 'pmu_name', which
will bring much more changes. I agree to make this change in follow-up patches.
Thanks
Jin Yao
On Thu, Apr 15, 2021 at 10:53:33PM +0800, Jin, Yao wrote:
SNIP
>
> With my current code,
>
> static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
> const char *str, char *pmu_name,
> struct list_head *list)
> {
> struct parse_events_state ps = {
> .list = LIST_HEAD_INIT(ps.list),
> .stoken = PE_START_EVENTS,
> .pmu_name = pmu_name,
> .idx = parse_state->idx,
> };
> int ret;
>
> ret = parse_events__scanner(str, &ps);
> perf_pmu__parse_cleanup();
>
> if (!ret) {
> if (!list_empty(&ps.list)) {
> list_splice(&ps.list, list);
> parse_state->idx = ps.idx;
> }
> }
>
> return ret;
> }
>
> The new created evsels are added to the tail of list (ps.list) and ps.list
> is joined to the list (the parameter 'list').
>
> If we want to reuse the __parse_events(), we may need to:
>
> struct evlist *evlist = evlist__new();
there's the original evlist pointer passed to the initial parser
that we should use no?
>
> __parse_events(evlist, str, NULL, NULL);
> Add the evsels in evlist to the tail of list (the parameter 'list')
> evlist__delete(evlist);
>
> Is my understanding correct?
>
> Yes, we have to change the interface of __parse_events() by adding a new
> parameter 'pmu_name', which will bring much more changes. I agree to make
> this change in follow-up patches.
let's check on this over the next version
thanks,
jirka
Hi Jiri,
On 4/16/2021 3:39 AM, Jiri Olsa wrote:
> On Thu, Apr 15, 2021 at 10:53:33PM +0800, Jin, Yao wrote:
>
> SNIP
>
>>
>> With my current code,
>>
>> static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
>> const char *str, char *pmu_name,
>> struct list_head *list)
>> {
>> struct parse_events_state ps = {
>> .list = LIST_HEAD_INIT(ps.list),
>> .stoken = PE_START_EVENTS,
>> .pmu_name = pmu_name,
>> .idx = parse_state->idx,
>> };
>> int ret;
>>
>> ret = parse_events__scanner(str, &ps);
>> perf_pmu__parse_cleanup();
>>
>> if (!ret) {
>> if (!list_empty(&ps.list)) {
>> list_splice(&ps.list, list);
>> parse_state->idx = ps.idx;
>> }
>> }
>>
>> return ret;
>> }
>>
>> The new created evsels are added to the tail of list (ps.list) and ps.list
>> is joined to the list (the parameter 'list').
>>
>> If we want to reuse the __parse_events(), we may need to:
>>
>> struct evlist *evlist = evlist__new();
>
> there's the original evlist pointer passed to the initial parser
> that we should use no?
>
Unfortunately the answer is no. :(
For "cpu_core/LLC-loads/", if we do the parser twice by just calling __parse_events, actually the
__parse_events will be called two times.
int __parse_events(struct evlist *evlist, const char *str,
struct parse_events_error *err, struct perf_pmu *fake_pmu,
char *pmu_name)
{
struct parse_events_state parse_state = {
.list = LIST_HEAD_INIT(parse_state.list),
...
};
ret = parse_events__scanner(str, &parse_state);
perf_pmu__parse_cleanup();
if (!ret && list_empty(&parse_state.list)) {
WARN_ONCE(true, "WARNING: event parser found nothing\n");
return -1;
}
...
}
When returning to the first __parse_events,'parse_state.list' is an empty list so it would return
"WARNING: event parser found nothing".
So in my patch, I pass a list pointer in and the new created evsels will be added to this list.
>>
>> __parse_events(evlist, str, NULL, NULL);
>> Add the evsels in evlist to the tail of list (the parameter 'list')
>> evlist__delete(evlist);
>>
>> Is my understanding correct?
>>
>> Yes, we have to change the interface of __parse_events() by adding a new
>> parameter 'pmu_name', which will bring much more changes. I agree to make
>> this change in follow-up patches.
>
> let's check on this over the next version
>
That's fine, thanks.
Thanks
Jin Yao
> thanks,
> jirka
>