From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: James Clark <james.clark@arm.com>
Cc: coresight@lists.linaro.org, al.grant@arm.com,
branislav.rankov@arm.com, denik@chromium.org,
suzuki.poulose@arm.com, Mike Leach <mike.leach@linaro.org>,
Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] perf cs-etm: Set time on synthesised samples to preserve ordering
Date: Thu, 15 Apr 2021 13:54:16 -0600 [thread overview]
Message-ID: <20210415195416.GB937505@xps15> (raw)
In-Reply-To: <20210414143919.12605-2-james.clark@arm.com>
On Wed, Apr 14, 2021 at 05:39:19PM +0300, James Clark wrote:
> The following attribute is set when synthesising samples in
> timed decoding mode:
>
> attr.sample_type |= PERF_SAMPLE_TIME;
>
> This results in new samples that appear to have timestamps but
> because we don't assign any timestamps to the samples, when the
> resulting inject file is opened again, the synthesised samples
> will be on the wrong side of the MMAP or COMM events.
>
I understand the problem. Once again an issue caused by CS and the kernel
having a different view of time.
> For example this results in the samples being associated with
> the perf binary, rather than the target of the record:
>
> perf record -e cs_etm/@tmc_etr0/u top
> perf inject -i perf.data -o perf.inject --itrace=i100il
> perf report -i perf.inject
>
> Where 'Command' == perf should show as 'top':
>
> # Overhead Command Source Shared Object Source Symbol Target Symbol Basic Block Cycles
> # ........ ....... .................... ...................... ...................... ..................
> #
> 31.08% perf [unknown] [.] 0x000000000040c3f8 [.] 0x000000000040c3e8 -
>
> If the perf.data file is opened directly with perf, without the
> inject step, then this already works correctly because the
> events are synthesised after the COMM and MMAP events and
> no second sorting happens. Re-sorting only happens when opening
> the perf.inject file for the second time so timestamps are
> needed.
>
> Using the timestamp from the AUX record mirrors the current
> behaviour when opening directly with perf, because the events
> are generated on the call to cs_etm__process_queues().
>
> Signed-off-by: James Clark <james.clark@arm.com>
> Co-developed-by: Al Grant <al.grant@arm.com>
> Signed-off-by: Al Grant <al.grant@arm.com>
Suzuki is correct, your name has to appear after Al's.
> ---
> tools/perf/util/cs-etm.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index c25da2ffa8f3..d0fa9dce47f1 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -54,6 +54,7 @@ struct cs_etm_auxtrace {
> u8 sample_instructions;
>
> int num_cpu;
> + u64 latest_kernel_timestamp;
> u32 auxtrace_type;
> u64 branches_sample_type;
> u64 branches_id;
> @@ -1192,6 +1193,8 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
> event->sample.header.misc = cs_etm__cpu_mode(etmq, addr);
> event->sample.header.size = sizeof(struct perf_event_header);
>
> + if (!etm->timeless_decoding)
> + sample.time = etm->latest_kernel_timestamp;
> sample.ip = addr;
> sample.pid = tidq->pid;
> sample.tid = tidq->tid;
> @@ -1248,6 +1251,8 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
> event->sample.header.misc = cs_etm__cpu_mode(etmq, ip);
> event->sample.header.size = sizeof(struct perf_event_header);
>
> + if (!etm->timeless_decoding)
> + sample.time = etm->latest_kernel_timestamp;
> sample.ip = ip;
> sample.pid = tidq->pid;
> sample.tid = tidq->tid;
> @@ -2412,9 +2417,10 @@ static int cs_etm__process_event(struct perf_session *session,
> else if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE)
> return cs_etm__process_switch_cpu_wide(etm, event);
>
> - if (!etm->timeless_decoding &&
> - event->header.type == PERF_RECORD_AUX)
> + if (!etm->timeless_decoding && event->header.type == PERF_RECORD_AUX) {
> + etm->latest_kernel_timestamp = sample_kernel_timestamp;
It will be fun to fix this when 8.4 comes out but for now it's the best we've
got.
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> return cs_etm__process_queues(etm);
> + }
>
> return 0;
> }
> --
> 2.28.0
>
next prev parent reply other threads:[~2021-04-15 19:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 14:39 [PATCH 1/2] perf cs-etm: Refactor timestamp variable names James Clark
2021-04-14 14:39 ` [PATCH 2/2] perf cs-etm: Set time on synthesised samples to preserve ordering James Clark
2021-04-14 14:41 ` James Clark
2021-04-15 12:39 ` Leo Yan
2021-04-15 12:51 ` James Clark
2021-04-15 14:33 ` Leo Yan
2021-04-16 9:55 ` James Clark
2021-04-14 15:54 ` Suzuki K Poulose
2021-04-15 12:30 ` Leo Yan
2021-04-15 19:54 ` Mathieu Poirier [this message]
2021-04-16 10:16 ` James Clark
2021-04-15 19:46 ` [PATCH 1/2] perf cs-etm: Refactor timestamp variable names Mathieu Poirier
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=20210415195416.GB937505@xps15 \
--to=mathieu.poirier@linaro.org \
--cc=al.grant@arm.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=branislav.rankov@arm.com \
--cc=coresight@lists.linaro.org \
--cc=denik@chromium.org \
--cc=james.clark@arm.com \
--cc=john.garry@huawei.com \
--cc=jolsa@redhat.com \
--cc=leo.yan@linaro.org \
--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@linaro.org \
--cc=namhyung@kernel.org \
--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;
as well as URLs for NNTP newsgroup(s).