From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751858AbdF1Sxs (ORCPT ); Wed, 28 Jun 2017 14:53:48 -0400 Received: from mail.kernel.org ([198.145.29.99]:50198 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751523AbdF1Sxm (ORCPT ); Wed, 28 Jun 2017 14:53:42 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EEA2B22B60 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 15:53:37 -0300 From: Arnaldo Carvalho de Melo To: Adrian Hunter 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: <20170628185337.GC3342@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: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 {