| 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
| ||
|
Message-ID: <1d99287a-e76d-9beb-f795-da5e34ab10fe@intel.com>
Date: Wed, 12 Jul 2023 18:03:56 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Yang Jihong <yangjihong1@...wei.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
On 12/07/23 17:44, Yang Jihong wrote:
> 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?
Yes
> 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()?
Sure
>
> Thanks,
> Yang
Powered by blists - more mailing lists