Skip to contentSkip to conversation

perf tools: Fix swap for samples with raw data

PATCH Applied Jiri Olsa 5 participants 6 messages Nov 29, 2017 – Dec 28, 2017 perf events

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

PATCH 1/1 perf tools: Fix swap for samples with raw data

When we detect different endianity we swap event before processing. It's tricky for samples because we have no idea what's inside. We treat it as an array of u64s, swap them and later on we swap back parts which are different.

We mangle this way also the tracepoint raw data, which ends up in report showing wrong data:

1.95% comm=Q^B pid=29285 prio=16777216 target_cpu=000 1.67% comm=l^B pid=0 prio=16777216 target_cpu=000

Luckily the traceevent library handles the endianity by itself (thank you Steven!), so we can pass the RAW data directly in the other endianity.

2.51% comm=beah-rhts-task pid=1175 prio=120 target_cpu=002 2.23% comm=kworker/0:0 pid=11566 prio=120 target_cpu=000

The fix is basically to swap back the raw data if different endianity is detected.

---
 tools/perf/util/evsel.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)
tools/perf/util/evsel.c +17 -3
index 95853c51c0ca..09c68cf2d9d2 100644
@@ -36,6 +36,7 @@
 #include "debug.h"
 #include "trace-event.h"
 #include "stat.h"
+#include "memswap.h"
 #include "util/parse-branch-options.h"
 
 #include "sane_ctype.h"
@@ -2131,14 +2132,27 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (type & PERF_SAMPLE_RAW) {
 		OVERFLOW_CHECK_u64(array);
 		u.val64 = *array;
-		if (WARN_ONCE(swapped,
-			      "Endianness of raw data not corrected!\n")) {
-			/* undo swap of u64, then swap on individual u32s */
+
+		/*
+		 * Undo swap of u64, then swap on individual u32s,
+		 * get the size of the raw area and undo all of the
+		 * swap. The pevent interface handles endianity by
+		 * itself.
+		 */
+		if (swapped) {
 			u.val64 = bswap_64(u.val64);
 			u.val32[0] = bswap_32(u.val32[0]);
 			u.val32[1] = bswap_32(u.val32[1]);
 		}
 		data->raw_size = u.val32[0];
+
+		/*
+		 * The raw data is aligned on 64bits including the
+		 * u32 size, so it's safe to use mem_bswap_64.
+		 */
+		if (swapped)
+			mem_bswap_64((void *) array, data->raw_size);
+
 		array = (void *)array + sizeof(u32);
 
 		OVERFLOW_CHECK(array, data->raw_size, max_size);
Signature · 1 line
2.13.6
#1 lore raw
Quoted 3 lines · On Wed, 29 Nov 2017 19:43:46 +0100 Jiri Olsa <[email protected]> wrote:

Luckily the traceevent library handles the endianity by itself (thank you Steven!), so we can pass the RAW data directly in the other endianity.

You are welcome ;-)

-- Steve

maintainer Applied
#2 lore raw
In reply to Jiri Olsa

Em Wed, Nov 29, 2017 at 07:43:46PM +0100, Jiri Olsa escreveu:

Quoted 5 lines

When we detect different endianity we swap event before processing. It's tricky for samples because we have no idea what's inside. We treat it as an array of u64s, swap them and later on we swap back parts which are different.

Thanks, applied to perf/core, now at:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=26ea2ece7802f8fdaaacf321dbfb22de3271ab82

- Arnaldo

Quoted 69 lines

We mangle this way also the tracepoint raw data, which ends up in report showing wrong data:

1.95% comm=Q^B pid=29285 prio=16777216 target_cpu=000 1.67% comm=l^B pid=0 prio=16777216 target_cpu=000

Luckily the traceevent library handles the endianity by itself (thank you Steven!), so we can pass the RAW data directly in the other endianity.

2.51% comm=beah-rhts-task pid=1175 prio=120 target_cpu=002 2.23% comm=kworker/0:0 pid=11566 prio=120 target_cpu=000

The fix is basically to swap back the raw data if different endianity is detected.

---
 tools/perf/util/evsel.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)
tools/perf/util/evsel.c +17 -3
index 95853c51c0ca..09c68cf2d9d2 100644
@@ -36,6 +36,7 @@
 #include "debug.h"
 #include "trace-event.h"
 #include "stat.h"
+#include "memswap.h"
 #include "util/parse-branch-options.h"
 
 #include "sane_ctype.h"
@@ -2131,14 +2132,27 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (type & PERF_SAMPLE_RAW) {
 		OVERFLOW_CHECK_u64(array);
 		u.val64 = *array;
-		if (WARN_ONCE(swapped,
-			      "Endianness of raw data not corrected!\n")) {
-			/* undo swap of u64, then swap on individual u32s */
+
+		/*
+		 * Undo swap of u64, then swap on individual u32s,
+		 * get the size of the raw area and undo all of the
+		 * swap. The pevent interface handles endianity by
+		 * itself.
+		 */
+		if (swapped) {
 			u.val64 = bswap_64(u.val64);
 			u.val32[0] = bswap_32(u.val32[0]);
 			u.val32[1] = bswap_32(u.val32[1]);
 		}
 		data->raw_size = u.val32[0];
+
+		/*
+		 * The raw data is aligned on 64bits including the
+		 * u32 size, so it's safe to use mem_bswap_64.
+		 */
+		if (swapped)
+			mem_bswap_64((void *) array, data->raw_size);
+
 		array = (void *)array + sizeof(u32);
 
 		OVERFLOW_CHECK(array, data->raw_size, max_size);
Signature · 1 line
2.13.6
#3 lore raw
In reply to Jiri Olsa

Em Wed, Nov 29, 2017 at 07:43:46PM +0100, Jiri Olsa escreveu:

Quoted 10 lines

The fix is basically to swap back the raw data if different endianity is detected.

+++ b/tools/perf/util/evsel.c

tools/perf/util/evsel.c +1 -0
@@ -36,6 +36,7 @@
 #include "trace-event.h"
 #include "stat.h"
+#include "memswap.h"
 

tools/perf/util/evsel.c int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event, · line 2136

Quoted 5 lines
tools/perf/util/evsel.c +3 -0
@@ -2131,14 +2132,27 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (type & PERF_SAMPLE_RAW) {
 
+		if (swapped)
+			mem_bswap_64((void *) array, data->raw_size);
+

you forgot to add util/memswap.c to tools/perf/util/python-ext-sources, breaking 'perf test python'.

Please run 'perf test' before sending pull requests :-)

Fixed it this time.

- Arnaldo

#4 lore raw
Quoted 21 lines · Arnaldo Carvalho de Melo

Em Wed, Nov 29, 2017 at 07:43:46PM +0100, Jiri Olsa escreveu:

Quoted 10 lines

The fix is basically to swap back the raw data if different endianity is detected.

+++ b/tools/perf/util/evsel.c

tools/perf/util/evsel.c +1 -0
@@ -36,6 +36,7 @@
 #include "trace-event.h"
 #include "stat.h"
+#include "memswap.h"
 

tools/perf/util/evsel.c int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event, · line 2136

Quoted 5 lines
tools/perf/util/evsel.c +3 -0
@@ -2131,14 +2132,27 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (type & PERF_SAMPLE_RAW) {
 
+		if (swapped)
+			mem_bswap_64((void *) array, data->raw_size);
+

you forgot to add util/memswap.c to tools/perf/util/python-ext-sources, breaking 'perf test python'.

Quoted 2 lines

Please run 'perf test' before sending pull requests :-)

ouch ;-)

Quoted 1 line

Fixed it this time.

thanks,
jirka

#5 lore raw

[tip:perf/core] perf evsel: Fix swap for samples with raw data

In reply to Jiri Olsa
perf evsel: Fix swap for samples with raw data
Commit-ID:  f9d8adb345d7adbb2d3431eea73beb89c8d6d612
Gitweb:     https://git.kernel.org/tip/f9d8adb345d7adbb2d3431eea73beb89c8d6d612
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 29 Nov 2017 19:43:46 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 Dec 2017 12:15:56 -0300

perf evsel: Fix swap for samples with raw data

When we detect a different endianity we swap event before processing. It's tricky for samples because we have no idea what's inside. We treat it as an array of u64s, swap them and later on we swap back parts which are different.

We mangle this way also the tracepoint raw data, which ends up in report showing wrong data:

1.95% comm=Q^B pid=29285 prio=16777216 target_cpu=000 1.67% comm=l^B pid=0 prio=16777216 target_cpu=000

Luckily the traceevent library handles the endianity by itself (thank you Steven!), so we can pass the RAW data directly in the other endianity.

2.51% comm=beah-rhts-task pid=1175 prio=120 target_cpu=002 2.23% comm=kworker/0:0 pid=11566 prio=120 target_cpu=000

The fix is basically to swap back the raw data if different endianity is detected.

Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Steven Rostedt <[email protected]>
[ Add util/memswap.c to python-ext-sources to link missing mem_bswap_64() ]
---
 tools/perf/util/evsel.c            | 20 +++++++++++++++++---
 tools/perf/util/python-ext-sources |  1 +
 2 files changed, 18 insertions(+), 3 deletions(-)
tools/perf/util/evsel.c +17 -3
index 4718f0a..1cf044c 100644
@@ -36,6 +36,7 @@
 #include "debug.h"
 #include "trace-event.h"
 #include "stat.h"
+#include "memswap.h"
 #include "util/parse-branch-options.h"
 
 #include "sane_ctype.h"
@@ -2131,14 +2132,27 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	if (type & PERF_SAMPLE_RAW) {
 		OVERFLOW_CHECK_u64(array);
 		u.val64 = *array;
-		if (WARN_ONCE(swapped,
-			      "Endianness of raw data not corrected!\n")) {
-			/* undo swap of u64, then swap on individual u32s */
+
+		/*
+		 * Undo swap of u64, then swap on individual u32s,
+		 * get the size of the raw area and undo all of the
+		 * swap. The pevent interface handles endianity by
+		 * itself.
+		 */
+		if (swapped) {
 			u.val64 = bswap_64(u.val64);
 			u.val32[0] = bswap_32(u.val32[0]);
 			u.val32[1] = bswap_32(u.val32[1]);
 		}
 		data->raw_size = u.val32[0];
+
+		/*
+		 * The raw data is aligned on 64bits including the
+		 * u32 size, so it's safe to use mem_bswap_64.
+		 */
+		if (swapped)
+			mem_bswap_64((void *) array, data->raw_size);
+
 		array = (void *)array + sizeof(u32);
 
 		OVERFLOW_CHECK(array, data->raw_size, max_size);
tools/perf/util/python-ext-sources +1 -0
index b4f2f06..7aa0ea6 100644
@@ -10,6 +10,7 @@ util/ctype.c
 util/evlist.c
 util/evsel.c
 util/cpumap.c
+util/memswap.c
 util/mmap.c
 util/namespaces.c
 ../lib/bitmap.c