Hello Tor,
While ramping up on the "perf report" and "perf script" features I fixed a few things and decided to play a little:
1/4: No influence on the current results but good to fix. 2/4: That changes a lot of things when multiple buffers have been collected. 3/4: No change in functionality, just cleanup. 4/4: Enhancement to make the output easier to read.
These changes can be found here[1]. If this is good with you I'll push them to gitHub for the 4.7 cycle.
Regards, Mathieu
[1]. https://git.linaro.org/people/mathieu.poirier/coresight.git/shortlog/refs/he...
Mathieu Poirier (4): cs-etm: avoid casting variable cs-etm: account for each trace buffer in the queue cs-etm: removing unecessary structure field cs-etm: associating output packet with CPU they executed on
tools/perf/scripts/python/cs-trace-disasm.py | 2 + tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 15 +++- tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 1 + tools/perf/util/cs-etm.c | 103 +++++++++++++++++++----- tools/perf/util/cs-etm.h | 7 ++ 5 files changed, 106 insertions(+), 22 deletions(-)
Because of two's complement reprensentation, casting an int to and unsigned value doesn't simply get rid of the negative sign. As such a value of -1 becomes 0xFFFFFFFF, which is clearly not the desired effect.
This patch deals with cases when @cpu has the value of -1. In those cases queue '0' is initially selected.
Signed-off-by: Mathieu Poirier mathieu.poirier@linaro.org --- tools/perf/util/cs-etm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 50fdd5405656..14bded991ff2 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -939,10 +939,12 @@ static struct cs_etm_queue *cs_etm__cpu_to_etmq(struct cs_etm_auxtrace *etm, { unsigned q,j;
- if (/*(cpu < 0) ||*/ (0 == etm->queues.nr_queues)) + if (etm->queues.nr_queues == 0) return NULL;
- if ((unsigned) cpu >= etm->queues.nr_queues) + if (cpu < 0) + q = 0; + else if ((unsigned) cpu >= etm->queues.nr_queues) q = etm->queues.nr_queues - 1; else q = cpu;
Function cs_etm__get_trace() picks up a single buffer from the current queue. As such when multiple buffers are present in the queue several iteration of the fetch-decode block need to be run in order to process all the trace data.
Signed-off-by: Mathieu Poirier mathieu.poirier@linaro.org --- tools/perf/util/cs-etm.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 14bded991ff2..1c37ca808398 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -326,7 +326,6 @@ static int cs_etm__get_trace(struct cs_etm_buffer *buff, struct cs_etm_queue *et buff->len = sizeof(cstrace); buff->buf = cstrace; */ - etmq->stop = true;
buff->ref_timestamp = aux_buffer->reference;
@@ -339,10 +338,7 @@ static int cs_etm__get_trace(struct cs_etm_buffer *buff, struct cs_etm_queue *et if (etmq->step_through_buffers) etmq->stop = true;
- if (buff->len == 0) - return cs_etm__get_trace(buff,etmq); - - return 0; + return buff->len; }
static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, @@ -820,12 +816,16 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
static int cs_etm__run_decoder(struct cs_etm_queue *etmq, u64 *timestamp) { - struct cs_etm_buffer buffer = {.buf = 0,}; - size_t buffer_used = 0; + struct cs_etm_buffer buffer; + size_t buffer_used; int err = 0;
+ /* Go through each buffer in the queue and decode them one by one */ +more: + buffer_used = 0; + memset(&buffer, 0, sizeof(buffer)); err = cs_etm__get_trace(&buffer,etmq); - if (err) + if (err <= 0) return err;
do { @@ -843,6 +843,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue *etmq, u64 *timestamp) cs_etm__sample(etmq); } } while (!etmq->eot && (buffer.len > buffer_used)); +goto more;
(void) timestamp;
Function cs_etm__sample() is called only from cs_etm__run_decoder() where cs_etm_queue::have_sample is set to 'true'. As such checking the value of the variable again in cs_etm__sample() is not needed.
Since the variable isn't used anywhere else, also removing it from the structure definition.
Signed-off-by: Mathieu Poirier mathieu.poirier@linaro.org --- tools/perf/util/cs-etm.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 1c37ca808398..1194e8a34a21 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -83,7 +83,6 @@ struct cs_etm_queue { u64 time; u64 timestamp; bool stop; - bool have_sample; struct cs_etm_decoder *decoder; u64 offset; bool eot; @@ -798,11 +797,6 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) //struct cs_etm_auxtrace *etm = etmq->etm; int err;
- if (!etmq->have_sample) - return 0; - - etmq->have_sample = false; - err = cs_etm_decoder__get_packet(etmq->decoder,&packet); // if there is no sample, it returns err = -1, no real error
@@ -838,10 +832,8 @@ more: err = etmq->state->err; etmq->offset += processed; buffer_used += processed; - if (!err) { - etmq->have_sample = true; + if (!err) cs_etm__sample(etmq); - } } while (!etmq->eot && (buffer.len > buffer_used)); goto more;
This patch adds the required mechanic to quickly lookup the CPU number associated with a traceID. That way the CPU that executed the code conveyed by a decoded packet can be identified, without having to do unecessary translations.
Using this new functionality the "cs-trace-disasm.py" script is enhanced to output the file and CPU number the code has been executed on:
FILE: /lib/aarch64-linux-gnu/ld-2.21.so CPU: 3 7fab57fd80: 910003e0 mov x0, sp 7fab57fd84: 94000d53 bl 7fab5832d0 <free@plt+0x3790> FILE: /lib/aarch64-linux-gnu/ld-2.21.so CPU: 3 7fab5832d0: d11203ff sub sp, sp, #0x480 FILE: /lib/aarch64-linux-gnu/ld-2.21.so CPU: 3 7fab5832d4: a9ba7bfd stp x29, x30, [sp,#-96]! 7fab5832d8: 910003fd mov x29, sp 7fab5832dc: a90363f7 stp x23, x24, [sp,#48] 7fab5832e0: 9101e3b7 add x23, x29, #0x78 7fab5832e4: a90573fb stp x27, x28, [sp,#80] 7fab5832e8: a90153f3 stp x19, x20, [sp,#16] 7fab5832ec: aa0003fb mov x27, x0 7fab5832f0: 910a82e1 add x1, x23, #0x2a0 7fab5832f4: a9025bf5 stp x21, x22, [sp,#32] 7fab5832f8: a9046bf9 stp x25, x26, [sp,#64] 7fab5832fc: 910102e0 add x0, x23, #0x40 7fab583300: f800841f str xzr, [x0],#8 7fab583304: eb01001f cmp x0, x1 7fab583308: 54ffffc1 b.ne 7fab583300 <free@plt+0x37c0>
Signed-off-by: Mathieu Poirier mathieu.poirier@linaro.org --- tools/perf/scripts/python/cs-trace-disasm.py | 2 + tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 15 +++++- tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 1 + tools/perf/util/cs-etm.c | 70 ++++++++++++++++++++++++- tools/perf/util/cs-etm.h | 7 +++ 5 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/tools/perf/scripts/python/cs-trace-disasm.py b/tools/perf/scripts/python/cs-trace-disasm.py index 175ae4bfa732..429d0d2d7a23 100644 --- a/tools/perf/scripts/python/cs-trace-disasm.py +++ b/tools/perf/scripts/python/cs-trace-disasm.py @@ -82,6 +82,7 @@ def process_event(t): if (len(disasm_cache) > cache_size): disasm_cache.clear();
+ cpu = format(sample['cpu'], "d"); addr_range = format(sample['ip'],"x") + ":" + format(sample['addr'],"x");
try: @@ -103,6 +104,7 @@ def process_event(t): disasm_output = check_output(disasm).split('\n') disasm_cache[addr_range] = disasm_output;
+ print "FILE: %s\tCPU: %s" % (dso, cpu); for line in disasm_output: m = disasm_re.search(line) if (m != None) : 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 2cc7ccd3c22a..c6f23d63590d 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -16,10 +16,13 @@ * with this program. If not, see http://www.gnu.org/licenses/. */
+#include <linux/err.h> #include <stdlib.h>
+#include "../cs-etm.h" #include "cs-etm-decoder.h" #include "../util.h" +#include "../util/intlist.h"
#include "c_api/opencsd_c_api.h" #include "ocsd_if_types.h" @@ -102,10 +105,12 @@ int cs_etm_decoder__flush(struct cs_etm_decoder *decoder)
static int cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder, const ocsd_generic_trace_elem *elem, + const uint8_t trace_chan_id, enum cs_etm_sample_type sample_type) { int err = 0; uint32_t et = 0; + struct int_node *inode = NULL;
if (decoder == NULL) return -1;
@@ -116,12 +121,18 @@ static int cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder, if (err) return err;
et = decoder->end_tail; + /* Search the RB tree for the cpu associated with this traceID */ + inode = intlist__find(traceid_list, trace_chan_id); + if (!inode) + return PTR_ERR(inode);
decoder->packet_buffer[et].sample_type = sample_type; decoder->packet_buffer[et].start_addr = elem->st_addr; decoder->packet_buffer[et].end_addr = elem->en_addr; decoder->packet_buffer[et].exc = false; decoder->packet_buffer[et].exc_ret = false; + decoder->packet_buffer[et].cpu = *((int*)inode->priv); + et = (et + 1) & (MAX_BUFFER - 1);
decoder->end_tail = et; @@ -177,7 +188,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( //decoder->discontinuity = true; //break; case OCSD_GEN_TRC_ELEM_INSTR_RANGE: - cs_etm_decoder__buffer_packet(decoder,elem, CS_ETM_RANGE); + cs_etm_decoder__buffer_packet(decoder,elem, + trace_chan_id, CS_ETM_RANGE); resp = OCSD_RESP_WAIT; break; case OCSD_GEN_TRC_ELEM_EXCEPTION: @@ -409,6 +421,7 @@ static void cs_etm_decoder__clear_buffer(struct cs_etm_decoder *decoder) decoder->packet_buffer[i].end_addr = 0xdeadbeefdeadbeefUL; decoder->packet_buffer[i].exc = false; decoder->packet_buffer[i].exc_ret = false; + decoder->packet_buffer[i].cpu = INT_MIN; } }
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h index 2d1013bcdbab..38c5ae84ed07 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h @@ -51,6 +51,7 @@ struct cs_etm_packet { uint64_t end_addr; bool exc; bool exc_ret; + int cpu; };
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 1194e8a34a21..551adc7bca6f 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -15,6 +15,7 @@ * this program. If not, see http://www.gnu.org/licenses/. */
+#include <linux/err.h> #include <linux/kernel.h> #include <linux/types.h> #include <linux/bitops.h> @@ -29,6 +30,7 @@ #include "evlist.h" #include "machine.h" #include "util.h" +#include "util/intlist.h" #include "color.h" #include "cs-etm.h" #include "cs-etm-decoder/cs-etm-decoder.h" @@ -234,12 +236,20 @@ static void cs_etm__free(struct perf_session *session) {
size_t i; + struct int_node *inode, *tmp; struct cs_etm_auxtrace *aux = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace); auxtrace_heap__free(&aux->heap); cs_etm__free_events(session); session->auxtrace = NULL; + + /* First remove all traceID/CPU# nodes from the RB tree */ + intlist__for_each_safe(inode, tmp, traceid_list) + intlist__remove(traceid_list, inode); + /* Then the RB tree itself */ + intlist__delete(traceid_list); + //thread__delete(aux->unknown_thread); for (i = 0; i < aux->num_cpu; ++i) { zfree(&aux->metadata[i]); @@ -613,7 +623,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, sample.id = etmq->etm->instructions_id; sample.stream_id = etmq->etm->instructions_id; sample.period = (end_addr - start_addr) >> 2; - sample.cpu = etmq->cpu; + sample.cpu = packet->cpu; sample.flags = 0; // etmq->flags; sample.insn_len = 1; // etmq->insn_len; sample.cpumode = event->header.misc; @@ -1327,12 +1337,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, size_t priv_size = 0; size_t num_cpu; struct cs_etm_auxtrace *etm = 0; - int err = 0; + int err = 0, idx = -1; u64 *ptr; u64 *hdr = NULL; u64 **metadata = NULL; size_t i,j,k; unsigned pmu_type; + struct int_node *inode; + + /* + * sizeof(auxtrace_info_event::type) + + * sizeof(auxtrace_info_event::reserved) == 8 + */ + info_header_size = 8;
if (total_size < (event_header_size + info_header_size)) return -EINVAL; @@ -1356,7 +1373,20 @@ int cs_etm__process_auxtrace_info(union perf_event *event, return -EINVAL; }
+ /* + * Create an RB tree for traceID-CPU# tuple. Since the conversion has + * to be made for each packet that gets decoded optimizing access in + * anything other than a sequential array is worth doing. + */ + traceid_list = intlist__new(NULL); + if (!traceid_list) + return -ENOMEM; + metadata = zalloc(sizeof(u64 *) * num_cpu); + if (!metadata) { + err = -ENOMEM; + goto err_free_traceid_list; + }
if (metadata == NULL) { return -EINVAL; @@ -1370,6 +1400,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event, for (k = 0; k < CS_ETM_PRIV_MAX; k++) { metadata[j][k] = ptr[i+k]; } + + /* The traceID is our handle */ + idx = metadata[j][CS_ETM_ETMIDR]; i += CS_ETM_PRIV_MAX; } else if (ptr[i] == __perf_cs_etmv4_magic) { metadata[j] = zalloc(sizeof(u64)*CS_ETMV4_PRIV_MAX); @@ -1378,8 +1411,33 @@ int cs_etm__process_auxtrace_info(union perf_event *event, for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) { metadata[j][k] = ptr[i+k]; } + + /* The traceID is our handle */ + idx = metadata[j][CS_ETMV4_TRCTRACEIDR]; i += CS_ETMV4_PRIV_MAX; } + + /* Get an RB node for this CPU */ + inode = intlist__findnew(traceid_list, idx); + + /* Something went wrong, no need to continue */ + if (!inode) { + err = PTR_ERR(inode); + goto err_free_metadata; + } + + /* + * The node for that CPU should not have been taken already. + * Backout if that's the case. + */ + if (inode->priv) { + err = -EINVAL; + goto err_free_metadata; + } + + /* All good, associate the traceID with the CPU# */ + inode->priv = &metadata[j][CS_ETM_CPU]; + }
if (i*8 != priv_size) @@ -1464,5 +1522,13 @@ err_free_queues: session->auxtrace = NULL; err_free: free(etm); +err_free_metadata: + /* No need to check @metadata[j], free(NULL) is supported */ + for (j = 0; j < num_cpu; ++j) + free(metadata[j]); + free(metadata); +err_free_traceid_list: + intlist__delete(traceid_list); + return err; } diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index 7b683a024690..ec6ff78f1905 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -18,6 +18,10 @@ #ifndef INCLUDE__UTIL_PERF_CS_ETM_H__ #define INCLUDE__UTIL_PERF_CS_ETM_H__
+#include "util/event.h" +#include "util/intlist.h" +#include "util/session.h" + /* Versionning header in case things need tro change in the future. That way * decoding of old snapshot is still possible. */ @@ -61,6 +65,9 @@ enum { CS_ETMV4_PRIV_MAX, };
+/* RB tree for quick conversion between traceID and CPUs */ +struct intlist *traceid_list; + #define KiB(x) ((x) * 1024) #define MiB(x) ((x) * 1024 * 1024)
-
Mathieu Poirier