* [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling
@ 2021-05-19 7:45 Adrian Hunter
2021-05-19 7:45 ` [PATCH 1/3] perf intel-pt: Fix " Adrian Hunter
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Adrian Hunter @ 2021-05-19 7:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Andi Kleen; +Cc: linux-kernel
Hi
Here are 2 fixes for stable and a subsequent tiny tidy-up.
Adrian Hunter (3):
perf intel-pt: Fix transaction abort handling
perf intel-pt: Fix sample instruction bytes
perf intel-pt: Remove redundant setting of ptq->insn_len
tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 6 +++++-
tools/perf/util/intel-pt.c | 6 ++++--
2 files changed, 9 insertions(+), 3 deletions(-)
Regards
Adrian
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] perf intel-pt: Fix transaction abort handling
2021-05-19 7:45 [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Adrian Hunter
@ 2021-05-19 7:45 ` Adrian Hunter
2021-05-19 7:45 ` [PATCH 2/3] perf intel-pt: Fix sample instruction bytes Adrian Hunter
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2021-05-19 7:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Andi Kleen; +Cc: linux-kernel
When adding support for power events, some handling of FUP packets was
unified. That resulted in breaking reporting of TSX aborts, by not
considering the associated TIP packet. Fix that.
Example:
A machne that supports TSX is required. It will have flag "rtm". Kernel
parameter tsx=on may be required.
# for w in `cat /proc/cpuinfo | grep -m1 flags `;do echo $w | grep rtm ; done
rtm
Test program:
#include <stdio.h>
#include <immintrin.h>
int main()
{
int x = 0;
if (_xbegin() == _XBEGIN_STARTED) {
x = 1;
_xabort(1);
} else {
printf("x = %d\n", x);
}
return 0;
}
Compile with -mrtm i.e.
gcc -Wall -Wextra -mrtm xabort.c -o xabort
Record:
perf record -e intel_pt/cyc/u --filter 'filter main @ ./xabort' ./xabort
Before:
# perf script --itrace=be -F+flags,+addr,-period,-event --ns
xabort 1478 [007] 92161.431348552: tr strt 0 [unknown] ([unknown]) => 400b6d main+0x0 (/root/xabort)
xabort 1478 [007] 92161.431348624: jmp 400b96 main+0x29 (/root/xabort) => 400bae main+0x41 (/root/xabort)
xabort 1478 [007] 92161.431348624: return 400bb4 main+0x47 (/root/xabort) => 400b87 main+0x1a (/root/xabort)
xabort 1478 [007] 92161.431348637: jcc 400b8a main+0x1d (/root/xabort) => 400b98 main+0x2b (/root/xabort)
xabort 1478 [007] 92161.431348644: tr end call 400ba9 main+0x3c (/root/xabort) => 40f690 printf+0x0 (/root/xabort)
xabort 1478 [007] 92161.431360859: tr strt 0 [unknown] ([unknown]) => 400bae main+0x41 (/root/xabort)
xabort 1478 [007] 92161.431360882: tr end return 400bb4 main+0x47 (/root/xabort) => 401139 __libc_start_main+0x309 (/root/xabort)
After:
# perf script --itrace=be -F+flags,+addr,-period,-event --ns
xabort 1478 [007] 92161.431348552: tr strt 0 [unknown] ([unknown]) => 400b6d main+0x0 (/root/xabort)
xabort 1478 [007] 92161.431348624: tx abrt 400b93 main+0x26 (/root/xabort) => 400b87 main+0x1a (/root/xabort)
xabort 1478 [007] 92161.431348637: jcc 400b8a main+0x1d (/root/xabort) => 400b98 main+0x2b (/root/xabort)
xabort 1478 [007] 92161.431348644: tr end call 400ba9 main+0x3c (/root/xabort) => 40f690 printf+0x0 (/root/xabort)
xabort 1478 [007] 92161.431360859: tr strt 0 [unknown] ([unknown]) => 400bae main+0x41 (/root/xabort)
xabort 1478 [007] 92161.431360882: tr end return 400bb4 main+0x47 (/root/xabort) => 401139 __libc_start_main+0x309 (/root/xabort)
Fixes: a472e65fc490a ("perf intel-pt: Add decoder support for ptwrite and power event packets")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org
---
tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
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 0db5f948801f..cb2520abf261 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1205,6 +1205,8 @@ static bool intel_pt_fup_event(struct intel_pt_decoder *decoder)
decoder->set_fup_tx_flags = false;
decoder->tx_flags = decoder->fup_tx_flags;
decoder->state.type = INTEL_PT_TRANSACTION;
+ if (decoder->fup_tx_flags & INTEL_PT_ABORT_TX)
+ decoder->state.type |= INTEL_PT_BRANCH;
decoder->state.from_ip = decoder->ip;
decoder->state.to_ip = 0;
decoder->state.flags = decoder->fup_tx_flags;
@@ -1279,8 +1281,10 @@ static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
return 0;
if (err == -EAGAIN ||
intel_pt_fup_with_nlip(decoder, &intel_pt_insn, ip, err)) {
+ bool no_tip = decoder->pkt_state != INTEL_PT_STATE_FUP;
+
decoder->pkt_state = INTEL_PT_STATE_IN_SYNC;
- if (intel_pt_fup_event(decoder))
+ if (intel_pt_fup_event(decoder) && no_tip)
return 0;
return -EAGAIN;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] perf intel-pt: Fix sample instruction bytes
2021-05-19 7:45 [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Adrian Hunter
2021-05-19 7:45 ` [PATCH 1/3] perf intel-pt: Fix " Adrian Hunter
@ 2021-05-19 7:45 ` Adrian Hunter
2021-05-19 7:45 ` [PATCH 3/3] perf intel-pt: Remove redundant setting of ptq->insn_len Adrian Hunter
2021-05-19 13:35 ` [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Arnaldo Carvalho de Melo
3 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2021-05-19 7:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Andi Kleen; +Cc: linux-kernel
The decoder reports the current instruction if it was decoded. In some
cases the current instruction is not decoded, in which case the instruction
bytes length must be set to zero. Ensure that is always done.
Note perf script can anyway get the instruction bytes for any samples where
they are not present.
Also note, that there is a redundant "ptq->insn_len = 0" statement which is
not removed until a subsequent patch in order to make this patch apply
cleanly to stable branches.
Example:
A machne that supports TSX is required. It will have flag "rtm". Kernel
parameter tsx=on may be required.
# for w in `cat /proc/cpuinfo | grep -m1 flags `;do echo $w | grep rtm ; done
rtm
Test program:
#include <stdio.h>
#include <immintrin.h>
int main()
{
int x = 0;
if (_xbegin() == _XBEGIN_STARTED) {
x = 1;
_xabort(1);
} else {
printf("x = %d\n", x);
}
return 0;
}
Compile with -mrtm i.e.
gcc -Wall -Wextra -mrtm xabort.c -o xabort
Record:
perf record -e intel_pt/cyc/u --filter 'filter main @ ./xabort' ./xabort
Before:
# perf script --itrace=xe -F+flags,+insn,-period --xed --ns
xabort 1478 [007] 92161.431348581: transactions: x 400b81 main+0x14 (/root/xabort) mov $0xffffffff, %eax
xabort 1478 [007] 92161.431348624: transactions: tx abrt 400b93 main+0x26 (/root/xabort) mov $0xffffffff, %eax
After:
# perf script --itrace=xe -F+flags,+insn,-period --xed --ns
xabort 1478 [007] 92161.431348581: transactions: x 400b81 main+0x14 (/root/xabort) xbegin 0x6
xabort 1478 [007] 92161.431348624: transactions: tx abrt 400b93 main+0x26 (/root/xabort) xabort $0x1
Fixes: faaa87680b25d ("perf intel-pt/bts: Report instruction bytes and length in sample")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org
---
tools/perf/util/intel-pt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 2a5fe1514e65..4428dba24aa7 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -778,8 +778,10 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
*ip += intel_pt_insn->length;
- if (to_ip && *ip == to_ip)
+ if (to_ip && *ip == to_ip) {
+ intel_pt_insn->length = 0;
goto out_no_cache;
+ }
if (*ip >= al.map->end)
break;
@@ -1301,6 +1303,7 @@ static void intel_pt_set_pid_tid_cpu(struct intel_pt *pt,
static void intel_pt_sample_flags(struct intel_pt_queue *ptq)
{
+ ptq->insn_len = 0;
if (ptq->state->flags & INTEL_PT_ABORT_TX) {
ptq->flags = PERF_IP_FLAG_BRANCH | PERF_IP_FLAG_TX_ABORT;
} else if (ptq->state->flags & INTEL_PT_ASYNC) {
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] perf intel-pt: Remove redundant setting of ptq->insn_len
2021-05-19 7:45 [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Adrian Hunter
2021-05-19 7:45 ` [PATCH 1/3] perf intel-pt: Fix " Adrian Hunter
2021-05-19 7:45 ` [PATCH 2/3] perf intel-pt: Fix sample instruction bytes Adrian Hunter
@ 2021-05-19 7:45 ` Adrian Hunter
2021-05-19 13:35 ` [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Arnaldo Carvalho de Melo
3 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2021-05-19 7:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Andi Kleen; +Cc: linux-kernel
Remove redundant "ptq->insn_len = 0" statement.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
tools/perf/util/intel-pt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 4428dba24aa7..154a1077f22e 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -1317,7 +1317,6 @@ static void intel_pt_sample_flags(struct intel_pt_queue *ptq)
ptq->flags = PERF_IP_FLAG_BRANCH | PERF_IP_FLAG_CALL |
PERF_IP_FLAG_ASYNC |
PERF_IP_FLAG_INTERRUPT;
- ptq->insn_len = 0;
} else {
if (ptq->state->from_ip)
ptq->flags = intel_pt_insn_type(ptq->state->insn_op);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling
2021-05-19 7:45 [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Adrian Hunter
` (2 preceding siblings ...)
2021-05-19 7:45 ` [PATCH 3/3] perf intel-pt: Remove redundant setting of ptq->insn_len Adrian Hunter
@ 2021-05-19 13:35 ` Arnaldo Carvalho de Melo
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-05-19 13:35 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Jiri Olsa, Andi Kleen, linux-kernel
Em Wed, May 19, 2021 at 10:45:12AM +0300, Adrian Hunter escreveu:
> Hi
>
> Here are 2 fixes for stable and a subsequent tiny tidy-up.
>
> Adrian Hunter (3):
> perf intel-pt: Fix transaction abort handling
> perf intel-pt: Fix sample instruction bytes
> perf intel-pt: Remove redundant setting of ptq->insn_len
>
> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 6 +++++-
> tools/perf/util/intel-pt.c | 6 ++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-19 13:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 7:45 [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Adrian Hunter
2021-05-19 7:45 ` [PATCH 1/3] perf intel-pt: Fix " Adrian Hunter
2021-05-19 7:45 ` [PATCH 2/3] perf intel-pt: Fix sample instruction bytes Adrian Hunter
2021-05-19 7:45 ` [PATCH 3/3] perf intel-pt: Remove redundant setting of ptq->insn_len Adrian Hunter
2021-05-19 13:35 ` [PATCH 0/3] perf intel-pt: Fixes relating to transaction abort handling Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).