From: Kan Liang <[email protected]>
As discussed in the below thread, the patch set is to clean up perf mem.
https://lore.kernel.org/lkml/[email protected]/
Introduce generic functions perf_mem_events__ptr(),
perf_mem_events__name() ,and is_mem_loads_aux_event() to replace the
ARCH specific ones.
Simplify the perf_mem_event__supported().
Only keeps the ARCH-specific perf_mem_events array in the corresponding
mem-events.c for each ARCH.
There is no functional change.
The patch set touches almost all the ARCHs, Intel, AMD, ARM, Power and
etc. But I can only test it on two Intel platforms.
Please give it try, if you have machines with other ARCHs.
Here are the test results:
Intel hybrid machine:
$perf mem record -e list
ldlat-loads : available
ldlat-stores : available
$perf mem record -e ldlat-loads -v --ldlat 50
calling: record -e cpu_atom/mem-loads,ldlat=50/P -e cpu_core/mem-loads,ldlat=50/P
$perf mem record -v
calling: record -e cpu_atom/mem-loads,ldlat=30/P -e cpu_atom/mem-stores/P -e cpu_core/mem-loads,ldlat=30/P -e cpu_core/mem-stores/P
$perf mem record -t store -v
calling: record -e cpu_atom/mem-stores/P -e cpu_core/mem-stores/P
Intel SPR:
$perf mem record -e list
ldlat-loads : available
ldlat-stores : available
$perf mem record -e ldlat-loads -v --ldlat 50
calling: record -e {cpu/mem-loads-aux/,cpu/mem-loads,ldlat=50/}:P
$perf mem record -v
calling: record -e {cpu/mem-loads-aux/,cpu/mem-loads,ldlat=30/}:P -e cpu/mem-stores/P
$perf mem record -t store -v
calling: record -e cpu/mem-stores/P
Kan Liang (6):
perf mem: Add mem_events into the supported perf_pmu
perf mem: Clean up perf_mem_events__ptr()
perf mem: Clean up perf_mem_events__name()
perf mem: Clean up perf_mem_event__supported()
perf mem: Clean up is_mem_loads_aux_event()
perf mem: Remove useless header files for X86
tools/perf/arch/arm64/util/mem-events.c | 36 +----
tools/perf/arch/arm64/util/mem-events.h | 7 +
tools/perf/arch/arm64/util/pmu.c | 6 +
tools/perf/arch/powerpc/util/mem-events.c | 13 +-
tools/perf/arch/powerpc/util/mem-events.h | 7 +
tools/perf/arch/powerpc/util/pmu.c | 11 ++
tools/perf/arch/s390/util/pmu.c | 3 +
tools/perf/arch/x86/util/mem-events.c | 99 +++---------
tools/perf/arch/x86/util/mem-events.h | 10 ++
tools/perf/arch/x86/util/pmu.c | 11 ++
tools/perf/builtin-c2c.c | 28 +++-
tools/perf/builtin-mem.c | 28 +++-
tools/perf/util/mem-events.c | 177 +++++++++++++---------
tools/perf/util/mem-events.h | 15 +-
tools/perf/util/pmu.c | 4 +-
tools/perf/util/pmu.h | 7 +
16 files changed, 248 insertions(+), 214 deletions(-)
create mode 100644 tools/perf/arch/arm64/util/mem-events.h
create mode 100644 tools/perf/arch/powerpc/util/mem-events.h
create mode 100644 tools/perf/arch/powerpc/util/pmu.c
create mode 100644 tools/perf/arch/x86/util/mem-events.h
--
2.35.1
From: Kan Liang <[email protected]>
The mem_events can be retrieved from the struct perf_pmu now. An ARCH
specific perf_mem_events__ptr() is not required anymore. Remove all of
them.
The Intel hybrid has multiple mem-events-supported PMUs. But they share
the same mem_events. Other ARCHs only support one mem-events-supported
PMU. In the configuration, it's good enough to only configure the
mem_events for one PMU. Add perf_mem_events_find_pmu() which returns the
first mem-events-supported PMU.
In the perf_mem_events__init(), the perf_pmus__scan() is not required
anymore. It avoids checking the sysfs for every PMU on the system.
Make the perf_mem_events__record_args() more generic. Remove the
perf_mem_events__print_unsupport_hybrid().
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/arch/arm64/util/mem-events.c | 10 +--
tools/perf/arch/x86/util/mem-events.c | 18 ++---
tools/perf/builtin-c2c.c | 28 +++++--
tools/perf/builtin-mem.c | 28 +++++--
tools/perf/util/mem-events.c | 103 ++++++++++++------------
tools/perf/util/mem-events.h | 9 ++-
6 files changed, 104 insertions(+), 92 deletions(-)
diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
index aaa4804922b4..2602e8688727 100644
--- a/tools/perf/arch/arm64/util/mem-events.c
+++ b/tools/perf/arch/arm64/util/mem-events.c
@@ -12,17 +12,9 @@ struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
static char mem_ev_name[100];
-struct perf_mem_event *perf_mem_events__ptr(int i)
-{
- if (i >= PERF_MEM_EVENTS__MAX)
- return NULL;
-
- return &perf_mem_events_arm[i];
-}
-
const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
{
- struct perf_mem_event *e = perf_mem_events__ptr(i);
+ struct perf_mem_event *e = &perf_mem_events_arm[i];
if (i >= PERF_MEM_EVENTS__MAX)
return NULL;
diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index 2b81d229982c..5fb41d50118d 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -28,17 +28,6 @@ struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
E("mem-ldst", "ibs_op//", "ibs_op"),
};
-struct perf_mem_event *perf_mem_events__ptr(int i)
-{
- if (i >= PERF_MEM_EVENTS__MAX)
- return NULL;
-
- if (x86__is_amd_cpu())
- return &perf_mem_events_amd[i];
-
- return &perf_mem_events_intel[i];
-}
-
bool is_mem_loads_aux_event(struct evsel *leader)
{
struct perf_pmu *pmu = perf_pmus__find("cpu");
@@ -54,7 +43,12 @@ bool is_mem_loads_aux_event(struct evsel *leader)
const char *perf_mem_events__name(int i, const char *pmu_name)
{
- struct perf_mem_event *e = perf_mem_events__ptr(i);
+ struct perf_mem_event *e;
+
+ if (x86__is_amd_cpu())
+ e = &perf_mem_events_amd[i];
+ else
+ e = &perf_mem_events_intel[i];
if (!e)
return NULL;
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index a4cf9de7a7b5..76c760be1bcf 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -3215,12 +3215,19 @@ static int parse_record_events(const struct option *opt,
const char *str, int unset __maybe_unused)
{
bool *event_set = (bool *) opt->value;
+ struct perf_pmu *pmu;
+
+ pmu = perf_mem_events_find_pmu();
+ if (!pmu) {
+ pr_err("failed: there is no PMU that supports perf c2c\n");
+ exit(-1);
+ }
if (!strcmp(str, "list")) {
- perf_mem_events__list();
+ perf_mem_events__list(pmu);
exit(0);
}
- if (perf_mem_events__parse(str))
+ if (perf_mem_events__parse(pmu, str))
exit(-1);
*event_set = true;
@@ -3245,6 +3252,7 @@ static int perf_c2c__record(int argc, const char **argv)
bool all_user = false, all_kernel = false;
bool event_set = false;
struct perf_mem_event *e;
+ struct perf_pmu *pmu;
struct option options[] = {
OPT_CALLBACK('e', "event", &event_set, "event",
"event selector. Use 'perf c2c record -e list' to list available events",
@@ -3256,7 +3264,13 @@ static int perf_c2c__record(int argc, const char **argv)
OPT_END()
};
- if (perf_mem_events__init()) {
+ pmu = perf_mem_events_find_pmu();
+ if (!pmu) {
+ pr_err("failed: no PMU supports the memory events\n");
+ return -1;
+ }
+
+ if (perf_mem_events__init(pmu)) {
pr_err("failed: memory events not supported\n");
return -1;
}
@@ -3280,7 +3294,7 @@ static int perf_c2c__record(int argc, const char **argv)
rec_argv[i++] = "record";
if (!event_set) {
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD_STORE);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
/*
* The load and store operations are required, use the event
* PERF_MEM_EVENTS__LOAD_STORE if it is supported.
@@ -3289,15 +3303,15 @@ static int perf_c2c__record(int argc, const char **argv)
e->record = true;
rec_argv[i++] = "-W";
} else {
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
e->record = true;
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__STORE);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__STORE);
e->record = true;
}
}
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
if (e->record)
rec_argv[i++] = "-W";
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 51499c20da01..8218c4721101 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -43,12 +43,19 @@ static int parse_record_events(const struct option *opt,
const char *str, int unset __maybe_unused)
{
struct perf_mem *mem = *(struct perf_mem **)opt->value;
+ struct perf_pmu *pmu;
+
+ pmu = perf_mem_events_find_pmu();
+ if (!pmu) {
+ pr_err("failed: there is no PMU that supports perf mem\n");
+ exit(-1);
+ }
if (!strcmp(str, "list")) {
- perf_mem_events__list();
+ perf_mem_events__list(pmu);
exit(0);
}
- if (perf_mem_events__parse(str))
+ if (perf_mem_events__parse(pmu, str))
exit(-1);
mem->operation = 0;
@@ -72,6 +79,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
int ret;
bool all_user = false, all_kernel = false;
struct perf_mem_event *e;
+ struct perf_pmu *pmu;
struct option options[] = {
OPT_CALLBACK('e', "event", &mem, "event",
"event selector. use 'perf mem record -e list' to list available events",
@@ -84,7 +92,13 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
OPT_END()
};
- if (perf_mem_events__init()) {
+ pmu = perf_mem_events_find_pmu();
+ if (!pmu) {
+ pr_err("failed: no PMU supports the memory events\n");
+ return -1;
+ }
+
+ if (perf_mem_events__init(pmu)) {
pr_err("failed: memory events not supported\n");
return -1;
}
@@ -113,7 +127,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
rec_argv[i++] = "record";
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD_STORE);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
/*
* The load and store operations are required, use the event
@@ -126,17 +140,17 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
rec_argv[i++] = "-W";
} else {
if (mem->operation & MEM_OPERATION_LOAD) {
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
e->record = true;
}
if (mem->operation & MEM_OPERATION_STORE) {
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__STORE);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__STORE);
e->record = true;
}
}
- e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
+ e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
if (e->record)
rec_argv[i++] = "-W";
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 0a8f415f5efe..887ffdcce338 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -29,17 +29,42 @@ struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
static char mem_loads_name[100];
static bool mem_loads_name__init;
-struct perf_mem_event * __weak perf_mem_events__ptr(int i)
+struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i)
{
- if (i >= PERF_MEM_EVENTS__MAX)
+ if (i >= PERF_MEM_EVENTS__MAX || !pmu)
return NULL;
- return &perf_mem_events[i];
+ return &pmu->mem_events[i];
+}
+
+static struct perf_pmu *perf_pmus__scan_mem(struct perf_pmu *pmu)
+{
+ while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+ if (pmu->mem_events)
+ return pmu;
+ }
+ return NULL;
+}
+
+struct perf_pmu *perf_mem_events_find_pmu(void)
+{
+ /*
+ * The current perf mem doesn't support per-PMU configuration.
+ * The exact same configuration is applied to all the
+ * mem_events supported PMUs.
+ * Return the first mem_events supported PMU.
+ *
+ * Notes: The only case which may support multiple mem_events
+ * supported PMUs is Intel hybrid. The exact same mem_events
+ * is shared among the PMUs. Only configure the first PMU
+ * is good enough as well.
+ */
+ return perf_pmus__scan_mem(NULL);
}
const char * __weak perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
{
- struct perf_mem_event *e = perf_mem_events__ptr(i);
+ struct perf_mem_event *e = &perf_mem_events[i];
if (!e)
return NULL;
@@ -61,7 +86,7 @@ __weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
return false;
}
-int perf_mem_events__parse(const char *str)
+int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
{
char *tok, *saveptr = NULL;
bool found = false;
@@ -79,7 +104,7 @@ int perf_mem_events__parse(const char *str)
while (tok) {
for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
- struct perf_mem_event *e = perf_mem_events__ptr(j);
+ struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
if (!e->tag)
continue;
@@ -112,7 +137,7 @@ static bool perf_mem_event__supported(const char *mnt, struct perf_pmu *pmu,
return !stat(path, &st);
}
-int perf_mem_events__init(void)
+int perf_mem_events__init(struct perf_pmu *pmu)
{
const char *mnt = sysfs__mount();
bool found = false;
@@ -122,8 +147,7 @@ int perf_mem_events__init(void)
return -ENOENT;
for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
- struct perf_mem_event *e = perf_mem_events__ptr(j);
- struct perf_pmu *pmu = NULL;
+ struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
/*
* If the event entry isn't valid, skip initialization
@@ -132,29 +156,20 @@ int perf_mem_events__init(void)
if (!e->tag)
continue;
- /*
- * Scan all PMUs not just core ones, since perf mem/c2c on
- * platforms like AMD uses IBS OP PMU which is independent
- * of core PMU.
- */
- while ((pmu = perf_pmus__scan(pmu)) != NULL) {
- e->supported |= perf_mem_event__supported(mnt, pmu, e);
- if (e->supported) {
- found = true;
- break;
- }
- }
+ e->supported |= perf_mem_event__supported(mnt, pmu, e);
+ if (e->supported)
+ found = true;
}
return found ? 0 : -ENOENT;
}
-void perf_mem_events__list(void)
+void perf_mem_events__list(struct perf_pmu *pmu)
{
int j;
for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
- struct perf_mem_event *e = perf_mem_events__ptr(j);
+ struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
fprintf(stderr, "%-*s%-*s%s",
e->tag ? 13 : 0,
@@ -165,50 +180,32 @@ void perf_mem_events__list(void)
}
}
-static void perf_mem_events__print_unsupport_hybrid(struct perf_mem_event *e,
- int idx)
-{
- const char *mnt = sysfs__mount();
- struct perf_pmu *pmu = NULL;
-
- while ((pmu = perf_pmus__scan(pmu)) != NULL) {
- if (!perf_mem_event__supported(mnt, pmu, e)) {
- pr_err("failed: event '%s' not supported\n",
- perf_mem_events__name(idx, pmu->name));
- }
- }
-}
-
int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
char **rec_tmp, int *tmp_nr)
{
const char *mnt = sysfs__mount();
+ struct perf_pmu *pmu = NULL;
int i = *argv_nr, k = 0;
struct perf_mem_event *e;
- for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
- e = perf_mem_events__ptr(j);
- if (!e->record)
- continue;
- if (perf_pmus__num_mem_pmus() == 1) {
- if (!e->supported) {
- pr_err("failed: event '%s' not supported\n",
- perf_mem_events__name(j, NULL));
- return -1;
- }
+ while ((pmu = perf_pmus__scan_mem(pmu)) != NULL) {
+ for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
+ e = perf_mem_events__ptr(pmu, j);
- rec_argv[i++] = "-e";
- rec_argv[i++] = perf_mem_events__name(j, NULL);
- } else {
- struct perf_pmu *pmu = NULL;
+ if (!e->record)
+ continue;
if (!e->supported) {
- perf_mem_events__print_unsupport_hybrid(e, j);
+ pr_err("failed: event '%s' not supported\n",
+ perf_mem_events__name(j, pmu->name));
return -1;
}
- while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+ if (perf_pmus__num_mem_pmus() == 1) {
+ rec_argv[i++] = "-e";
+ rec_argv[i++] = perf_mem_events__name(j, NULL);
+ } else {
const char *s = perf_mem_events__name(j, pmu->name);
if (!perf_mem_event__supported(mnt, pmu, e))
diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
index 8c5694b2d0b0..59a4303aac96 100644
--- a/tools/perf/util/mem-events.h
+++ b/tools/perf/util/mem-events.h
@@ -36,14 +36,15 @@ enum {
extern unsigned int perf_mem_events__loads_ldlat;
extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
-int perf_mem_events__parse(const char *str);
-int perf_mem_events__init(void);
+int perf_mem_events__parse(struct perf_pmu *pmu, const char *str);
+int perf_mem_events__init(struct perf_pmu *pmu);
const char *perf_mem_events__name(int i, const char *pmu_name);
-struct perf_mem_event *perf_mem_events__ptr(int i);
+struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i);
+struct perf_pmu *perf_mem_events_find_pmu(void);
bool is_mem_loads_aux_event(struct evsel *leader);
-void perf_mem_events__list(void);
+void perf_mem_events__list(struct perf_pmu *pmu);
int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
char **rec_tmp, int *tmp_nr);
--
2.35.1
From: Kan Liang <[email protected]>
The aux_event can be retrieved from the perf_pmu now. Implement a
generic support.
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/arch/x86/util/mem-events.c | 13 -------------
tools/perf/util/mem-events.c | 14 ++++++++++++--
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index b776d849fc64..71ffe16de751 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -28,16 +28,3 @@ struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
E(NULL, NULL, NULL, false, 0),
E("mem-ldst", "%s//", NULL, false, 0),
};
-
-bool is_mem_loads_aux_event(struct evsel *leader)
-{
- struct perf_pmu *pmu = perf_pmus__find("cpu");
-
- if (!pmu)
- pmu = perf_pmus__find("cpu_core");
-
- if (pmu && !perf_pmu__have_event(pmu, "mem-loads-aux"))
- return false;
-
- return leader->core.attr.config == MEM_LOADS_AUX;
-}
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 9ea9e9a868c4..336d1109b3a5 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -103,9 +103,19 @@ static const char *perf_mem_events__name(int i, struct perf_pmu *pmu)
return NULL;
}
-__weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
+bool is_mem_loads_aux_event(struct evsel *leader)
{
- return false;
+ struct perf_pmu *pmu = leader->pmu;
+ struct perf_mem_event *e;
+
+ if (!pmu || !pmu->mem_events)
+ return false;
+
+ e = &pmu->mem_events[PERF_MEM_EVENTS__LOAD];
+ if (!e->aux_event)
+ return false;
+
+ return leader->core.attr.config == e->aux_event;
}
int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
--
2.35.1
From: Kan Liang <[email protected]>
The X86 mem-events.c only has perf_mem_events array now. Remove useless
header files.
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/arch/x86/util/mem-events.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index 71ffe16de751..62df03e91c7e 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -1,11 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
-#include "util/pmu.h"
-#include "util/pmus.h"
-#include "util/env.h"
-#include "map_symbol.h"
-#include "mem-events.h"
#include "linux/string.h"
-#include "env.h"
+#include "util/map_symbol.h"
+#include "util/mem-events.h"
+#include "mem-events.h"
+
#define MEM_LOADS_AUX 0x8203
--
2.35.1
From: Kan Liang <[email protected]>
For some ARCHs, e.g., ARM and AMD, to get the availability of the
mem-events, perf checks the existence of a specific PMU. For the other
ARCHs, e.g., Intel and Power, perf has to check the existence of some
specific events.
The current perf only iterates the mem-events-supported PMUs. It's not
required to check the existence of a specific PMU anymore.
Rename sysfs_name to event_name, which stores the specific mem-events.
Perf only needs to check those events for the availability of the
mem-events.
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/arch/arm64/util/mem-events.c | 8 ++++----
tools/perf/arch/powerpc/util/mem-events.c | 8 ++++----
tools/perf/arch/x86/util/mem-events.c | 20 ++++++++++----------
tools/perf/util/mem-events.c | 16 +++++++++-------
tools/perf/util/mem-events.h | 2 +-
5 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
index eb2ef84f0fc8..590dddd6b0ab 100644
--- a/tools/perf/arch/arm64/util/mem-events.c
+++ b/tools/perf/arch/arm64/util/mem-events.c
@@ -2,10 +2,10 @@
#include "map_symbol.h"
#include "mem-events.h"
-#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
- E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0", true, 0),
- E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0", false, 0),
- E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0", true, 0),
+ E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", NULL, true, 0),
+ E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", NULL, false, 0),
+ E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", NULL, true, 0),
};
diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
index b7883e38950f..72a6ac2b52f5 100644
--- a/tools/perf/arch/powerpc/util/mem-events.c
+++ b/tools/perf/arch/powerpc/util/mem-events.c
@@ -2,10 +2,10 @@
#include "map_symbol.h"
#include "mem-events.h"
-#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "%s/mem-loads/", "cpu/events/mem-loads", false, 0),
- E("ldlat-stores", "%s/mem-stores/", "cpu/events/mem-stores", false, 0),
- E(NULL, NULL, NULL, false, 0),
+ E("ldlat-loads", "%s/mem-loads/", "mem-loads", false, 0),
+ E("ldlat-stores", "%s/mem-stores/", "mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index f0e66a0151a0..b776d849fc64 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -9,24 +9,24 @@
#define MEM_LOADS_AUX 0x8203
-#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads", true, 0),
- E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
- E(NULL, NULL, NULL, false, 0),
+ E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "mem-loads", true, 0),
+ E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "%s/events/mem-loads", true, MEM_LOADS_AUX),
- E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
- E(NULL, NULL, NULL, false, 0),
+ E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "mem-loads", true, MEM_LOADS_AUX),
+ E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
- E(NULL, NULL, NULL, false, 0),
- E(NULL, NULL, NULL, false, 0),
- E("mem-ldst", "%s//", "ibs_op", false, 0),
+ E(NULL, NULL, NULL, false, 0),
+ E(NULL, NULL, NULL, false, 0),
+ E("mem-ldst", "%s//", NULL, false, 0),
};
bool is_mem_loads_aux_event(struct evsel *leader)
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 3a60cbcd6d8e..9ea9e9a868c4 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -17,12 +17,12 @@
unsigned int perf_mem_events__loads_ldlat = 30;
-#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "cpu/events/mem-loads", true, 0),
- E("ldlat-stores", "%s/mem-stores/P", "cpu/events/mem-stores", false, 0),
- E(NULL, NULL, NULL, false, 0),
+ E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "mem-loads", true, 0),
+ E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
#undef E
@@ -150,12 +150,14 @@ int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
static bool perf_mem_event__supported(const char *mnt, struct perf_pmu *pmu,
struct perf_mem_event *e)
{
- char sysfs_name[100];
char path[PATH_MAX];
struct stat st;
- scnprintf(sysfs_name, sizeof(sysfs_name), e->sysfs_name, pmu->name);
- scnprintf(path, PATH_MAX, "%s/devices/%s", mnt, sysfs_name);
+ if (!e->event_name)
+ return true;
+
+ scnprintf(path, PATH_MAX, "%s/devices/%s/events/%s", mnt, pmu->name, e->event_name);
+
return !stat(path, &st);
}
diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
index d257cf67d6d9..d2875d731da8 100644
--- a/tools/perf/util/mem-events.h
+++ b/tools/perf/util/mem-events.h
@@ -18,7 +18,7 @@ struct perf_mem_event {
u32 aux_event;
const char *tag;
const char *name;
- const char *sysfs_name;
+ const char *event_name;
};
struct mem_info {
--
2.35.1
From: Kan Liang <[email protected]>
Introduce a generic perf_mem_events__name(). Remove the ARCH-specific
one.
The mem_load events may have a different format. Add ldlat and aux_event
in the struct perf_mem_event to indicate the format and the extra aux
event.
Add perf_mem_events_intel_aux[] to support the extra mem_load_aux event.
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/arch/arm64/util/mem-events.c | 26 ++-------
tools/perf/arch/powerpc/util/mem-events.c | 13 ++---
tools/perf/arch/powerpc/util/mem-events.h | 7 +++
tools/perf/arch/powerpc/util/pmu.c | 11 ++++
tools/perf/arch/x86/util/mem-events.c | 70 +++++------------------
tools/perf/arch/x86/util/mem-events.h | 1 +
tools/perf/arch/x86/util/pmu.c | 8 ++-
tools/perf/util/mem-events.c | 56 ++++++++++++------
tools/perf/util/mem-events.h | 3 +-
9 files changed, 89 insertions(+), 106 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/mem-events.h
create mode 100644 tools/perf/arch/powerpc/util/pmu.c
diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
index 2602e8688727..eb2ef84f0fc8 100644
--- a/tools/perf/arch/arm64/util/mem-events.c
+++ b/tools/perf/arch/arm64/util/mem-events.c
@@ -2,28 +2,10 @@
#include "map_symbol.h"
#include "mem-events.h"
-#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
- E("spe-load", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0"),
- E("spe-store", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0"),
- E("spe-ldst", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0"),
+ E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0", true, 0),
+ E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0", false, 0),
+ E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0", true, 0),
};
-
-static char mem_ev_name[100];
-
-const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
-{
- struct perf_mem_event *e = &perf_mem_events_arm[i];
-
- if (i >= PERF_MEM_EVENTS__MAX)
- return NULL;
-
- if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE)
- scnprintf(mem_ev_name, sizeof(mem_ev_name),
- e->name, perf_mem_events__loads_ldlat);
- else /* PERF_MEM_EVENTS__STORE */
- scnprintf(mem_ev_name, sizeof(mem_ev_name), e->name);
-
- return mem_ev_name;
-}
diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
index 78b986e5268d..b7883e38950f 100644
--- a/tools/perf/arch/powerpc/util/mem-events.c
+++ b/tools/perf/arch/powerpc/util/mem-events.c
@@ -2,11 +2,10 @@
#include "map_symbol.h"
#include "mem-events.h"
-/* PowerPC does not support 'ldlat' parameter. */
-const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
-{
- if (i == PERF_MEM_EVENTS__LOAD)
- return "cpu/mem-loads/";
+#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
- return "cpu/mem-stores/";
-}
+struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX] = {
+ E("ldlat-loads", "%s/mem-loads/", "cpu/events/mem-loads", false, 0),
+ E("ldlat-stores", "%s/mem-stores/", "cpu/events/mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
+};
diff --git a/tools/perf/arch/powerpc/util/mem-events.h b/tools/perf/arch/powerpc/util/mem-events.h
new file mode 100644
index 000000000000..6acc3d1b6873
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/mem-events.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _POWER_MEM_EVENTS_H
+#define _POWER_MEM_EVENTS_H
+
+extern struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX];
+
+#endif /* _POWER_MEM_EVENTS_H */
diff --git a/tools/perf/arch/powerpc/util/pmu.c b/tools/perf/arch/powerpc/util/pmu.c
new file mode 100644
index 000000000000..168173f88ddb
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/pmu.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <string.h>
+
+#include "../../../util/pmu.h"
+
+void perf_pmu__arch_init(struct perf_pmu *pmu)
+{
+ if (pmu->is_core)
+ pmu->mem_events = perf_mem_events_power;
+}
diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index 5fb41d50118d..f0e66a0151a0 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -7,25 +7,26 @@
#include "linux/string.h"
#include "env.h"
-static char mem_loads_name[100];
-static bool mem_loads_name__init;
-static char mem_stores_name[100];
-
#define MEM_LOADS_AUX 0x8203
-#define MEM_LOADS_AUX_NAME "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P"
-#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads"),
- E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores"),
- E(NULL, NULL, NULL),
+ E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads", true, 0),
+ E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
+};
+
+struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
+ E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "%s/events/mem-loads", true, MEM_LOADS_AUX),
+ E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
- E(NULL, NULL, NULL),
- E(NULL, NULL, NULL),
- E("mem-ldst", "ibs_op//", "ibs_op"),
+ E(NULL, NULL, NULL, false, 0),
+ E(NULL, NULL, NULL, false, 0),
+ E("mem-ldst", "%s//", "ibs_op", false, 0),
};
bool is_mem_loads_aux_event(struct evsel *leader)
@@ -40,48 +41,3 @@ bool is_mem_loads_aux_event(struct evsel *leader)
return leader->core.attr.config == MEM_LOADS_AUX;
}
-
-const char *perf_mem_events__name(int i, const char *pmu_name)
-{
- struct perf_mem_event *e;
-
- if (x86__is_amd_cpu())
- e = &perf_mem_events_amd[i];
- else
- e = &perf_mem_events_intel[i];
-
- if (!e)
- return NULL;
-
- if (i == PERF_MEM_EVENTS__LOAD) {
- if (mem_loads_name__init && !pmu_name)
- return mem_loads_name;
-
- if (!pmu_name) {
- mem_loads_name__init = true;
- pmu_name = "cpu";
- }
-
- if (perf_pmus__have_event(pmu_name, "mem-loads-aux")) {
- scnprintf(mem_loads_name, sizeof(mem_loads_name),
- MEM_LOADS_AUX_NAME, pmu_name, pmu_name,
- perf_mem_events__loads_ldlat);
- } else {
- scnprintf(mem_loads_name, sizeof(mem_loads_name),
- e->name, pmu_name,
- perf_mem_events__loads_ldlat);
- }
- return mem_loads_name;
- }
-
- if (i == PERF_MEM_EVENTS__STORE) {
- if (!pmu_name)
- pmu_name = "cpu";
-
- scnprintf(mem_stores_name, sizeof(mem_stores_name),
- e->name, pmu_name);
- return mem_stores_name;
- }
-
- return e->name;
-}
diff --git a/tools/perf/arch/x86/util/mem-events.h b/tools/perf/arch/x86/util/mem-events.h
index 3959e427f482..f55c8d3b7d59 100644
--- a/tools/perf/arch/x86/util/mem-events.h
+++ b/tools/perf/arch/x86/util/mem-events.h
@@ -3,6 +3,7 @@
#define _X86_MEM_EVENTS_H
extern struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX];
+extern struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX];
extern struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX];
diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index 7e69f4f2e363..446f8197aae4 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -35,8 +35,12 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
if (x86__is_amd_cpu()) {
if (strcmp(pmu->name, "ibs_op"))
pmu->mem_events = perf_mem_events_amd;
- } else if (pmu->is_core)
- pmu->mem_events = perf_mem_events_intel;
+ } else if (pmu->is_core) {
+ if (perf_pmu__have_event(pmu, "mem-loads-aux"))
+ pmu->mem_events = perf_mem_events_intel_aux;
+ else
+ pmu->mem_events = perf_mem_events_intel;
+ }
}
int perf_pmus__num_mem_pmus(void)
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 887ffdcce338..3a60cbcd6d8e 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -17,17 +17,17 @@
unsigned int perf_mem_events__loads_ldlat = 30;
-#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
+#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "cpu/events/mem-loads"),
- E("ldlat-stores", "cpu/mem-stores/P", "cpu/events/mem-stores"),
- E(NULL, NULL, NULL),
+ E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "cpu/events/mem-loads", true, 0),
+ E("ldlat-stores", "%s/mem-stores/P", "cpu/events/mem-stores", false, 0),
+ E(NULL, NULL, NULL, false, 0),
};
#undef E
static char mem_loads_name[100];
-static bool mem_loads_name__init;
+static char mem_stores_name[100];
struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i)
{
@@ -62,23 +62,45 @@ struct perf_pmu *perf_mem_events_find_pmu(void)
return perf_pmus__scan_mem(NULL);
}
-const char * __weak perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
+static const char *perf_mem_events__name(int i, struct perf_pmu *pmu)
{
- struct perf_mem_event *e = &perf_mem_events[i];
+ struct perf_mem_event *e = &pmu->mem_events[i];
if (!e)
return NULL;
- if (i == PERF_MEM_EVENTS__LOAD) {
- if (!mem_loads_name__init) {
- mem_loads_name__init = true;
- scnprintf(mem_loads_name, sizeof(mem_loads_name),
- e->name, perf_mem_events__loads_ldlat);
+ if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE) {
+ if (e->ldlat) {
+ if (!e->aux_event) {
+ /* ARM and Most of Intel */
+ scnprintf(mem_loads_name, sizeof(mem_loads_name),
+ e->name, pmu->name,
+ perf_mem_events__loads_ldlat);
+ } else {
+ /* Intel with mem-loads-aux event */
+ scnprintf(mem_loads_name, sizeof(mem_loads_name),
+ e->name, pmu->name, pmu->name,
+ perf_mem_events__loads_ldlat);
+ }
+ } else {
+ if (!e->aux_event) {
+ /* AMD and POWER */
+ scnprintf(mem_loads_name, sizeof(mem_loads_name),
+ e->name, pmu->name);
+ } else
+ return NULL;
}
+
return mem_loads_name;
}
- return e->name;
+ if (i == PERF_MEM_EVENTS__STORE) {
+ scnprintf(mem_stores_name, sizeof(mem_stores_name),
+ e->name, pmu->name);
+ return mem_stores_name;
+ }
+
+ return NULL;
}
__weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
@@ -175,7 +197,7 @@ void perf_mem_events__list(struct perf_pmu *pmu)
e->tag ? 13 : 0,
e->tag ? : "",
e->tag && verbose > 0 ? 25 : 0,
- e->tag && verbose > 0 ? perf_mem_events__name(j, NULL) : "",
+ e->tag && verbose > 0 ? perf_mem_events__name(j, pmu) : "",
e->supported ? ": available\n" : "");
}
}
@@ -198,15 +220,15 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
if (!e->supported) {
pr_err("failed: event '%s' not supported\n",
- perf_mem_events__name(j, pmu->name));
+ perf_mem_events__name(j, pmu));
return -1;
}
if (perf_pmus__num_mem_pmus() == 1) {
rec_argv[i++] = "-e";
- rec_argv[i++] = perf_mem_events__name(j, NULL);
+ rec_argv[i++] = perf_mem_events__name(j, pmu);
} else {
- const char *s = perf_mem_events__name(j, pmu->name);
+ const char *s = perf_mem_events__name(j, pmu);
if (!perf_mem_event__supported(mnt, pmu, e))
continue;
diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
index 59a4303aac96..d257cf67d6d9 100644
--- a/tools/perf/util/mem-events.h
+++ b/tools/perf/util/mem-events.h
@@ -14,6 +14,8 @@
struct perf_mem_event {
bool record;
bool supported;
+ bool ldlat;
+ u32 aux_event;
const char *tag;
const char *name;
const char *sysfs_name;
@@ -39,7 +41,6 @@ extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
int perf_mem_events__parse(struct perf_pmu *pmu, const char *str);
int perf_mem_events__init(struct perf_pmu *pmu);
-const char *perf_mem_events__name(int i, const char *pmu_name);
struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i);
struct perf_pmu *perf_mem_events_find_pmu(void);
bool is_mem_loads_aux_event(struct evsel *leader);
--
2.35.1
Em Wed, Dec 06, 2023 at 12:13:24PM -0800, [email protected] escreveu:
> From: Kan Liang <[email protected]>
>
> The X86 mem-events.c only has perf_mem_events array now. Remove useless
> header files.
It would be great that those were removed while you made the cleanups,
i.e. removed the need for one of them, remove it together with the
refactorings, etc.
But I don't think this is a requirement, just would make it cleaner.
Will wait for reviews now.
- Arnaldo
> Signed-off-by: Kan Liang <[email protected]>
> ---
> tools/perf/arch/x86/util/mem-events.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> index 71ffe16de751..62df03e91c7e 100644
> --- a/tools/perf/arch/x86/util/mem-events.c
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -1,11 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
> -#include "util/pmu.h"
> -#include "util/pmus.h"
> -#include "util/env.h"
> -#include "map_symbol.h"
> -#include "mem-events.h"
> #include "linux/string.h"
> -#include "env.h"
> +#include "util/map_symbol.h"
> +#include "util/mem-events.h"
> +#include "mem-events.h"
> +
>
> #define MEM_LOADS_AUX 0x8203
>
> --
> 2.35.1
>
--
- Arnaldo
On Wed, Dec 6, 2023 at 12:13 PM <[email protected]> wrote:
>
> From: Kan Liang <[email protected]>
>
> The mem_events can be retrieved from the struct perf_pmu now. An ARCH
> specific perf_mem_events__ptr() is not required anymore. Remove all of
> them.
>
> The Intel hybrid has multiple mem-events-supported PMUs. But they share
> the same mem_events. Other ARCHs only support one mem-events-supported
> PMU. In the configuration, it's good enough to only configure the
> mem_events for one PMU. Add perf_mem_events_find_pmu() which returns the
> first mem-events-supported PMU.
>
> In the perf_mem_events__init(), the perf_pmus__scan() is not required
> anymore. It avoids checking the sysfs for every PMU on the system.
>
> Make the perf_mem_events__record_args() more generic. Remove the
> perf_mem_events__print_unsupport_hybrid().
>
> Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
> ---
> tools/perf/arch/arm64/util/mem-events.c | 10 +--
> tools/perf/arch/x86/util/mem-events.c | 18 ++---
> tools/perf/builtin-c2c.c | 28 +++++--
> tools/perf/builtin-mem.c | 28 +++++--
> tools/perf/util/mem-events.c | 103 ++++++++++++------------
> tools/perf/util/mem-events.h | 9 ++-
> 6 files changed, 104 insertions(+), 92 deletions(-)
>
> diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
> index aaa4804922b4..2602e8688727 100644
> --- a/tools/perf/arch/arm64/util/mem-events.c
> +++ b/tools/perf/arch/arm64/util/mem-events.c
> @@ -12,17 +12,9 @@ struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
>
> static char mem_ev_name[100];
>
> -struct perf_mem_event *perf_mem_events__ptr(int i)
> -{
> - if (i >= PERF_MEM_EVENTS__MAX)
> - return NULL;
> -
> - return &perf_mem_events_arm[i];
> -}
> -
> const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
> {
> - struct perf_mem_event *e = perf_mem_events__ptr(i);
> + struct perf_mem_event *e = &perf_mem_events_arm[i];
>
> if (i >= PERF_MEM_EVENTS__MAX)
> return NULL;
> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> index 2b81d229982c..5fb41d50118d 100644
> --- a/tools/perf/arch/x86/util/mem-events.c
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -28,17 +28,6 @@ struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
> E("mem-ldst", "ibs_op//", "ibs_op"),
> };
>
> -struct perf_mem_event *perf_mem_events__ptr(int i)
> -{
> - if (i >= PERF_MEM_EVENTS__MAX)
> - return NULL;
> -
> - if (x86__is_amd_cpu())
> - return &perf_mem_events_amd[i];
> -
> - return &perf_mem_events_intel[i];
> -}
> -
> bool is_mem_loads_aux_event(struct evsel *leader)
> {
> struct perf_pmu *pmu = perf_pmus__find("cpu");
> @@ -54,7 +43,12 @@ bool is_mem_loads_aux_event(struct evsel *leader)
>
> const char *perf_mem_events__name(int i, const char *pmu_name)
> {
> - struct perf_mem_event *e = perf_mem_events__ptr(i);
> + struct perf_mem_event *e;
> +
> + if (x86__is_amd_cpu())
> + e = &perf_mem_events_amd[i];
> + else
> + e = &perf_mem_events_intel[i];
>
> if (!e)
> return NULL;
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index a4cf9de7a7b5..76c760be1bcf 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -3215,12 +3215,19 @@ static int parse_record_events(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> bool *event_set = (bool *) opt->value;
> + struct perf_pmu *pmu;
> +
> + pmu = perf_mem_events_find_pmu();
> + if (!pmu) {
> + pr_err("failed: there is no PMU that supports perf c2c\n");
> + exit(-1);
> + }
>
> if (!strcmp(str, "list")) {
> - perf_mem_events__list();
> + perf_mem_events__list(pmu);
> exit(0);
> }
> - if (perf_mem_events__parse(str))
> + if (perf_mem_events__parse(pmu, str))
> exit(-1);
>
> *event_set = true;
> @@ -3245,6 +3252,7 @@ static int perf_c2c__record(int argc, const char **argv)
> bool all_user = false, all_kernel = false;
> bool event_set = false;
> struct perf_mem_event *e;
> + struct perf_pmu *pmu;
> struct option options[] = {
> OPT_CALLBACK('e', "event", &event_set, "event",
> "event selector. Use 'perf c2c record -e list' to list available events",
> @@ -3256,7 +3264,13 @@ static int perf_c2c__record(int argc, const char **argv)
> OPT_END()
> };
>
> - if (perf_mem_events__init()) {
> + pmu = perf_mem_events_find_pmu();
> + if (!pmu) {
> + pr_err("failed: no PMU supports the memory events\n");
> + return -1;
> + }
> +
> + if (perf_mem_events__init(pmu)) {
> pr_err("failed: memory events not supported\n");
> return -1;
> }
> @@ -3280,7 +3294,7 @@ static int perf_c2c__record(int argc, const char **argv)
> rec_argv[i++] = "record";
>
> if (!event_set) {
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD_STORE);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
> /*
> * The load and store operations are required, use the event
> * PERF_MEM_EVENTS__LOAD_STORE if it is supported.
> @@ -3289,15 +3303,15 @@ static int perf_c2c__record(int argc, const char **argv)
> e->record = true;
> rec_argv[i++] = "-W";
> } else {
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
Fwiw, it seems strange in cases like this that the function isn't:
perf_pmu__mem_events_ptr(pmu, PERF_MEM_EVENTS__LOAD)
Thanks,
Ian
> e->record = true;
>
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__STORE);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__STORE);
> e->record = true;
> }
> }
>
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
> if (e->record)
> rec_argv[i++] = "-W";
>
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index 51499c20da01..8218c4721101 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -43,12 +43,19 @@ static int parse_record_events(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> struct perf_mem *mem = *(struct perf_mem **)opt->value;
> + struct perf_pmu *pmu;
> +
> + pmu = perf_mem_events_find_pmu();
> + if (!pmu) {
> + pr_err("failed: there is no PMU that supports perf mem\n");
> + exit(-1);
> + }
>
> if (!strcmp(str, "list")) {
> - perf_mem_events__list();
> + perf_mem_events__list(pmu);
> exit(0);
> }
> - if (perf_mem_events__parse(str))
> + if (perf_mem_events__parse(pmu, str))
> exit(-1);
>
> mem->operation = 0;
> @@ -72,6 +79,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> int ret;
> bool all_user = false, all_kernel = false;
> struct perf_mem_event *e;
> + struct perf_pmu *pmu;
> struct option options[] = {
> OPT_CALLBACK('e', "event", &mem, "event",
> "event selector. use 'perf mem record -e list' to list available events",
> @@ -84,7 +92,13 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> OPT_END()
> };
>
> - if (perf_mem_events__init()) {
> + pmu = perf_mem_events_find_pmu();
> + if (!pmu) {
> + pr_err("failed: no PMU supports the memory events\n");
> + return -1;
> + }
> +
> + if (perf_mem_events__init(pmu)) {
> pr_err("failed: memory events not supported\n");
> return -1;
> }
> @@ -113,7 +127,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>
> rec_argv[i++] = "record";
>
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD_STORE);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
>
> /*
> * The load and store operations are required, use the event
> @@ -126,17 +140,17 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> rec_argv[i++] = "-W";
> } else {
> if (mem->operation & MEM_OPERATION_LOAD) {
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
> e->record = true;
> }
>
> if (mem->operation & MEM_OPERATION_STORE) {
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__STORE);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__STORE);
> e->record = true;
> }
> }
>
> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
> if (e->record)
> rec_argv[i++] = "-W";
>
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 0a8f415f5efe..887ffdcce338 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -29,17 +29,42 @@ struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> static char mem_loads_name[100];
> static bool mem_loads_name__init;
>
> -struct perf_mem_event * __weak perf_mem_events__ptr(int i)
> +struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i)
> {
> - if (i >= PERF_MEM_EVENTS__MAX)
> + if (i >= PERF_MEM_EVENTS__MAX || !pmu)
> return NULL;
>
> - return &perf_mem_events[i];
> + return &pmu->mem_events[i];
> +}
> +
> +static struct perf_pmu *perf_pmus__scan_mem(struct perf_pmu *pmu)
> +{
> + while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> + if (pmu->mem_events)
> + return pmu;
> + }
> + return NULL;
> +}
> +
> +struct perf_pmu *perf_mem_events_find_pmu(void)
> +{
> + /*
> + * The current perf mem doesn't support per-PMU configuration.
> + * The exact same configuration is applied to all the
> + * mem_events supported PMUs.
> + * Return the first mem_events supported PMU.
> + *
> + * Notes: The only case which may support multiple mem_events
> + * supported PMUs is Intel hybrid. The exact same mem_events
> + * is shared among the PMUs. Only configure the first PMU
> + * is good enough as well.
> + */
> + return perf_pmus__scan_mem(NULL);
> }
>
> const char * __weak perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
> {
> - struct perf_mem_event *e = perf_mem_events__ptr(i);
> + struct perf_mem_event *e = &perf_mem_events[i];
>
> if (!e)
> return NULL;
> @@ -61,7 +86,7 @@ __weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
> return false;
> }
>
> -int perf_mem_events__parse(const char *str)
> +int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
> {
> char *tok, *saveptr = NULL;
> bool found = false;
> @@ -79,7 +104,7 @@ int perf_mem_events__parse(const char *str)
>
> while (tok) {
> for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> - struct perf_mem_event *e = perf_mem_events__ptr(j);
> + struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
>
> if (!e->tag)
> continue;
> @@ -112,7 +137,7 @@ static bool perf_mem_event__supported(const char *mnt, struct perf_pmu *pmu,
> return !stat(path, &st);
> }
>
> -int perf_mem_events__init(void)
> +int perf_mem_events__init(struct perf_pmu *pmu)
> {
> const char *mnt = sysfs__mount();
> bool found = false;
> @@ -122,8 +147,7 @@ int perf_mem_events__init(void)
> return -ENOENT;
>
> for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> - struct perf_mem_event *e = perf_mem_events__ptr(j);
> - struct perf_pmu *pmu = NULL;
> + struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
>
> /*
> * If the event entry isn't valid, skip initialization
> @@ -132,29 +156,20 @@ int perf_mem_events__init(void)
> if (!e->tag)
> continue;
>
> - /*
> - * Scan all PMUs not just core ones, since perf mem/c2c on
> - * platforms like AMD uses IBS OP PMU which is independent
> - * of core PMU.
> - */
> - while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> - e->supported |= perf_mem_event__supported(mnt, pmu, e);
> - if (e->supported) {
> - found = true;
> - break;
> - }
> - }
> + e->supported |= perf_mem_event__supported(mnt, pmu, e);
> + if (e->supported)
> + found = true;
> }
>
> return found ? 0 : -ENOENT;
> }
>
> -void perf_mem_events__list(void)
> +void perf_mem_events__list(struct perf_pmu *pmu)
> {
> int j;
>
> for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> - struct perf_mem_event *e = perf_mem_events__ptr(j);
> + struct perf_mem_event *e = perf_mem_events__ptr(pmu, j);
>
> fprintf(stderr, "%-*s%-*s%s",
> e->tag ? 13 : 0,
> @@ -165,50 +180,32 @@ void perf_mem_events__list(void)
> }
> }
>
> -static void perf_mem_events__print_unsupport_hybrid(struct perf_mem_event *e,
> - int idx)
> -{
> - const char *mnt = sysfs__mount();
> - struct perf_pmu *pmu = NULL;
> -
> - while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> - if (!perf_mem_event__supported(mnt, pmu, e)) {
> - pr_err("failed: event '%s' not supported\n",
> - perf_mem_events__name(idx, pmu->name));
> - }
> - }
> -}
> -
> int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
> char **rec_tmp, int *tmp_nr)
> {
> const char *mnt = sysfs__mount();
> + struct perf_pmu *pmu = NULL;
> int i = *argv_nr, k = 0;
> struct perf_mem_event *e;
>
> - for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> - e = perf_mem_events__ptr(j);
> - if (!e->record)
> - continue;
>
> - if (perf_pmus__num_mem_pmus() == 1) {
> - if (!e->supported) {
> - pr_err("failed: event '%s' not supported\n",
> - perf_mem_events__name(j, NULL));
> - return -1;
> - }
> + while ((pmu = perf_pmus__scan_mem(pmu)) != NULL) {
> + for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> + e = perf_mem_events__ptr(pmu, j);
>
> - rec_argv[i++] = "-e";
> - rec_argv[i++] = perf_mem_events__name(j, NULL);
> - } else {
> - struct perf_pmu *pmu = NULL;
> + if (!e->record)
> + continue;
>
> if (!e->supported) {
> - perf_mem_events__print_unsupport_hybrid(e, j);
> + pr_err("failed: event '%s' not supported\n",
> + perf_mem_events__name(j, pmu->name));
> return -1;
> }
>
> - while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> + if (perf_pmus__num_mem_pmus() == 1) {
> + rec_argv[i++] = "-e";
> + rec_argv[i++] = perf_mem_events__name(j, NULL);
> + } else {
> const char *s = perf_mem_events__name(j, pmu->name);
>
> if (!perf_mem_event__supported(mnt, pmu, e))
> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index 8c5694b2d0b0..59a4303aac96 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -36,14 +36,15 @@ enum {
> extern unsigned int perf_mem_events__loads_ldlat;
> extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
>
> -int perf_mem_events__parse(const char *str);
> -int perf_mem_events__init(void);
> +int perf_mem_events__parse(struct perf_pmu *pmu, const char *str);
> +int perf_mem_events__init(struct perf_pmu *pmu);
>
> const char *perf_mem_events__name(int i, const char *pmu_name);
> -struct perf_mem_event *perf_mem_events__ptr(int i);
> +struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i);
> +struct perf_pmu *perf_mem_events_find_pmu(void);
> bool is_mem_loads_aux_event(struct evsel *leader);
>
> -void perf_mem_events__list(void);
> +void perf_mem_events__list(struct perf_pmu *pmu);
> int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
> char **rec_tmp, int *tmp_nr);
>
> --
> 2.35.1
>
On Wed, Dec 6, 2023 at 12:13 PM <[email protected]> wrote:
>
> From: Kan Liang <[email protected]>
>
> Introduce a generic perf_mem_events__name(). Remove the ARCH-specific
> one.
>
> The mem_load events may have a different format. Add ldlat and aux_event
> in the struct perf_mem_event to indicate the format and the extra aux
> event.
>
> Add perf_mem_events_intel_aux[] to support the extra mem_load_aux event.
>
> Signed-off-by: Kan Liang <[email protected]>
> ---
> tools/perf/arch/arm64/util/mem-events.c | 26 ++-------
> tools/perf/arch/powerpc/util/mem-events.c | 13 ++---
> tools/perf/arch/powerpc/util/mem-events.h | 7 +++
> tools/perf/arch/powerpc/util/pmu.c | 11 ++++
> tools/perf/arch/x86/util/mem-events.c | 70 +++++------------------
> tools/perf/arch/x86/util/mem-events.h | 1 +
> tools/perf/arch/x86/util/pmu.c | 8 ++-
> tools/perf/util/mem-events.c | 56 ++++++++++++------
> tools/perf/util/mem-events.h | 3 +-
> 9 files changed, 89 insertions(+), 106 deletions(-)
> create mode 100644 tools/perf/arch/powerpc/util/mem-events.h
> create mode 100644 tools/perf/arch/powerpc/util/pmu.c
>
> diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
> index 2602e8688727..eb2ef84f0fc8 100644
> --- a/tools/perf/arch/arm64/util/mem-events.c
> +++ b/tools/perf/arch/arm64/util/mem-events.c
> @@ -2,28 +2,10 @@
> #include "map_symbol.h"
> #include "mem-events.h"
>
> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
> - E("spe-load", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0"),
> - E("spe-store", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0"),
> - E("spe-ldst", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0"),
> + E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0", true, 0),
> + E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0", false, 0),
> + E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0", true, 0),
> };
> -
> -static char mem_ev_name[100];
> -
> -const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
> -{
> - struct perf_mem_event *e = &perf_mem_events_arm[i];
> -
> - if (i >= PERF_MEM_EVENTS__MAX)
> - return NULL;
> -
> - if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE)
> - scnprintf(mem_ev_name, sizeof(mem_ev_name),
> - e->name, perf_mem_events__loads_ldlat);
> - else /* PERF_MEM_EVENTS__STORE */
> - scnprintf(mem_ev_name, sizeof(mem_ev_name), e->name);
> -
> - return mem_ev_name;
> -}
> diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
> index 78b986e5268d..b7883e38950f 100644
> --- a/tools/perf/arch/powerpc/util/mem-events.c
> +++ b/tools/perf/arch/powerpc/util/mem-events.c
> @@ -2,11 +2,10 @@
> #include "map_symbol.h"
> #include "mem-events.h"
>
> -/* PowerPC does not support 'ldlat' parameter. */
> -const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
> -{
> - if (i == PERF_MEM_EVENTS__LOAD)
> - return "cpu/mem-loads/";
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>
> - return "cpu/mem-stores/";
> -}
> +struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX] = {
> + E("ldlat-loads", "%s/mem-loads/", "cpu/events/mem-loads", false, 0),
> + E("ldlat-stores", "%s/mem-stores/", "cpu/events/mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> +};
> diff --git a/tools/perf/arch/powerpc/util/mem-events.h b/tools/perf/arch/powerpc/util/mem-events.h
> new file mode 100644
> index 000000000000..6acc3d1b6873
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/mem-events.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _POWER_MEM_EVENTS_H
> +#define _POWER_MEM_EVENTS_H
> +
> +extern struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX];
> +
> +#endif /* _POWER_MEM_EVENTS_H */
> diff --git a/tools/perf/arch/powerpc/util/pmu.c b/tools/perf/arch/powerpc/util/pmu.c
> new file mode 100644
> index 000000000000..168173f88ddb
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/pmu.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <string.h>
> +
> +#include "../../../util/pmu.h"
> +
> +void perf_pmu__arch_init(struct perf_pmu *pmu)
> +{
> + if (pmu->is_core)
> + pmu->mem_events = perf_mem_events_power;
> +}
> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> index 5fb41d50118d..f0e66a0151a0 100644
> --- a/tools/perf/arch/x86/util/mem-events.c
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -7,25 +7,26 @@
> #include "linux/string.h"
> #include "env.h"
>
> -static char mem_loads_name[100];
> -static bool mem_loads_name__init;
> -static char mem_stores_name[100];
> -
> #define MEM_LOADS_AUX 0x8203
> -#define MEM_LOADS_AUX_NAME "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P"
>
> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads"),
> - E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores"),
> - E(NULL, NULL, NULL),
> + E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads", true, 0),
> + E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> +};
> +
> +struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
> + E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "%s/events/mem-loads", true, MEM_LOADS_AUX),
> + E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
>
> struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
> - E(NULL, NULL, NULL),
> - E(NULL, NULL, NULL),
> - E("mem-ldst", "ibs_op//", "ibs_op"),
> + E(NULL, NULL, NULL, false, 0),
> + E(NULL, NULL, NULL, false, 0),
> + E("mem-ldst", "%s//", "ibs_op", false, 0),
> };
>
> bool is_mem_loads_aux_event(struct evsel *leader)
> @@ -40,48 +41,3 @@ bool is_mem_loads_aux_event(struct evsel *leader)
>
> return leader->core.attr.config == MEM_LOADS_AUX;
> }
> -
> -const char *perf_mem_events__name(int i, const char *pmu_name)
> -{
> - struct perf_mem_event *e;
> -
> - if (x86__is_amd_cpu())
> - e = &perf_mem_events_amd[i];
> - else
> - e = &perf_mem_events_intel[i];
> -
> - if (!e)
> - return NULL;
> -
> - if (i == PERF_MEM_EVENTS__LOAD) {
> - if (mem_loads_name__init && !pmu_name)
> - return mem_loads_name;
> -
> - if (!pmu_name) {
> - mem_loads_name__init = true;
> - pmu_name = "cpu";
> - }
> -
> - if (perf_pmus__have_event(pmu_name, "mem-loads-aux")) {
> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
> - MEM_LOADS_AUX_NAME, pmu_name, pmu_name,
> - perf_mem_events__loads_ldlat);
> - } else {
> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
> - e->name, pmu_name,
> - perf_mem_events__loads_ldlat);
> - }
> - return mem_loads_name;
> - }
> -
> - if (i == PERF_MEM_EVENTS__STORE) {
> - if (!pmu_name)
> - pmu_name = "cpu";
> -
> - scnprintf(mem_stores_name, sizeof(mem_stores_name),
> - e->name, pmu_name);
> - return mem_stores_name;
> - }
> -
> - return e->name;
> -}
> diff --git a/tools/perf/arch/x86/util/mem-events.h b/tools/perf/arch/x86/util/mem-events.h
> index 3959e427f482..f55c8d3b7d59 100644
> --- a/tools/perf/arch/x86/util/mem-events.h
> +++ b/tools/perf/arch/x86/util/mem-events.h
> @@ -3,6 +3,7 @@
> #define _X86_MEM_EVENTS_H
>
> extern struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX];
> +extern struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX];
>
> extern struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX];
>
> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index 7e69f4f2e363..446f8197aae4 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c
> @@ -35,8 +35,12 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
> if (x86__is_amd_cpu()) {
> if (strcmp(pmu->name, "ibs_op"))
> pmu->mem_events = perf_mem_events_amd;
> - } else if (pmu->is_core)
> - pmu->mem_events = perf_mem_events_intel;
> + } else if (pmu->is_core) {
> + if (perf_pmu__have_event(pmu, "mem-loads-aux"))
> + pmu->mem_events = perf_mem_events_intel_aux;
> + else
> + pmu->mem_events = perf_mem_events_intel;
> + }
> }
>
> int perf_pmus__num_mem_pmus(void)
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 887ffdcce338..3a60cbcd6d8e 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -17,17 +17,17 @@
>
> unsigned int perf_mem_events__loads_ldlat = 30;
>
> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "cpu/events/mem-loads"),
> - E("ldlat-stores", "cpu/mem-stores/P", "cpu/events/mem-stores"),
> - E(NULL, NULL, NULL),
> + E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "cpu/events/mem-loads", true, 0),
Is there a mistake here "/," looks like it should just be ",".
Thanks,
Ian
> + E("ldlat-stores", "%s/mem-stores/P", "cpu/events/mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
> #undef E
>
> static char mem_loads_name[100];
> -static bool mem_loads_name__init;
> +static char mem_stores_name[100];
>
> struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i)
> {
> @@ -62,23 +62,45 @@ struct perf_pmu *perf_mem_events_find_pmu(void)
> return perf_pmus__scan_mem(NULL);
> }
>
> -const char * __weak perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
> +static const char *perf_mem_events__name(int i, struct perf_pmu *pmu)
> {
> - struct perf_mem_event *e = &perf_mem_events[i];
> + struct perf_mem_event *e = &pmu->mem_events[i];
>
> if (!e)
> return NULL;
>
> - if (i == PERF_MEM_EVENTS__LOAD) {
> - if (!mem_loads_name__init) {
> - mem_loads_name__init = true;
> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
> - e->name, perf_mem_events__loads_ldlat);
> + if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE) {
> + if (e->ldlat) {
> + if (!e->aux_event) {
> + /* ARM and Most of Intel */
> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
> + e->name, pmu->name,
> + perf_mem_events__loads_ldlat);
> + } else {
> + /* Intel with mem-loads-aux event */
> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
> + e->name, pmu->name, pmu->name,
> + perf_mem_events__loads_ldlat);
> + }
> + } else {
> + if (!e->aux_event) {
> + /* AMD and POWER */
> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
> + e->name, pmu->name);
> + } else
> + return NULL;
> }
> +
> return mem_loads_name;
> }
>
> - return e->name;
> + if (i == PERF_MEM_EVENTS__STORE) {
> + scnprintf(mem_stores_name, sizeof(mem_stores_name),
> + e->name, pmu->name);
> + return mem_stores_name;
> + }
> +
> + return NULL;
> }
>
> __weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
> @@ -175,7 +197,7 @@ void perf_mem_events__list(struct perf_pmu *pmu)
> e->tag ? 13 : 0,
> e->tag ? : "",
> e->tag && verbose > 0 ? 25 : 0,
> - e->tag && verbose > 0 ? perf_mem_events__name(j, NULL) : "",
> + e->tag && verbose > 0 ? perf_mem_events__name(j, pmu) : "",
> e->supported ? ": available\n" : "");
> }
> }
> @@ -198,15 +220,15 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
>
> if (!e->supported) {
> pr_err("failed: event '%s' not supported\n",
> - perf_mem_events__name(j, pmu->name));
> + perf_mem_events__name(j, pmu));
> return -1;
> }
>
> if (perf_pmus__num_mem_pmus() == 1) {
> rec_argv[i++] = "-e";
> - rec_argv[i++] = perf_mem_events__name(j, NULL);
> + rec_argv[i++] = perf_mem_events__name(j, pmu);
> } else {
> - const char *s = perf_mem_events__name(j, pmu->name);
> + const char *s = perf_mem_events__name(j, pmu);
>
> if (!perf_mem_event__supported(mnt, pmu, e))
> continue;
> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index 59a4303aac96..d257cf67d6d9 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -14,6 +14,8 @@
> struct perf_mem_event {
> bool record;
> bool supported;
> + bool ldlat;
> + u32 aux_event;
> const char *tag;
> const char *name;
> const char *sysfs_name;
> @@ -39,7 +41,6 @@ extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
> int perf_mem_events__parse(struct perf_pmu *pmu, const char *str);
> int perf_mem_events__init(struct perf_pmu *pmu);
>
> -const char *perf_mem_events__name(int i, const char *pmu_name);
> struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i);
> struct perf_pmu *perf_mem_events_find_pmu(void);
> bool is_mem_loads_aux_event(struct evsel *leader);
> --
> 2.35.1
>
On Wed, Dec 6, 2023 at 12:13 PM <[email protected]> wrote:
>
> From: Kan Liang <[email protected]>
>
> For some ARCHs, e.g., ARM and AMD, to get the availability of the
> mem-events, perf checks the existence of a specific PMU. For the other
> ARCHs, e.g., Intel and Power, perf has to check the existence of some
> specific events.
>
> The current perf only iterates the mem-events-supported PMUs. It's not
> required to check the existence of a specific PMU anymore.
>
> Rename sysfs_name to event_name, which stores the specific mem-events.
> Perf only needs to check those events for the availability of the
> mem-events.
>
> Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/arch/arm64/util/mem-events.c | 8 ++++----
> tools/perf/arch/powerpc/util/mem-events.c | 8 ++++----
> tools/perf/arch/x86/util/mem-events.c | 20 ++++++++++----------
> tools/perf/util/mem-events.c | 16 +++++++++-------
> tools/perf/util/mem-events.h | 2 +-
> 5 files changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
> index eb2ef84f0fc8..590dddd6b0ab 100644
> --- a/tools/perf/arch/arm64/util/mem-events.c
> +++ b/tools/perf/arch/arm64/util/mem-events.c
> @@ -2,10 +2,10 @@
> #include "map_symbol.h"
> #include "mem-events.h"
>
> -#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
> - E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0", true, 0),
> - E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0", false, 0),
> - E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0", true, 0),
> + E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", NULL, true, 0),
> + E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", NULL, false, 0),
> + E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", NULL, true, 0),
> };
> diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
> index b7883e38950f..72a6ac2b52f5 100644
> --- a/tools/perf/arch/powerpc/util/mem-events.c
> +++ b/tools/perf/arch/powerpc/util/mem-events.c
> @@ -2,10 +2,10 @@
> #include "map_symbol.h"
> #include "mem-events.h"
>
> -#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "%s/mem-loads/", "cpu/events/mem-loads", false, 0),
> - E("ldlat-stores", "%s/mem-stores/", "cpu/events/mem-stores", false, 0),
> - E(NULL, NULL, NULL, false, 0),
> + E("ldlat-loads", "%s/mem-loads/", "mem-loads", false, 0),
> + E("ldlat-stores", "%s/mem-stores/", "mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> index f0e66a0151a0..b776d849fc64 100644
> --- a/tools/perf/arch/x86/util/mem-events.c
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -9,24 +9,24 @@
>
> #define MEM_LOADS_AUX 0x8203
>
> -#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads", true, 0),
> - E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
> - E(NULL, NULL, NULL, false, 0),
> + E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "mem-loads", true, 0),
> + E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
>
> struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "%s/events/mem-loads", true, MEM_LOADS_AUX),
> - E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
> - E(NULL, NULL, NULL, false, 0),
> + E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "mem-loads", true, MEM_LOADS_AUX),
> + E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
>
> struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
> - E(NULL, NULL, NULL, false, 0),
> - E(NULL, NULL, NULL, false, 0),
> - E("mem-ldst", "%s//", "ibs_op", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> + E(NULL, NULL, NULL, false, 0),
> + E("mem-ldst", "%s//", NULL, false, 0),
> };
>
> bool is_mem_loads_aux_event(struct evsel *leader)
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 3a60cbcd6d8e..9ea9e9a868c4 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -17,12 +17,12 @@
>
> unsigned int perf_mem_events__loads_ldlat = 30;
>
> -#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
> +#define E(t, n, s, l, a) { .tag = t, .name = n, .event_name = s, .ldlat = l, .aux_event = a }
>
> struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> - E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "cpu/events/mem-loads", true, 0),
> - E("ldlat-stores", "%s/mem-stores/P", "cpu/events/mem-stores", false, 0),
> - E(NULL, NULL, NULL, false, 0),
> + E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "mem-loads", true, 0),
> + E("ldlat-stores", "%s/mem-stores/P", "mem-stores", false, 0),
> + E(NULL, NULL, NULL, false, 0),
> };
> #undef E
>
> @@ -150,12 +150,14 @@ int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
> static bool perf_mem_event__supported(const char *mnt, struct perf_pmu *pmu,
> struct perf_mem_event *e)
> {
> - char sysfs_name[100];
> char path[PATH_MAX];
> struct stat st;
>
> - scnprintf(sysfs_name, sizeof(sysfs_name), e->sysfs_name, pmu->name);
> - scnprintf(path, PATH_MAX, "%s/devices/%s", mnt, sysfs_name);
> + if (!e->event_name)
> + return true;
> +
> + scnprintf(path, PATH_MAX, "%s/devices/%s/events/%s", mnt, pmu->name, e->event_name);
> +
> return !stat(path, &st);
> }
>
> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index d257cf67d6d9..d2875d731da8 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -18,7 +18,7 @@ struct perf_mem_event {
> u32 aux_event;
> const char *tag;
> const char *name;
> - const char *sysfs_name;
> + const char *event_name;
> };
>
> struct mem_info {
> --
> 2.35.1
>
On Wed, Dec 6, 2023 at 12:13 PM <[email protected]> wrote:
>
> From: Kan Liang <[email protected]>
>
> The aux_event can be retrieved from the perf_pmu now. Implement a
> generic support.
>
> Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/arch/x86/util/mem-events.c | 13 -------------
> tools/perf/util/mem-events.c | 14 ++++++++++++--
> 2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> index b776d849fc64..71ffe16de751 100644
> --- a/tools/perf/arch/x86/util/mem-events.c
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -28,16 +28,3 @@ struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
> E(NULL, NULL, NULL, false, 0),
> E("mem-ldst", "%s//", NULL, false, 0),
> };
> -
> -bool is_mem_loads_aux_event(struct evsel *leader)
> -{
> - struct perf_pmu *pmu = perf_pmus__find("cpu");
> -
> - if (!pmu)
> - pmu = perf_pmus__find("cpu_core");
> -
> - if (pmu && !perf_pmu__have_event(pmu, "mem-loads-aux"))
> - return false;
> -
> - return leader->core.attr.config == MEM_LOADS_AUX;
> -}
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 9ea9e9a868c4..336d1109b3a5 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -103,9 +103,19 @@ static const char *perf_mem_events__name(int i, struct perf_pmu *pmu)
> return NULL;
> }
>
> -__weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
> +bool is_mem_loads_aux_event(struct evsel *leader)
> {
> - return false;
> + struct perf_pmu *pmu = leader->pmu;
> + struct perf_mem_event *e;
> +
> + if (!pmu || !pmu->mem_events)
> + return false;
> +
> + e = &pmu->mem_events[PERF_MEM_EVENTS__LOAD];
> + if (!e->aux_event)
> + return false;
> +
> + return leader->core.attr.config == e->aux_event;
> }
>
> int perf_mem_events__parse(struct perf_pmu *pmu, const char *str)
> --
> 2.35.1
>
On 2023-12-06 4:07 p.m., Ian Rogers wrote:
> On Wed, Dec 6, 2023 at 12:13 PM <[email protected]> wrote:
>>
>> From: Kan Liang <[email protected]>
>>
>> Introduce a generic perf_mem_events__name(). Remove the ARCH-specific
>> one.
>>
>> The mem_load events may have a different format. Add ldlat and aux_event
>> in the struct perf_mem_event to indicate the format and the extra aux
>> event.
>>
>> Add perf_mem_events_intel_aux[] to support the extra mem_load_aux event.
>>
>> Signed-off-by: Kan Liang <[email protected]>
>> ---
>> tools/perf/arch/arm64/util/mem-events.c | 26 ++-------
>> tools/perf/arch/powerpc/util/mem-events.c | 13 ++---
>> tools/perf/arch/powerpc/util/mem-events.h | 7 +++
>> tools/perf/arch/powerpc/util/pmu.c | 11 ++++
>> tools/perf/arch/x86/util/mem-events.c | 70 +++++------------------
>> tools/perf/arch/x86/util/mem-events.h | 1 +
>> tools/perf/arch/x86/util/pmu.c | 8 ++-
>> tools/perf/util/mem-events.c | 56 ++++++++++++------
>> tools/perf/util/mem-events.h | 3 +-
>> 9 files changed, 89 insertions(+), 106 deletions(-)
>> create mode 100644 tools/perf/arch/powerpc/util/mem-events.h
>> create mode 100644 tools/perf/arch/powerpc/util/pmu.c
>>
>> diff --git a/tools/perf/arch/arm64/util/mem-events.c b/tools/perf/arch/arm64/util/mem-events.c
>> index 2602e8688727..eb2ef84f0fc8 100644
>> --- a/tools/perf/arch/arm64/util/mem-events.c
>> +++ b/tools/perf/arch/arm64/util/mem-events.c
>> @@ -2,28 +2,10 @@
>> #include "map_symbol.h"
>> #include "mem-events.h"
>>
>> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
>> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>>
>> struct perf_mem_event perf_mem_events_arm[PERF_MEM_EVENTS__MAX] = {
>> - E("spe-load", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0"),
>> - E("spe-store", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0"),
>> - E("spe-ldst", "arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0"),
>> + E("spe-load", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=0,min_latency=%u/", "arm_spe_0", true, 0),
>> + E("spe-store", "%s/ts_enable=1,pa_enable=1,load_filter=0,store_filter=1/", "arm_spe_0", false, 0),
>> + E("spe-ldst", "%s/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=%u/", "arm_spe_0", true, 0),
>> };
>> -
>> -static char mem_ev_name[100];
>> -
>> -const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
>> -{
>> - struct perf_mem_event *e = &perf_mem_events_arm[i];
>> -
>> - if (i >= PERF_MEM_EVENTS__MAX)
>> - return NULL;
>> -
>> - if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE)
>> - scnprintf(mem_ev_name, sizeof(mem_ev_name),
>> - e->name, perf_mem_events__loads_ldlat);
>> - else /* PERF_MEM_EVENTS__STORE */
>> - scnprintf(mem_ev_name, sizeof(mem_ev_name), e->name);
>> -
>> - return mem_ev_name;
>> -}
>> diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
>> index 78b986e5268d..b7883e38950f 100644
>> --- a/tools/perf/arch/powerpc/util/mem-events.c
>> +++ b/tools/perf/arch/powerpc/util/mem-events.c
>> @@ -2,11 +2,10 @@
>> #include "map_symbol.h"
>> #include "mem-events.h"
>>
>> -/* PowerPC does not support 'ldlat' parameter. */
>> -const char *perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
>> -{
>> - if (i == PERF_MEM_EVENTS__LOAD)
>> - return "cpu/mem-loads/";
>> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>>
>> - return "cpu/mem-stores/";
>> -}
>> +struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX] = {
>> + E("ldlat-loads", "%s/mem-loads/", "cpu/events/mem-loads", false, 0),
>> + E("ldlat-stores", "%s/mem-stores/", "cpu/events/mem-stores", false, 0),
>> + E(NULL, NULL, NULL, false, 0),
>> +};
>> diff --git a/tools/perf/arch/powerpc/util/mem-events.h b/tools/perf/arch/powerpc/util/mem-events.h
>> new file mode 100644
>> index 000000000000..6acc3d1b6873
>> --- /dev/null
>> +++ b/tools/perf/arch/powerpc/util/mem-events.h
>> @@ -0,0 +1,7 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _POWER_MEM_EVENTS_H
>> +#define _POWER_MEM_EVENTS_H
>> +
>> +extern struct perf_mem_event perf_mem_events_power[PERF_MEM_EVENTS__MAX];
>> +
>> +#endif /* _POWER_MEM_EVENTS_H */
>> diff --git a/tools/perf/arch/powerpc/util/pmu.c b/tools/perf/arch/powerpc/util/pmu.c
>> new file mode 100644
>> index 000000000000..168173f88ddb
>> --- /dev/null
>> +++ b/tools/perf/arch/powerpc/util/pmu.c
>> @@ -0,0 +1,11 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <string.h>
>> +
>> +#include "../../../util/pmu.h"
>> +
>> +void perf_pmu__arch_init(struct perf_pmu *pmu)
>> +{
>> + if (pmu->is_core)
>> + pmu->mem_events = perf_mem_events_power;
>> +}
>> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
>> index 5fb41d50118d..f0e66a0151a0 100644
>> --- a/tools/perf/arch/x86/util/mem-events.c
>> +++ b/tools/perf/arch/x86/util/mem-events.c
>> @@ -7,25 +7,26 @@
>> #include "linux/string.h"
>> #include "env.h"
>>
>> -static char mem_loads_name[100];
>> -static bool mem_loads_name__init;
>> -static char mem_stores_name[100];
>> -
>> #define MEM_LOADS_AUX 0x8203
>> -#define MEM_LOADS_AUX_NAME "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P"
>>
>> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
>> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>>
>> struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX] = {
>> - E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads"),
>> - E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores"),
>> - E(NULL, NULL, NULL),
>> + E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads", true, 0),
>> + E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
>> + E(NULL, NULL, NULL, false, 0),
>> +};
>> +
>> +struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
>> + E("ldlat-loads", "{%s/mem-loads-aux/,%s/mem-loads,ldlat=%u/}:P", "%s/events/mem-loads", true, MEM_LOADS_AUX),
>> + E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores", false, 0),
>> + E(NULL, NULL, NULL, false, 0),
>> };
>>
>> struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
>> - E(NULL, NULL, NULL),
>> - E(NULL, NULL, NULL),
>> - E("mem-ldst", "ibs_op//", "ibs_op"),
>> + E(NULL, NULL, NULL, false, 0),
>> + E(NULL, NULL, NULL, false, 0),
>> + E("mem-ldst", "%s//", "ibs_op", false, 0),
>> };
>>
>> bool is_mem_loads_aux_event(struct evsel *leader)
>> @@ -40,48 +41,3 @@ bool is_mem_loads_aux_event(struct evsel *leader)
>>
>> return leader->core.attr.config == MEM_LOADS_AUX;
>> }
>> -
>> -const char *perf_mem_events__name(int i, const char *pmu_name)
>> -{
>> - struct perf_mem_event *e;
>> -
>> - if (x86__is_amd_cpu())
>> - e = &perf_mem_events_amd[i];
>> - else
>> - e = &perf_mem_events_intel[i];
>> -
>> - if (!e)
>> - return NULL;
>> -
>> - if (i == PERF_MEM_EVENTS__LOAD) {
>> - if (mem_loads_name__init && !pmu_name)
>> - return mem_loads_name;
>> -
>> - if (!pmu_name) {
>> - mem_loads_name__init = true;
>> - pmu_name = "cpu";
>> - }
>> -
>> - if (perf_pmus__have_event(pmu_name, "mem-loads-aux")) {
>> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> - MEM_LOADS_AUX_NAME, pmu_name, pmu_name,
>> - perf_mem_events__loads_ldlat);
>> - } else {
>> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> - e->name, pmu_name,
>> - perf_mem_events__loads_ldlat);
>> - }
>> - return mem_loads_name;
>> - }
>> -
>> - if (i == PERF_MEM_EVENTS__STORE) {
>> - if (!pmu_name)
>> - pmu_name = "cpu";
>> -
>> - scnprintf(mem_stores_name, sizeof(mem_stores_name),
>> - e->name, pmu_name);
>> - return mem_stores_name;
>> - }
>> -
>> - return e->name;
>> -}
>> diff --git a/tools/perf/arch/x86/util/mem-events.h b/tools/perf/arch/x86/util/mem-events.h
>> index 3959e427f482..f55c8d3b7d59 100644
>> --- a/tools/perf/arch/x86/util/mem-events.h
>> +++ b/tools/perf/arch/x86/util/mem-events.h
>> @@ -3,6 +3,7 @@
>> #define _X86_MEM_EVENTS_H
>>
>> extern struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX];
>> +extern struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX];
>>
>> extern struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX];
>>
>> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
>> index 7e69f4f2e363..446f8197aae4 100644
>> --- a/tools/perf/arch/x86/util/pmu.c
>> +++ b/tools/perf/arch/x86/util/pmu.c
>> @@ -35,8 +35,12 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
>> if (x86__is_amd_cpu()) {
>> if (strcmp(pmu->name, "ibs_op"))
>> pmu->mem_events = perf_mem_events_amd;
>> - } else if (pmu->is_core)
>> - pmu->mem_events = perf_mem_events_intel;
>> + } else if (pmu->is_core) {
>> + if (perf_pmu__have_event(pmu, "mem-loads-aux"))
>> + pmu->mem_events = perf_mem_events_intel_aux;
>> + else
>> + pmu->mem_events = perf_mem_events_intel;
>> + }
>> }
>>
>> int perf_pmus__num_mem_pmus(void)
>> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
>> index 887ffdcce338..3a60cbcd6d8e 100644
>> --- a/tools/perf/util/mem-events.c
>> +++ b/tools/perf/util/mem-events.c
>> @@ -17,17 +17,17 @@
>>
>> unsigned int perf_mem_events__loads_ldlat = 30;
>>
>> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
>> +#define E(t, n, s, l, a) { .tag = t, .name = n, .sysfs_name = s, .ldlat = l, .aux_event = a }
>>
>> struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
>> - E("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "cpu/events/mem-loads"),
>> - E("ldlat-stores", "cpu/mem-stores/P", "cpu/events/mem-stores"),
>> - E(NULL, NULL, NULL),
>> + E("ldlat-loads", "%s/mem-loads/,ldlat=%u/P", "cpu/events/mem-loads", true, 0),
>
> Is there a mistake here "/," looks like it should just be ",".
Right, a typo. I will fix it in V2.
Thanks,
Kan
>
> Thanks,
> Ian
>
>> + E("ldlat-stores", "%s/mem-stores/P", "cpu/events/mem-stores", false, 0),
>> + E(NULL, NULL, NULL, false, 0),
>> };
>> #undef E
>>
>> static char mem_loads_name[100];
>> -static bool mem_loads_name__init;
>> +static char mem_stores_name[100];
>>
>> struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i)
>> {
>> @@ -62,23 +62,45 @@ struct perf_pmu *perf_mem_events_find_pmu(void)
>> return perf_pmus__scan_mem(NULL);
>> }
>>
>> -const char * __weak perf_mem_events__name(int i, const char *pmu_name __maybe_unused)
>> +static const char *perf_mem_events__name(int i, struct perf_pmu *pmu)
>> {
>> - struct perf_mem_event *e = &perf_mem_events[i];
>> + struct perf_mem_event *e = &pmu->mem_events[i];
>>
>> if (!e)
>> return NULL;
>>
>> - if (i == PERF_MEM_EVENTS__LOAD) {
>> - if (!mem_loads_name__init) {
>> - mem_loads_name__init = true;
>> - scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> - e->name, perf_mem_events__loads_ldlat);
>> + if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE) {
>> + if (e->ldlat) {
>> + if (!e->aux_event) {
>> + /* ARM and Most of Intel */
>> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> + e->name, pmu->name,
>> + perf_mem_events__loads_ldlat);
>> + } else {
>> + /* Intel with mem-loads-aux event */
>> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> + e->name, pmu->name, pmu->name,
>> + perf_mem_events__loads_ldlat);
>> + }
>> + } else {
>> + if (!e->aux_event) {
>> + /* AMD and POWER */
>> + scnprintf(mem_loads_name, sizeof(mem_loads_name),
>> + e->name, pmu->name);
>> + } else
>> + return NULL;
>> }
>> +
>> return mem_loads_name;
>> }
>>
>> - return e->name;
>> + if (i == PERF_MEM_EVENTS__STORE) {
>> + scnprintf(mem_stores_name, sizeof(mem_stores_name),
>> + e->name, pmu->name);
>> + return mem_stores_name;
>> + }
>> +
>> + return NULL;
>> }
>>
>> __weak bool is_mem_loads_aux_event(struct evsel *leader __maybe_unused)
>> @@ -175,7 +197,7 @@ void perf_mem_events__list(struct perf_pmu *pmu)
>> e->tag ? 13 : 0,
>> e->tag ? : "",
>> e->tag && verbose > 0 ? 25 : 0,
>> - e->tag && verbose > 0 ? perf_mem_events__name(j, NULL) : "",
>> + e->tag && verbose > 0 ? perf_mem_events__name(j, pmu) : "",
>> e->supported ? ": available\n" : "");
>> }
>> }
>> @@ -198,15 +220,15 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
>>
>> if (!e->supported) {
>> pr_err("failed: event '%s' not supported\n",
>> - perf_mem_events__name(j, pmu->name));
>> + perf_mem_events__name(j, pmu));
>> return -1;
>> }
>>
>> if (perf_pmus__num_mem_pmus() == 1) {
>> rec_argv[i++] = "-e";
>> - rec_argv[i++] = perf_mem_events__name(j, NULL);
>> + rec_argv[i++] = perf_mem_events__name(j, pmu);
>> } else {
>> - const char *s = perf_mem_events__name(j, pmu->name);
>> + const char *s = perf_mem_events__name(j, pmu);
>>
>> if (!perf_mem_event__supported(mnt, pmu, e))
>> continue;
>> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
>> index 59a4303aac96..d257cf67d6d9 100644
>> --- a/tools/perf/util/mem-events.h
>> +++ b/tools/perf/util/mem-events.h
>> @@ -14,6 +14,8 @@
>> struct perf_mem_event {
>> bool record;
>> bool supported;
>> + bool ldlat;
>> + u32 aux_event;
>> const char *tag;
>> const char *name;
>> const char *sysfs_name;
>> @@ -39,7 +41,6 @@ extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
>> int perf_mem_events__parse(struct perf_pmu *pmu, const char *str);
>> int perf_mem_events__init(struct perf_pmu *pmu);
>>
>> -const char *perf_mem_events__name(int i, const char *pmu_name);
>> struct perf_mem_event *perf_mem_events__ptr(struct perf_pmu *pmu, int i);
>> struct perf_pmu *perf_mem_events_find_pmu(void);
>> bool is_mem_loads_aux_event(struct evsel *leader);
>> --
>> 2.35.1
>>
On 2023-12-06 3:38 p.m., Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 06, 2023 at 12:13:24PM -0800, [email protected] escreveu:
>> From: Kan Liang <[email protected]>
>>
>> The X86 mem-events.c only has perf_mem_events array now. Remove useless
>> header files.
>
> It would be great that those were removed while you made the cleanups,
> i.e. removed the need for one of them, remove it together with the
> refactorings, etc.
>
> But I don't think this is a requirement, just would make it cleaner.
Sure, I will merge it with the previous patch in V2.
Thanks,
Kan
>
> Will wait for reviews now.
>
> - Arnaldo
>
>> Signed-off-by: Kan Liang <[email protected]>
>> ---
>> tools/perf/arch/x86/util/mem-events.c | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
>> index 71ffe16de751..62df03e91c7e 100644
>> --- a/tools/perf/arch/x86/util/mem-events.c
>> +++ b/tools/perf/arch/x86/util/mem-events.c
>> @@ -1,11 +1,9 @@
>> // SPDX-License-Identifier: GPL-2.0
>> -#include "util/pmu.h"
>> -#include "util/pmus.h"
>> -#include "util/env.h"
>> -#include "map_symbol.h"
>> -#include "mem-events.h"
>> #include "linux/string.h"
>> -#include "env.h"
>> +#include "util/map_symbol.h"
>> +#include "util/mem-events.h"
>> +#include "mem-events.h"
>> +
>>
>> #define MEM_LOADS_AUX 0x8203
>>
>> --
>> 2.35.1
>>
>
On 2023-12-06 4:04 p.m., Ian Rogers wrote:
>> * The load and store operations are required, use the event
>> * PERF_MEM_EVENTS__LOAD_STORE if it is supported.
>> @@ -3289,15 +3303,15 @@ static int perf_c2c__record(int argc, const char **argv)
>> e->record = true;
>> rec_argv[i++] = "-W";
>> } else {
>> - e = perf_mem_events__ptr(PERF_MEM_EVENTS__LOAD);
>> + e = perf_mem_events__ptr(pmu, PERF_MEM_EVENTS__LOAD);
> Fwiw, it seems strange in cases like this that the function isn't:
> perf_pmu__mem_events_ptr(pmu, PERF_MEM_EVENTS__LOAD)
Not just this one. The PMU is also added into other functions.
I will change the name for all of them in V2.
diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
index 8c5694b2d0b0..0ad301a2e424 100644
--- a/tools/perf/util/mem-events.h
+++ b/tools/perf/util/mem-events.h
@@ -36,14 +36,15 @@ enum {
extern unsigned int perf_mem_events__loads_ldlat;
extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
-int perf_mem_events__parse(const char *str);
-int perf_mem_events__init(void);
+int perf_pmu__mem_events_parse(struct perf_pmu *pmu, const char *str);
+int perf_pmu__mem_events_init(struct perf_pmu *pmu);
const char *perf_mem_events__name(int i, const char *pmu_name);
-struct perf_mem_event *perf_mem_events__ptr(int i);
+struct perf_mem_event *perf_pmu__mem_events_ptr(struct perf_pmu *pmu,
int i);
+struct perf_pmu *perf_mem_events_find_pmu(void);
bool is_mem_loads_aux_event(struct evsel *leader);
-void perf_mem_events__list(void);
+void perf_pmu__mem_events_list(struct perf_pmu *pmu);
int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
char **rec_tmp, int *tmp_nr);
Thanks,
Kan
> The patch set touches almost all the ARCHs, Intel, AMD, ARM, Power and
> etc. But I can only test it on two Intel platforms.
> Please give it try, if you have machines with other ARCHs.
I did a quick perf mem and perf c2c test, with the fix I suggested
in patch #1, and things seems to be working fine.
[For AMD specific changes]
Tested-by: Ravi Bangoria <[email protected]>
On 2023-12-07 9:44 a.m., Ravi Bangoria wrote:
>> The patch set touches almost all the ARCHs, Intel, AMD, ARM, Power and
>> etc. But I can only test it on two Intel platforms.
>> Please give it try, if you have machines with other ARCHs.
>
> I did a quick perf mem and perf c2c test, with the fix I suggested
> in patch #1, and things seems to be working fine.
>
> [For AMD specific changes]
> Tested-by: Ravi Bangoria <[email protected]>
Thanks Ravi.
Kan