Re: [PATCH v6 3/7] perf record: Move setting dummy tracking before record__init_thread_masks()
From: Ian Rogers
Date: Fri Aug 25 2023 - 01:18:31 EST
On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@xxxxxxxxxx> wrote:
>
> User space tasks can migrate between CPUs, so when tracing selected CPUs,
> sideband for all CPUs is needed. In this case set the cpu map of the evsel
> to all online CPUs. This may modify the original cpu map of the evlist.
> Therefore, need to check whether the preceding scenario exists before
> record__init_thread_masks().
> Dummy tracking has been set in record__open(), move it before
> record__init_thread_masks() and add a helper for unified processing.
I have a hard time understanding this commit message. Let me have a go:
In order to set cpu maps correctly on dummy events they need to set up
later. Factor out the dummy event initialization from record__open and
move it to its own function record__setup_dummy_event. Move the call
to the record__setup_dummy to before the call to
record__init_thread_masks in cmd_record.
> The sys_perf_event_open invoked is as follows:
>
> # perf --debug verbose=3 record -e cpu-clock -D 100 true
> <SNIP>
> Opening: cpu-clock
> ------------------------------------------------------------
> perf_event_attr:
> type 1 (PERF_TYPE_SOFTWARE)
> size 136
> config 0 (PERF_COUNT_SW_CPU_CLOCK)
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|PERIOD|IDENTIFIER
> read_format ID|LOST
> disabled 1
> inherit 1
> freq 1
> sample_id_all 1
> exclude_guest 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 10318 cpu 0 group_fd -1 flags 0x8 = 5
> sys_perf_event_open: pid 10318 cpu 1 group_fd -1 flags 0x8 = 6
> sys_perf_event_open: pid 10318 cpu 2 group_fd -1 flags 0x8 = 7
> sys_perf_event_open: pid 10318 cpu 3 group_fd -1 flags 0x8 = 9
> sys_perf_event_open: pid 10318 cpu 4 group_fd -1 flags 0x8 = 10
> sys_perf_event_open: pid 10318 cpu 5 group_fd -1 flags 0x8 = 11
> sys_perf_event_open: pid 10318 cpu 6 group_fd -1 flags 0x8 = 12
> sys_perf_event_open: pid 10318 cpu 7 group_fd -1 flags 0x8 = 13
> Opening: dummy:u
> ------------------------------------------------------------
> perf_event_attr:
> type 1 (PERF_TYPE_SOFTWARE)
> size 136
> config 0x9 (PERF_COUNT_SW_DUMMY)
> { sample_period, sample_freq } 1
> sample_type IP|TID|TIME|IDENTIFIER
> read_format ID|LOST
> disabled 1
> inherit 1
> exclude_kernel 1
> exclude_hv 1
> mmap 1
> comm 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> exclude_guest 1
> mmap2 1
> comm_exec 1
> ksymbol 1
> bpf_event 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 10318 cpu 0 group_fd -1 flags 0x8 = 14
> sys_perf_event_open: pid 10318 cpu 1 group_fd -1 flags 0x8 = 15
> sys_perf_event_open: pid 10318 cpu 2 group_fd -1 flags 0x8 = 16
> sys_perf_event_open: pid 10318 cpu 3 group_fd -1 flags 0x8 = 17
> sys_perf_event_open: pid 10318 cpu 4 group_fd -1 flags 0x8 = 18
> sys_perf_event_open: pid 10318 cpu 5 group_fd -1 flags 0x8 = 19
> sys_perf_event_open: pid 10318 cpu 6 group_fd -1 flags 0x8 = 20
> sys_perf_event_open: pid 10318 cpu 7 group_fd -1 flags 0x8 = 21
> <SNIP>
>
> Signed-off-by: Yang Jihong <yangjihong1@xxxxxxxxxx>
> Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
> tools/perf/builtin-record.c | 59 +++++++++++++++++++++++--------------
> 1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 12edad8392cc..4ee94058028f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -906,6 +906,37 @@ 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 record_opts *opts = &rec->opts;
> + struct evlist *evlist = rec->evlist;
> + struct evsel *evsel;
> +
> + /*
> + * For initial_delay, system wide or a hybrid system, we need to add a
> + * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
This code is explicitly talking about dummy events but the function is
now renamed to call it a tracking event. I think the code should be
consistent.
> + * of waiting or event synthesis.
> + */
> + if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
> + perf_pmus__num_core_pmus() > 1) {
> + evsel = evlist__findnew_tracking_event(evlist, false);
> + if (!evsel)
> + return -ENOMEM;
> +
> + /*
> + * Enable the dummy event when the process is forked for
Dummy again.
Thanks,
Ian
> + * initial_delay, immediately for system wide.
> + */
> + if (opts->target.initial_delay && !evsel->immediate &&
> + !target__has_cpu(&opts->target))
> + evsel->core.attr.enable_on_exec = 1;
> + else
> + evsel->immediate = 1;
> + }
> +
> + return 0;
> +}
> +
> static bool record__kcore_readable(struct machine *machine)
> {
> char kcore[PATH_MAX];
> @@ -1286,28 +1317,6 @@ static int record__open(struct record *rec)
> struct record_opts *opts = &rec->opts;
> int rc = 0;
>
> - /*
> - * For initial_delay, system wide or a hybrid system, we need to add a
> - * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
> - * of waiting or event synthesis.
> - */
> - if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
> - perf_pmus__num_core_pmus() > 1) {
> - pos = evlist__findnew_tracking_event(evlist, false);
> - if (!pos)
> - return -ENOMEM;
> -
> - /*
> - * Enable the dummy event when the process is forked for
> - * initial_delay, immediately for system wide.
> - */
> - if (opts->target.initial_delay && !pos->immediate &&
> - !target__has_cpu(&opts->target))
> - pos->core.attr.enable_on_exec = 1;
> - else
> - pos->immediate = 1;
> - }
> -
> evlist__config(evlist, opts, &callchain_param);
>
> evlist__for_each_entry(evlist, pos) {
> @@ -4190,6 +4199,12 @@ int cmd_record(int argc, const char **argv)
> goto out;
> }
>
> + err = record__config_tracking_events(rec);
> + if (err) {
> + pr_err("record__config_tracking_events failed, error %d\n", err);
> + goto out;
> + }
> +
> err = record__init_thread_masks(rec);
> if (err) {
> pr_err("Failed to initialize parallel data streaming masks\n");
> --
> 2.30.GIT
>