Em Wed, Jun 28, 2017 at 08:21:37PM +0000, Hunter, Adrian escreveu: > Sorry for the top-post... > > Yeah, I've now mixed up the variable attribute: > > > https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes > > with the type attribute: > > > https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#Common-Type-Attributes > > Late here, so maybe it will make more sense tomorrow.
Right, and I've not been able to focus on this, but I think the problem is with packed mixed with unnamed unions :-\ - Arnaldo > -----Original Message----- > From: Arnaldo Carvalho de Melo [mailto:[email protected]] > Sent: Wednesday, June 28, 2017 9:54 PM > To: Hunter, Adrian <[email protected]> > Cc: Andi Kleen <[email protected]>; [email protected] > Subject: Re: [PATCH V2 25/37] perf script: Add synthesized Intel PT power and > ptwrite events > > Em Wed, Jun 28, 2017 at 08:40:25PM +0300, Adrian Hunter escreveu: > > 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: > > Humm, can you provide the URL for such docs? I always saw packed as an > attribute for a struct, not for a member... For members "aligned" is what I'm > used to see: > > __attribute__ ((aligned (8))) > > In the kernel sources there are a few such cases as you suggest: > > [acme@jouet linux]$ find include/ -name "*.h"| xargs grep -w __packed | grep > -v } | grep -v "struct __packed" | wc -l > 12 > [acme@jouet linux]$ > > But most are the other way, i.e. tagging the packed attribute to the whole > struct, as you originally did :-\ > > - Arnaldo > > > 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 {
