[PATCH v2] perf evlist: Fix 'perf record -C xx' failed issue

[PATCH v2] perf evlist: Fix 'perf record -C xx' failed issue

[Chunxin Zang]

The cpu has 8 performance-cores and 8 efficient-cores on my pc.
0-15 are performance-cores
16-23 are 8 efficient-cores

When I run "perf record -C xxx", the command fails if xxx all belong to
performance cores or efficient cores

The results are as follows

  # perf record -C 12
  WARNING: A requested CPU in '12' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
  Error:
  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles:P/).
  /bin/dmesg | grep -i perf may provide additional information.

  # perf record -C 14-17
  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_core' (CPUs 0-15) for event 'cycles:P'
  ^C[ perf record: Woken up 1 times to write data ]

The reason is that the cpu_map of 'cpu_atom'-evsel is empty, causing
the sys_perf_event_open() result to fail.

Changes in v2:
- fix memory leak about 'intersect'

Signed-off-by: Chunxin Zang <spring.cxz@gmail.com>
Reviewed-by: Chen Yang <yangchen11@lixiang.com>
---
 tools/perf/util/evlist.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 55a300a0977b..babbde29341f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -2499,7 +2499,7 @@ void evlist__check_mem_load_aux(struct evlist *evlist)
 void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list)
 {
 	struct perf_cpu_map *user_requested_cpus;
-	struct evsel *pos;
+	struct evsel *pos, *tmp;
 
 	if (!cpu_list)
 		return;
@@ -2508,18 +2508,28 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
 	if (!user_requested_cpus)
 		return;
 
-	evlist__for_each_entry(evlist, pos) {
+	evlist__for_each_entry_safe(evlist, tmp, pos) {
 		struct perf_cpu_map *intersect, *to_test;
 		const struct perf_pmu *pmu = evsel__find_pmu(pos);
 
 		to_test = pmu && pmu->is_core ? pmu->cpus : cpu_map__online();
 		intersect = perf_cpu_map__intersect(to_test, user_requested_cpus);
-		if (!perf_cpu_map__equal(intersect, user_requested_cpus)) {
-			char buf[128];
+		if (!intersect) {
+			evlist__remove(evlist, pos);
+			evsel__delete(pos);
+			perf_cpu_map__put(intersect);
+			continue;
+		}
+
+		if (!perf_cpu_map__is_subset(user_requested_cpus, intersect)) {
+			char buf_test[128];
+			char buf_intersect[128];
 
-			cpu_map__snprint(to_test, buf, sizeof(buf));
-			pr_warning("WARNING: A requested CPU in '%s' is not supported by PMU '%s' (CPUs %s) for event '%s'\n",
-				cpu_list, pmu ? pmu->name : "cpu", buf, evsel__name(pos));
+			cpu_map__snprint(to_test, buf_test, sizeof(buf_test));
+			cpu_map__snprint(intersect, buf_intersect, sizeof(buf_intersect));
+			pr_warning("WARNING: A requested CPU '%s' in '%s' is not supported by "
+				   "PMU '%s' (CPUs %s) for event '%s'\n", buf_intersect, cpu_list,
+				   pmu ? pmu->name : "cpu", buf_test, evsel__name(pos));
 		}
 		perf_cpu_map__put(intersect);
 	}
-- 
2.34.1

Re: [PATCH v2] perf evlist: Fix 'perf record -C xx' failed issue

[Chunxin Zang]

Sorry, I'm a little out of it, maybe because I didn't sleep at noon. This version is deprecated, v1 is correct.

v1: https://lore.kernel.org/linux-kernel/20240319084022.2036296-1-spring.cxz@gmail.com/T/#u

Best wishes
Chunxin


> On Mar 19, 2024, at 17:14, Chunxin Zang <spring.cxz@gmail.com> wrote:
> 
> The cpu has 8 performance-cores and 8 efficient-cores on my pc.
> 0-15 are performance-cores
> 16-23 are 8 efficient-cores
> 
> When I run "perf record -C xxx", the command fails if xxx all belong to
> performance cores or efficient cores
> 
> The results are as follows
> 
>  # perf record -C 12
>  WARNING: A requested CPU in '12' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>  Error:
>  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles:P/).
>  /bin/dmesg | grep -i perf may provide additional information.
> 
>  # perf record -C 14-17
>  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_core' (CPUs 0-15) for event 'cycles:P'
>  ^C[ perf record: Woken up 1 times to write data ]
> 
> The reason is that the cpu_map of 'cpu_atom'-evsel is empty, causing
> the sys_perf_event_open() result to fail.
> 
> Changes in v2:
> - fix memory leak about 'intersect'
> 
> Signed-off-by: Chunxin Zang <spring.cxz@gmail.com>
> Reviewed-by: Chen Yang <yangchen11@lixiang.com>
> ---
> tools/perf/util/evlist.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 55a300a0977b..babbde29341f 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -2499,7 +2499,7 @@ void evlist__check_mem_load_aux(struct evlist *evlist)
> void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list)
> {
> struct perf_cpu_map *user_requested_cpus;
> - struct evsel *pos;
> + struct evsel *pos, *tmp;
> 
> if (!cpu_list)
> return;
> @@ -2508,18 +2508,28 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
> if (!user_requested_cpus)
> return;
> 
> - evlist__for_each_entry(evlist, pos) {
> + evlist__for_each_entry_safe(evlist, tmp, pos) {
> struct perf_cpu_map *intersect, *to_test;
> const struct perf_pmu *pmu = evsel__find_pmu(pos);
> 
> to_test = pmu && pmu->is_core ? pmu->cpus : cpu_map__online();
> intersect = perf_cpu_map__intersect(to_test, user_requested_cpus);
> - if (!perf_cpu_map__equal(intersect, user_requested_cpus)) {
> - char buf[128];
> + if (!intersect) {
> + evlist__remove(evlist, pos);
> + evsel__delete(pos);
> + perf_cpu_map__put(intersect);
> + continue;
> + }
> +
> + if (!perf_cpu_map__is_subset(user_requested_cpus, intersect)) {
> + char buf_test[128];
> + char buf_intersect[128];
> 
> - cpu_map__snprint(to_test, buf, sizeof(buf));
> - pr_warning("WARNING: A requested CPU in '%s' is not supported by PMU '%s' (CPUs %s) for event '%s'\n",
> - cpu_list, pmu ? pmu->name : "cpu", buf, evsel__name(pos));
> + cpu_map__snprint(to_test, buf_test, sizeof(buf_test));
> + cpu_map__snprint(intersect, buf_intersect, sizeof(buf_intersect));
> + pr_warning("WARNING: A requested CPU '%s' in '%s' is not supported by "
> +   "PMU '%s' (CPUs %s) for event '%s'\n", buf_intersect, cpu_list,
> +   pmu ? pmu->name : "cpu", buf_test, evsel__name(pos));
> }
> perf_cpu_map__put(intersect);
> }
> -- 
> 2.34.1
> 

Re: [PATCH v2] perf evlist: Fix 'perf record -C xx' failed issue

[Ian Rogers]

On Tue, Mar 19, 2024 at 2:14 AM Chunxin Zang <spring.cxz@gmail.com> wrote:
>
> The cpu has 8 performance-cores and 8 efficient-cores on my pc.
> 0-15 are performance-cores
> 16-23 are 8 efficient-cores
>
> When I run "perf record -C xxx", the command fails if xxx all belong to
> performance cores or efficient cores
>
> The results are as follows
>
>   # perf record -C 12
>   WARNING: A requested CPU in '12' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>   Error:
>   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles:P/).
>   /bin/dmesg | grep -i perf may provide additional information.
>
>   # perf record -C 14-17
>   WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>   WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_core' (CPUs 0-15) for event 'cycles:P'
>   ^C[ perf record: Woken up 1 times to write data ]
>
> The reason is that the cpu_map of 'cpu_atom'-evsel is empty, causing
> the sys_perf_event_open() result to fail.
>
> Changes in v2:
> - fix memory leak about 'intersect'
>
> Signed-off-by: Chunxin Zang <spring.cxz@gmail.com>
> Reviewed-by: Chen Yang <yangchen11@lixiang.com>
> ---
>  tools/perf/util/evlist.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 55a300a0977b..babbde29341f 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -2499,7 +2499,7 @@ void evlist__check_mem_load_aux(struct evlist *evlist)
>  void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list)
>  {
>         struct perf_cpu_map *user_requested_cpus;
> -       struct evsel *pos;
> +       struct evsel *pos, *tmp;
>
>         if (!cpu_list)
>                 return;
> @@ -2508,18 +2508,28 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
>         if (!user_requested_cpus)
>                 return;
>
> -       evlist__for_each_entry(evlist, pos) {
> +       evlist__for_each_entry_safe(evlist, tmp, pos) {
>                 struct perf_cpu_map *intersect, *to_test;
>                 const struct perf_pmu *pmu = evsel__find_pmu(pos);
>
>                 to_test = pmu && pmu->is_core ? pmu->cpus : cpu_map__online();

Perhaps this would be clearer if we just made sure requested uncore
CPUs were online. Uncore cpu maps are weird, they say the default CPU
but other CPUs are valid. It'd be worth testing the impact of this
change on uncore events.

>                 intersect = perf_cpu_map__intersect(to_test, user_requested_cpus);
> -               if (!perf_cpu_map__equal(intersect, user_requested_cpus)) {
> -                       char buf[128];
> +               if (!intersect) {
> +                       evlist__remove(evlist, pos);
> +                       evsel__delete(pos);
> +                       perf_cpu_map__put(intersect);

evlist__warn_user_requested_cpus seems like a wrong function name if
evsels are being removed. Perhaps something like
evlist__remove_empty_cpu_map_evsels. It seems this change will remove
warnings in cases like:

$ perf record cpu_atom/cycles/ -C 0 ...

I wonder that we need a flag in the evsel to say when an event comes
from wildcard expansion so we only don't warn/remove in that case.

Wrt memory leaks, try compiling with EXTRA_CFLAGS="-fsanitize=address"
which also incorporates leak sanitizer.

Thanks,
Ian

> +                       continue;
> +               }
> +
> +               if (!perf_cpu_map__is_subset(user_requested_cpus, intersect)) {
> +                       char buf_test[128];
> +                       char buf_intersect[128];
>
> -                       cpu_map__snprint(to_test, buf, sizeof(buf));
> -                       pr_warning("WARNING: A requested CPU in '%s' is not supported by PMU '%s' (CPUs %s) for event '%s'\n",
> -                               cpu_list, pmu ? pmu->name : "cpu", buf, evsel__name(pos));
> +                       cpu_map__snprint(to_test, buf_test, sizeof(buf_test));
> +                       cpu_map__snprint(intersect, buf_intersect, sizeof(buf_intersect));
> +                       pr_warning("WARNING: A requested CPU '%s' in '%s' is not supported by "
> +                                  "PMU '%s' (CPUs %s) for event '%s'\n", buf_intersect, cpu_list,
> +                                  pmu ? pmu->name : "cpu", buf_test, evsel__name(pos));
>                 }
>                 perf_cpu_map__put(intersect);
>         }
> --
> 2.34.1
>

Re: [PATCH v2] perf evlist: Fix 'perf record -C xx' failed issue

[Chunxin Zang]

> On Mar 20, 2024, at 04:33, Ian Rogers <irogers@google.com> wrote:
> 
> On Tue, Mar 19, 2024 at 2:14 AM Chunxin Zang <spring.cxz@gmail.com> wrote:
>> 
>> The cpu has 8 performance-cores and 8 efficient-cores on my pc.
>> 0-15 are performance-cores
>> 16-23 are 8 efficient-cores
>> 
>> When I run "perf record -C xxx", the command fails if xxx all belong to
>> performance cores or efficient cores
>> 
>> The results are as follows
>> 
>>  # perf record -C 12
>>  WARNING: A requested CPU in '12' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>>  Error:
>>  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles:P/).
>>  /bin/dmesg | grep -i perf may provide additional information.
>> 
>>  # perf record -C 14-17
>>  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_atom' (CPUs 16-23) for event 'cycles:P'
>>  WARNING: A requested CPU in '14-17' is not supported by PMU 'cpu_core' (CPUs 0-15) for event 'cycles:P'
>>  ^C[ perf record: Woken up 1 times to write data ]
>> 
>> The reason is that the cpu_map of 'cpu_atom'-evsel is empty, causing
>> the sys_perf_event_open() result to fail.
>> 
>> Changes in v2:
>> - fix memory leak about 'intersect'
>> 
>> Signed-off-by: Chunxin Zang <spring.cxz@gmail.com>
>> Reviewed-by: Chen Yang <yangchen11@lixiang.com>
>> ---
>> tools/perf/util/evlist.c | 24 +++++++++++++++++-------
>> 1 file changed, 17 insertions(+), 7 deletions(-)
>> 
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 55a300a0977b..babbde29341f 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -2499,7 +2499,7 @@ void evlist__check_mem_load_aux(struct evlist *evlist)
>> void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_list)
>> {
>>        struct perf_cpu_map *user_requested_cpus;
>> -       struct evsel *pos;
>> +       struct evsel *pos, *tmp;
>> 
>>        if (!cpu_list)
>>                return;
>> @@ -2508,18 +2508,28 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
>>        if (!user_requested_cpus)
>>                return;
>> 
>> -       evlist__for_each_entry(evlist, pos) {
>> +       evlist__for_each_entry_safe(evlist, tmp, pos) {
>>                struct perf_cpu_map *intersect, *to_test;
>>                const struct perf_pmu *pmu = evsel__find_pmu(pos);
>> 
>>                to_test = pmu && pmu->is_core ? pmu->cpus : cpu_map__online();
> 
> Perhaps this would be clearer if we just made sure requested uncore
> CPUs were online. Uncore cpu maps are weird, they say the default CPU
> but other CPUs are valid. It'd be worth testing the impact of this
> change on uncore events.

Hi  Ian,

I tested the "uncore_clock" event and it seems to work fine.
The results are as follows:

   $ ~/perf stat  -e uncore_clock/clockticks/  -C 12


    ^C
     Performance counter stats for 'CPU(s) 12':

           740,036,624      uncore_clock/clockticks/

           4.135909502 seconds time elapsed

> 
>>                intersect = perf_cpu_map__intersect(to_test, user_requested_cpus);
>> -               if (!perf_cpu_map__equal(intersect, user_requested_cpus)) {
>> -                       char buf[128];
>> +               if (!intersect) {
>> +                       evlist__remove(evlist, pos);
>> +                       evsel__delete(pos);
>> +                       perf_cpu_map__put(intersect);
> 
> evlist__warn_user_requested_cpus seems like a wrong function name if
> evsels are being removed. Perhaps something like
> evlist__remove_empty_cpu_map_evsels. It seems this change will remove
> warnings in cases like:
> 
> $ perf record cpu_atom/cycles/ -C 0 ...
> 
Yes ,I will fix it.

> I wonder that we need a flag in the evsel to say when an event comes
> from wildcard expansion so we only don't warn/remove in that case.
> 
That sounds very reasonable. Furthermore, if the event is not from wildcard and it's 
cpu maps is empty, the evsel needs to be deleted and the process should be stoped.
I tried to implement it in v3 patch.

> Wrt memory leaks, try compiling with EXTRA_CFLAGS="-fsanitize=address"
> which also incorporates leak sanitizer.

Got it,  thanks.


Best wishes
Chunxin

> 
> Thanks,
> Ian
> 
>> +                       continue;
>> +               }
>> +
>> +               if (!perf_cpu_map__is_subset(user_requested_cpus, intersect)) {
>> +                       char buf_test[128];
>> +                       char buf_intersect[128];
>> 
>> -                       cpu_map__snprint(to_test, buf, sizeof(buf));
>> -                       pr_warning("WARNING: A requested CPU in '%s' is not supported by PMU '%s' (CPUs %s) for event '%s'\n",
>> -                               cpu_list, pmu ? pmu->name : "cpu", buf, evsel__name(pos));
>> +                       cpu_map__snprint(to_test, buf_test, sizeof(buf_test));
>> +                       cpu_map__snprint(intersect, buf_intersect, sizeof(buf_intersect));
>> +                       pr_warning("WARNING: A requested CPU '%s' in '%s' is not supported by "
>> +                                  "PMU '%s' (CPUs %s) for event '%s'\n", buf_intersect, cpu_list,
>> +                                  pmu ? pmu->name : "cpu", buf_test, evsel__name(pos));
>>                }
>>                perf_cpu_map__put(intersect);
>>        }
>> --
>> 2.34.1
>>