Re: [PATCH 4/6] perf: cs-etm: Respect --no-itrace option
Adrian Hunter <[email protected]>
| Newsgroups | org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 21/08/2026 12:50, James Clark wrote: > If there is a bug in decoding then Perf will exit early and you can't > investigate by using the raw dump mode. Make the --no-itrace option stop > Coresight decoding after printing the aux header so decode errors don't > stop the rest of the samples being printed. > > pmu_type and evsel_is_auxtrace need to be set so that the > unleader_auxtrace() behavior is the same as without --no-itrace. > > Signed-off-by: James Clark <[email protected]> For auxtrace: Reviewed-by: Adrian Hunter <[email protected]> > --- > tools/perf/util/auxtrace.c | 2 +- > tools/perf/util/auxtrace.h | 1 + > tools/perf/util/cs-etm.c | 66 +++++++++++++++++++++++++--------------------- > 3 files changed, 38 insertions(+), 31 deletions(-) > > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c > index e3c770b46e94..caa53056d349 100644 > --- a/tools/perf/util/auxtrace.c > +++ b/tools/perf/util/auxtrace.c > @@ -123,7 +123,7 @@ static int evlist__regroup(struct evlist *evlist, struct evsel *leader, struct e > return 0; > } > > -static bool auxtrace__dont_decode(struct perf_session *session) > +bool auxtrace__dont_decode(struct perf_session *session) > { > return !session->itrace_synth_opts || > session->itrace_synth_opts->dont_decode; > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h > index 971b817d3396..47fb97136836 100644 > --- a/tools/perf/util/auxtrace.h > +++ b/tools/perf/util/auxtrace.h > @@ -651,6 +651,7 @@ void auxtrace__free(struct perf_session *session); > bool auxtrace__evsel_is_auxtrace(struct perf_session *session, > struct evsel *evsel); > u64 auxtrace_synth_id_range_start(struct evsel *evsel); > +bool auxtrace__dont_decode(struct perf_session *session); > > #define ITRACE_HELP \ > " i[period]: synthesize instructions events\n" \ > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index dd15e91b7b38..22884c9fdff8 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -969,7 +969,7 @@ static int cs_etm__flush_events(struct perf_session *session, > struct cs_etm_auxtrace *etm = container_of(session->auxtrace, > struct cs_etm_auxtrace, > auxtrace); > - if (dump_trace) > + if (dump_trace || auxtrace__dont_decode(session)) > return 0; > > if (!tool->ordered_events) > @@ -3040,7 +3040,7 @@ static int cs_etm__process_event(struct perf_session *session, > struct cs_etm_auxtrace, > auxtrace); > > - if (dump_trace) > + if (dump_trace || auxtrace__dont_decode(session)) > return 0; > > if (!tool->ordered_events) { > @@ -3603,27 +3603,49 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > int err = 0; > int aux_hw_id_found; > int i; > - u64 *ptr = NULL; > + u64 *ptr = (u64 *) auxtrace_info->priv; > u64 **metadata = NULL; > > - /* First the global part */ > - ptr = (u64 *) auxtrace_info->priv; > - num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; > + etm = zalloc(sizeof(*etm)); > + if (!etm) > + return -ENOMEM; > + > + session->auxtrace = &etm->auxtrace; > + etm->auxtrace.free = cs_etm__free; > + etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace; > + etm->auxtrace.process_event = cs_etm__process_event; > + etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event; > + etm->auxtrace.flush_events = cs_etm__flush_events; > + etm->auxtrace.free_events = cs_etm__free_events; > + etm->pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) & 0xffffffff); > + > + /* > + * Don't go further than the minimum required to identify this event as > + * auxtrace with cs_etm__evsel_is_auxtrace() so unleader_auxtrace() > + * works. > + */ > + if (auxtrace__dont_decode(session)) > + return 0; > > /* > * Bound num_cpu by the event size: the global header consumes > * CS_ETM_HEADER_SIZE bytes, and each CPU needs at least one u64 > * metadata entry after that. > */ > + num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; > priv_size = total_size - event_header_size - INFO_HEADER_SIZE - > CS_ETM_HEADER_SIZE; > if (num_cpu <= 0 || priv_size <= 0 || > - num_cpu > priv_size / (int)sizeof(u64)) > - return -EINVAL; > + num_cpu > priv_size / (int)sizeof(u64)) { > + err = -EINVAL; > + goto err_free_etm; > + } > > metadata = zalloc(sizeof(*metadata) * num_cpu); > - if (!metadata) > - return -ENOMEM; > + if (!metadata) { > + err = -ENOMEM; > + goto err_free_etm; > + } > > /* Start parsing after the common part of the header */ > i = CS_HEADER_VERSION_MAX; > @@ -3682,13 +3704,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > goto err_free_metadata; > } > > - etm = zalloc(sizeof(*etm)); > - > - if (!etm) { > - err = -ENOMEM; > - goto err_free_metadata; > - } > - > /* > * As all the ETMs run at the same exception level, the system should > * have the same PID format crossing CPUs. So cache the PID format > @@ -3698,7 +3713,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > > err = auxtrace_queues__init_nr(&etm->queues, max_cpu + 1); > if (err) > - goto err_free_etm; > + goto err_free_metadata; > > for (unsigned int j = 0; j < etm->queues.nr_queues; ++j) { > err = cs_etm__setup_queue(etm, &etm->queues.queue_array[j], j); > @@ -3736,7 +3751,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > etm->session = session; > > etm->num_cpu = num_cpu; > - etm->pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) & 0xffffffff); > etm->snapshot_mode = (ptr[CS_ETM_SNAPSHOT] != 0); > etm->metadata = metadata; > etm->auxtrace_type = auxtrace_info->type; > @@ -3763,14 +3777,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > "you can specify the itrace option 'T' for timestamp decoding\n" > "if the Coresight timestamp on the platform is same with the kernel time.\n\n"); > > - etm->auxtrace.process_event = cs_etm__process_event; > - etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event; > - etm->auxtrace.flush_events = cs_etm__flush_events; > - etm->auxtrace.free_events = cs_etm__free_events; > - etm->auxtrace.free = cs_etm__free; > - etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace; > - session->auxtrace = &etm->auxtrace; > - > cs_etm__setup_timeless_decoding(etm); > > etm->tc.time_shift = tc->time_shift; > @@ -3853,13 +3859,13 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > > err_free_queues: > cs_etm__free_queues(etm); > - session->auxtrace = NULL; > -err_free_etm: > - zfree(&etm); > err_free_metadata: > /* No need to check @metadata[j], free(NULL) is supported */ > for (int j = 0; j < num_cpu; j++) > zfree(&metadata[j]); > zfree(&metadata); > +err_free_etm: > + session->auxtrace = NULL; > + zfree(&etm); > return err; > } >