| 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: Wed, 12 Jul 2023 22:44:22 +0800
From: Yang Jihong <yangjihong1@...wei.com>
To: Adrian Hunter <adrian.hunter@...el.com>, <peterz@...radead.org>,
<mingo@...hat.com>, <acme@...nel.org>, <mark.rutland@....com>,
<alexander.shishkin@...ux.intel.com>, <jolsa@...nel.org>,
<namhyung@...nel.org>, <irogers@...gle.com>,
<kan.liang@...ux.intel.com>, <linux-kernel@...r.kernel.org>,
<linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH 3/5] perf record: Tracking side-band events for all CPUs
when tracing selected CPUs
Hello,
On 2023/7/11 21:13, Adrian Hunter wrote:
> On 4/07/23 10:42, Yang Jihong wrote:
>> User space tasks can migrate between CPUs, we need to track side-band
>> events for all CPUs.
>>
>> The specific scenarios are as follows:
>>
>> CPU0 CPU1
>> perf record -C 0 start
>> taskA starts to be created and executed
>> -> PERF_RECORD_COMM and PERF_RECORD_MMAP
>> events only deliver to CPU1
>> ......
>> |
>> migrate to CPU0
>> |
>> Running on CPU0 <----------/
>> ...
>>
>> perf record -C 0 stop
>>
>> Now perf samples the PC of taskA. However, perf does not record the
>> PERF_RECORD_COMM and PERF_RECORD_COMM events of taskA.
>> Therefore, the comm and symbols of taskA cannot be parsed.
>>
>> The sys_perf_event_open invoked is as follows:
>>
>> # perf --debug verbose=3 record -e cpu-clock -C 1 true
>> <SNIP>
>> Opening: cpu-clock
>> ------------------------------------------------------------
>> perf_event_attr:
>> type 1
>> size 136
>> { sample_period, sample_freq } 4000
>> sample_type IP|TID|TIME|ID|CPU|PERIOD
>> read_format ID|LOST
>> disabled 1
>> inherit 1
>> freq 1
>> sample_id_all 1
>> exclude_guest 1
>> ------------------------------------------------------------
>> sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 5
>> Opening: dummy:HG
>> ------------------------------------------------------------
>> perf_event_attr:
>> type 1
>> size 136
>> config 0x9
>> { sample_period, sample_freq } 4000
>> sample_type IP|TID|TIME|ID|CPU|PERIOD
>> read_format ID|LOST
>> inherit 1
>> mmap 1
>> comm 1
>> freq 1
>> task 1
>> sample_id_all 1
>> mmap2 1
>> comm_exec 1
>> ksymbol 1
>> bpf_event 1
>> ------------------------------------------------------------
>> sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 6
>> sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 7
>> sys_perf_event_open: pid -1 cpu 2 group_fd -1 flags 0x8 = 9
>> sys_perf_event_open: pid -1 cpu 3 group_fd -1 flags 0x8 = 10
>> sys_perf_event_open: pid -1 cpu 4 group_fd -1 flags 0x8 = 11
>> sys_perf_event_open: pid -1 cpu 5 group_fd -1 flags 0x8 = 12
>> sys_perf_event_open: pid -1 cpu 6 group_fd -1 flags 0x8 = 13
>> sys_perf_event_open: pid -1 cpu 7 group_fd -1 flags 0x8 = 14
>> <SNIP>
>>
>> Signed-off-by: Yang Jihong <yangjihong1@...wei.com>
>> ---
>> tools/perf/builtin-record.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 8872cd037f2c..69e0d8c75aab 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -908,6 +908,31 @@ static int record__config_off_cpu(struct record *rec)
>> return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>> }
>>
>> +static int record__config_tracking_events(struct record *rec)
>> +{
>> + struct evsel *evsel;
>> + struct evlist *evlist = rec->evlist;
>> + struct record_opts *opts = &rec->opts;
>> +
>> + /*
>> + * User space tasks can migrate between CPUs, so when tracing
>> + * selected CPUs, sideband for all CPUs is still needed.
>> + */
>> + if (opts->target.cpu_list) {
>
> I am not sure if anyone minds doing this by default, but perhaps
> we should say something about it on the perf record man page.
>
Okay, will add comments to the man page.
>> + evsel = evlist__findnew_tracking_event(evlist);
>> + if (!evsel)
>> + return -ENOMEM;
>> +
>> + if (!evsel->core.system_wide) {
>> + evsel->core.system_wide = true;
>> + evsel__set_sample_bit(evsel, TIME);
>> + perf_evlist__propagate_maps(&evlist->core, &evsel->core);
>> + }
>
> Perhaps better to export via internel/evsel.h
>
> void perf_evsel__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel)
> {
> if (!evsel->system_wide) {
> evsel->system_wide = true;
> if (evlist->needs_map_propagation)
> __perf_evlist__propagate_maps(evlist, evsel);
> }
> }
>
> As suggested in response to patch 2, perhaps deal with system_wide
> inside evlist__findnew_tracking_event()
>
Okay, I'll modify it as above, so maybe we need to export
perf_evlist__propagate_maps().
As mentioned in the patch 1, __perf_evlist__propagate_maps is low-level
and avoid to export it.
Or can we export perf_evsel__go_system_wide() via through internel/evlist.h?
In this way, we do not need to export perf_evlist__propagate_maps().
If so, would it be more appropriate to call perf_evlist__go_system_wide()?
Thanks,
Yang
Powered by blists - more mailing lists