1. Patchew
  2. linux
  3. [PATCH 0/7] perf: cs-etm: Fixes around timestamped and timeless decoding
  4. View patch

[PATCH 1/7] perf: cs-etm: Fix timeless decode mode detection

James Clark posted 7 patches 2 years, 7 months ago
[PATCH 1/7] perf: cs-etm: Fix timeless decode mode detection
Posted by James Clark 2 years, 7 months ago 
In this context, timeless refers to the trace data rather than the perf
event data. But when detecting whether there are timestamps in the trace
data or not, the presence of a timestamp flag on any perf event is used.

Since commit f42c0ce573df ("perf record: Always get text_poke events
with --kcore option") timestamps were added to a tracking event when
--kcore is used which breaks this detection mechanism. Fix it by
detecting if trace timestamps exist by looking at the ETM config flags.
This would have always been a more accurate way of doing it anyway.

This fixes the following error message when using --kcore with
Coresight:

  $ perf record --kcore -e cs_etm// --per-thread
  $ perf report
  The perf.data/data data has no samples!

Fixes: f42c0ce573df ("perf record: Always get text_poke events with --kcore option")
Reported-by: Yang Shi <shy828301@gmail.com>
Link: https://lore.kernel.org/lkml/CAHbLzkrJQTrYBtPkf=jf3OpQ-yBcJe7XkvQstX9j2frz4WF-SQ@mail.gmail.com/
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 8dd81ddd9e4e..50593289d53c 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2684,26 +2684,29 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session,
 	return 0;
 }
 
-static bool cs_etm__is_timeless_decoding(struct cs_etm_auxtrace *etm)
+static int cs_etm__setup_timeless_decoding(struct cs_etm_auxtrace *etm)
 {
 	struct evsel *evsel;
 	struct evlist *evlist = etm->session->evlist;
-	bool timeless_decoding = true;
 
 	/* Override timeless mode with user input from --itrace=Z */
-	if (etm->synth_opts.timeless_decoding)
-		return true;
+	if (etm->synth_opts.timeless_decoding) {
+		etm->timeless_decoding = true;
+		return 0;
+	}
 
 	/*
-	 * Circle through the list of event and complain if we find one
-	 * with the time bit set.
+	 * Find the cs_etm evsel and look at what its timestamp setting was
 	 */
-	evlist__for_each_entry(evlist, evsel) {
-		if ((evsel->core.attr.sample_type & PERF_SAMPLE_TIME))
-			timeless_decoding = false;
-	}
+	evlist__for_each_entry(evlist, evsel)
+		if (cs_etm__evsel_is_auxtrace(etm->session, evsel)) {
+			etm->timeless_decoding =
+				!(evsel->core.attr.config & BIT(ETM_OPT_TS));
+			return 0;
+		}
 
-	return timeless_decoding;
+	pr_err("CS ETM: Couldn't find ETM evsel\n");
+	return -EINVAL;
 }
 
 /*
@@ -3155,7 +3158,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	etm->snapshot_mode = (ptr[CS_ETM_SNAPSHOT] != 0);
 	etm->metadata = metadata;
 	etm->auxtrace_type = auxtrace_info->type;
-	etm->timeless_decoding = cs_etm__is_timeless_decoding(etm);
 
 	/* Use virtual timestamps if all ETMs report ts_source = 1 */
 	etm->has_virtual_ts = cs_etm__has_virtual_ts(metadata, num_cpu);
@@ -3172,6 +3174,10 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace;
 	session->auxtrace = &etm->auxtrace;
 
+	err = cs_etm__setup_timeless_decoding(etm);
+	if (err)
+		return err;
+
 	etm->unknown_thread = thread__new(999999999, 999999999);
 	if (!etm->unknown_thread) {
 		err = -ENOMEM;
-- 
2.34.1
Re: [PATCH 1/7] perf: cs-etm: Fix timeless decode mode detection
Posted by Suzuki K Poulose 2 years, 7 months ago 
On 24/04/2023 14:47, James Clark wrote:
> In this context, timeless refers to the trace data rather than the perf
> event data. But when detecting whether there are timestamps in the trace
> data or not, the presence of a timestamp flag on any perf event is used.
> 
> Since commit f42c0ce573df ("perf record: Always get text_poke events
> with --kcore option") timestamps were added to a tracking event when
> --kcore is used which breaks this detection mechanism. Fix it by
> detecting if trace timestamps exist by looking at the ETM config flags.
> This would have always been a more accurate way of doing it anyway.
> 
> This fixes the following error message when using --kcore with
> Coresight:
> 
>    $ perf record --kcore -e cs_etm// --per-thread
>    $ perf report
>    The perf.data/data data has no samples!
> 
> Fixes: f42c0ce573df ("perf record: Always get text_poke events with --kcore option")
> Reported-by: Yang Shi <shy828301@gmail.com>
> Link: https://lore.kernel.org/lkml/CAHbLzkrJQTrYBtPkf=jf3OpQ-yBcJe7XkvQstX9j2frz4WF-SQ@mail.gmail.com/
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>   tools/perf/util/cs-etm.c | 30 ++++++++++++++++++------------
>   1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 8dd81ddd9e4e..50593289d53c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -2684,26 +2684,29 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session,
>   	return 0;
>   }
>   
> -static bool cs_etm__is_timeless_decoding(struct cs_etm_auxtrace *etm)
> +static int cs_etm__setup_timeless_decoding(struct cs_etm_auxtrace *etm)

minor nit: "setup" sound more like prepare to do what is required to
do a timeless decoding, while we are doing more like, check if we
have to do a timeless decoding. So may be:

cs_etm_check_timeless_decoding() ?

>   {
>   	struct evsel *evsel;
>   	struct evlist *evlist = etm->session->evlist;
> -	bool timeless_decoding = true;
>   
>   	/* Override timeless mode with user input from --itrace=Z */
> -	if (etm->synth_opts.timeless_decoding)
> -		return true;
> +	if (etm->synth_opts.timeless_decoding) {
> +		etm->timeless_decoding = true;
> +		return 0;
> +	}
>   
>   	/*
> -	 * Circle through the list of event and complain if we find one
> -	 * with the time bit set.
> +	 * Find the cs_etm evsel and look at what its timestamp setting was
>   	 */
> -	evlist__for_each_entry(evlist, evsel) {
> -		if ((evsel->core.attr.sample_type & PERF_SAMPLE_TIME))
> -			timeless_decoding = false;
> -	}
> +	evlist__for_each_entry(evlist, evsel)

minor nit: please retain the braces

> +		if (cs_etm__evsel_is_auxtrace(etm->session, evsel)) {
> +			etm->timeless_decoding =
> +				!(evsel->core.attr.config & BIT(ETM_OPT_TS));


> +			return 0;
> +		}

Otherwise, looks good to me

Suzuki
Re: [PATCH 1/7] perf: cs-etm: Fix timeless decode mode detection
Posted by Denis Nikitin 2 years, 7 months ago 
On Mon, Apr 24, 2023 at 8:14 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 24/04/2023 14:47, James Clark wrote:
> > In this context, timeless refers to the trace data rather than the perf
> > event data. But when detecting whether there are timestamps in the trace
> > data or not, the presence of a timestamp flag on any perf event is used.
> >
> > Since commit f42c0ce573df ("perf record: Always get text_poke events
> > with --kcore option") timestamps were added to a tracking event when
> > --kcore is used which breaks this detection mechanism. Fix it by
> > detecting if trace timestamps exist by looking at the ETM config flags.
> > This would have always been a more accurate way of doing it anyway.
> >
> > This fixes the following error message when using --kcore with
> > Coresight:
> >
> >    $ perf record --kcore -e cs_etm// --per-thread
> >    $ perf report
> >    The perf.data/data data has no samples!
> >
> > Fixes: f42c0ce573df ("perf record: Always get text_poke events with --kcore option")
> > Reported-by: Yang Shi <shy828301@gmail.com>
> > Link: https://lore.kernel.org/lkml/CAHbLzkrJQTrYBtPkf=jf3OpQ-yBcJe7XkvQstX9j2frz4WF-SQ@mail.gmail.com/
> > Signed-off-by: James Clark <james.clark@arm.com>
> > ---
> >   tools/perf/util/cs-etm.c | 30 ++++++++++++++++++------------
> >   1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 8dd81ddd9e4e..50593289d53c 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -2684,26 +2684,29 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session,
> >       return 0;
> >   }
> >
> > -static bool cs_etm__is_timeless_decoding(struct cs_etm_auxtrace *etm)
> > +static int cs_etm__setup_timeless_decoding(struct cs_etm_auxtrace *etm)
>
> minor nit: "setup" sound more like prepare to do what is required to
> do a timeless decoding, while we are doing more like, check if we
> have to do a timeless decoding. So may be:
>
> cs_etm_check_timeless_decoding() ?
>

I didn't catch that "setup_timeless_decoding" can be treated as the
initialization
of the timeless decoding. On the other hand _check_ doesn't imply that
it changes
the flag.
Maybe "_setup_timeless_decoding_flag" will be more specific about its intention?

- Denis

>
> Otherwise, looks good to me
>
> Suzuki
>
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.