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

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

[Ian Rogers]

When there are multiple PMUs differing by ordered suffixes only
display one. This avoids repeated listing of events, in particular
when there are 10s of uncore PMUs. This also 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]
```

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

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 PMU suffix in list

 tools/perf/util/pmus.c | 107 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 102 insertions(+), 5 deletions(-)

-- 
2.41.0.390.g38632f3daf-goog

[PATCH v1 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".

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.390.g38632f3daf-goog

[PATCH v1 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, 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.

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

diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 3581710667b0..87e5fb74e121 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;
@@ -432,7 +476,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 
 	pmu = NULL;
 	len = 0;
-	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+	while ((pmu = perf_pmus__scan_skip_duplicates(pmu)) != NULL) {
 		list_for_each_entry(event, &pmu->aliases, list)
 			len++;
 		if (pmu->selectable)
@@ -445,7 +489,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 = perf_pmus__scan_skip_duplicates(pmu)) != NULL) {
 		bool is_cpu = pmu->is_core;
 
 		list_for_each_entry(event, &pmu->aliases, list) {
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v1 3/3] perf pmus: Don't print PMU suffix in list

[Ian Rogers]

Duplicate PMUs are no longer printed but the suffix of the first is
printed. Avoid printing the suffix as multiple PMUs are matched.

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]
```

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

diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 87e5fb74e121..c25b9cb70050 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -443,7 +443,8 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
 			  const struct perf_pmu_alias *alias)
 {
 	struct parse_events_term *term;
-	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
+	int pmu_name_len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
+	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)
@@ -512,6 +513,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;
 
@@ -522,7 +524,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) {
@@ -548,8 +551,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.390.g38632f3daf-goog

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

[John Garry]

On 11/07/2023 06:58, Ian Rogers wrote:
> When there are multiple PMUs differing by ordered suffixes only
> display one. This avoids repeated listing of events, in particular
> when there are 10s of uncore PMUs. This also 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]

So with this change can we run something like:

perf stat -e uncore_imc_free_running/data_read/

?

If so, does that match all PMUs whose name beings with 
"uncore_imc_free_running" (and give aggregate result for those PMUs)?

Thanks,
John

> ```
> 
> The PMUs are sorted by name then suffix as a part of this change.
> 
> 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 PMU suffix in list
> 
>   tools/perf/util/pmus.c | 107 +++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 102 insertions(+), 5 deletions(-)
> 

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

[Ian Rogers]

On Tue, Jul 11, 2023 at 1:26 AM John Garry <john.g.garry@oracle.com> wrote:
>
> On 11/07/2023 06:58, Ian Rogers wrote:
> > When there are multiple PMUs differing by ordered suffixes only
> > display one. This avoids repeated listing of events, in particular
> > when there are 10s of uncore PMUs. This also 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]
>
> So with this change can we run something like:
>
> perf stat -e uncore_imc_free_running/data_read/
>
> ?

It is a long standing behavior of the event parser that we match the
numeric suffixes, so:

```
$ sudo perf stat -e uncore_imc_free_running/data_read/ -a sleep 1

Performance counter stats for 'system wide':

         6,969.93 MiB  uncore_imc_free_running/data_read/

      1.001163027 seconds time elapsed
```

The "uncore_" at the beginning is also optional, I kind of wish the
"free_running" was too. The code doing this is:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.y?h=perf-tools-next#n316
adding a * after the PMU name in:
asprintf(&pattern, "%s*", $1)
Then using fnmatch here:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next#n1707

> If so, does that match all PMUs whose name beings with
> "uncore_imc_free_running" (and give aggregate result for those PMUs)?

Yep. As we're matching with a filename '*' glob then it will actually
potentially grab a bunch more. I think this should likely be made a
lot more precise.

The merging of the counters happens throughout the code, but it is set up here:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next#n559

I didn't write this behavior, it has pre-existed my contributions. I'm
hoping to change the perf list behavior as we're seeing large server
systems with getting on toward 100 PMUs, the events are replicated for
each one and the perf list and testing behaviors are somewhat
exploding in size.

Thanks,
Ian

> Thanks,
> John
>
> > ```
> >
> > The PMUs are sorted by name then suffix as a part of this change.
> >
> > 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 PMU suffix in list
> >
> >   tools/perf/util/pmus.c | 107 +++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 102 insertions(+), 5 deletions(-)
> >
>

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

[John Garry]

>>> ```
>>> $ 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]
>> So with this change can we run something like:
>>
>> perf stat -e uncore_imc_free_running/data_read/
>>
>> ?
> It is a long standing behavior of the event parser that we match the
> numeric suffixes, so:

I guess that I missed this as I assume that it would not handle more 
complex names, like hisi_sccl1_ddr3, which I was then interested in.

> 
> ```
> $ sudo perf stat -e uncore_imc_free_running/data_read/ -a sleep 1
> 
> Performance counter stats for 'system wide':
> 
>           6,969.93 MiB  uncore_imc_free_running/data_read/
> 
>        1.001163027 seconds time elapsed
> ```
> 
> The "uncore_" at the beginning is also optional, I kind of wish the
> "free_running" was too. The code doing this is:
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.y?h=perf-tools-next*n316__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNFJ13LGk$  
> adding a * after the PMU name in:
> asprintf(&pattern, "%s*", $1)
> Then using fnmatch here:
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next*n1707__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNa2_VzYE$  
> 
>> If so, does that match all PMUs whose name beings with
>> "uncore_imc_free_running" (and give aggregate result for those PMUs)?
> Yep. As we're matching with a filename '*' glob then it will actually
> potentially grab a bunch more. I think this should likely be made a
> lot more precise.
> 
> The merging of the counters happens throughout the code, but it is set up here:
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next*n559__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNiVEZvEE$  
> 
> I didn't write this behavior, it has pre-existed my contributions. I'm
> hoping to change the perf list behavior as we're seeing large server
> systems with getting on toward 100 PMUs, the events are replicated for
> each one and the perf list and testing behaviors are somewhat
> exploding in size.

Sure, that is why I was advised PMU kernel drivers event names to be 
unique per PMU, so that we can add an event alias in a JSON and then 
kernel events are matched and removed from perf list.

I suppose that your changes are an alternative to the problem of 
mushrooming kernel event list.

Thanks,
John

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

[Ian Rogers]

On Tue, Jul 11, 2023 at 8:24 AM John Garry <john.g.garry@oracle.com> wrote:
>
>
> >>> ```
> >>> $ 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]
> >> So with this change can we run something like:
> >>
> >> perf stat -e uncore_imc_free_running/data_read/
> >>
> >> ?
> > It is a long standing behavior of the event parser that we match the
> > numeric suffixes, so:
>
> I guess that I missed this as I assume that it would not handle more
> complex names, like hisi_sccl1_ddr3, which I was then interested in.
>
> >
> > ```
> > $ sudo perf stat -e uncore_imc_free_running/data_read/ -a sleep 1
> >
> > Performance counter stats for 'system wide':
> >
> >           6,969.93 MiB  uncore_imc_free_running/data_read/
> >
> >        1.001163027 seconds time elapsed
> > ```
> >
> > The "uncore_" at the beginning is also optional, I kind of wish the
> > "free_running" was too. The code doing this is:
> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.y?h=perf-tools-next*n316__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNFJ13LGk$
> > adding a * after the PMU name in:
> > asprintf(&pattern, "%s*", $1)
> > Then using fnmatch here:
> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmu.c?h=perf-tools-next*n1707__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNa2_VzYE$
> >
> >> If so, does that match all PMUs whose name beings with
> >> "uncore_imc_free_running" (and give aggregate result for those PMUs)?
> > Yep. As we're matching with a filename '*' glob then it will actually
> > potentially grab a bunch more. I think this should likely be made a
> > lot more precise.
> >
> > The merging of the counters happens throughout the code, but it is set up here:
> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat.c?h=perf-tools-next*n559__;Iw!!ACWV5N9M2RV99hQ!JduVayRc--qLXHsoXWTlMUsO4NBUoBnKQHqP2sx7VuwZiZzfVXaQZNBZuzO2Ie-twWQ1xu7nycBNiVEZvEE$
> >
> > I didn't write this behavior, it has pre-existed my contributions. I'm
> > hoping to change the perf list behavior as we're seeing large server
> > systems with getting on toward 100 PMUs, the events are replicated for
> > each one and the perf list and testing behaviors are somewhat
> > exploding in size.
>
> Sure, that is why I was advised PMU kernel drivers event names to be
> unique per PMU, so that we can add an event alias in a JSON and then
> kernel events are matched and removed from perf list.
>
> I suppose that your changes are an alternative to the problem of
> mushrooming kernel event list.

Thanks John, yep this is going after that problem. Could I get a
reviewed/acked/tested-by for these changes?

Thanks,
Ian

> Thanks,
> John

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

[John Garry]

On 01/08/2023 18:39, Ian Rogers wrote:
>>> I didn't write this behavior, it has pre-existed my contributions. I'm
>>> hoping to change the perf list behavior as we're seeing large server
>>> systems with getting on toward 100 PMUs, the events are replicated for
>>> each one and the perf list and testing behaviors are somewhat
>>> exploding in size.
>> Sure, that is why I was advised PMU kernel drivers event names to be
>> unique per PMU, so that we can add an event alias in a JSON and then
>> kernel events are matched and removed from perf list.
>>
>> I suppose that your changes are an alternative to the problem of
>> mushrooming kernel event list.
> Thanks John, yep this is going after that problem. Could I get a
> reviewed/acked/tested-by for these changes?

I'll try to help.

So we have:

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]
```
How about keep listing uncore_imc_free_running_0 and the rest for perf 
list -v? Or something like that.

I find that the perf tool has lots of veiled tricks in terms of usage 
and describing events available and how to use them.

Thanks,
John