Re: [PATCH V2 25/37] perf script: Add synthesized Intel PT power and ptwrite events

Adrian Hunter
2017-06-28T17:46:38Z

On 06/28/2017 04:04 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 26, 2017 at 11:17:26AM +0300, Adrian Hunter escreveu:
>> Add definitions for synthesized Intel PT events for power and ptwrite.
>
>> +++ b/tools/perf/util/event.h
>> +/*
>> + * Raw data formats for synthesized events. Note that raw data plus the raw data
>> + * size (4 bytes) must align to 8-bytes.
>> + */
>> +
>> +struct perf_synth_intel_ptwrite {
>> + union {
>> + struct {
>> + u32 ip : 1,
>> + reserved : 31;
>> + };
>> + u32 flags;
>> + };
>> + u64 payload;
>> +} __packed;
>
>
> some versions of clang and gcc dislike this __packed here:
>
> In file included from builtin-script.c:5:
> In file included from /git/linux/tools/perf/util/debug.h:8:
> /git/linux/tools/perf/util/event.h:274:2: error: packed attribute is unnecessary for (null) [-Werror,-Wpacked]
> union {
> ^
> /git/linux/tools/perf/util/event.h:285:6: error: packed attribute is unnecessary for 'reserved' [-Werror,-Wpacked]
> u32 reserved;
> ^
> /git/linux/tools/perf/util/event.h:298:6: error: packed attribute is unnecessary for 'reserved' [-Werror,-Wpacked]
> u32 reserved;
> ^
> /git/linux/tools/perf/util/event.h:322:6: error: packed attribute is unnecessary for 'reserved' [-Werror,-Wpacked]
> u32 reserved;
> ^
> 4 errors generated.
> mv: can't rename '/tmp/build/perf/.builtin-script.o.tmp': No such file or directory
>
> /git/linux/tools/build/Makefile.build:101: recipe for target '/tmp/build/perf/builtin-script.o' failed
>
> Failing in various distros:
>
> [root@jouet ~]# waitp 3940 ; time dm
> 1 92.3684147260 alpine:3.4: FAIL
> 2 95.9136365930 alpine:3.5: FAIL
> 3 104.8328303770 alpine:3.6: FAIL
> 4 121.6584964930 alpine:edge: FAIL
> 5 37.2536373490 android-ndk:r12b-arm: Ok
> 6 83.9077612370 archlinux:latest: Ok
> 7 14.7094639200 centos:5: FAIL
> 8 16.6371634320 centos:6: FAIL
>
> Investigating...

Re-reading the documentation for __packed, it seems like the following
might be better:

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index c283603f59c7..a7547cb3b760 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -278,8 +278,8 @@ struct perf_synth_intel_ptwrite {
};
u32 flags;
};
- u64 payload;
-} __packed;
+ u64 payload __packed;
+};

struct perf_synth_intel_mwait {
u32 reserved;
@@ -291,8 +291,8 @@ struct perf_synth_intel_mwait {
reserved2 : 30;
};
u64 payload;
- };
-} __packed;
+ } __packed;
+};

struct perf_synth_intel_pwre {
u32 reserved;
@@ -305,8 +305,8 @@ struct perf_synth_intel_pwre {
reserved2 : 48;
};
u64 payload;
- };
-} __packed;
+ } __packed;
+};

struct perf_synth_intel_exstop {
union {
@@ -328,8 +328,8 @@ struct perf_synth_intel_pwrx {
reserved1 : 52;
};
u64 payload;
- };
-} __packed;
+ } __packed;
+};

struct perf_synth_intel_cbr {
union {