Messages in this thread Patch in this message | | | Date | Wed, 28 Jun 2017 15:53:37 -0300 | From | Arnaldo Carvalho de Melo <> | 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 {
| |