Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: James Clark <james.clark@linaro.org>,
	John Garry <john.g.garry@oracle.com>,
	Will Deacon <will@kernel.org>, Mike Leach <mike.leach@arm.com>,
	Leo Yan <leo.yan@linux.dev>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Leo Yan <leo.yan@arm.com>, Suyash Mahar <smahar@meta.com>,
	Amir Ayupov <aaupov@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<coresight@lists.linaro.org>, <linux-perf-users@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/6] perf: cs-etm: Respect --no-itrace option
Date: Mon, 24 Aug 2026 12:53:16 +0300	[thread overview]
Message-ID: <1986a53d-9167-4f7a-8b1b-2c09f962a9e2@intel.com> (raw)
In-Reply-To: <20260821-james-cs-hw_id-output-failure-v1-4-9d532ddabcc3@linaro.org>

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 <james.clark@linaro.org>

For auxtrace:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  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;
>  }
> 



  reply	other threads:[~2026-08-24  9:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:50 [PATCH 0/6] perf: cs-etm: HW_ID improvements James Clark
2026-08-21  9:50 ` [PATCH 1/6] perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues James Clark
2026-08-21  9:50 ` [PATCH 2/6] perf cs-etm: Free partially created queues James Clark
2026-08-21  9:50 ` [PATCH 3/6] perf cs-etm: Synthesize missing HW_ID mappings for raw trace James Clark
2026-08-21  9:50 ` [PATCH 4/6] perf: cs-etm: Respect --no-itrace option James Clark
2026-08-24  9:53   ` Adrian Hunter [this message]
2026-08-21  9:50 ` [PATCH 5/6] perf/core: Return errors from perf_report_aux_output_id() James Clark
2026-08-21  9:50 ` [PATCH 6/6] coresight: perf: Retry failed HW_ID writes James Clark
2026-08-21 10:01   ` James Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1986a53d-9167-4f7a-8b1b-2c09f962a9e2@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=aaupov@fb.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=leo.yan@arm.com \
    --cc=leo.yan@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mike.leach@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=smahar@meta.com \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox