Hi Denis,

On Tue, May 11, 2021 at 01:06:03AM -0700, Denis Nikitin wrote:

Hi Leo,

quoted

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.

quoted

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