FLAT: allow arches to declare a larger alignment than the slab
in [Kernel]
|
From: Geert Uytterhoeven on 25 May 2010 16:00 On Tue, May 25, 2010 at 21:24, Mike Frysinger <vapier(a)gentoo.org> wrote: > From: Jie Zhang <jie.zhang(a)analog.com> > > The recent commit 1f0ce8b3dd667dca7 which moved the ARCH_SLAB_MINALIGN > default into the global header inadvertently broke FLAT for a bunch of > systems. Blackfin systems now fail on any FLAT exec with: > Unable to read code+data+bss, errno 14 > When your /init is a FLAT binary, obviously this can be annoying ;). > > This stems from the alignment usage in the FLAT loader. The behavior > before was that FLAT would default to ARCH_SLAB_MINALIGN only if it was > defined, and this was only defined by arches when they wanted a larger > alignment value. Otherwise it'd default to pointer alignment. Arguably, ^^^^^^^^^^^^^^^^^ That would be __alignof__(void *) > +#elif defined(ARCH_SLAB_MINALIGN) > #define FLAT_DATA_ALIGN (ARCH_SLAB_MINALIGN) > #else > #define FLAT_DATA_ALIGN (sizeof(void *)) Not sizeof(void *) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mike Frysinger on 25 May 2010 19:20 On Tue, May 25, 2010 at 17:07, Paul Mundt wrote: > On Tue, May 25, 2010 at 03:24:27PM -0400, Mike Frysinger wrote: >> This stems from the alignment usage in the FLAT loader. The behavior >> before was that FLAT would default to ARCH_SLAB_MINALIGN only if it was >> defined, and this was only defined by arches when they wanted a larger >> alignment value. Otherwise it'd default to pointer alignment. Arguably, >> this is kind of hokey that the FLAT is semi-abusing defines it shouldn't. > > This needs some explaining. What exactly do you find problematic with > ARCH_SLAB_MINALIGN in this case? For the case that was introduced leading > up to the wrapping of the minalign value it was absolutely the proper > thing to use. If blackfin has special alignment requirements on top of > that, then that's certainly fine, but it doesn't negate the validity of > the minalign wrapping for the other platforms. the Blackfin processor only requires alignment according to the natural type sizes for 8, 16, and 32 bits. so "char" needs no alignment, "short" needs 2 byte alignment, and "int" & "void*" need 4 byte alignment. these translate directly into a minimum aligned size for .data sections as 4 bytes, and similarly for the stack. FLAT is using ARCH_SLAB_MINALIGN to align the stack and align the data section. as such, Blackfin needs 4 byte alignment here. the previous FLAT behavior was "use arch slab sizes if defined, otherwise use sizeof(void*)". this worked fine for us size sizeof(void*) == 4. now with ARCH_SLAB_MINALIGN being in global space, this defaults to 0 for us and the manual stack & data alignment no longer work. i'm a schlub when it comes to these allocators, so i know as much as the documentation states. slab_def.h says: * Enforce a minimum alignment for all caches. * Intended for archs that get misalignment faults even for BYTES_PER_WORD * aligned buffers. this comment does not seem to apply to Blackfin as BYTES_PER_WORD is 4 and we can work with anything aligned to 4 bytes. >> /* >> - * User data (stack, data section and bss) needs to be aligned >> - * for the same reasons as SLAB memory is, and to the same amount. >> + * User data (stack, data section and bss) needs to be aligned. >> + * If ARCH_FLAT_DATA_ALIGN is defined, use it. >> + */ > > If you're going to update the comment, the update should at least serve > some purpose. This not only obscures the reason for the slab minalign > wrapping, it also fails to suggest why anyone would deviate from that. > > If the intention is that ARCH_FLAT_DATA_ALIGN provides cacheline > alignment on blackfin, then use ARCH_KMALLOC_MINALIGN like everyone else. i do not believe that is the reason for this, but unfortunately Jie is about the only one atm who knows the inner details as for why shared FLAT libraries requires 0x20 rather than just 0x4 alignment. i do know that there are some gcc fortran tests that fail otherwise. hopefully he can remember details ;). to be sure, we dont need 0x20 alignment in general. i just figured kill two birds with one patch here. and Blackfin is already setting ARCH_KMALLOC_MINALIGN to cacheline size, but that wouldnt make any difference in these issues. -mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Geert Uytterhoeven on 26 May 2010 03:00 On Wed, May 26, 2010 at 04:23, Jie Zhang <jie(a)codesourcery.com> wrote: > On 05/26/2010 07:17 AM, Mike Frysinger wrote: >> >> i do not believe that is the reason for this, but unfortunately Jie is >> about the only one atm who knows the inner details as for why shared >> FLAT libraries requires 0x20 rather than just 0x4 alignment. i do >> know that there are some gcc fortran tests that fail otherwise. >> hopefully he can remember details ;). >> > I encountered this issue when investigating some GCC test failures when > using FLAT. I don't remember if they were in GCC Fortran testsuite. Some > variables in those test cases were required to be aligned at a large > boundary, for example 16-byte. I found 0x20 was a reasonably large alignment > to fix all such failures in GCC testsuite. I'm no FLAT expert (except for the AmigaOS HUNK loader :-), but isn't the core of the issue that alignment requirements in the object file are no longer fulfilled after loading, as a FLAT segment in memory is just allocated using kmalloc(), which may now return 4-byte aligned blocks? From looking at <linux/flat.h>, it looks like the FLAT binary format doesn't contain any alignment information? So if I put __attribute__((aligned(4096))) in a file, there's still no guarantee it will actually be in memory at a 4Ki-aligned address? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mike Frysinger on 26 May 2010 03:30 On Wed, May 26, 2010 at 02:59, Geert Uytterhoeven wrote: > On Wed, May 26, 2010 at 04:23, Jie Zhang wrote: >> On 05/26/2010 07:17 AM, Mike Frysinger wrote: >>> >>> i do not believe that is the reason for this, but unfortunately Jie is >>> about the only one atm who knows the inner details as for why shared >>> FLAT libraries requires 0x20 rather than just 0x4 alignment. i do >>> know that there are some gcc fortran tests that fail otherwise. >>> hopefully he can remember details ;). >>> >> I encountered this issue when investigating some GCC test failures when >> using FLAT. I don't remember if they were in GCC Fortran testsuite. Some >> variables in those test cases were required to be aligned at a large >> boundary, for example 16-byte. I found 0x20 was a reasonably large alignment >> to fix all such failures in GCC testsuite. > > I'm no FLAT expert (except for the AmigaOS HUNK loader :-), but isn't > the core of the > issue that alignment requirements in the object file are no longer > fulfilled after loading, > as a FLAT segment in memory is just allocated using kmalloc(), which may now > return 4-byte aligned blocks? there are two issues. first, the ARCH_SLAB_MINALIGN define is used to align dynamically packed data on the stack. the alignment of kmalloc has no bearing here because things like the env get packed in and then the ARCH_SLAB_MINALIGN value is manually used to re-align the stack. second, the define is further used to manually set up alignment of the data section after it has been mmapped in anonymously and made room for shared library data. this too has no kmalloc bearing because mmap sucks things in by grabbing pages manually. > From looking at <linux/flat.h>, it looks like the FLAT binary format > doesn't contain any > alignment information? So if I put __attribute__((aligned(4096))) in a > file, there's still > no guarantee it will actually be in memory at a 4Ki-aligned address? i believe that is correct. FLAT behavior today provides alignment of either sizeof(unsigned long) or ARCH_SLAB_MINALIGN. i imagine something like this would work today because everyone defines it to a constant: -#ifdef ARCH_SLAB_MINALIGN +#if defined(ARCH_SLAB_MINALIGN) && ARCH_SLAB_MINALIGN != 0 but this would break if someone tried using gcc sizeof/alignof/etc... -mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mike Frysinger on 26 May 2010 03:40 On Wed, May 26, 2010 at 03:33, Paul Mundt wrote: > On Wed, May 26, 2010 at 03:23:02AM -0400, Mike Frysinger wrote: >> On Wed, May 26, 2010 at 02:59, Geert Uytterhoeven wrote: >> > From looking at <linux/flat.h>, it looks like the FLAT binary format >> > doesn't contain any >> > alignment information? So if I put __attribute__((aligned(4096))) in a >> > file, there's still >> > no guarantee it will actually be in memory at a 4Ki-aligned address? >> >> i believe that is correct. FLAT behavior today provides alignment of >> either sizeof(unsigned long) or ARCH_SLAB_MINALIGN. >> >> i imagine something like this would work today because everyone >> defines it to a constant: >> -#ifdef ARCH_SLAB_MINALIGN >> +#if defined(ARCH_SLAB_MINALIGN) && ARCH_SLAB_MINALIGN != 0 >> but this would break if someone tried using gcc sizeof/alignof/etc... > > alignof is used by SLUB/SLOB to set the ARCH_SLAB_MINALIGN value if the > architecture hasn't already specified one, so that wouldn't work. sorry, the implied file here is the FLAT loader, not some other header. and the FLAT loader either uses the define directly in its math, or uses the ALIGN() macro. so alignof shouldnt matter. -mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Next
|
Last
Pages: 1 2 Prev: sata_nv times out for BD-ROM iHOS104-08 Next: powernow-k8: section mismatch |