From: Leo Yan <leo.yan@linaro.org> To: Denis Nikitin <denik@google.com> Cc: James Clark <james.clark@arm.com>, coresight@lists.linaro.org, Mathieu Poirier <mathieu.poirier@linaro.org>, Al Grant <al.grant@arm.com>, Branislav Rankov <branislav.rankov@arm.com>, Denis Nikitin <denik@chromium.org>, Suzuki Poulose <suzuki.poulose@arm.com>, anshuman.khandual@arm.com, Mike Leach <mike.leach@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: [RFC PATCH] perf cs-etm: Handle valid-but-zero timestamps Date: Tue, 11 May 2021 16:26:31 +0800 [thread overview] Message-ID: <20210511082631.GD8273@leoy-ThinkPad-X240s> (raw) In-Reply-To: <CAOYpmdFYs8=FzpiA-mMNZ=L8B9GRXLfQuEnMeDsvWeeKg2PWtA@mail.gmail.com> Hi Denis, On Tue, May 11, 2021 at 01:06:03AM -0700, Denis Nikitin wrote: > Hi Leo, > > > Just remind, as Mike has mentioned that if the timestamp is zero, it > > means the hardware setting for timestamp is not enabled properly. So > > for system wide or per CPU mode tracing, it's better to double check > > what's the reason the timestamp is not enabled properly. > > The bug is confirmed by HW verification. Yeah. > > IIUC, this patch breaks the existed rational in the code. Let's think > > about there have 4 CPUs, every CPU has its own AUX trace buffer, and > > when decode the trace data, it will use 4 queues to track the packets > > and every queue has its timestamp. > > > > CPU0: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU1: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU2: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU3: cs_etm_queue -> ... -> packet_queue->timestamp > > > > The issue is if all CPUs' timestamp are zero, it's impossible to find > > a way to synthesize samples in the right time order. > > Is it really impossible or it just can lead to incorrect decoding? Thanks for correcting. Just clarifying: with this change, perf can decode and synthesize samples, but the sequence of samples is not time-based ordering. > I verified the profiles generated with zero timestamps and this patch > on Trogdor (8 CPU cores) and I don't see any significant difference > in the quality of the AutoFDO profiles. > > If mixed packets don't cause errors in reconstructing the branches > but instead mess up with their timeline then it probably won't matter > for AutoFDO which just collects statistics of the branches. > What do you think? Understand. CoreSight trace data can be used for two purposes: tracing and profiling. For AutoFDO profiling, it's okay for with zero timestamps based on your conclusion; on the other hand, the change can introduce confusion if any user wants to use CoreSight for tracing and review the program flow (like use "perf script") command. The change in this patch is valid for me, but it's better to connect with a new option (like "--force-aux-ts-zero" mentioned in my another replying), this can allow users to explictly to force AUX trace timestamp as zero (or in other word, users can use this option to ignore AUX timestamp if the timestamp is not reliable). Thanks, Leo
WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan@linaro.org> To: Denis Nikitin <denik@google.com> Cc: James Clark <james.clark@arm.com>, coresight@lists.linaro.org, Mathieu Poirier <mathieu.poirier@linaro.org>, Al Grant <al.grant@arm.com>, Branislav Rankov <branislav.rankov@arm.com>, Denis Nikitin <denik@chromium.org>, Suzuki Poulose <suzuki.poulose@arm.com>, anshuman.khandual@arm.com, Mike Leach <mike.leach@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: [RFC PATCH] perf cs-etm: Handle valid-but-zero timestamps Date: Tue, 11 May 2021 16:26:31 +0800 [thread overview] Message-ID: <20210511082631.GD8273@leoy-ThinkPad-X240s> (raw) In-Reply-To: <CAOYpmdFYs8=FzpiA-mMNZ=L8B9GRXLfQuEnMeDsvWeeKg2PWtA@mail.gmail.com> Hi Denis, On Tue, May 11, 2021 at 01:06:03AM -0700, Denis Nikitin wrote: > Hi Leo, > > > Just remind, as Mike has mentioned that if the timestamp is zero, it > > means the hardware setting for timestamp is not enabled properly. So > > for system wide or per CPU mode tracing, it's better to double check > > what's the reason the timestamp is not enabled properly. > > The bug is confirmed by HW verification. Yeah. > > IIUC, this patch breaks the existed rational in the code. Let's think > > about there have 4 CPUs, every CPU has its own AUX trace buffer, and > > when decode the trace data, it will use 4 queues to track the packets > > and every queue has its timestamp. > > > > CPU0: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU1: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU2: cs_etm_queue -> ... -> packet_queue->timestamp > > CPU3: cs_etm_queue -> ... -> packet_queue->timestamp > > > > The issue is if all CPUs' timestamp are zero, it's impossible to find > > a way to synthesize samples in the right time order. > > Is it really impossible or it just can lead to incorrect decoding? Thanks for correcting. Just clarifying: with this change, perf can decode and synthesize samples, but the sequence of samples is not time-based ordering. > I verified the profiles generated with zero timestamps and this patch > on Trogdor (8 CPU cores) and I don't see any significant difference > in the quality of the AutoFDO profiles. > > If mixed packets don't cause errors in reconstructing the branches > but instead mess up with their timeline then it probably won't matter > for AutoFDO which just collects statistics of the branches. > What do you think? Understand. CoreSight trace data can be used for two purposes: tracing and profiling. For AutoFDO profiling, it's okay for with zero timestamps based on your conclusion; on the other hand, the change can introduce confusion if any user wants to use CoreSight for tracing and review the program flow (like use "perf script") command. The change in this patch is valid for me, but it's better to connect with a new option (like "--force-aux-ts-zero" mentioned in my another replying), this can allow users to explictly to force AUX trace timestamp as zero (or in other word, users can use this option to ignore AUX timestamp if the timestamp is not reliable). Thanks, Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-11 8:26 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-07 9:58 [RFC PATCH] perf cs-etm: Handle valid-but-zero timestamps James Clark 2021-05-07 9:58 ` James Clark [not found] ` <3926c523-3fdb-66de-8b9c-b68290a5053e@arm.com> 2021-05-07 14:09 ` Mike Leach 2021-05-07 14:09 ` Mike Leach 2021-05-11 7:39 ` Denis Nikitin 2021-05-11 7:39 ` Denis Nikitin 2021-05-11 10:07 ` James Clark 2021-05-11 10:07 ` James Clark 2021-05-10 5:39 ` Leo Yan 2021-05-10 5:39 ` Leo Yan 2021-05-11 8:05 ` Leo Yan 2021-05-11 8:05 ` Leo Yan 2021-05-11 10:00 ` James Clark 2021-05-11 10:00 ` James Clark 2021-05-11 8:06 ` Denis Nikitin 2021-05-11 8:06 ` Denis Nikitin 2021-05-11 8:26 ` Leo Yan [this message] 2021-05-11 8:26 ` Leo Yan 2021-05-11 13:53 ` James Clark 2021-05-11 13:53 ` James Clark 2021-05-12 1:20 ` Leo Yan 2021-05-12 1:20 ` Leo Yan 2021-05-13 13:57 ` James Clark 2021-05-13 13:57 ` James Clark 2021-05-12 2:08 ` Leo Yan 2021-05-12 2:08 ` Leo Yan 2021-05-13 13:10 ` James Clark 2021-05-13 13:10 ` 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=20210511082631.GD8273@leoy-ThinkPad-X240s \ --to=leo.yan@linaro.org \ --cc=al.grant@arm.com \ --cc=alexander.shishkin@linux.intel.com \ --cc=anshuman.khandual@arm.com \ --cc=branislav.rankov@arm.com \ --cc=coresight@lists.linaro.org \ --cc=denik@chromium.org \ --cc=denik@google.com \ --cc=james.clark@arm.com \ --cc=john.garry@huawei.com \ --cc=jolsa@redhat.com \ --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=mathieu.poirier@linaro.org \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.