Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
On Thu, Dec 13, 2018 at 10:26:33AM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Thu, Dec 13, 2018 at 09:09:49PM +0800, leo@linaro.org escreveu: > > > Thanks a lot for helping. If you prefer me to resend the patch set, > > > also let me know. > > > > I can fix it, here. > > Can I take a look at the result: > > https://git.kernel.org/acme/c/222b0bead7e4 Yeah, looks good to me :) Thanks, Leo Yan
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
Em Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Thu, Dec 13, 2018 at 09:09:49PM +0800, leo@linaro.org escreveu: > > Thanks a lot for helping. If you prefer me to resend the patch set, > > also let me know. > > I can fix it, here. Can I take a look at the result: https://git.kernel.org/acme/c/222b0bead7e4 ? - Arnaldo
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
On Thu, Dec 13, 2018 at 10:21:26AM -0300, Arnaldo Carvalho de Melo wrote: [...] > > > commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59 > > > Author: Leo Yan > > > Date: Tue Dec 11 15:38:26 2018 +0800 > > > > > > perf cs-etm: Treat NO_SYNC element as trace discontinuity > > > > > > The CoreSight tracer driver might insert a barrier packets between > > packet > > I'll remove the 'a' instead, ok? Yeah. > > > different buffers, thus the decoder can spot the boundaries based on > > > the > > > barrier packet; it is possible for the decoder 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 the > > > trace > > > again but does not have the TRACE_ON element as indicator - usually > > > because this trace block has wrapped the buffer so we have lost the > > > original point when the trace was enabled. > > > > > > In the first case it causes the insertion of a > > > OCSD_GEN_TRC_ELEM_NO_SYNC > > former > > > in the middle of the the tracing stream, but as we were npt handling > > > the > > not > > > NO_SYNC element properly which ends up making users miss the > > > discontinuity indication"? > > s/?/. > > > > Thanks a lot for helping. If you prefer me to resend the patch set, > > also let me know. > > I can fix it, here. Thanks a lot! > > > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON > > > when > > > output from the decoder, both indicate that the trace data is > > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace > > > discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so > > > cs-etm can handle the discontinuity for this case, finally it saves > > > the > > > last trace data for the previous trace block and restart samples for > > > the > > > new block. > > > > > > Signed-off-by: Leo Yan > > > Reviewed-by: Mathieu Poirier > > > Cc: Alexander Shishkin > > > Cc: Jiri Olsa > > > Cc: Mike Leach > > > Cc: Namhyung Kim > > > Cc: Robert Walker > > > Cc: coresight ml > > > Cc: linux-arm-ker...@lists.infradead.org > > > Link: > > > http://lkml.kernel.org/r/1544513908-16805-7-git-send-email-leo@linaro.org > > > Signed-off-by: Arnaldo Carvalho de Melo > > > > > > 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 1039f364f4cc..bee026e76a4c 100644 > > > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > > > @@ -410,7 +410,6 @@ 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: > > > - break; > > > case OCSD_GEN_TRC_ELEM_TRACE_ON: > > > resp = cs_etm_decoder__buffer_discontinuity(decoder, > > > trace_chan_id); > > > > > > > > -- > > - Arnaldo
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
Em Thu, Dec 13, 2018 at 09:09:49PM +0800, leo@linaro.org escreveu: > Hi Arnaldo, > > On Thu, Dec 13, 2018 at 09:41:54AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu: > > > > 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. > > > > > > > > > "In upper case"? Maybe: > > > > > > In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC > > > in the middle of the the tracing stream, but as we were npt handling the > > > NO_SYNC element properly which ends up making users miss the > > > discontinuity indication"? > > > > > > 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 > > > > > > can we remove the "but" and "of them" (redundant) above? > > > > > > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace > > >a > > > > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so > > > > cs-etm can handle discontinuity for this case, finally it saves the last > > >it (way too many "discontinuity") > > > > trace data for previous trace block and restart samples for new block. > > > > I've tentatively done those changes (and a few more) in my local branch, > > resulting in the wording below, plese let me know if you are ok with it: > > Very appreciate your helping. Please see some minor typos in below > commit: > > > commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59 > > Author: Leo Yan > > Date: Tue Dec 11 15:38:26 2018 +0800 > > > > perf cs-etm: Treat NO_SYNC element as trace discontinuity > > > > The CoreSight tracer driver might insert a barrier packets between > packet I'll remove the 'a' instead, ok? > > different buffers, thus the decoder can spot the boundaries based on the > > barrier packet; it is possible for the decoder 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 the trace > > again but does not have the TRACE_ON element as indicator - usually > > because this trace block has wrapped the buffer so we have lost the > > original point when the trace was enabled. > > > > In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC > former > > in the middle of the the tracing stream, but as we were npt handling the > not > > NO_SYNC element properly which ends up making users miss the > > discontinuity indication"? > s/?/. > > Thanks a lot for helping. If you prefer me to resend the patch set, > also let me know. I can fix it, here. > > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when > > output from the decoder, both indicate that the trace data is > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace > > discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so > > cs-etm can handle the discontinuity for this case, finally it saves the > > last trace data for the previous trace block and restart samples for the > > new block. > > > > Signed-off-by: Leo Yan > > Reviewed-by: Mathieu Poirier > > Cc: Alexander Shishkin > > Cc: Jiri Olsa > > Cc: Mike Leach > > Cc: Namhyung Kim > > Cc: Robert Walker > > Cc: coresight ml > > Cc: linux-arm-ker...@lists.infradead.org > > Link: > > http://lkml.kernel.org/r/1544513908-16805-7-git-send-email-leo@linaro.org > > Signed-off-by: Arnaldo Carvalho de Melo > > > > 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 1039f364f4cc..bee026e76a4c 100644 > > ---
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
Hi Arnaldo, On Thu, Dec 13, 2018 at 09:41:54AM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu: > > > 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. > > > > > > "In upper case"? Maybe: > > > > In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC > > in the middle of the the tracing stream, but as we were npt handling the > > NO_SYNC element properly which ends up making users miss the > > discontinuity indication"? > > > > 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 > > > > can we remove the "but" and "of them" (redundant) above? > > > > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace > >a > > > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so > > > cs-etm can handle discontinuity for this case, finally it saves the last > >it (way too many "discontinuity") > > > trace data for previous trace block and restart samples for new block. > > I've tentatively done those changes (and a few more) in my local branch, > resulting in the wording below, plese let me know if you are ok with it: Very appreciate your helping. Please see some minor typos in below commit: > commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59 > Author: Leo Yan > Date: Tue Dec 11 15:38:26 2018 +0800 > > perf cs-etm: Treat NO_SYNC element as trace discontinuity > > The CoreSight tracer driver might insert a barrier packets between packet > different buffers, thus the decoder can spot the boundaries based on the > barrier packet; it is possible for the decoder 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 the trace > again but does not have the TRACE_ON element as indicator - usually > because this trace block has wrapped the buffer so we have lost the > original point when the trace was enabled. > > In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC former > in the middle of the the tracing stream, but as we were npt handling the not > NO_SYNC element properly which ends up making users miss the > discontinuity indication"? s/?/. Thanks a lot for helping. If you prefer me to resend the patch set, also let me know. > Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when > output from the decoder, both indicate that the trace data is > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace > discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so > cs-etm can handle the discontinuity for this case, finally it saves the > last trace data for the previous trace block and restart samples for the > new block. > > Signed-off-by: Leo Yan > Reviewed-by: Mathieu Poirier > Cc: Alexander Shishkin > Cc: Jiri Olsa > Cc: Mike Leach > Cc: Namhyung Kim > Cc: Robert Walker > Cc: coresight ml > Cc: linux-arm-ker...@lists.infradead.org > Link: > http://lkml.kernel.org/r/1544513908-16805-7-git-send-email-leo@linaro.org > Signed-off-by: Arnaldo Carvalho de Melo > > 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 1039f364f4cc..bee026e76a4c 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -410,7 +410,6 @@ 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: > - break; > case OCSD_GEN_TRC_ELEM_TRACE_ON: > resp =
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
Em Thu, Dec 13, 2018 at 09:38:54AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu: > > 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. > > > "In upper case"? Maybe: > > In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC > in the middle of the the tracing stream, but as we were npt handling the > NO_SYNC element properly which ends up making users miss the > discontinuity indication"? > > 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 > > can we remove the "but" and "of them" (redundant) above? > > > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace >a > > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so > > cs-etm can handle discontinuity for this case, finally it saves the last >it (way too many "discontinuity") > > trace data for previous trace block and restart samples for new block. I've tentatively done those changes (and a few more) in my local branch, resulting in the wording below, plese let me know if you are ok with it: commit 148068b45fe2e93b19c06cfc1140ea12ca72eb59 Author: Leo Yan Date: Tue Dec 11 15:38:26 2018 +0800 perf cs-etm: Treat NO_SYNC element as trace discontinuity The CoreSight tracer driver might insert a barrier packets between different buffers, thus the decoder can spot the boundaries based on the barrier packet; it is possible for the decoder 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 the trace again but does not have the TRACE_ON element as indicator - usually because this trace block has wrapped the buffer so we have lost the original point when the trace was enabled. In the first case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC in the middle of the the tracing stream, but as we were npt handling the NO_SYNC element properly which ends up making users miss the discontinuity indication"? Though OCSD_GEN_TRC_ELEM_NO_SYNC is different from CS_ETM_TRACE_ON when output from the decoder, both indicate that the trace data is discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as a trace discontinuity and generates a CS_ETM_DISCONTINUITY packet for it, so cs-etm can handle the discontinuity for this case, finally it saves the last trace data for the previous trace block and restart samples for the new block. Signed-off-by: Leo Yan Reviewed-by: Mathieu Poirier Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Mike Leach Cc: Namhyung Kim Cc: Robert Walker Cc: coresight ml Cc: linux-arm-ker...@lists.infradead.org Link: http://lkml.kernel.org/r/1544513908-16805-7-git-send-email-leo@linaro.org Signed-off-by: Arnaldo Carvalho de Melo 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 1039f364f4cc..bee026e76a4c 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -410,7 +410,6 @@ 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: - break; case OCSD_GEN_TRC_ELEM_TRACE_ON: resp = cs_etm_decoder__buffer_discontinuity(decoder, trace_chan_id);
Re: [PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
Em Tue, Dec 11, 2018 at 03:38:26PM +0800, Leo Yan escreveu: > 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. "In upper case"? Maybe: In the former case it causes the insertion of a OCSD_GEN_TRC_ELEM_NO_SYNC in the middle of the the tracing stream, but as we were npt handling the NO_SYNC element properly which ends up making users miss the discontinuity indication"? > 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 can we remove the "but" and "of them" (redundant) above? > discontinuous; this patch treats OCSD_GEN_TRC_ELEM_NO_SYNC as trace a > discontinuity and generates CS_ETM_DISCONTINUITY packet for it, so > cs-etm can handle discontinuity for this case, finally it saves the last it (way too many "discontinuity") > trace data for previous trace block and restart samples for new block. > > Signed-off-by: Leo Yan > Reviewed-by: Mathieu Poirier > Cc: Mike Leach > Cc: Robert Walker > --- > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 - > 1 file changed, 1 deletion(-) > > 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 1039f364..bee026e 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -410,7 +410,6 @@ 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: > - break; > case OCSD_GEN_TRC_ELEM_TRACE_ON: > resp = cs_etm_decoder__buffer_discontinuity(decoder, > trace_chan_id); > -- > 2.7.4 -- - Arnaldo
[PATCH v3 6/8] perf cs-etm: Treat NO_SYNC element as trace discontinuity
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. Signed-off-by: Leo Yan Reviewed-by: Mathieu Poirier Cc: Mike Leach Cc: Robert Walker --- tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 - 1 file changed, 1 deletion(-) 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 1039f364..bee026e 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -410,7 +410,6 @@ 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: - break; case OCSD_GEN_TRC_ELEM_TRACE_ON: resp = cs_etm_decoder__buffer_discontinuity(decoder, trace_chan_id); -- 2.7.4
