Hi Mathieu,
On 15 June 2017 at 18:42, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 13 June 2017 at 02:55, Mike Leach mike.leach@linaro.org wrote:
Adds a compile-time option to print out the raw trace data bytes when dumping the trace packets using the perf report --dump.
Unpacked raw trace data is output by default, but packed trace frame output can be added.
Controlled by environment var "CSTRACE_RAW", checked in Makefile.config
Signed-off-by: Mike Leach mike.leach@linaro.org
tools/perf/Makefile.config | 6 ++++ tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 47 +++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 432e25f..6a53e4b 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -586,6 +586,12 @@ ifdef CSTRACE_PATH $(call detected,CSTRACE) $(call detected_var,CSTRACE_PATH) EXTLIBS += -L$(CSTRACE_LIB_PATH) $(LIBCSTRACE) -lstdc++
- ifdef CSTRACE_RAW
- CFLAGS += -DCS_DEBUG_RAW
- ifeq (${CSTRACE_RAW}, packed)
CFLAGS += -DCS_RAW_PACKED- endif
- endif
endif
ifdef NO_LIBPERL 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 bbe6923..d5d39c0 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -31,6 +31,16 @@
#define MAX_BUFFER 1024
+/* use raw logging */ +#ifdef CS_DEBUG_RAW +#define CS_LOG_RAW_FRAMES +#ifdef CS_RAW_PACKED +#define CS_RAW_DEBUG_FLAGS (OCSD_DFRMTR_UNPACKED_RAW_OUT | OCSD_DFRMTR_PACKED_RAW_OUT) +#else +#define CS_RAW_DEBUG_FLAGS (OCSD_DFRMTR_UNPACKED_RAW_OUT) +#endif +#endif
The above two hunks won't be pleasant to upstream... Do we absolutely need packed and unpacked mode?
There is a small subset of cases where packed-raw is useful (suspected memory corruption / trace hardware faults), otherwise it is a huge amount of distracting white noise. Effectively with both printing out each byte is output twice. My preference here would be to never upstream this but convert to a command line option rather than compile time - but at present I am not sure how welcome that might be either.
struct cs_etm_decoder;
struct cs_etm_channel { @@ -269,8 +279,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__etmv4i_packet_printer( channel->cs_id); break; case OCSD_OP_RESET:
sprintf(packet_str, "**** RESET DECODER id[%02X] ****",channel->cs_id);
sprintf(packet_str+offset, "**** RESET DECODER ****");Code added in an earlier patch and deleted in a subsequent one is tolerated only when it shows a progression and ease review. Here the changes are small enough that is it now required - please put this in the previous patch.
break; default: break;@@ -281,6 +290,16 @@ static ocsd_datapath_resp_t cs_etm_decoder__etmv4i_packet_printer( return ret; }
+#ifdef CS_LOG_RAW_FRAMES +static void cs_etm_decoder__print_str_cb(const void *p_context,
const char *psz_msg_str,const int str_len)+{
if (p_context && str_len)((struct cs_etm_decoder*)p_context)->packet_printer(psz_msg_str); +} +#endif
static struct cs_etm_channel *cs_etm_decoder__create_channel_item( struct cs_etm_decoder *decoder, unsigned char cs_id) { @@ -530,6 +549,30 @@ struct cs_etm_decoder *cs_etm_decoder__new(uint32_t num_cpu, struct cs_etm_decod /* Create decode tree for the data source */ decoder->dcd_tree = ocsd_create_dcd_tree(format,flags);
+#ifdef CS_LOG_RAW_FRAMES
/* Only log these during a --dump operation */if (d_params->operation == CS_ETM_OPERATION_PRINT) {/* set up a library default logger to process the* raw frame printer we add later*/ocsd_def_errlog_init(OCSD_ERR_SEV_ERROR, 1);/* no stdout / err / file output */ocsd_def_errlog_config_output(C_API_MSGLOGOUT_FLG_NONE,NULL);
/* set the string CB for the default logger,* passes strings to perf print logger.*/ocsd_def_errlog_set_strprint_cb(decoder->dcd_tree,(void *)decoder,cs_etm_decoder__print_str_cb);
/* use the built in library printer for the raw frames */ocsd_dt_set_raw_frame_printer(decoder->dcd_tree,CS_RAW_DEBUG_FLAGS);}+#endif
The goal is to reduce the amount of #if/#endif in the code. Take this hunk, create a function with it and move it to the previous hunk. Then create an empty function with the same name and put it in and #else condition. That way said function can be called in cs_etm_decoder__new() without adding conditional defines.
Will do.
Thanks for the code, Mathieu
if (decoder->dcd_tree == 0) { goto err_free_decoder; }-- 2.7.4
Mike