2021-09-02 07:06:45

by Jin Yao

[permalink] [raw]
Subject: [PATCH v7 1/2] perf pmu: Add PMU alias support

From: Kan Liang <[email protected]>

A perf uncore PMU may have two PMU names, a real name and an alias. The
alias is exported at /sys/bus/event_source/devices/uncore_*/alias.
The perf tool should support the alias as well.

Add alias_name in the struct perf_pmu to store the alias. For the PMU
which doesn't have an alias. It's NULL.

Introduce two X86 specific functions to retrieve the real name and the
alias separately.

Only go through the sysfs to retrieve the mapping between the real name
and the alias once. The result is cached in a list, uncore_pmu_list.

Nothing changed for the other ARCHs.

With the patch, the perf tool can monitor the PMU with either the real
name or the alias.

Use the real name,
$ perf stat -e uncore_cha_2/event=1/ -x,
4044879584,,uncore_cha_2/event=1/,2528059205,100.00,,

Use the alias,
$ perf stat -e uncore_type_0_2/event=1/ -x,
3659675336,,uncore_type_0_2/event=1/,2287306455,100.00,,

Co-developed-by: Jin Yao <[email protected]>
Co-developed-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Jin Yao <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
---
v7:
- Create 'struct perf_pmu_alias_name' constructor/destructor.
- Return '-errno' if opendir() is failed.

v6:
- Call setup_pmu_alias_list in pmu_find_alias_name.
- Check pmu->name and pmu->alias_name after strdup in pmu_lookup.

v5:
- Don't use strdup in pmu_find_real_name and pmu_find_alias_name.
Just return name and keep the 'pmu->name = strdup(name);' in
pmu_lookup.

- Remove invalid comment for alias_name in struct perf_pmu.

v4:
- Fix memory leaks in pmu_lookup.
- Rebase to perf/core.

v3:
- Use fgets to read alias string from sysfs.
- Resource cleanup.

v2:
- No change.

tools/perf/arch/x86/util/pmu.c | 155 ++++++++++++++++++++++++++++++++-
tools/perf/util/parse-events.y | 3 +-
tools/perf/util/pmu.c | 40 ++++++++-
tools/perf/util/pmu.h | 5 ++
4 files changed, 198 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index d48d608517fd..d4554b58c889 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -1,12 +1,30 @@
// SPDX-License-Identifier: GPL-2.0
#include <string.h>
-
+#include <stdio.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <fcntl.h>
#include <linux/stddef.h>
#include <linux/perf_event.h>
+#include <linux/zalloc.h>
+#include <api/fs/fs.h>
+#include <errno.h>

#include "../../../util/intel-pt.h"
#include "../../../util/intel-bts.h"
#include "../../../util/pmu.h"
+#include "../../../util/fncache.h"
+
+#define TEMPLATE_ALIAS "%s/bus/event_source/devices/%s/alias"
+
+struct perf_pmu_alias_name {
+ char *name;
+ char *alias;
+ struct list_head list;
+};
+
+static LIST_HEAD(pmu_alias_name_list);
+static bool cached_list;

struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
{
@@ -18,3 +36,138 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
#endif
return NULL;
}
+
+static void pmu_alias__delete(struct perf_pmu_alias_name *pmu)
+{
+ if (!pmu)
+ return;
+
+ zfree(&pmu->name);
+ zfree(&pmu->alias);
+ free(pmu);
+}
+
+static struct perf_pmu_alias_name *pmu_alias__new(char *name, char *alias)
+{
+ struct perf_pmu_alias_name *pmu = zalloc(sizeof(*pmu));
+
+ if (pmu) {
+ pmu->name = strdup(name);
+ if (!pmu->name)
+ goto out_delete;
+
+ pmu->alias = strdup(alias);
+ if (!pmu->alias)
+ goto out_delete;
+ }
+ return pmu;
+
+out_delete:
+ pmu_alias__delete(pmu);
+ return NULL;
+}
+
+static int setup_pmu_alias_list(void)
+{
+ char path[PATH_MAX];
+ DIR *dir;
+ struct dirent *dent;
+ const char *sysfs = sysfs__mountpoint();
+ struct perf_pmu_alias_name *pmu;
+ char buf[MAX_PMU_NAME_LEN];
+ FILE *file;
+ int ret = -ENOMEM;
+
+ if (!sysfs)
+ return -1;
+
+ snprintf(path, PATH_MAX,
+ "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
+
+ dir = opendir(path);
+ if (!dir)
+ return -errno;
+
+ while ((dent = readdir(dir))) {
+ if (!strcmp(dent->d_name, ".") ||
+ !strcmp(dent->d_name, ".."))
+ continue;
+
+ snprintf(path, PATH_MAX,
+ TEMPLATE_ALIAS, sysfs, dent->d_name);
+
+ if (!file_available(path))
+ continue;
+
+ file = fopen(path, "r");
+ if (!file)
+ continue;
+
+ if (!fgets(buf, sizeof(buf), file)) {
+ fclose(file);
+ continue;
+ }
+
+ fclose(file);
+
+ /* Remove the last '\n' */
+ buf[strlen(buf) - 1] = 0;
+
+ pmu = pmu_alias__new(dent->d_name, buf);
+ if (!pmu)
+ goto close_dir;
+
+ list_add_tail(&pmu->list, &pmu_alias_name_list);
+ }
+
+ ret = 0;
+
+close_dir:
+ closedir(dir);
+ return ret;
+}
+
+static char *__pmu_find_real_name(const char *name)
+{
+ struct perf_pmu_alias_name *pmu;
+
+ list_for_each_entry(pmu, &pmu_alias_name_list, list) {
+ if (!strcmp(name, pmu->alias))
+ return pmu->name;
+ }
+
+ return (char *)name;
+}
+
+char *pmu_find_real_name(const char *name)
+{
+ if (cached_list)
+ return __pmu_find_real_name(name);
+
+ setup_pmu_alias_list();
+ cached_list = true;
+
+ return __pmu_find_real_name(name);
+}
+
+static char *__pmu_find_alias_name(const char *name)
+{
+ struct perf_pmu_alias_name *pmu;
+
+ list_for_each_entry(pmu, &pmu_alias_name_list, list) {
+ if (!strcmp(name, pmu->name))
+ return pmu->alias;
+ }
+ return NULL;
+}
+
+char *pmu_find_alias_name(const char *name)
+{
+ if (cached_list)
+ return __pmu_find_alias_name(name);
+
+ setup_pmu_alias_list();
+ cached_list = true;
+
+ return __pmu_find_alias_name(name);
+}
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 9321bd0e2f76..d94e48e1ff9b 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -316,7 +316,8 @@ event_pmu_name opt_pmu_config
if (!strncmp(name, "uncore_", 7) &&
strncmp($1, "uncore_", 7))
name += 7;
- if (!perf_pmu__match(pattern, name, $1)) {
+ if (!perf_pmu__match(pattern, name, $1) ||
+ !perf_pmu__match(pattern, pmu->alias_name, $1)) {
if (parse_events_copy_term_list(orig_terms, &terms))
CLEANUP_YYABORT;
if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6cdbee8a12e7..1a35915edf68 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -945,6 +945,18 @@ perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
return NULL;
}

+char * __weak
+pmu_find_real_name(const char *name)
+{
+ return (char *)name;
+}
+
+char * __weak
+pmu_find_alias_name(const char *name __maybe_unused)
+{
+ return NULL;
+}
+
static int pmu_max_precise(const char *name)
{
char path[PATH_MAX];
@@ -958,13 +970,15 @@ static int pmu_max_precise(const char *name)
return max_precise;
}

-static struct perf_pmu *pmu_lookup(const char *name)
+static struct perf_pmu *pmu_lookup(const char *lookup_name)
{
struct perf_pmu *pmu;
LIST_HEAD(format);
LIST_HEAD(aliases);
__u32 type;
+ char *name = pmu_find_real_name(lookup_name);
bool is_hybrid = perf_pmu__hybrid_mounted(name);
+ char *alias_name;

/*
* Check pmu name for hybrid and the pmu may be invalid in sysfs
@@ -995,6 +1009,16 @@ static struct perf_pmu *pmu_lookup(const char *name)

pmu->cpus = pmu_cpumask(name);
pmu->name = strdup(name);
+ if (!pmu->name)
+ goto err;
+
+ alias_name = pmu_find_alias_name(name);
+ if (alias_name) {
+ pmu->alias_name = strdup(alias_name);
+ if (!pmu->alias_name)
+ goto err;
+ }
+
pmu->type = type;
pmu->is_uncore = pmu_is_uncore(name);
if (pmu->is_uncore)
@@ -1017,15 +1041,22 @@ static struct perf_pmu *pmu_lookup(const char *name)
pmu->default_config = perf_pmu__get_default_config(pmu);

return pmu;
+err:
+ if (pmu->name)
+ free(pmu->name);
+ free(pmu);
+ return NULL;
}

static struct perf_pmu *pmu_find(const char *name)
{
struct perf_pmu *pmu;

- list_for_each_entry(pmu, &pmus, list)
- if (!strcmp(pmu->name, name))
+ list_for_each_entry(pmu, &pmus, list) {
+ if (!strcmp(pmu->name, name) ||
+ (pmu->alias_name && !strcmp(pmu->alias_name, name)))
return pmu;
+ }

return NULL;
}
@@ -1919,6 +1950,9 @@ bool perf_pmu__has_hybrid(void)

int perf_pmu__match(char *pattern, char *name, char *tok)
{
+ if (!name)
+ return -1;
+
if (fnmatch(pattern, name, 0))
return -1;

diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 033e8211c025..6b122f97acf3 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -21,6 +21,7 @@ enum {
#define PERF_PMU_FORMAT_BITS 64
#define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
#define CPUS_TEMPLATE_CPU "%s/bus/event_source/devices/%s/cpus"
+#define MAX_PMU_NAME_LEN 128

struct perf_event_attr;

@@ -32,6 +33,7 @@ struct perf_pmu_caps {

struct perf_pmu {
char *name;
+ char *alias_name;
char *id;
__u32 type;
bool selectable;
@@ -136,4 +138,7 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
bool perf_pmu__has_hybrid(void);
int perf_pmu__match(char *pattern, char *name, char *tok);

+char *pmu_find_real_name(const char *name);
+char *pmu_find_alias_name(const char *name);
+
#endif /* __PMU_H */
--
2.17.1


2021-09-03 12:54:01

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] perf pmu: Add PMU alias support

Em Thu, Sep 02, 2021 at 02:59:54PM +0800, Jin Yao escreveu:
> From: Kan Liang <[email protected]>
>
> A perf uncore PMU may have two PMU names, a real name and an alias. The
> alias is exported at /sys/bus/event_source/devices/uncore_*/alias.
> The perf tool should support the alias as well.
>
> Add alias_name in the struct perf_pmu to store the alias. For the PMU
> which doesn't have an alias. It's NULL.
>
> Introduce two X86 specific functions to retrieve the real name and the
> alias separately.
>
> Only go through the sysfs to retrieve the mapping between the real name
> and the alias once. The result is cached in a list, uncore_pmu_list.
>
> Nothing changed for the other ARCHs.
>
> With the patch, the perf tool can monitor the PMU with either the real
> name or the alias.
>
> Use the real name,
> $ perf stat -e uncore_cha_2/event=1/ -x,
> 4044879584,,uncore_cha_2/event=1/,2528059205,100.00,,
>
> Use the alias,
> $ perf stat -e uncore_type_0_2/event=1/ -x,
> 3659675336,,uncore_type_0_2/event=1/,2287306455,100.00,,
>
> Co-developed-by: Jin Yao <[email protected]>
> Co-developed-by: Arnaldo Carvalho de Melo <[email protected]>
> Signed-off-by: Jin Yao <[email protected]>
> Signed-off-by: Kan Liang <[email protected]>
> Reviewed-by: Andi Kleen <[email protected]>
> ---
> v7:
> - Create 'struct perf_pmu_alias_name' constructor/destructor.
> - Return '-errno' if opendir() is failed.

Thanks for addressing those, I did a v8 here with the changes described
in my Committer notes below, please holler if you disagree.

commit 13d60ba0738b0532edab3e1492b2005d36ba0802
Author: Kan Liang <[email protected]>
Date: Thu Sep 2 14:59:54 2021 +0800

perf pmu: Add PMU alias support

A perf uncore PMU may have two PMU names, a real name and an alias. The
alias is exported at /sys/bus/event_source/devices/uncore_*/alias.
The perf tool should support the alias as well.

Add alias_name in the struct perf_pmu to store the alias. For the PMU
which doesn't have an alias. It's NULL.

Introduce two X86 specific functions to retrieve the real name and the
alias separately.

Only go through the sysfs to retrieve the mapping between the real name
and the alias once. The result is cached in a list, uncore_pmu_list.

Nothing changed for the other ARCHs.

With the patch, the perf tool can monitor the PMU with either the real
name or the alias.

Use the real name,
$ perf stat -e uncore_cha_2/event=1/ -x,
4044879584,,uncore_cha_2/event=1/,2528059205,100.00,,

Use the alias,
$ perf stat -e uncore_type_0_2/event=1/ -x,
3659675336,,uncore_type_0_2/event=1/,2287306455,100.00,,

Committer notes:

Rename 'struct perf_pmu_alias_name' to 'pmu_alias', the 'perf_' prefix
should be used for libperf, things inside just tools/perf/ are being
moved away from that prefix.

Also 'pmu_alias' is shorter and reflects the abstraction.

Also don't use 'pmu' as the name for variables for that type, we should
use that for the 'struct perf_pmu' variables, avoiding confusion. Use
'pmu_alias' for 'struct pmu_alias' variables.

Co-developed-by: Jin Yao <[email protected]>
Co-developed-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: John Garry <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Riccardo Mancini <[email protected]>
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Jin Yao <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index d48d608517fd2732..74d69db1ea99df17 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -1,12 +1,30 @@
// SPDX-License-Identifier: GPL-2.0
#include <string.h>
-
+#include <stdio.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <fcntl.h>
#include <linux/stddef.h>
#include <linux/perf_event.h>
+#include <linux/zalloc.h>
+#include <api/fs/fs.h>
+#include <errno.h>

#include "../../../util/intel-pt.h"
#include "../../../util/intel-bts.h"
#include "../../../util/pmu.h"
+#include "../../../util/fncache.h"
+
+#define TEMPLATE_ALIAS "%s/bus/event_source/devices/%s/alias"
+
+struct pmu_alias {
+ char *name;
+ char *alias;
+ struct list_head list;
+};
+
+static LIST_HEAD(pmu_alias_name_list);
+static bool cached_list;

struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
{
@@ -18,3 +36,138 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
#endif
return NULL;
}
+
+static void pmu_alias__delete(struct pmu_alias *pmu_alias)
+{
+ if (!pmu_alias)
+ return;
+
+ zfree(&pmu_alias->name);
+ zfree(&pmu_alias->alias);
+ free(pmu_alias);
+}
+
+static struct pmu_alias *pmu_alias__new(char *name, char *alias)
+{
+ struct pmu_alias *pmu_alias = zalloc(sizeof(*pmu_alias));
+
+ if (pmu_alias) {
+ pmu_alias->name = strdup(name);
+ if (!pmu_alias->name)
+ goto out_delete;
+
+ pmu_alias->alias = strdup(alias);
+ if (!pmu_alias->alias)
+ goto out_delete;
+ }
+ return pmu_alias;
+
+out_delete:
+ pmu_alias__delete(pmu_alias);
+ return NULL;
+}
+
+static int setup_pmu_alias_list(void)
+{
+ char path[PATH_MAX];
+ DIR *dir;
+ struct dirent *dent;
+ const char *sysfs = sysfs__mountpoint();
+ struct pmu_alias *pmu_alias;
+ char buf[MAX_PMU_NAME_LEN];
+ FILE *file;
+ int ret = -ENOMEM;
+
+ if (!sysfs)
+ return -1;
+
+ snprintf(path, PATH_MAX,
+ "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
+
+ dir = opendir(path);
+ if (!dir)
+ return -errno;
+
+ while ((dent = readdir(dir))) {
+ if (!strcmp(dent->d_name, ".") ||
+ !strcmp(dent->d_name, ".."))
+ continue;
+
+ snprintf(path, PATH_MAX,
+ TEMPLATE_ALIAS, sysfs, dent->d_name);
+
+ if (!file_available(path))
+ continue;
+
+ file = fopen(path, "r");
+ if (!file)
+ continue;
+
+ if (!fgets(buf, sizeof(buf), file)) {
+ fclose(file);
+ continue;
+ }
+
+ fclose(file);
+
+ /* Remove the last '\n' */
+ buf[strlen(buf) - 1] = 0;
+
+ pmu_alias = pmu_alias__new(dent->d_name, buf);
+ if (!pmu_alias)
+ goto close_dir;
+
+ list_add_tail(&pmu_alias->list, &pmu_alias_name_list);
+ }
+
+ ret = 0;
+
+close_dir:
+ closedir(dir);
+ return ret;
+}
+
+static char *__pmu_find_real_name(const char *name)
+{
+ struct pmu_alias *pmu_alias;
+
+ list_for_each_entry(pmu_alias, &pmu_alias_name_list, list) {
+ if (!strcmp(name, pmu_alias->alias))
+ return pmu_alias->name;
+ }
+
+ return (char *)name;
+}
+
+char *pmu_find_real_name(const char *name)
+{
+ if (cached_list)
+ return __pmu_find_real_name(name);
+
+ setup_pmu_alias_list();
+ cached_list = true;
+
+ return __pmu_find_real_name(name);
+}
+
+static char *__pmu_find_alias_name(const char *name)
+{
+ struct pmu_alias *pmu_alias;
+
+ list_for_each_entry(pmu_alias, &pmu_alias_name_list, list) {
+ if (!strcmp(name, pmu_alias->name))
+ return pmu_alias->alias;
+ }
+ return NULL;
+}
+
+char *pmu_find_alias_name(const char *name)
+{
+ if (cached_list)
+ return __pmu_find_alias_name(name);
+
+ setup_pmu_alias_list();
+ cached_list = true;
+
+ return __pmu_find_alias_name(name);
+}
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 9321bd0e2f76321a..d94e48e1ff9b22dd 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -316,7 +316,8 @@ event_pmu_name opt_pmu_config
if (!strncmp(name, "uncore_", 7) &&
strncmp($1, "uncore_", 7))
name += 7;
- if (!perf_pmu__match(pattern, name, $1)) {
+ if (!perf_pmu__match(pattern, name, $1) ||
+ !perf_pmu__match(pattern, pmu->alias_name, $1)) {
if (parse_events_copy_term_list(orig_terms, &terms))
CLEANUP_YYABORT;
if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 5f486ccb6fe67b58..bdabd62170d2cf1f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -945,6 +945,18 @@ perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
return NULL;
}

+char * __weak
+pmu_find_real_name(const char *name)
+{
+ return (char *)name;
+}
+
+char * __weak
+pmu_find_alias_name(const char *name __maybe_unused)
+{
+ return NULL;
+}
+
static int pmu_max_precise(const char *name)
{
char path[PATH_MAX];
@@ -958,13 +970,15 @@ static int pmu_max_precise(const char *name)
return max_precise;
}

-static struct perf_pmu *pmu_lookup(const char *name)
+static struct perf_pmu *pmu_lookup(const char *lookup_name)
{
struct perf_pmu *pmu;
LIST_HEAD(format);
LIST_HEAD(aliases);
__u32 type;
+ char *name = pmu_find_real_name(lookup_name);
bool is_hybrid = perf_pmu__hybrid_mounted(name);
+ char *alias_name;

/*
* Check pmu name for hybrid and the pmu may be invalid in sysfs
@@ -995,6 +1009,16 @@ static struct perf_pmu *pmu_lookup(const char *name)

pmu->cpus = pmu_cpumask(name);
pmu->name = strdup(name);
+ if (!pmu->name)
+ goto err;
+
+ alias_name = pmu_find_alias_name(name);
+ if (alias_name) {
+ pmu->alias_name = strdup(alias_name);
+ if (!pmu->alias_name)
+ goto err;
+ }
+
pmu->type = type;
pmu->is_uncore = pmu_is_uncore(name);
if (pmu->is_uncore)
@@ -1017,15 +1041,22 @@ static struct perf_pmu *pmu_lookup(const char *name)
pmu->default_config = perf_pmu__get_default_config(pmu);

return pmu;
+err:
+ if (pmu->name)
+ free(pmu->name);
+ free(pmu);
+ return NULL;
}

static struct perf_pmu *pmu_find(const char *name)
{
struct perf_pmu *pmu;

- list_for_each_entry(pmu, &pmus, list)
- if (!strcmp(pmu->name, name))
+ list_for_each_entry(pmu, &pmus, list) {
+ if (!strcmp(pmu->name, name) ||
+ (pmu->alias_name && !strcmp(pmu->alias_name, name)))
return pmu;
+ }

return NULL;
}
@@ -1919,6 +1950,9 @@ bool perf_pmu__has_hybrid(void)

int perf_pmu__match(char *pattern, char *name, char *tok)
{
+ if (!name)
+ return -1;
+
if (fnmatch(pattern, name, 0))
return -1;

diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 5133bc45603492f7..394898b07fd9874b 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -22,6 +22,7 @@ enum {
#define PERF_PMU_FORMAT_BITS 64
#define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
#define CPUS_TEMPLATE_CPU "%s/bus/event_source/devices/%s/cpus"
+#define MAX_PMU_NAME_LEN 128

struct perf_event_attr;

@@ -33,6 +34,7 @@ struct perf_pmu_caps {

struct perf_pmu {
char *name;
+ char *alias_name;
char *id;
__u32 type;
bool selectable;
@@ -140,4 +142,7 @@ int perf_pmu__match(char *pattern, char *name, char *tok);
int perf_pmu__cpus_match(struct perf_pmu *pmu, struct perf_cpu_map *cpus,
struct perf_cpu_map **mcpus_ptr,
struct perf_cpu_map **ucpus_ptr);
+
+char *pmu_find_real_name(const char *name);
+char *pmu_find_alias_name(const char *name);
#endif /* __PMU_H */

2021-09-03 13:10:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] perf pmu: Add PMU alias support

Em Fri, Sep 03, 2021 at 08:38:23AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Sep 02, 2021 at 02:59:54PM +0800, Jin Yao escreveu:
> > From: Kan Liang <[email protected]>
> >
> > A perf uncore PMU may have two PMU names, a real name and an alias. The
> > alias is exported at /sys/bus/event_source/devices/uncore_*/alias.
> > The perf tool should support the alias as well.
> >
> > Add alias_name in the struct perf_pmu to store the alias. For the PMU
> > which doesn't have an alias. It's NULL.
> >
> > Introduce two X86 specific functions to retrieve the real name and the
> > alias separately.
> >
> > Only go through the sysfs to retrieve the mapping between the real name
> > and the alias once. The result is cached in a list, uncore_pmu_list.
> >
> > Nothing changed for the other ARCHs.
> >
> > With the patch, the perf tool can monitor the PMU with either the real
> > name or the alias.
> >
> > Use the real name,
> > $ perf stat -e uncore_cha_2/event=1/ -x,
> > 4044879584,,uncore_cha_2/event=1/,2528059205,100.00,,
> >
> > Use the alias,
> > $ perf stat -e uncore_type_0_2/event=1/ -x,
> > 3659675336,,uncore_type_0_2/event=1/,2287306455,100.00,,
> >
> > Co-developed-by: Jin Yao <[email protected]>
> > Co-developed-by: Arnaldo Carvalho de Melo <[email protected]>
> > Signed-off-by: Jin Yao <[email protected]>
> > Signed-off-by: Kan Liang <[email protected]>
> > Reviewed-by: Andi Kleen <[email protected]>
> > ---
> > v7:
> > - Create 'struct perf_pmu_alias_name' constructor/destructor.
> > - Return '-errno' if opendir() is failed.
>
> Thanks for addressing those, I did a v8 here with the changes described
> in my Committer notes below, please holler if you disagree.

Please take a look at my tmp.perf/core branch, it has the patch below
and the test one plus what I'm about to push to Linus.

- Arnaldo

> commit 13d60ba0738b0532edab3e1492b2005d36ba0802
> Author: Kan Liang <[email protected]>
> Date: Thu Sep 2 14:59:54 2021 +0800
>
> perf pmu: Add PMU alias support
>
> A perf uncore PMU may have two PMU names, a real name and an alias. The
> alias is exported at /sys/bus/event_source/devices/uncore_*/alias.
> The perf tool should support the alias as well.
>
> Add alias_name in the struct perf_pmu to store the alias. For the PMU
> which doesn't have an alias. It's NULL.
>
> Introduce two X86 specific functions to retrieve the real name and the
> alias separately.
>
> Only go through the sysfs to retrieve the mapping between the real name
> and the alias once. The result is cached in a list, uncore_pmu_list.
>
> Nothing changed for the other ARCHs.
>
> With the patch, the perf tool can monitor the PMU with either the real
> name or the alias.
>
> Use the real name,
> $ perf stat -e uncore_cha_2/event=1/ -x,
> 4044879584,,uncore_cha_2/event=1/,2528059205,100.00,,
>
> Use the alias,
> $ perf stat -e uncore_type_0_2/event=1/ -x,
> 3659675336,,uncore_type_0_2/event=1/,2287306455,100.00,,
>
> Committer notes:
>
> Rename 'struct perf_pmu_alias_name' to 'pmu_alias', the 'perf_' prefix
> should be used for libperf, things inside just tools/perf/ are being
> moved away from that prefix.
>
> Also 'pmu_alias' is shorter and reflects the abstraction.
>
> Also don't use 'pmu' as the name for variables for that type, we should
> use that for the 'struct perf_pmu' variables, avoiding confusion. Use
> 'pmu_alias' for 'struct pmu_alias' variables.
>
> Co-developed-by: Jin Yao <[email protected]>
> Co-developed-by: Arnaldo Carvalho de Melo <[email protected]>
> Signed-off-by: Kan Liang <[email protected]>
> Reviewed-by: Andi Kleen <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: John Garry <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Riccardo Mancini <[email protected]>
> Link: http://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Jin Yao <[email protected]>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index d48d608517fd2732..74d69db1ea99df17 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c
> @@ -1,12 +1,30 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <string.h>
> -
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <fcntl.h>
> #include <linux/stddef.h>
> #include <linux/perf_event.h>
> +#include <linux/zalloc.h>
> +#include <api/fs/fs.h>
> +#include <errno.h>
>
> #include "../../../util/intel-pt.h"
> #include "../../../util/intel-bts.h"
> #include "../../../util/pmu.h"
> +#include "../../../util/fncache.h"
> +
> +#define TEMPLATE_ALIAS "%s/bus/event_source/devices/%s/alias"
> +
> +struct pmu_alias {
> + char *name;
> + char *alias;
> + struct list_head list;
> +};
> +
> +static LIST_HEAD(pmu_alias_name_list);
> +static bool cached_list;
>
> struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
> {
> @@ -18,3 +36,138 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
> #endif
> return NULL;
> }
> +
> +static void pmu_alias__delete(struct pmu_alias *pmu_alias)
> +{
> + if (!pmu_alias)
> + return;
> +
> + zfree(&pmu_alias->name);
> + zfree(&pmu_alias->alias);
> + free(pmu_alias);
> +}
> +
> +static struct pmu_alias *pmu_alias__new(char *name, char *alias)
> +{
> + struct pmu_alias *pmu_alias = zalloc(sizeof(*pmu_alias));
> +
> + if (pmu_alias) {
> + pmu_alias->name = strdup(name);
> + if (!pmu_alias->name)
> + goto out_delete;
> +
> + pmu_alias->alias = strdup(alias);
> + if (!pmu_alias->alias)
> + goto out_delete;
> + }
> + return pmu_alias;
> +
> +out_delete:
> + pmu_alias__delete(pmu_alias);
> + return NULL;
> +}
> +
> +static int setup_pmu_alias_list(void)
> +{
> + char path[PATH_MAX];
> + DIR *dir;
> + struct dirent *dent;
> + const char *sysfs = sysfs__mountpoint();
> + struct pmu_alias *pmu_alias;
> + char buf[MAX_PMU_NAME_LEN];
> + FILE *file;
> + int ret = -ENOMEM;
> +
> + if (!sysfs)
> + return -1;
> +
> + snprintf(path, PATH_MAX,
> + "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
> +
> + dir = opendir(path);
> + if (!dir)
> + return -errno;
> +
> + while ((dent = readdir(dir))) {
> + if (!strcmp(dent->d_name, ".") ||
> + !strcmp(dent->d_name, ".."))
> + continue;
> +
> + snprintf(path, PATH_MAX,
> + TEMPLATE_ALIAS, sysfs, dent->d_name);
> +
> + if (!file_available(path))
> + continue;
> +
> + file = fopen(path, "r");
> + if (!file)
> + continue;
> +
> + if (!fgets(buf, sizeof(buf), file)) {
> + fclose(file);
> + continue;
> + }
> +
> + fclose(file);
> +
> + /* Remove the last '\n' */
> + buf[strlen(buf) - 1] = 0;
> +
> + pmu_alias = pmu_alias__new(dent->d_name, buf);
> + if (!pmu_alias)
> + goto close_dir;
> +
> + list_add_tail(&pmu_alias->list, &pmu_alias_name_list);
> + }
> +
> + ret = 0;
> +
> +close_dir:
> + closedir(dir);
> + return ret;
> +}
> +
> +static char *__pmu_find_real_name(const char *name)
> +{
> + struct pmu_alias *pmu_alias;
> +
> + list_for_each_entry(pmu_alias, &pmu_alias_name_list, list) {
> + if (!strcmp(name, pmu_alias->alias))
> + return pmu_alias->name;
> + }
> +
> + return (char *)name;
> +}
> +
> +char *pmu_find_real_name(const char *name)
> +{
> + if (cached_list)
> + return __pmu_find_real_name(name);
> +
> + setup_pmu_alias_list();
> + cached_list = true;
> +
> + return __pmu_find_real_name(name);
> +}
> +
> +static char *__pmu_find_alias_name(const char *name)
> +{
> + struct pmu_alias *pmu_alias;
> +
> + list_for_each_entry(pmu_alias, &pmu_alias_name_list, list) {
> + if (!strcmp(name, pmu_alias->name))
> + return pmu_alias->alias;
> + }
> + return NULL;
> +}
> +
> +char *pmu_find_alias_name(const char *name)
> +{
> + if (cached_list)
> + return __pmu_find_alias_name(name);
> +
> + setup_pmu_alias_list();
> + cached_list = true;
> +
> + return __pmu_find_alias_name(name);
> +}
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 9321bd0e2f76321a..d94e48e1ff9b22dd 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -316,7 +316,8 @@ event_pmu_name opt_pmu_config
> if (!strncmp(name, "uncore_", 7) &&
> strncmp($1, "uncore_", 7))
> name += 7;
> - if (!perf_pmu__match(pattern, name, $1)) {
> + if (!perf_pmu__match(pattern, name, $1) ||
> + !perf_pmu__match(pattern, pmu->alias_name, $1)) {
> if (parse_events_copy_term_list(orig_terms, &terms))
> CLEANUP_YYABORT;
> if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 5f486ccb6fe67b58..bdabd62170d2cf1f 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -945,6 +945,18 @@ perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
> return NULL;
> }
>
> +char * __weak
> +pmu_find_real_name(const char *name)
> +{
> + return (char *)name;
> +}
> +
> +char * __weak
> +pmu_find_alias_name(const char *name __maybe_unused)
> +{
> + return NULL;
> +}
> +
> static int pmu_max_precise(const char *name)
> {
> char path[PATH_MAX];
> @@ -958,13 +970,15 @@ static int pmu_max_precise(const char *name)
> return max_precise;
> }
>
> -static struct perf_pmu *pmu_lookup(const char *name)
> +static struct perf_pmu *pmu_lookup(const char *lookup_name)
> {
> struct perf_pmu *pmu;
> LIST_HEAD(format);
> LIST_HEAD(aliases);
> __u32 type;
> + char *name = pmu_find_real_name(lookup_name);
> bool is_hybrid = perf_pmu__hybrid_mounted(name);
> + char *alias_name;
>
> /*
> * Check pmu name for hybrid and the pmu may be invalid in sysfs
> @@ -995,6 +1009,16 @@ static struct perf_pmu *pmu_lookup(const char *name)
>
> pmu->cpus = pmu_cpumask(name);
> pmu->name = strdup(name);
> + if (!pmu->name)
> + goto err;
> +
> + alias_name = pmu_find_alias_name(name);
> + if (alias_name) {
> + pmu->alias_name = strdup(alias_name);
> + if (!pmu->alias_name)
> + goto err;
> + }
> +
> pmu->type = type;
> pmu->is_uncore = pmu_is_uncore(name);
> if (pmu->is_uncore)
> @@ -1017,15 +1041,22 @@ static struct perf_pmu *pmu_lookup(const char *name)
> pmu->default_config = perf_pmu__get_default_config(pmu);
>
> return pmu;
> +err:
> + if (pmu->name)
> + free(pmu->name);
> + free(pmu);
> + return NULL;
> }
>
> static struct perf_pmu *pmu_find(const char *name)
> {
> struct perf_pmu *pmu;
>
> - list_for_each_entry(pmu, &pmus, list)
> - if (!strcmp(pmu->name, name))
> + list_for_each_entry(pmu, &pmus, list) {
> + if (!strcmp(pmu->name, name) ||
> + (pmu->alias_name && !strcmp(pmu->alias_name, name)))
> return pmu;
> + }
>
> return NULL;
> }
> @@ -1919,6 +1950,9 @@ bool perf_pmu__has_hybrid(void)
>
> int perf_pmu__match(char *pattern, char *name, char *tok)
> {
> + if (!name)
> + return -1;
> +
> if (fnmatch(pattern, name, 0))
> return -1;
>
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 5133bc45603492f7..394898b07fd9874b 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -22,6 +22,7 @@ enum {
> #define PERF_PMU_FORMAT_BITS 64
> #define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
> #define CPUS_TEMPLATE_CPU "%s/bus/event_source/devices/%s/cpus"
> +#define MAX_PMU_NAME_LEN 128
>
> struct perf_event_attr;
>
> @@ -33,6 +34,7 @@ struct perf_pmu_caps {
>
> struct perf_pmu {
> char *name;
> + char *alias_name;
> char *id;
> __u32 type;
> bool selectable;
> @@ -140,4 +142,7 @@ int perf_pmu__match(char *pattern, char *name, char *tok);
> int perf_pmu__cpus_match(struct perf_pmu *pmu, struct perf_cpu_map *cpus,
> struct perf_cpu_map **mcpus_ptr,
> struct perf_cpu_map **ucpus_ptr);
> +
> +char *pmu_find_real_name(const char *name);
> +char *pmu_find_alias_name(const char *name);
> #endif /* __PMU_H */

--

- Arnaldo

2021-09-05 09:13:29

by Jin Yao

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] perf pmu: Add PMU alias support

Hi Arnaldo,

On 9/3/2021 7:38 PM, Arnaldo Carvalho de Melo wrote:
> Committer notes:
>
> Rename 'struct perf_pmu_alias_name' to 'pmu_alias', the 'perf_' prefix
> should be used for libperf, things inside just tools/perf/ are being
> moved away from that prefix.
>
> Also 'pmu_alias' is shorter and reflects the abstraction.
>
> Also don't use 'pmu' as the name for variables for that type, we should
> use that for the 'struct perf_pmu' variables, avoiding confusion. Use
> 'pmu_alias' for 'struct pmu_alias' variables.

I fully agree your recommended changes. I think Kan will agree too. :) Thanks so much!

Thanks
Jin Yao

2021-09-05 09:44:41

by Jin Yao

[permalink] [raw]
Subject: Re: [PATCH v7 1/2] perf pmu: Add PMU alias support

Hi Arnaldo,

On 9/3/2021 7:52 PM, Arnaldo Carvalho de Melo wrote:
> Please take a look at my tmp.perf/core branch, it has the patch below
> and the test one plus what I'm about to push to Linus.
>
> - Arnaldo

I've pulled tmp.perf/core branch and checked the patch. It looks good to me.

Thanks
Jin Yao