Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965147Ab3IDRx6 (ORCPT ); Wed, 4 Sep 2013 13:53:58 -0400 Received: from na3sys009aog135.obsmtp.com ([74.125.149.84]:49242 "HELO na3sys009aog135.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S964786Ab3IDRxv (ORCPT ); Wed, 4 Sep 2013 13:53:51 -0400 MIME-Version: 1.0 In-Reply-To: References: <20130829.181001.145658561677052552.davem@davemloft.net> From: Jesse Gross Date: Wed, 4 Sep 2013 10:53:29 -0700 Message-ID: Subject: Re: [-next] openvswitch BUILD_BUG_ON failed To: Geert Uytterhoeven Cc: David Miller , Andy Zhou , "dev@openvswitch.org" , netdev , Linux Kernel Mailing List , Linux-Next Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3314 Lines: 70 On Tue, Sep 3, 2013 at 11:55 PM, Geert Uytterhoeven wrote: > On Tue, Sep 3, 2013 at 11:44 PM, Jesse Gross wrote: >> On Sat, Aug 31, 2013 at 5:11 AM, Geert Uytterhoeven >> wrote: >>> On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross wrote: >>>> On Thu, Aug 29, 2013 at 3:10 PM, David Miller wrote: >>>>> From: Jesse Gross >>>>> Date: Thu, 29 Aug 2013 14:42:22 -0700 >>>>> >>>>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven >>>>>> wrote: >>>>>>> However, I have some doubts about other alignment "enforcements": >>>>>>> >>>>>>> "__aligned(__alignof__(long))" makes the whole struct aligned to the >>>>>>> alignment rule for "long": >>>>>>> 1. This is only 2 bytes on m68k, i.e. != sizeof(long). >>>>>>> 2. This is 4 bytes on many 32-bit platforms, which may be less than the >>>>>>> default alignment for "__be64" (cfr. some members of struct >>>>>>> ovs_key_ipv4_tunnel), so this may make those 64-bit members unaligned. >>>>>> >>>>>> Do any of those 32-bit architectures actually care about alignment of >>>>>> 64 bit values? On 32-bit x86, a long is 32 bits but the alignment >>>>>> requirement of __be64 is also 32 bit. >>>>> >>>>> All except x86-32 do, it is in fact the odd man out with respect to this >>>>> issue. >>>> >>>> Thanks, good to know. >>>> >>>> Andy, do you want to modify your patch to just drop the alignment >>>> specification as Geert suggested (but definitely keep the new build >>>> assert that you added)? It's probably better to just send the patch to >>>> netdev (against net-next) as well since you'll likely get better >>>> comments there and we can fix this faster if you cut out the >>>> middleman. >>> >>> Why do you want to keep the build asserts? >>> Is this in-memory structure also transfered as-is over the network? >>> If yes, you definitely want the padding. >> >> Well they caught this bug and really don't cost anything. >> >>> Nevertheless, as the struct contains u32 and even __be64 members, the >>> size of the struct will always be a multiple of the alignment unit for >>> 64-bit quantities (and thus also for long), as per the C standard. >>> Hence the check >>> >>> BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long)); >>> >>> will only catch bad compiler bugs or people adding __packed to the struct. >> >> It's possible that we might want to pack the structure in the future. >> More generally though, the contents of the struct is really >> independent of the alignment requirements here because we're accessing >> it as an array of bytes in long-sized chunks so implicitly depending >> on the size of the members is not that great. > > So you're accessing it as an array of bytes in long-sized chunks. > What are you doing with this accessed data? > Transfering over the network? > Storing on disk? > Then it must be portable across machines and architectures, right? It's just an in-memory hash table lookup. No one else ever sees it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/