Re: [PATCH 1/2] perf intel-pt-decoder: Report instruction bytes and length in sample

From: Arnaldo Carvalho de Melo
Date: Wed Oct 05 2016 - 07:36:24 EST


Em Mon, Oct 03, 2016 at 05:30:32PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>
> Change the Intel PT decoder to pass up the length and the instruction
> bytes of the decoded or sampled instruction in the perf sample.
>
> The decoder already knows this information, we just need to pass it
> up. Since it is only a couple of movs it is not very expensive.
>
> Used in the next patch.

Adrian, Ack?

- Arnaldo

> v2: Handle instruction cache too. Make sure ilen is always initialized.
> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> ---
> tools/perf/util/event.h | 3 +++
> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 ++
> tools/perf/util/intel-pt-decoder/intel-pt-decoder.h | 3 +++
> tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h | 2 +-
> tools/perf/util/intel-pt.c | 10 ++++++++++
> 5 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index 8d363d5e65a2..c735c53a26f8 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -177,6 +177,8 @@ enum {
> PERF_IP_FLAG_TRACE_BEGIN |\
> PERF_IP_FLAG_TRACE_END)
>
> +#define MAX_INSN 16
> +
> struct perf_sample {
> u64 ip;
> u32 pid, tid;
> @@ -193,6 +195,7 @@ struct perf_sample {
> u32 flags;
> u16 insn_len;
> u8 cpumode;
> + char insn[MAX_INSN];
> void *raw_data;
> struct ip_callchain *callchain;
> struct branch_stack *branch_stack;
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> index 8ff6c6a61291..8a5e21abb790 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> @@ -949,6 +949,8 @@ out:
> out_no_progress:
> decoder->state.insn_op = intel_pt_insn->op;
> decoder->state.insn_len = intel_pt_insn->length;
> + memcpy(decoder->state.insn, intel_pt_insn->buf,
> + sizeof(decoder->state.insn));
>
> if (decoder->tx_flags & INTEL_PT_IN_TX)
> decoder->state.flags |= INTEL_PT_IN_TX;
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.h b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.h
> index 02c38fec1c37..fbd7d08d97d5 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.h
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.h
> @@ -20,6 +20,8 @@
> #include <stddef.h>
> #include <stdbool.h>
>
> +#define MAX_INSN 16
> +
> #include "intel-pt-insn-decoder.h"
>
> #define INTEL_PT_IN_TX (1 << 0)
> @@ -66,6 +68,7 @@ struct intel_pt_state {
> uint32_t flags;
> enum intel_pt_insn_op insn_op;
> int insn_len;
> + char insn[MAX_INSN];
> };
>
> struct intel_pt_insn;
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h
> index b0adbf37323e..47e196dec224 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h
> @@ -20,7 +20,7 @@
> #include <stdint.h>
>
> #define INTEL_PT_INSN_DESC_MAX 32
> -#define INTEL_PT_INSN_DBG_BUF_SZ 16
> +#define INTEL_PT_INSN_DBG_BUF_SZ 16 /* Must be >= MAX_INSN */
>
> enum intel_pt_insn_op {
> INTEL_PT_OP_OTHER,
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index b9cc353cace2..4b9d0086a383 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -140,6 +140,7 @@ struct intel_pt_queue {
> u32 flags;
> u16 insn_len;
> u64 last_insn_cnt;
> + char insn[MAX_INSN];
> };
>
> static void intel_pt_dump(struct intel_pt *pt __maybe_unused,
> @@ -305,6 +306,7 @@ struct intel_pt_cache_entry {
> enum intel_pt_insn_branch branch;
> int length;
> int32_t rel;
> + char insn[MAX_INSN];
> };
>
> static int intel_pt_config_div(const char *var, const char *value, void *data)
> @@ -390,6 +392,7 @@ static int intel_pt_cache_add(struct dso *dso, struct machine *machine,
> e->branch = intel_pt_insn->branch;
> e->length = intel_pt_insn->length;
> e->rel = intel_pt_insn->rel;
> + memcpy(e->insn, intel_pt_insn->buf, MAX_INSN);
>
> err = auxtrace_cache__add(c, offset, &e->entry);
> if (err)
> @@ -427,6 +430,8 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
> u64 insn_cnt = 0;
> bool one_map = true;
>
> + intel_pt_insn->length = 0;
> +
> if (to_ip && *ip == to_ip)
> goto out_no_cache;
>
> @@ -468,6 +473,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
> intel_pt_insn->branch = e->branch;
> intel_pt_insn->length = e->length;
> intel_pt_insn->rel = e->rel;
> + memcpy(intel_pt_insn->buf, e->insn, MAX_INSN);
> intel_pt_log_insn_no_data(intel_pt_insn, *ip);
> return 0;
> }
> @@ -817,6 +823,7 @@ static void intel_pt_sample_flags(struct intel_pt_queue *ptq)
> if (ptq->state->flags & INTEL_PT_IN_TX)
> ptq->flags |= PERF_IP_FLAG_IN_TX;
> ptq->insn_len = ptq->state->insn_len;
> + memcpy(ptq->insn, ptq->state->insn, MAX_INSN);
> }
> }
>
> @@ -997,6 +1004,7 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
> sample.cpu = ptq->cpu;
> sample.flags = ptq->flags;
> sample.insn_len = ptq->insn_len;
> + memcpy(sample.insn, ptq->insn, MAX_INSN);
>
> /*
> * perf report cannot handle events without a branch stack when using
> @@ -1058,6 +1066,7 @@ static int intel_pt_synth_instruction_sample(struct intel_pt_queue *ptq)
> sample.cpu = ptq->cpu;
> sample.flags = ptq->flags;
> sample.insn_len = ptq->insn_len;
> + memcpy(sample.insn, ptq->insn, MAX_INSN);
>
> ptq->last_insn_cnt = ptq->state->tot_insn_cnt;
>
> @@ -1120,6 +1129,7 @@ static int intel_pt_synth_transaction_sample(struct intel_pt_queue *ptq)
> sample.cpu = ptq->cpu;
> sample.flags = ptq->flags;
> sample.insn_len = ptq->insn_len;
> + memcpy(sample.insn, ptq->insn, MAX_INSN);
>
> if (pt->synth_opts.callchain) {
> thread_stack__sample(ptq->thread, ptq->chain,
> --
> 2.5.5