On Oct 4, 2012, at 8:04 PM, Rob Herring <robherri...@gmail.com> wrote:
> On 10/04/2012 09:25 PM, Mans Rullgard wrote: >> On 5 October 2012 02:56, Rob Herring <robherri...@gmail.com> wrote: >>> On 10/04/2012 08:26 PM, Mans Rullgard wrote: >>>> On 5 October 2012 01:58, Michael Hope <michael.h...@linaro.org> wrote: >>>>> On 5 October 2012 12:10, Rob Herring <robherri...@gmail.com> wrote: >>>>>> I've been scratching my head with a "scheduling while atomic" bug I >>>>>> started seeing on 3.6. I can easily reproduce this problem when doing a >>>>>> wget on my system. It ultimately seems to be a combination of factors. >>>>>> The "scheduling while atomic" bug is triggered in do_alignment which >>>>>> gets triggered by this code in net/ipv4/af_inet.c, line 1356: >>>>>> >>>>>> id = ntohl(*(__be32 *)&iph->id); >>>>>> flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (id ^ IP_DF)); >>>>>> id >>= 16; >>>>>> >>>>>> This code compiles into this using "gcc version 4.6.3 (Ubuntu/Linaro >>>>>> 4.6.3-1ubuntu5)": >>>>>> >>>>>> c02ac020: e8920840 ldm r2, {r6, fp} >>>>>> c02ac024: e6bfbf3b rev fp, fp >>>>>> c02ac028: e6bf6f36 rev r6, r6 >>>>>> c02ac02c: e22bc901 eor ip, fp, #16384 ; 0x4000 >>>>>> c02ac030: e0266008 eor r6, r6, r8 >>>>>> c02ac034: e18c6006 orr r6, ip, r6 >>>>>> >>>>>> which generates alignment faults on the ldm. These are silent until this >>>>>> commit is applied: >>>>> >>>>> Hi Rob. I assume that iph is something like: >>>>> >>>>> struct foo { >>>>> u32 x; >>>>> char id[8]; >>>>> }; >>>>> >>>>> struct foo *iph; >>>>> >>>>> GCC merged the two adjacent loads of x and id into one ldm. This is >>>>> an ARM specific optimisation done in load_multiple_sequence() and >>>>> enabled with -fpeephole2. >>>>> >>>>> I think the assembly is correct - GCC knows that iph is aligned and >>>>> knows the offsets of both x and id. Happy to be corrected if I'm >>>>> wrong, but I think the assembly is valid given the C code. >>>> >>>> The struct looks like this: >>>> >>>> struct iphdr { >>>> #if defined(__LITTLE_ENDIAN_BITFIELD) >>>> __u8 ihl:4, >>>> version:4; >>>> #elif defined (__BIG_ENDIAN_BITFIELD) >>>> __u8 version:4, >>>> ihl:4; >>>> #else >>>> #error "Please fix <asm/byteorder.h>" >>>> #endif >>>> __u8 tos; >>>> __be16 tot_len; >>>> __be16 id; >>>> __be16 frag_off; >>>> __u8 ttl; >>>> __u8 protocol; >>>> __sum16 check; >>>> __be32 saddr; >>>> __be32 daddr; >>>> /*The options start here. */ >>>> }; >>>> >>>> In a normal build (there's some magic for special checkers) __be32 is a >>>> plain >>>> __u32 so the struct should be at least 4-byte aligned. If somehow it is >>>> not, >>>> that is the real bug. >>> >>> 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). > > Unfortunately, declaring the struct or __be32* cast as packed have no > effect. I still get an ldm emitted. what is value of r2 ? and can you pastebin the .i file somewhere ? > > Rob > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linaro-toolchain mailing list linaro-toolchain@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-toolchain