Re: [PATCH] perf tools: Fix swap for samples with raw data

[Arnaldo Carvalho de Melo]

Em Wed, Nov 29, 2017 at 07:43:46PM +0100, Jiri Olsa escreveu:
> 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
 
> 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 <dsahern@xxxxxxxxx>
> Cc: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> Link: http://lkml.kernel.org/n/tip-0zcnwqf2k4pq854huwxc1v4l@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  tools/perf/util/evsel.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 95853c51c0ca..09c68cf2d9d2 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -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);
> -- 
> 2.13.6