| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 8 Jun 2021 21:24:49 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
mingo@...hat.com, alexander.shishkin@...ux.intel.com,
Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH] perf evsel: Adjust hybrid event and global event mixed
group
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@...ux.intel.com>
>> Reviewed-by: Andi Kleen <ak@...ux.intel.com>
>> ---
>> 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
>>
>
Powered by blists - more mailing lists