Skip to content

Commit 20aa397

Browse files
ahunter6acmel
authored andcommitted
perf intel-pt: Fix premature IPC
The code assumed a change in cycle count means accurate IPC. That is not correct, for example when sampling both branches and instructions, or at a FUP packet (which is not CYC-eligible) address. Fix by using an explicit flag to indicate when IPC can be sampled. Fixes: 5b1dc0f ("perf intel-pt: Add support for samples to contain IPC ratio") Signed-off-by: Adrian Hunter <[email protected]> Reviewed-by: Andi Kleen <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: [email protected] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 03fb0f8 commit 20aa397

File tree

3 files changed

+17
-11
lines changed

3 files changed

+17
-11
lines changed

tools/perf/util/intel-pt-decoder/intel-pt-decoder.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2814,9 +2814,18 @@ const struct intel_pt_state *intel_pt_decode(struct intel_pt_decoder *decoder)
28142814
}
28152815
if (intel_pt_sample_time(decoder->pkt_state)) {
28162816
intel_pt_update_sample_time(decoder);
2817-
if (decoder->sample_cyc)
2817+
if (decoder->sample_cyc) {
28182818
decoder->sample_tot_cyc_cnt = decoder->tot_cyc_cnt;
2819+
decoder->state.flags |= INTEL_PT_SAMPLE_IPC;
2820+
decoder->sample_cyc = false;
2821+
}
28192822
}
2823+
/*
2824+
* When using only TSC/MTC to compute cycles, IPC can be
2825+
* sampled as soon as the cycle count changes.
2826+
*/
2827+
if (!decoder->have_cyc)
2828+
decoder->state.flags |= INTEL_PT_SAMPLE_IPC;
28202829
}
28212830

28222831
decoder->state.timestamp = decoder->sample_timestamp;

tools/perf/util/intel-pt-decoder/intel-pt-decoder.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#define INTEL_PT_ABORT_TX (1 << 1)
1818
#define INTEL_PT_ASYNC (1 << 2)
1919
#define INTEL_PT_FUP_IP (1 << 3)
20+
#define INTEL_PT_SAMPLE_IPC (1 << 4)
2021

2122
enum intel_pt_sample_type {
2223
INTEL_PT_BRANCH = 1 << 0,

tools/perf/util/intel-pt.c

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,7 +1381,8 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
13811381
sample.branch_stack = (struct branch_stack *)&dummy_bs;
13821382
}
13831383

1384-
sample.cyc_cnt = ptq->ipc_cyc_cnt - ptq->last_br_cyc_cnt;
1384+
if (ptq->state->flags & INTEL_PT_SAMPLE_IPC)
1385+
sample.cyc_cnt = ptq->ipc_cyc_cnt - ptq->last_br_cyc_cnt;
13851386
if (sample.cyc_cnt) {
13861387
sample.insn_cnt = ptq->ipc_insn_cnt - ptq->last_br_insn_cnt;
13871388
ptq->last_br_insn_cnt = ptq->ipc_insn_cnt;
@@ -1431,7 +1432,8 @@ static int intel_pt_synth_instruction_sample(struct intel_pt_queue *ptq)
14311432
else
14321433
sample.period = ptq->state->tot_insn_cnt - ptq->last_insn_cnt;
14331434

1434-
sample.cyc_cnt = ptq->ipc_cyc_cnt - ptq->last_in_cyc_cnt;
1435+
if (ptq->state->flags & INTEL_PT_SAMPLE_IPC)
1436+
sample.cyc_cnt = ptq->ipc_cyc_cnt - ptq->last_in_cyc_cnt;
14351437
if (sample.cyc_cnt) {
14361438
sample.insn_cnt = ptq->ipc_insn_cnt - ptq->last_in_insn_cnt;
14371439
ptq->last_in_insn_cnt = ptq->ipc_insn_cnt;
@@ -1983,14 +1985,8 @@ static int intel_pt_sample(struct intel_pt_queue *ptq)
19831985

19841986
ptq->have_sample = false;
19851987

1986-
if (ptq->state->tot_cyc_cnt > ptq->ipc_cyc_cnt) {
1987-
/*
1988-
* Cycle count and instruction count only go together to create
1989-
* a valid IPC ratio when the cycle count changes.
1990-
*/
1991-
ptq->ipc_insn_cnt = ptq->state->tot_insn_cnt;
1992-
ptq->ipc_cyc_cnt = ptq->state->tot_cyc_cnt;
1993-
}
1988+
ptq->ipc_insn_cnt = ptq->state->tot_insn_cnt;
1989+
ptq->ipc_cyc_cnt = ptq->state->tot_cyc_cnt;
19941990

19951991
/*
19961992
* Do PEBS first to allow for the possibility that the PEBS timestamp

0 commit comments

Comments
 (0)