Messages in this thread | | | Date | Wed, 11 May 2022 10:02:13 +0300 | Subject | Re: [PATCH V2 10/23] perf auxtrace: Add mmap_needed to auxtrace_mmap_params | From | Adrian Hunter <> |
| |
On 6/05/22 23:16, Ian Rogers wrote: > On Fri, May 6, 2022 at 5:26 AM Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> Add mmap_needed to auxtrace_mmap_params. >> >> Currently an auxtrace mmap is always attempted even if the event is not an >> auxtrace event. That works because, when AUX area tracing, there is always >> an auxtrace event first for every mmap. Prepare for that not being the >> case, which it won't be when sideband tracking events are allowed on >> all CPUs even when auxtrace is limited to selected CPUs. > > Should there be a later patch to remove this option and just not do > the auxtrace mmap when it's not necessary?
The same code path is used for non-auxtrace mmap as auxtrace mmap. I have amended the comment to explain.
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> tools/perf/util/auxtrace.c | 10 ++++++++-- >> tools/perf/util/auxtrace.h | 8 ++++++-- >> tools/perf/util/evlist.c | 5 +++-- >> tools/perf/util/mmap.c | 1 + >> 4 files changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c >> index b11549ae39df..b446cfa66469 100644 >> --- a/tools/perf/util/auxtrace.c >> +++ b/tools/perf/util/auxtrace.c >> @@ -125,7 +125,7 @@ int auxtrace_mmap__mmap(struct auxtrace_mmap *mm, >> mm->tid = mp->tid; >> mm->cpu = mp->cpu.cpu; >> >> - if (!mp->len) { >> + if (!mp->len || !mp->mmap_needed) { >> mm->base = NULL; >> return 0; >> } >> @@ -168,9 +168,15 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp, >> } >> >> void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp, >> - struct evlist *evlist, int idx, >> + struct evlist *evlist, >> + struct evsel *evsel, int idx, >> bool per_cpu) >> { >> + mp->mmap_needed = evsel->needs_auxtrace_mmap; >> + >> + if (!mp->mmap_needed) >> + return; >> + >> mp->idx = idx; >> >> if (per_cpu) { >> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h >> index dc38b6f57232..37feae003904 100644 >> --- a/tools/perf/util/auxtrace.h >> +++ b/tools/perf/util/auxtrace.h >> @@ -344,6 +344,7 @@ struct auxtrace_mmap { >> * @idx: index of this mmap >> * @tid: tid for a per-thread mmap (also set if there is only 1 tid on a per-cpu >> * mmap) otherwise %0 >> + * @mmap_needed: set to %false for non-auxtrace events > > Could you add something like: > (ie. we don't want the mmap but we do want the side effects of ... > > Thanks, > Ian > >> * @cpu: cpu number for a per-cpu mmap otherwise %-1 >> */ >> struct auxtrace_mmap_params { >> @@ -353,6 +354,7 @@ struct auxtrace_mmap_params { >> int prot; >> int idx; >> pid_t tid; >> + bool mmap_needed; >> struct perf_cpu cpu; >> }; >> >> @@ -490,7 +492,8 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp, >> unsigned int auxtrace_pages, >> bool auxtrace_overwrite); >> void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp, >> - struct evlist *evlist, int idx, >> + struct evlist *evlist, >> + struct evsel *evsel, int idx, >> bool per_cpu); >> >> typedef int (*process_auxtrace_t)(struct perf_tool *tool, >> @@ -863,7 +866,8 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp, >> unsigned int auxtrace_pages, >> bool auxtrace_overwrite); >> void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp, >> - struct evlist *evlist, int idx, >> + struct evlist *evlist, >> + struct evsel *evsel, int idx, >> bool per_cpu); >> >> #define ITRACE_HELP "" >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index 7ae56b062f44..996bdc203616 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -747,15 +747,16 @@ static struct mmap *evlist__alloc_mmap(struct evlist *evlist, >> >> static void >> perf_evlist__mmap_cb_idx(struct perf_evlist *_evlist, >> - struct perf_evsel *_evsel __maybe_unused, >> + struct perf_evsel *_evsel, >> struct perf_mmap_param *_mp, >> int idx) >> { >> struct evlist *evlist = container_of(_evlist, struct evlist, core); >> struct mmap_params *mp = container_of(_mp, struct mmap_params, core); >> bool per_cpu = !perf_cpu_map__empty(_evlist->user_requested_cpus); >> + struct evsel *evsel = container_of(_evsel, struct evsel, core); >> >> - auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, idx, per_cpu); >> + auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, evsel, idx, per_cpu); >> } >> >> static struct perf_mmap* >> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c >> index 50502b4a7ca4..de59c4da852b 100644 >> --- a/tools/perf/util/mmap.c >> +++ b/tools/perf/util/mmap.c >> @@ -62,6 +62,7 @@ void __weak auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp __maybe_u >> >> void __weak auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp __maybe_unused, >> struct evlist *evlist __maybe_unused, >> + struct evsel *evsel __maybe_unused, >> int idx __maybe_unused, >> bool per_cpu __maybe_unused) >> { >> -- >> 2.25.1 >>
| |