diff mbox series

[v2,4/6] perf cs-etm: Treat NO_SYNC element as trace discontinuity

Message ID 1544431981-24144-5-git-send-email-leo.yan@linaro.org
State New
Headers show
Series perf cs-etm: Correct packets handling | expand

Commit Message

Leo Yan Dec. 10, 2018, 8:52 a.m. UTC
CoreSight tracer driver might insert barrier packet between different
buffers, thus the decoder can spot the boundaries based on the barrier
packet; the decoder is possible to hit a barrier packet and emit a
NO_SYNC element, then the decoder will find a periodic synchronisation
point inside that next trace block that starts trace again but does not
have the TRACE_ON element as indicator - usually because this block of
trace has wrapped the buffer so we have lost the original point that
trace was enabled.

In upper case, it results in the trace stream only inserts the
OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but
we don't handle NO_SYNC element properly and at the end users miss to
see the info for trace discontinuity.

Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when
output from the decoder, but both of them indicate the trace data is
discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace
discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so
cs-etm can handle discontinuity for this case, finally it saves the last
trace data for previous trace block and restart samples for new block.

Credit to Mike Leach and Robert Walker who made me clear for underlying
mechanism for NO_SYNC element, Mike also shared with me for detailed
explanation for why we can treat NO_SYNC and TRACE_ON elements as the
same, so this commit log reused most of his explanation.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Robert Walker <robert.walker@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>

---
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.7.4

Comments

Mathieu Poirier Dec. 10, 2018, 10:53 p.m. UTC | #1
On Mon, Dec 10, 2018 at 04:52:59PM +0800, Leo Yan wrote:
> CoreSight tracer driver might insert barrier packet between different

> buffers, thus the decoder can spot the boundaries based on the barrier

> packet; the decoder is possible to hit a barrier packet and emit a

> NO_SYNC element, then the decoder will find a periodic synchronisation

> point inside that next trace block that starts trace again but does not

> have the TRACE_ON element as indicator - usually because this block of

> trace has wrapped the buffer so we have lost the original point that

> trace was enabled.

> 

> In upper case, it results in the trace stream only inserts the

> OCSD_GEN_TRC_ELEM_NO_SYNC element in the middle of tracing stream, but

> we don't handle NO_SYNC element properly and at the end users miss to

> see the info for trace discontinuity.

> 

> Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when

> output from the decoder, but both of them indicate the trace data is

> discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace

> discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so

> cs-etm can handle discontinuity for this case, finally it saves the last

> trace data for previous trace block and restart samples for new block.

> 

> Credit to Mike Leach and Robert Walker who made me clear for underlying

> mechanism for NO_SYNC element, Mike also shared with me for detailed

> explanation for why we can treat NO_SYNC and TRACE_ON elements as the

> same, so this commit log reused most of his explanation.


Credit and thank you note such as this one go in the cover letter.  With this
change:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>


> 

> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>

> Cc: Mike Leach <mike.leach@linaro.org>

> Cc: Robert Walker <robert.walker@arm.com>

> Signed-off-by: Leo Yan <leo.yan@linaro.org>

> ---

>  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c

> index a3994f1..46b67f1 100644

> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c

> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c

> @@ -411,6 +411,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(

>  	case OCSD_GEN_TRC_ELEM_UNKNOWN:

>  		break;

>  	case OCSD_GEN_TRC_ELEM_NO_SYNC:

> +		resp = cs_etm_decoder__buffer_discontinuity(decoder,

> +							    trace_chan_id);

>  		decoder->trace_on = false;

>  		break;

>  	case OCSD_GEN_TRC_ELEM_TRACE_ON:

> -- 

> 2.7.4

>
diff mbox series

Patch

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index a3994f1..46b67f1 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -411,6 +411,8 @@  static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
 	case OCSD_GEN_TRC_ELEM_UNKNOWN:
 		break;
 	case OCSD_GEN_TRC_ELEM_NO_SYNC:
+		resp = cs_etm_decoder__buffer_discontinuity(decoder,
+							    trace_chan_id);
 		decoder->trace_on = false;
 		break;
 	case OCSD_GEN_TRC_ELEM_TRACE_ON: