Re: [PATCH] perf evsel: Adjust hybrid event and global event mixed group

From: Jin, Yao
Date: Tue Jun 08 2021 - 09:25:01 EST

Next message: Zhou Yanjie: "Re: [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support."
Previous message: Steven Rostedt: "Re: [PATCH] tracing: Export tracing_start() and tracing_stop()"
In reply to: Jiri Olsa: "Re: [PATCH] perf evsel: Adjust hybrid event and global event mixed group"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jiri,

On 6/8/2021 6:56 PM, Jiri Olsa wrote:

On Tue, Jun 01, 2021 at 09:38:27AM +0800, Jin Yao wrote:

A group mixed with hybrid event and global event is allowed. For example,
group leader is 'intel_pt//' and the group member is 'cpu_atom/cycles/'.

e.g.
perf record --aux-sample -e '{intel_pt//,cpu_atom/cycles/}:u'

The challenge is their available cpus are not fully matched. For example,
'intel_pt//' is available on CPU0-CPU23, but 'cpu_atom/cycles/' is
available on CPU16-CPU23.

When getting the group id for group member, we must be very careful.
Because the cpu for 'intel_pt//' is not equal to the cpu for
'cpu_atom/cycles/'. Actually the cpu here is the index of evsel->core.cpus,
not the real CPU ID.

e.g. cpu0 for 'intel_pt//' is CPU0, but cpu0 for 'cpu_atom/cycles/' is CPU16.

Before:

# perf record --aux-sample -e '{intel_pt//,cpu_atom/cycles/}:u' -vv uname
...
------------------------------------------------------------
perf_event_attr:
type 10
size 128
config 0xe601
{ sample_period, sample_freq } 1
sample_type IP|TID|TIME|CPU|IDENTIFIER
read_format ID
disabled 1
inherit 1
exclude_kernel 1
exclude_hv 1
enable_on_exec 1
sample_id_all 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid 4084 cpu 0 group_fd -1 flags 0x8 = 5
sys_perf_event_open: pid 4084 cpu 1 group_fd -1 flags 0x8 = 6
sys_perf_event_open: pid 4084 cpu 2 group_fd -1 flags 0x8 = 7
sys_perf_event_open: pid 4084 cpu 3 group_fd -1 flags 0x8 = 9
sys_perf_event_open: pid 4084 cpu 4 group_fd -1 flags 0x8 = 10
sys_perf_event_open: pid 4084 cpu 5 group_fd -1 flags 0x8 = 11
sys_perf_event_open: pid 4084 cpu 6 group_fd -1 flags 0x8 = 12
sys_perf_event_open: pid 4084 cpu 7 group_fd -1 flags 0x8 = 13
sys_perf_event_open: pid 4084 cpu 8 group_fd -1 flags 0x8 = 14
sys_perf_event_open: pid 4084 cpu 9 group_fd -1 flags 0x8 = 15
sys_perf_event_open: pid 4084 cpu 10 group_fd -1 flags 0x8 = 16
sys_perf_event_open: pid 4084 cpu 11 group_fd -1 flags 0x8 = 17
sys_perf_event_open: pid 4084 cpu 12 group_fd -1 flags 0x8 = 18
sys_perf_event_open: pid 4084 cpu 13 group_fd -1 flags 0x8 = 19
sys_perf_event_open: pid 4084 cpu 14 group_fd -1 flags 0x8 = 20
sys_perf_event_open: pid 4084 cpu 15 group_fd -1 flags 0x8 = 21
sys_perf_event_open: pid 4084 cpu 16 group_fd -1 flags 0x8 = 22
sys_perf_event_open: pid 4084 cpu 17 group_fd -1 flags 0x8 = 23
sys_perf_event_open: pid 4084 cpu 18 group_fd -1 flags 0x8 = 24
sys_perf_event_open: pid 4084 cpu 19 group_fd -1 flags 0x8 = 25
sys_perf_event_open: pid 4084 cpu 20 group_fd -1 flags 0x8 = 26
sys_perf_event_open: pid 4084 cpu 21 group_fd -1 flags 0x8 = 27
sys_perf_event_open: pid 4084 cpu 22 group_fd -1 flags 0x8 = 28
sys_perf_event_open: pid 4084 cpu 23 group_fd -1 flags 0x8 = 29
------------------------------------------------------------
perf_event_attr:
size 128
config 0x800000000
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|PERIOD|IDENTIFIER|AUX
read_format ID
inherit 1
exclude_kernel 1
exclude_hv 1
freq 1
sample_id_all 1
exclude_guest 1
aux_sample_size 4096
------------------------------------------------------------
sys_perf_event_open: pid 4084 cpu 16 group_fd 5 flags 0x8
sys_perf_event_open failed, error -22

The group_fd 5 is not correct. It should be 22 (the fd of
'intel_pt' on CPU16).

After:

# perf record --aux-sample -e '{intel_pt//,cpu_atom/cycles/}:u' -vv uname
...
------------------------------------------------------------
perf_event_attr:
type 10
size 128
config 0xe601
{ sample_period, sample_freq } 1
sample_type IP|TID|TIME|CPU|IDENTIFIER
read_format ID
disabled 1
inherit 1
exclude_kernel 1
exclude_hv 1
enable_on_exec 1
sample_id_all 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid 5162 cpu 0 group_fd -1 flags 0x8 = 5
sys_perf_event_open: pid 5162 cpu 1 group_fd -1 flags 0x8 = 6
sys_perf_event_open: pid 5162 cpu 2 group_fd -1 flags 0x8 = 7
sys_perf_event_open: pid 5162 cpu 3 group_fd -1 flags 0x8 = 9
sys_perf_event_open: pid 5162 cpu 4 group_fd -1 flags 0x8 = 10
sys_perf_event_open: pid 5162 cpu 5 group_fd -1 flags 0x8 = 11
sys_perf_event_open: pid 5162 cpu 6 group_fd -1 flags 0x8 = 12
sys_perf_event_open: pid 5162 cpu 7 group_fd -1 flags 0x8 = 13
sys_perf_event_open: pid 5162 cpu 8 group_fd -1 flags 0x8 = 14
sys_perf_event_open: pid 5162 cpu 9 group_fd -1 flags 0x8 = 15
sys_perf_event_open: pid 5162 cpu 10 group_fd -1 flags 0x8 = 16
sys_perf_event_open: pid 5162 cpu 11 group_fd -1 flags 0x8 = 17
sys_perf_event_open: pid 5162 cpu 12 group_fd -1 flags 0x8 = 18
sys_perf_event_open: pid 5162 cpu 13 group_fd -1 flags 0x8 = 19
sys_perf_event_open: pid 5162 cpu 14 group_fd -1 flags 0x8 = 20
sys_perf_event_open: pid 5162 cpu 15 group_fd -1 flags 0x8 = 21
sys_perf_event_open: pid 5162 cpu 16 group_fd -1 flags 0x8 = 22
sys_perf_event_open: pid 5162 cpu 17 group_fd -1 flags 0x8 = 23
sys_perf_event_open: pid 5162 cpu 18 group_fd -1 flags 0x8 = 24
sys_perf_event_open: pid 5162 cpu 19 group_fd -1 flags 0x8 = 25
sys_perf_event_open: pid 5162 cpu 20 group_fd -1 flags 0x8 = 26
sys_perf_event_open: pid 5162 cpu 21 group_fd -1 flags 0x8 = 27
sys_perf_event_open: pid 5162 cpu 22 group_fd -1 flags 0x8 = 28
sys_perf_event_open: pid 5162 cpu 23 group_fd -1 flags 0x8 = 29
------------------------------------------------------------
perf_event_attr:
size 128
config 0x800000000
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|PERIOD|IDENTIFIER|AUX
read_format ID
inherit 1
exclude_kernel 1
exclude_hv 1
freq 1
sample_id_all 1
exclude_guest 1
aux_sample_size 4096
------------------------------------------------------------
sys_perf_event_open: pid 5162 cpu 16 group_fd 22 flags 0x8 = 30
sys_perf_event_open: pid 5162 cpu 17 group_fd 23 flags 0x8 = 31
sys_perf_event_open: pid 5162 cpu 18 group_fd 24 flags 0x8 = 32
sys_perf_event_open: pid 5162 cpu 19 group_fd 25 flags 0x8 = 33
sys_perf_event_open: pid 5162 cpu 20 group_fd 26 flags 0x8 = 34
sys_perf_event_open: pid 5162 cpu 21 group_fd 27 flags 0x8 = 35
sys_perf_event_open: pid 5162 cpu 22 group_fd 28 flags 0x8 = 36
sys_perf_event_open: pid 5162 cpu 23 group_fd 29 flags 0x8 = 37
------------------------------------------------------------
...

Signed-off-by: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
---
tools/perf/util/evsel.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4a3cd1b5bb33..a9cf615fe580 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1581,6 +1581,15 @@ int __evsel__read_on_cpu(struct evsel *evsel, int cpu, int thread, bool scale)
return 0;
}
+static int evsel_cpuid_match(struct evsel *evsel1, struct evsel *evsel2,
+ int cpu)
+{

could this be better name:

evsel__match_other_cpu(struct evsel *evsel, struct evsel *other, int cpu)

OK. I will use evsel__match_other_cpu in v2.

+ int cpuid;
+
+ cpuid = perf_cpu_map__cpu(evsel1->core.cpus, cpu);
+ return perf_cpu_map__idx(evsel2->core.cpus, cpuid);
+}
+
static int get_group_fd(struct evsel *evsel, int cpu, int thread)
{
struct evsel *leader = evsel->leader;
@@ -1595,6 +1604,26 @@ static int get_group_fd(struct evsel *evsel, int cpu, int thread)
*/
BUG_ON(!leader->core.fd);
+ /*
+ * If leader is global event (e.g. 'intel_pt//'), but member is
+ * hybrid event. Need to get the leader's fd from correct cpu.
+ */
+ if (evsel__is_hybrid(evsel) &&
+ !evsel__is_hybrid(leader)) {
+ cpu = evsel_cpuid_match(evsel, leader, cpu);
+ BUG_ON(cpu == -1);
+ }
+
+ /*
+ * Leader is hybrid event but member is global event.
+ */
+ if (!evsel__is_hybrid(evsel) &&
+ evsel__is_hybrid(leader)) {
+ cpu = evsel_cpuid_match(evsel, leader, cpu);
+ if (cpu == -1)
+ return -1;
+ }

why do we call BUG_ON on the first one and return -1 on the other?
they are equally bad no?

For first one, evsel1 is hybrid event and evsel2 is global event. The evsel1 is only available on part of CPUs and evsel2 is avaialbe on total CPUs.

So 'perf_cpu_map__idx(evsel2->core.cpus, cpuid);' must return a valid cpu index otherwise there should be a bug here.

For second one, evsel1 is global event and evsel2 is hybrid event. The evsel2 is only available on part of CPUs. It's possible that cpuid is not in the range of evsel2->core.cpus. So it's not a bug.

could you put that into separate function, like

cpu = evsel__hybrid_group_fd(evsel, cpu);

Sure, that's OK.

Thanks
Jin Yao

jirka

+
fd = FD(leader, cpu, thread);
BUG_ON(fd == -1);
--
2.17.1

Next message: Zhou Yanjie: "Re: [PATCH 2/2] net: stmmac: Add Ingenic SoCs MAC support."
Previous message: Steven Rostedt: "Re: [PATCH] tracing: Export tracing_start() and tracing_stop()"
In reply to: Jiri Olsa: "Re: [PATCH] perf evsel: Adjust hybrid event and global event mixed group"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]