From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751888AbdF1U0n (ORCPT ); Wed, 28 Jun 2017 16:26:43 -0400 Received: from mail.kernel.org ([198.145.29.99]:56272 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751808AbdF1U0b (ORCPT ); Wed, 28 Jun 2017 16:26:31 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 56AAC22B5A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Wed, 28 Jun 2017 17:26:21 -0300 From: Arnaldo Carvalho de Melo To: "Hunter, Adrian" Cc: Andi Kleen , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH V2 25/37] perf script: Add synthesized Intel PT power and ptwrite events Message-ID: <20170628202621.GA12064@kernel.org> References: <1495786658-18063-1-git-send-email-adrian.hunter@intel.com> <1495786658-18063-26-git-send-email-adrian.hunter@intel.com> <20170628130438.GB3342@kernel.org> <20170628185337.GC3342@kernel.org> <363DA0ED52042842948283D2FC38E4649BEC774E@IRSMSX106.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <363DA0ED52042842948283D2FC38E4649BEC774E@IRSMSX106.ger.corp.intel.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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:acme@kernel.org] > Sent: Wednesday, June 28, 2017 9:54 PM > To: Hunter, Adrian > Cc: Andi Kleen ; linux-kernel@vger.kernel.org > 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 {