[PATCH v3 0/3] perf list: Remove duplicate PMUs

[PATCH v3 0/3] perf list: Remove duplicate PMUs

[Ian Rogers]

When there are multiple PMUs, differing by ordered suffixes, by
default only display one. This avoids repeated listing of events, in
particular when there are 10s of uncore PMUs. If "-v" is passed to
"perf list" then still list all PMUs.

Listing fewer PMU/event combinations helps speed the all PMU event
tests.

Before:
```
$ perf list
...
  uncore_imc_free_running_0/data_read/               [Kernel PMU event]
  uncore_imc_free_running_0/data_total/              [Kernel PMU event]
  uncore_imc_free_running_0/data_write/              [Kernel PMU event]
  uncore_imc_free_running_1/data_read/               [Kernel PMU event]
  uncore_imc_free_running_1/data_total/              [Kernel PMU event]
  uncore_imc_free_running_1/data_write/              [Kernel PMU event]
```

After:
```
$ perf list
...
  uncore_imc_free_running/data_read/                 [Kernel PMU event]
  uncore_imc_free_running/data_total/                [Kernel PMU event]
  uncore_imc_free_running/data_write/                [Kernel PMU event]
...
$ perf list -v
...
  uncore_imc_free_running_0/data_read/               [Kernel PMU event]
  uncore_imc_free_running_0/data_total/              [Kernel PMU event]
  uncore_imc_free_running_0/data_write/              [Kernel PMU event]
  uncore_imc_free_running_1/data_read/               [Kernel PMU event]
  uncore_imc_free_running_1/data_total/              [Kernel PMU event]
  uncore_imc_free_running_1/data_write/              [Kernel PMU event]
...
```

The PMUs are sorted by name then suffix as a part of this change.

v3: Add detail to patch 1 sorting commit message about the suffix and
    why sorting is necessary.
v2: List all PMUs when "-v" is passed as suggested by John Garry
    <john.g.garry@oracle.com>.

Ian Rogers (3):
  perf pmus: Sort pmus by name then suffix
  perf pmus: Add scan that ignores duplicates, use for perf list
  perf pmus: Don't print duplicate PMU suffix in list by default

 tools/perf/builtin-list.c      |   8 +++
 tools/perf/util/pmus.c         | 120 +++++++++++++++++++++++++++++++--
 tools/perf/util/print-events.h |   1 +
 3 files changed, 122 insertions(+), 7 deletions(-)

-- 
2.41.0.640.ga95def55d0-goog

[PATCH v3 1/3] perf pmus: Sort pmus by name then suffix

[Ian Rogers]

Sort PMUs by name. If two PMUs have the same name but differ by
suffix, sort the suffixes numerically. For example, "breakpoint" comes
before "cpu", "uncore_imc_free_running_0" comes before
"uncore_imc_free_running_1". Suffixes need to be treated specially as
otherwise they will be ordered like 0, 1, 10, 11, .., 2, 20, 21, ..,
etc. Only PMUs starting 'uncore_' are considered to have a potential
suffix.

Sorting of PMUs is done so that later patches can skip duplicate
uncore PMUs that differ only by there suffix.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmus.c | 48 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index c58ba9fb6a36..3581710667b0 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/list.h>
+#include <linux/list_sort.h>
 #include <linux/zalloc.h>
 #include <subcmd/pager.h>
 #include <sys/types.h>
+#include <ctype.h>
 #include <dirent.h>
 #include <pthread.h>
 #include <string.h>
@@ -33,6 +35,31 @@ static LIST_HEAD(other_pmus);
 static bool read_sysfs_core_pmus;
 static bool read_sysfs_all_pmus;
 
+static int pmu_name_len_no_suffix(const char *str, unsigned long *num)
+{
+	int orig_len, len;
+
+	orig_len = len = strlen(str);
+
+	/* Non-uncore PMUs have their full length, for example, i915. */
+	if (strncmp(str, "uncore_", 7))
+		return len;
+
+	/*
+	 * Count trailing digits and '_', if '_{num}' suffix isn't present use
+	 * the full length.
+	 */
+	while (len > 0 && isdigit(str[len - 1]))
+		len--;
+
+	if (len > 0 && len != orig_len && str[len - 1] == '_') {
+		if (num)
+			*num = strtoul(&str[len], NULL, 10);
+		return len - 1;
+	}
+	return orig_len;
+}
+
 void perf_pmus__destroy(void)
 {
 	struct perf_pmu *pmu, *tmp;
@@ -122,6 +149,25 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
 	return perf_pmu__lookup(core_pmu ? &core_pmus : &other_pmus, dirfd, name);
 }
 
+static int pmus_cmp(void *priv __maybe_unused,
+		    const struct list_head *lhs, const struct list_head *rhs)
+{
+	unsigned long lhs_num, rhs_num;
+	struct perf_pmu *lhs_pmu = container_of(lhs, struct perf_pmu, list);
+	struct perf_pmu *rhs_pmu = container_of(rhs, struct perf_pmu, list);
+	const char *lhs_pmu_name = lhs_pmu->name ?: "";
+	const char *rhs_pmu_name = rhs_pmu->name ?: "";
+	int lhs_pmu_name_len = pmu_name_len_no_suffix(lhs_pmu_name, &lhs_num);
+	int rhs_pmu_name_len = pmu_name_len_no_suffix(rhs_pmu_name, &rhs_num);
+	int ret = strncmp(lhs_pmu_name, rhs_pmu_name,
+			lhs_pmu_name_len < rhs_pmu_name_len ? lhs_pmu_name_len : rhs_pmu_name_len);
+
+	if (lhs_pmu_name_len != rhs_pmu_name_len || ret != 0 || lhs_pmu_name_len == 0)
+		return ret;
+
+	return lhs_num < rhs_num ? -1 : (lhs_num > rhs_num ? 1 : 0);
+}
+
 /* Add all pmus in sysfs to pmu list: */
 static void pmu_read_sysfs(bool core_only)
 {
@@ -156,6 +202,8 @@ static void pmu_read_sysfs(bool core_only)
 		if (!perf_pmu__create_placeholder_core_pmu(&core_pmus))
 			pr_err("Failure to set up any core PMUs\n");
 	}
+	list_sort(NULL, &core_pmus, pmus_cmp);
+	list_sort(NULL, &other_pmus, pmus_cmp);
 	if (!list_empty(&core_pmus)) {
 		read_sysfs_core_pmus = true;
 		if (!core_only)
-- 
2.41.0.640.ga95def55d0-goog

[PATCH v3 2/3] perf pmus: Add scan that ignores duplicates, use for perf list

[Ian Rogers]

When there are multiple PMUs that differ only by suffix, by default
just list the first one and skip all others. As the PMUs are sorted,
the scan routine checks that the PMU names match and the numbers are
consecutive. If "-v" is passed to "perf list" then list all PMUs.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-list.c      |  8 +++++
 tools/perf/util/pmus.c         | 54 ++++++++++++++++++++++++++++++++--
 tools/perf/util/print-events.h |  1 +
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 7fec2cca759f..8fe4ddf02c14 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
 	strbuf_release(&buf);
 }
 
+static bool default_skip_duplicate_pmus(void *ps)
+{
+	struct print_state *print_state = ps;
+
+	return !print_state->long_desc;
+}
+
 int cmd_list(int argc, const char **argv)
 {
 	int i, ret = 0;
@@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
 		.print_end = default_print_end,
 		.print_event = default_print_event,
 		.print_metric = default_print_metric,
+		.skip_duplicate_pmus = default_skip_duplicate_pmus,
 	};
 	const char *cputype = NULL;
 	const char *unit_name = NULL;
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 3581710667b0..5073843aca19 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -275,6 +275,50 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
 	return NULL;
 }
 
+static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
+{
+	bool use_core_pmus = !pmu || pmu->is_core;
+	int last_pmu_name_len = 0;
+	unsigned long last_pmu_num = 0;
+	const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
+
+	if (!pmu) {
+		pmu_read_sysfs(/*core_only=*/false);
+		pmu = list_prepare_entry(pmu, &core_pmus, list);
+	} else
+		last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
+
+	if (use_core_pmus) {
+		list_for_each_entry_continue(pmu, &core_pmus, list) {
+			unsigned long pmu_num = 0;
+			int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
+
+			if (last_pmu_name_len == pmu_name_len &&
+			    (last_pmu_num + 1 == pmu_num) &&
+			    !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
+				last_pmu_num++;
+				continue;
+			}
+			return pmu;
+		}
+		pmu = NULL;
+		pmu = list_prepare_entry(pmu, &other_pmus, list);
+	}
+	list_for_each_entry_continue(pmu, &other_pmus, list) {
+		unsigned long pmu_num = 0;
+		int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
+
+		if (last_pmu_name_len == pmu_name_len &&
+		    (last_pmu_num + 1 == pmu_num) &&
+		    !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
+			last_pmu_num++;
+			continue;
+		}
+		return pmu;
+	}
+	return NULL;
+}
+
 const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
 {
 	struct perf_pmu *pmu = NULL;
@@ -429,10 +473,16 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 	int printed = 0;
 	int len, j;
 	struct sevent *aliases;
+	struct perf_pmu *(*scan_fn)(struct perf_pmu *);
+
+	if (print_cb->skip_duplicate_pmus(print_state))
+		scan_fn = perf_pmus__scan_skip_duplicates;
+	else
+		scan_fn = perf_pmus__scan;
 
 	pmu = NULL;
 	len = 0;
-	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+	while ((pmu = scan_fn(pmu)) != NULL) {
 		list_for_each_entry(event, &pmu->aliases, list)
 			len++;
 		if (pmu->selectable)
@@ -445,7 +495,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 	}
 	pmu = NULL;
 	j = 0;
-	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+	while ((pmu = scan_fn(pmu)) != NULL) {
 		bool is_cpu = pmu->is_core;
 
 		list_for_each_entry(event, &pmu->aliases, list) {
diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
index d7fab411e75c..bf4290bef0cd 100644
--- a/tools/perf/util/print-events.h
+++ b/tools/perf/util/print-events.h
@@ -26,6 +26,7 @@ struct print_callbacks {
 			const char *expr,
 			const char *threshold,
 			const char *unit);
+	bool (*skip_duplicate_pmus)(void *print_state);
 };
 
 /** Print all events, the default when no options are specified. */
-- 
2.41.0.640.ga95def55d0-goog

[PATCH v3 3/3] perf pmus: Don't print duplicate PMU suffix in list by default

[Ian Rogers]

Duplicate PMUs are no longer printed by default but the suffix of the
first is printed. When duplicate PMUs are being skipped avoid printing
the suffix.

Before:
```
$ perf list
...
  uncore_imc_free_running_0/data_read/               [Kernel PMU event]
  uncore_imc_free_running_0/data_total/              [Kernel PMU event]
  uncore_imc_free_running_0/data_write/              [Kernel PMU event]
```

After:
```
$ perf list
...
  uncore_imc_free_running/data_read/                 [Kernel PMU event]
  uncore_imc_free_running/data_total/                [Kernel PMU event]
  uncore_imc_free_running/data_write/                [Kernel PMU event]
...
$ perf list -v
  uncore_imc_free_running_0/data_read/               [Kernel PMU event]
  uncore_imc_free_running_0/data_total/              [Kernel PMU event]
  uncore_imc_free_running_0/data_write/              [Kernel PMU event]
  uncore_imc_free_running_1/data_read/               [Kernel PMU event]
  uncore_imc_free_running_1/data_total/              [Kernel PMU event]
  uncore_imc_free_running_1/data_write/              [Kernel PMU event]
...
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmus.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 5073843aca19..b0ecb2e5bdcc 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -440,10 +440,13 @@ static int sub_non_neg(int a, int b)
 }
 
 static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
-			  const struct perf_pmu_alias *alias)
+			  const struct perf_pmu_alias *alias, bool skip_duplicate_pmus)
 {
 	struct parse_events_term *term;
-	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
+	int pmu_name_len = skip_duplicate_pmus
+		? pmu_name_len_no_suffix(pmu->name, /*num=*/NULL)
+		: (int)strlen(pmu->name);
+	int used = snprintf(buf, len, "%.*s/%s", pmu_name_len, pmu->name, alias->name);
 
 	list_for_each_entry(term, &alias->terms, list) {
 		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
@@ -473,9 +476,10 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 	int printed = 0;
 	int len, j;
 	struct sevent *aliases;
+	bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
 	struct perf_pmu *(*scan_fn)(struct perf_pmu *);
 
-	if (print_cb->skip_duplicate_pmus(print_state))
+	if (skip_duplicate_pmus)
 		scan_fn = perf_pmus__scan_skip_duplicates;
 	else
 		scan_fn = perf_pmus__scan;
@@ -518,6 +522,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 			*desc = NULL, *long_desc = NULL,
 			*encoding_desc = NULL, *topic = NULL,
 			*pmu_name = NULL;
+		int pmu_name_len;
 		bool deprecated = false;
 		size_t buf_used;
 
@@ -528,7 +533,8 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 		if (!aliases[j].event) {
 			/* A selectable event. */
 			pmu_name = aliases[j].pmu->name;
-			buf_used = snprintf(buf, sizeof(buf), "%s//", pmu_name) + 1;
+			pmu_name_len = pmu_name_len_no_suffix(pmu_name, /*num=*/NULL);
+			buf_used = snprintf(buf, sizeof(buf), "%.*s//", pmu_name_len, pmu_name) + 1;
 			name = buf;
 		} else {
 			if (aliases[j].event->desc) {
@@ -536,7 +542,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 				buf_used = 0;
 			} else {
 				name = format_alias(buf, sizeof(buf), aliases[j].pmu,
-						    aliases[j].event);
+						    aliases[j].event, skip_duplicate_pmus);
 				if (aliases[j].is_cpu) {
 					alias = name;
 					name = aliases[j].event->name;
@@ -554,8 +560,10 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 			long_desc = aliases[j].event->long_desc;
 			topic = aliases[j].event->topic;
 			encoding_desc = buf + buf_used;
+			pmu_name_len = pmu_name_len_no_suffix(pmu_name, /*num=*/NULL);
 			buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
-					"%s/%s/", pmu_name, aliases[j].event->str) + 1;
+					"%.*s/%s/", pmu_name_len, pmu_name,
+					aliases[j].event->str) + 1;
 			deprecated = aliases[j].event->deprecated;
 		}
 		print_cb->print_event(print_state,
-- 
2.41.0.640.ga95def55d0-goog

Re: [PATCH v3 0/3] perf list: Remove duplicate PMUs

[Ian Rogers]

On Mon, Aug 14, 2023 at 9:38 AM Ian Rogers <irogers@google.com> wrote:
>
> When there are multiple PMUs, differing by ordered suffixes, by
> default only display one. This avoids repeated listing of events, in
> particular when there are 10s of uncore PMUs. If "-v" is passed to
> "perf list" then still list all PMUs.
>
> Listing fewer PMU/event combinations helps speed the all PMU event
> tests.
>
> Before:
> ```
> $ perf list
> ...
>   uncore_imc_free_running_0/data_read/               [Kernel PMU event]
>   uncore_imc_free_running_0/data_total/              [Kernel PMU event]
>   uncore_imc_free_running_0/data_write/              [Kernel PMU event]
>   uncore_imc_free_running_1/data_read/               [Kernel PMU event]
>   uncore_imc_free_running_1/data_total/              [Kernel PMU event]
>   uncore_imc_free_running_1/data_write/              [Kernel PMU event]
> ```
>
> After:
> ```
> $ perf list
> ...
>   uncore_imc_free_running/data_read/                 [Kernel PMU event]
>   uncore_imc_free_running/data_total/                [Kernel PMU event]
>   uncore_imc_free_running/data_write/                [Kernel PMU event]
> ...
> $ perf list -v
> ...
>   uncore_imc_free_running_0/data_read/               [Kernel PMU event]
>   uncore_imc_free_running_0/data_total/              [Kernel PMU event]
>   uncore_imc_free_running_0/data_write/              [Kernel PMU event]
>   uncore_imc_free_running_1/data_read/               [Kernel PMU event]
>   uncore_imc_free_running_1/data_total/              [Kernel PMU event]
>   uncore_imc_free_running_1/data_write/              [Kernel PMU event]
> ...
> ```
>
> The PMUs are sorted by name then suffix as a part of this change.
>
> v3: Add detail to patch 1 sorting commit message about the suffix and
>     why sorting is necessary.
> v2: List all PMUs when "-v" is passed as suggested by John Garry
>     <john.g.garry@oracle.com>.
>
> Ian Rogers (3):
>   perf pmus: Sort pmus by name then suffix
>   perf pmus: Add scan that ignores duplicates, use for perf list
>   perf pmus: Don't print duplicate PMU suffix in list by default

Ping. Thanks,
Ian

>  tools/perf/builtin-list.c      |   8 +++
>  tools/perf/util/pmus.c         | 120 +++++++++++++++++++++++++++++++--
>  tools/perf/util/print-events.h |   1 +
>  3 files changed, 122 insertions(+), 7 deletions(-)
>
> --
> 2.41.0.640.ga95def55d0-goog
>

Re: [PATCH v3 2/3] perf pmus: Add scan that ignores duplicates, use for perf list

[Liang, Kan]

On 2023-08-14 12:37 p.m., Ian Rogers wrote:
> When there are multiple PMUs that differ only by suffix, by default
> just list the first one and skip all others. As the PMUs are sorted,
> the scan routine checks that the PMU names match and the numbers are
> consecutive. 

The suffix number may not be consecutive, especially for SPR and later
platforms. Because the IDs are from the discovery table now, which is
assigned by the HW. The physic IDs are not guaranteed to be consecutive.

I don't think there is a plan to change it to logical IDs. Because
sometimes people want to know the physic IDs. So they can locate the
specific unit quickly.

Thanks,
Kan

> If "-v" is passed to "perf list" then list all PMUs.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-list.c      |  8 +++++
>  tools/perf/util/pmus.c         | 54 ++++++++++++++++++++++++++++++++--
>  tools/perf/util/print-events.h |  1 +
>  3 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 7fec2cca759f..8fe4ddf02c14 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
>  	strbuf_release(&buf);
>  }
>  
> +static bool default_skip_duplicate_pmus(void *ps)
> +{
> +	struct print_state *print_state = ps;
> +
> +	return !print_state->long_desc;
> +}
> +
>  int cmd_list(int argc, const char **argv)
>  {
>  	int i, ret = 0;
> @@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
>  		.print_end = default_print_end,
>  		.print_event = default_print_event,
>  		.print_metric = default_print_metric,
> +		.skip_duplicate_pmus = default_skip_duplicate_pmus,
>  	};
>  	const char *cputype = NULL;
>  	const char *unit_name = NULL;
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 3581710667b0..5073843aca19 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -275,6 +275,50 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
>  	return NULL;
>  }
>  
> +static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> +{
> +	bool use_core_pmus = !pmu || pmu->is_core;
> +	int last_pmu_name_len = 0;
> +	unsigned long last_pmu_num = 0;
> +	const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
> +
> +	if (!pmu) {
> +		pmu_read_sysfs(/*core_only=*/false);
> +		pmu = list_prepare_entry(pmu, &core_pmus, list);
> +	} else
> +		last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
> +
> +	if (use_core_pmus) {
> +		list_for_each_entry_continue(pmu, &core_pmus, list) {
> +			unsigned long pmu_num = 0;
> +			int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> +
> +			if (last_pmu_name_len == pmu_name_len &&
> +			    (last_pmu_num + 1 == pmu_num) &&
> +			    !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> +				last_pmu_num++;
> +				continue;
> +			}
> +			return pmu;
> +		}
> +		pmu = NULL;
> +		pmu = list_prepare_entry(pmu, &other_pmus, list);
> +	}
> +	list_for_each_entry_continue(pmu, &other_pmus, list) {
> +		unsigned long pmu_num = 0;
> +		int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> +
> +		if (last_pmu_name_len == pmu_name_len &&
> +		    (last_pmu_num + 1 == pmu_num) &&
> +		    !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> +			last_pmu_num++;
> +			continue;
> +		}
> +		return pmu;
> +	}
> +	return NULL;
> +}
> +
>  const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
>  {
>  	struct perf_pmu *pmu = NULL;
> @@ -429,10 +473,16 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>  	int printed = 0;
>  	int len, j;
>  	struct sevent *aliases;
> +	struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> +
> +	if (print_cb->skip_duplicate_pmus(print_state))
> +		scan_fn = perf_pmus__scan_skip_duplicates;
> +	else
> +		scan_fn = perf_pmus__scan;
>  
>  	pmu = NULL;
>  	len = 0;
> -	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> +	while ((pmu = scan_fn(pmu)) != NULL) {
>  		list_for_each_entry(event, &pmu->aliases, list)
>  			len++;
>  		if (pmu->selectable)
> @@ -445,7 +495,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>  	}
>  	pmu = NULL;
>  	j = 0;
> -	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> +	while ((pmu = scan_fn(pmu)) != NULL) {
>  		bool is_cpu = pmu->is_core;
>  
>  		list_for_each_entry(event, &pmu->aliases, list) {
> diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
> index d7fab411e75c..bf4290bef0cd 100644
> --- a/tools/perf/util/print-events.h
> +++ b/tools/perf/util/print-events.h
> @@ -26,6 +26,7 @@ struct print_callbacks {
>  			const char *expr,
>  			const char *threshold,
>  			const char *unit);
> +	bool (*skip_duplicate_pmus)(void *print_state);
>  };
>  
>  /** Print all events, the default when no options are specified. */

Re: [PATCH v3 2/3] perf pmus: Add scan that ignores duplicates, use for perf list

[Ian Rogers]

On Thu, Aug 24, 2023 at 7:01 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2023-08-14 12:37 p.m., Ian Rogers wrote:
> > When there are multiple PMUs that differ only by suffix, by default
> > just list the first one and skip all others. As the PMUs are sorted,
> > the scan routine checks that the PMU names match and the numbers are
> > consecutive.
>
> The suffix number may not be consecutive, especially for SPR and later
> platforms. Because the IDs are from the discovery table now, which is
> assigned by the HW. The physic IDs are not guaranteed to be consecutive.
>
> I don't think there is a plan to change it to logical IDs. Because
> sometimes people want to know the physic IDs. So they can locate the
> specific unit quickly.

Thanks Kan,

I think this could lead to perf list merging some PMUs into one name
and not doing this for others. We could keep the existing behavior by
checking the numbers are consecutive but it'd come with some
complexity and runtime cost. We could just ignore the consecutive
property. We could just not try to solve the problem. What do you
think is the right strategy?

On other architectures they encode these numbers in different places
but generally with no underscore, so this change has no impact for
them. I'm keen to solve this problem as we're seeing large numbers of
PMUs that cause perf list to be spammy and the all PMU events test to
run for too long.

Thanks,
Ian

> Thanks,
> Kan
>
> > If "-v" is passed to "perf list" then list all PMUs.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/builtin-list.c      |  8 +++++
> >  tools/perf/util/pmus.c         | 54 ++++++++++++++++++++++++++++++++--
> >  tools/perf/util/print-events.h |  1 +
> >  3 files changed, 61 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> > index 7fec2cca759f..8fe4ddf02c14 100644
> > --- a/tools/perf/builtin-list.c
> > +++ b/tools/perf/builtin-list.c
> > @@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> >       strbuf_release(&buf);
> >  }
> >
> > +static bool default_skip_duplicate_pmus(void *ps)
> > +{
> > +     struct print_state *print_state = ps;
> > +
> > +     return !print_state->long_desc;
> > +}
> > +
> >  int cmd_list(int argc, const char **argv)
> >  {
> >       int i, ret = 0;
> > @@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
> >               .print_end = default_print_end,
> >               .print_event = default_print_event,
> >               .print_metric = default_print_metric,
> > +             .skip_duplicate_pmus = default_skip_duplicate_pmus,
> >       };
> >       const char *cputype = NULL;
> >       const char *unit_name = NULL;
> > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> > index 3581710667b0..5073843aca19 100644
> > --- a/tools/perf/util/pmus.c
> > +++ b/tools/perf/util/pmus.c
> > @@ -275,6 +275,50 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
> >       return NULL;
> >  }
> >
> > +static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> > +{
> > +     bool use_core_pmus = !pmu || pmu->is_core;
> > +     int last_pmu_name_len = 0;
> > +     unsigned long last_pmu_num = 0;
> > +     const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
> > +
> > +     if (!pmu) {
> > +             pmu_read_sysfs(/*core_only=*/false);
> > +             pmu = list_prepare_entry(pmu, &core_pmus, list);
> > +     } else
> > +             last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
> > +
> > +     if (use_core_pmus) {
> > +             list_for_each_entry_continue(pmu, &core_pmus, list) {
> > +                     unsigned long pmu_num = 0;
> > +                     int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> > +
> > +                     if (last_pmu_name_len == pmu_name_len &&
> > +                         (last_pmu_num + 1 == pmu_num) &&
> > +                         !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> > +                             last_pmu_num++;
> > +                             continue;
> > +                     }
> > +                     return pmu;
> > +             }
> > +             pmu = NULL;
> > +             pmu = list_prepare_entry(pmu, &other_pmus, list);
> > +     }
> > +     list_for_each_entry_continue(pmu, &other_pmus, list) {
> > +             unsigned long pmu_num = 0;
> > +             int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> > +
> > +             if (last_pmu_name_len == pmu_name_len &&
> > +                 (last_pmu_num + 1 == pmu_num) &&
> > +                 !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> > +                     last_pmu_num++;
> > +                     continue;
> > +             }
> > +             return pmu;
> > +     }
> > +     return NULL;
> > +}
> > +
> >  const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
> >  {
> >       struct perf_pmu *pmu = NULL;
> > @@ -429,10 +473,16 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> >       int printed = 0;
> >       int len, j;
> >       struct sevent *aliases;
> > +     struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> > +
> > +     if (print_cb->skip_duplicate_pmus(print_state))
> > +             scan_fn = perf_pmus__scan_skip_duplicates;
> > +     else
> > +             scan_fn = perf_pmus__scan;
> >
> >       pmu = NULL;
> >       len = 0;
> > -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> > +     while ((pmu = scan_fn(pmu)) != NULL) {
> >               list_for_each_entry(event, &pmu->aliases, list)
> >                       len++;
> >               if (pmu->selectable)
> > @@ -445,7 +495,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> >       }
> >       pmu = NULL;
> >       j = 0;
> > -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> > +     while ((pmu = scan_fn(pmu)) != NULL) {
> >               bool is_cpu = pmu->is_core;
> >
> >               list_for_each_entry(event, &pmu->aliases, list) {
> > diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
> > index d7fab411e75c..bf4290bef0cd 100644
> > --- a/tools/perf/util/print-events.h
> > +++ b/tools/perf/util/print-events.h
> > @@ -26,6 +26,7 @@ struct print_callbacks {
> >                       const char *expr,
> >                       const char *threshold,
> >                       const char *unit);
> > +     bool (*skip_duplicate_pmus)(void *print_state);
> >  };
> >
> >  /** Print all events, the default when no options are specified. */

Re: [PATCH v3 2/3] perf pmus: Add scan that ignores duplicates, use for perf list

[Liang, Kan]

On 2023-08-24 1:30 p.m., Ian Rogers wrote:
> On Thu, Aug 24, 2023 at 7:01 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>>
>>
>> On 2023-08-14 12:37 p.m., Ian Rogers wrote:
>>> When there are multiple PMUs that differ only by suffix, by default
>>> just list the first one and skip all others. As the PMUs are sorted,
>>> the scan routine checks that the PMU names match and the numbers are
>>> consecutive.
>>
>> The suffix number may not be consecutive, especially for SPR and later
>> platforms. Because the IDs are from the discovery table now, which is
>> assigned by the HW. The physic IDs are not guaranteed to be consecutive.
>>
>> I don't think there is a plan to change it to logical IDs. Because
>> sometimes people want to know the physic IDs. So they can locate the
>> specific unit quickly.
> 
> Thanks Kan,
> 
> I think this could lead to perf list merging some PMUs into one name
> and not doing this for others. We could keep the existing behavior by
> checking the numbers are consecutive but it'd come with some
> complexity and runtime cost. We could just ignore the consecutive
> property. We could just not try to solve the problem. What do you
> think is the right strategy?

I like the idea of merging the duplicate PMUs. My only concern is that
the assumption of the consecutive may not work for all the uncore cases.
If the IDs are 0,2,4,6, they cannot be merged successfully, right?

Can we just ignore the consecutive check?
Is there a problem if we just simply remove the "(last_pmu_num + 1 ==
pmu_num) &&", and only compare the no_suffix name?

Thanks,
Kan

> 
> On other architectures they encode these numbers in different places
> but generally with no underscore, so this change has no impact for
> them. I'm keen to solve this problem as we're seeing large numbers of
> PMUs that cause perf list to be spammy and the all PMU events test to
> run for too long.>
> Thanks,
> Ian
> 
>> Thanks,
>> Kan
>>
>>> If "-v" is passed to "perf list" then list all PMUs.
>>>
>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>> ---
>>>  tools/perf/builtin-list.c      |  8 +++++
>>>  tools/perf/util/pmus.c         | 54 ++++++++++++++++++++++++++++++++--
>>>  tools/perf/util/print-events.h |  1 +
>>>  3 files changed, 61 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
>>> index 7fec2cca759f..8fe4ddf02c14 100644
>>> --- a/tools/perf/builtin-list.c
>>> +++ b/tools/perf/builtin-list.c
>>> @@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
>>>       strbuf_release(&buf);
>>>  }
>>>
>>> +static bool default_skip_duplicate_pmus(void *ps)
>>> +{
>>> +     struct print_state *print_state = ps;
>>> +
>>> +     return !print_state->long_desc;
>>> +}
>>> +
>>>  int cmd_list(int argc, const char **argv)
>>>  {
>>>       int i, ret = 0;
>>> @@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
>>>               .print_end = default_print_end,
>>>               .print_event = default_print_event,
>>>               .print_metric = default_print_metric,
>>> +             .skip_duplicate_pmus = default_skip_duplicate_pmus,
>>>       };
>>>       const char *cputype = NULL;
>>>       const char *unit_name = NULL;
>>> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
>>> index 3581710667b0..5073843aca19 100644
>>> --- a/tools/perf/util/pmus.c
>>> +++ b/tools/perf/util/pmus.c
>>> @@ -275,6 +275,50 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
>>>       return NULL;
>>>  }
>>>
>>> +static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
>>> +{
>>> +     bool use_core_pmus = !pmu || pmu->is_core;
>>> +     int last_pmu_name_len = 0;
>>> +     unsigned long last_pmu_num = 0;
>>> +     const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
>>> +
>>> +     if (!pmu) {
>>> +             pmu_read_sysfs(/*core_only=*/false);
>>> +             pmu = list_prepare_entry(pmu, &core_pmus, list);
>>> +     } else
>>> +             last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
>>> +
>>> +     if (use_core_pmus) {
>>> +             list_for_each_entry_continue(pmu, &core_pmus, list) {
>>> +                     unsigned long pmu_num = 0;
>>> +                     int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
>>> +
>>> +                     if (last_pmu_name_len == pmu_name_len &&
>>> +                         (last_pmu_num + 1 == pmu_num) &&
>>> +                         !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
>>> +                             last_pmu_num++;
>>> +                             continue;
>>> +                     }
>>> +                     return pmu;
>>> +             }
>>> +             pmu = NULL;
>>> +             pmu = list_prepare_entry(pmu, &other_pmus, list);
>>> +     }
>>> +     list_for_each_entry_continue(pmu, &other_pmus, list) {
>>> +             unsigned long pmu_num = 0;
>>> +             int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
>>> +
>>> +             if (last_pmu_name_len == pmu_name_len &&
>>> +                 (last_pmu_num + 1 == pmu_num) &&
>>> +                 !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
>>> +                     last_pmu_num++;
>>> +                     continue;
>>> +             }
>>> +             return pmu;
>>> +     }
>>> +     return NULL;
>>> +}
>>> +
>>>  const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
>>>  {
>>>       struct perf_pmu *pmu = NULL;
>>> @@ -429,10 +473,16 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>>>       int printed = 0;
>>>       int len, j;
>>>       struct sevent *aliases;
>>> +     struct perf_pmu *(*scan_fn)(struct perf_pmu *);
>>> +
>>> +     if (print_cb->skip_duplicate_pmus(print_state))
>>> +             scan_fn = perf_pmus__scan_skip_duplicates;
>>> +     else
>>> +             scan_fn = perf_pmus__scan;
>>>
>>>       pmu = NULL;
>>>       len = 0;
>>> -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
>>> +     while ((pmu = scan_fn(pmu)) != NULL) {
>>>               list_for_each_entry(event, &pmu->aliases, list)
>>>                       len++;
>>>               if (pmu->selectable)
>>> @@ -445,7 +495,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>>>       }
>>>       pmu = NULL;
>>>       j = 0;
>>> -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
>>> +     while ((pmu = scan_fn(pmu)) != NULL) {
>>>               bool is_cpu = pmu->is_core;
>>>
>>>               list_for_each_entry(event, &pmu->aliases, list) {
>>> diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
>>> index d7fab411e75c..bf4290bef0cd 100644
>>> --- a/tools/perf/util/print-events.h
>>> +++ b/tools/perf/util/print-events.h
>>> @@ -26,6 +26,7 @@ struct print_callbacks {
>>>                       const char *expr,
>>>                       const char *threshold,
>>>                       const char *unit);
>>> +     bool (*skip_duplicate_pmus)(void *print_state);
>>>  };
>>>
>>>  /** Print all events, the default when no options are specified. */

Re: [PATCH v3 2/3] perf pmus: Add scan that ignores duplicates, use for perf list

[Ian Rogers]

On Thu, Aug 24, 2023 at 12:44 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2023-08-24 1:30 p.m., Ian Rogers wrote:
> > On Thu, Aug 24, 2023 at 7:01 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >>
> >>
> >>
> >> On 2023-08-14 12:37 p.m., Ian Rogers wrote:
> >>> When there are multiple PMUs that differ only by suffix, by default
> >>> just list the first one and skip all others. As the PMUs are sorted,
> >>> the scan routine checks that the PMU names match and the numbers are
> >>> consecutive.
> >>
> >> The suffix number may not be consecutive, especially for SPR and later
> >> platforms. Because the IDs are from the discovery table now, which is
> >> assigned by the HW. The physic IDs are not guaranteed to be consecutive.
> >>
> >> I don't think there is a plan to change it to logical IDs. Because
> >> sometimes people want to know the physic IDs. So they can locate the
> >> specific unit quickly.
> >
> > Thanks Kan,
> >
> > I think this could lead to perf list merging some PMUs into one name
> > and not doing this for others. We could keep the existing behavior by
> > checking the numbers are consecutive but it'd come with some
> > complexity and runtime cost. We could just ignore the consecutive
> > property. We could just not try to solve the problem. What do you
> > think is the right strategy?
>
> I like the idea of merging the duplicate PMUs. My only concern is that
> the assumption of the consecutive may not work for all the uncore cases.
> If the IDs are 0,2,4,6, they cannot be merged successfully, right?
>
> Can we just ignore the consecutive check?
> Is there a problem if we just simply remove the "(last_pmu_num + 1 ==
> pmu_num) &&", and only compare the no_suffix name?

Sgtm. I'll update the patch to do this in v4.

Thanks,
Ian

> Thanks,
> Kan
>
> >
> > On other architectures they encode these numbers in different places
> > but generally with no underscore, so this change has no impact for
> > them. I'm keen to solve this problem as we're seeing large numbers of
> > PMUs that cause perf list to be spammy and the all PMU events test to
> > run for too long.>
> > Thanks,
> > Ian
> >
> >> Thanks,
> >> Kan
> >>
> >>> If "-v" is passed to "perf list" then list all PMUs.
> >>>
> >>> Signed-off-by: Ian Rogers <irogers@google.com>
> >>> ---
> >>>  tools/perf/builtin-list.c      |  8 +++++
> >>>  tools/perf/util/pmus.c         | 54 ++++++++++++++++++++++++++++++++--
> >>>  tools/perf/util/print-events.h |  1 +
> >>>  3 files changed, 61 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> >>> index 7fec2cca759f..8fe4ddf02c14 100644
> >>> --- a/tools/perf/builtin-list.c
> >>> +++ b/tools/perf/builtin-list.c
> >>> @@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> >>>       strbuf_release(&buf);
> >>>  }
> >>>
> >>> +static bool default_skip_duplicate_pmus(void *ps)
> >>> +{
> >>> +     struct print_state *print_state = ps;
> >>> +
> >>> +     return !print_state->long_desc;
> >>> +}
> >>> +
> >>>  int cmd_list(int argc, const char **argv)
> >>>  {
> >>>       int i, ret = 0;
> >>> @@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
> >>>               .print_end = default_print_end,
> >>>               .print_event = default_print_event,
> >>>               .print_metric = default_print_metric,
> >>> +             .skip_duplicate_pmus = default_skip_duplicate_pmus,
> >>>       };
> >>>       const char *cputype = NULL;
> >>>       const char *unit_name = NULL;
> >>> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> >>> index 3581710667b0..5073843aca19 100644
> >>> --- a/tools/perf/util/pmus.c
> >>> +++ b/tools/perf/util/pmus.c
> >>> @@ -275,6 +275,50 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
> >>>       return NULL;
> >>>  }
> >>>
> >>> +static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> >>> +{
> >>> +     bool use_core_pmus = !pmu || pmu->is_core;
> >>> +     int last_pmu_name_len = 0;
> >>> +     unsigned long last_pmu_num = 0;
> >>> +     const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
> >>> +
> >>> +     if (!pmu) {
> >>> +             pmu_read_sysfs(/*core_only=*/false);
> >>> +             pmu = list_prepare_entry(pmu, &core_pmus, list);
> >>> +     } else
> >>> +             last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
> >>> +
> >>> +     if (use_core_pmus) {
> >>> +             list_for_each_entry_continue(pmu, &core_pmus, list) {
> >>> +                     unsigned long pmu_num = 0;
> >>> +                     int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> >>> +
> >>> +                     if (last_pmu_name_len == pmu_name_len &&
> >>> +                         (last_pmu_num + 1 == pmu_num) &&
> >>> +                         !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> >>> +                             last_pmu_num++;
> >>> +                             continue;
> >>> +                     }
> >>> +                     return pmu;
> >>> +             }
> >>> +             pmu = NULL;
> >>> +             pmu = list_prepare_entry(pmu, &other_pmus, list);
> >>> +     }
> >>> +     list_for_each_entry_continue(pmu, &other_pmus, list) {
> >>> +             unsigned long pmu_num = 0;
> >>> +             int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &pmu_num);
> >>> +
> >>> +             if (last_pmu_name_len == pmu_name_len &&
> >>> +                 (last_pmu_num + 1 == pmu_num) &&
> >>> +                 !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> >>> +                     last_pmu_num++;
> >>> +                     continue;
> >>> +             }
> >>> +             return pmu;
> >>> +     }
> >>> +     return NULL;
> >>> +}
> >>> +
> >>>  const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
> >>>  {
> >>>       struct perf_pmu *pmu = NULL;
> >>> @@ -429,10 +473,16 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> >>>       int printed = 0;
> >>>       int len, j;
> >>>       struct sevent *aliases;
> >>> +     struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> >>> +
> >>> +     if (print_cb->skip_duplicate_pmus(print_state))
> >>> +             scan_fn = perf_pmus__scan_skip_duplicates;
> >>> +     else
> >>> +             scan_fn = perf_pmus__scan;
> >>>
> >>>       pmu = NULL;
> >>>       len = 0;
> >>> -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> >>> +     while ((pmu = scan_fn(pmu)) != NULL) {
> >>>               list_for_each_entry(event, &pmu->aliases, list)
> >>>                       len++;
> >>>               if (pmu->selectable)
> >>> @@ -445,7 +495,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> >>>       }
> >>>       pmu = NULL;
> >>>       j = 0;
> >>> -     while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> >>> +     while ((pmu = scan_fn(pmu)) != NULL) {
> >>>               bool is_cpu = pmu->is_core;
> >>>
> >>>               list_for_each_entry(event, &pmu->aliases, list) {
> >>> diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
> >>> index d7fab411e75c..bf4290bef0cd 100644
> >>> --- a/tools/perf/util/print-events.h
> >>> +++ b/tools/perf/util/print-events.h
> >>> @@ -26,6 +26,7 @@ struct print_callbacks {
> >>>                       const char *expr,
> >>>                       const char *threshold,
> >>>                       const char *unit);
> >>> +     bool (*skip_duplicate_pmus)(void *print_state);
> >>>  };
> >>>
> >>>  /** Print all events, the default when no options are specified. */