From mboxrd@z Thu Jan 1 00:00:00 1970 From: robherring2@gmail.com (Rob Herring) Date: Fri, 05 Oct 2012 07:24:44 -0500 Subject: alignment faults in 3.6 In-Reply-To: <20121005082439.GF4625@n2100.arm.linux.org.uk> References: <506E1762.3010601@gmail.com> <506E3E58.80703@gmail.com> <20121005071216.GD4625@n2100.arm.linux.org.uk> <20121005082439.GF4625@n2100.arm.linux.org.uk> Message-ID: <506ED18C.3010009@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 10/05/2012 03:24 AM, Russell King - ARM Linux wrote: > On Fri, Oct 05, 2012 at 09:20:56AM +0100, Mans Rullgard wrote: >> On 5 October 2012 08:12, Russell King - ARM Linux >> wrote: >>> On Fri, Oct 05, 2012 at 03:25:16AM +0100, Mans Rullgard wrote: >>>> On 5 October 2012 02:56, Rob Herring wrote: >>>>> This struct is the IP header, so a struct ptr is just set to the >>>>> beginning of the received data. Since ethernet headers are 14 bytes, >>>>> often the IP header is not aligned unless the NIC can place the frame at >>>>> a 2 byte offset (which is something I need to investigate). So this >>>>> function cannot make any assumptions about the alignment. Does the ABI >>>>> define structs have some minimum alignment? Does the struct need to be >>>>> declared as packed or something? >>>> >>>> The ABI defines the alignment of structs as the maximum alignment of its >>>> members. Since this struct contains 32-bit members, the alignment for the >>>> whole struct becomes 32 bits as well. Declaring it as packed tells gcc it >>>> might be unaligned (in addition to removing any holes within). >>> >>> This has come up before in the past. >>> >>> The Linux network folk will _not_ allow - in any shape or form - for >>> this struct to be marked packed (it's the struct which needs to be >>> marked packed) because by doing so, it causes GCC to issue byte loads/ >>> stores on architectures where there isn't a problem, and that decreases >>> the performance of the Linux IP stack unnecessarily. >> >> Which architectures? I have never seen anything like that. > > Does it matter? I'm just relaying the argument against adding __packed > which was used before we were forced (by the networking folk) to implement > the alignment fault handler. It doesn't really matter what will be accepted or not as adding __packed to struct iphdr doesn't fix the problem anyway. gcc still emits a ldm. The only way I've found to eliminate the alignment fault is adding a barrier between the 2 loads. That seems like a compiler issue to me if there is not a better fix. Rob