[PATCH] FLAT: allow arches to declare a larger alignment than the slab

[PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

From: Jie Zhang <jie.zhang@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,
this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.

But let's ignore that and let arches declare a larger FLAT alignment
specifically anyways as some arches are OK with the default slab alignment
but need stricter FLAT alignments for shared FLAT libraries.

The nommu arches might want to check to see if they need to declare this
in their headers as well ...

Signed-off-by: Jie Zhang <jie.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/include/asm/flat.h |    2 ++
 fs/binfmt_flat.c                 |   11 ++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/blackfin/include/asm/flat.h b/arch/blackfin/include/asm/flat.h
index c1314c5..cf2a73e 100644
--- a/arch/blackfin/include/asm/flat.h
+++ b/arch/blackfin/include/asm/flat.h
@@ -11,6 +11,8 @@
 
 #include <asm/unaligned.h>
 
+#define ARCH_FLAT_DATA_ALIGN			0x20
+
 #define	flat_argvp_envp_on_stack()		0
 #define	flat_old_ram_flag(flags)		(flags)
 
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 49566c1..6906170 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -56,12 +56,17 @@
 #endif
 
 /*
- * 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.
+ */
+#ifdef ARCH_FLAT_DATA_ALIGN
+#define FLAT_DATA_ALIGN	(ARCH_FLAT_DATA_ALIGN)
+/* Otherwise user data nees to be aligned for the same reasons
+ * as SLAB memory is aligned, and to the same amount.
  * Avoid duplicating architecture specific code by using the same
  * macro as with SLAB allocation:
  */
-#ifdef ARCH_SLAB_MINALIGN
+#elif defined(ARCH_SLAB_MINALIGN)
 #define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
 #else
 #define FLAT_DATA_ALIGN	(sizeof(void *))
-- 
1.7.1

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Geert Uytterhoeven]

On Tue, May 25, 2010 at 21:24, Mike Frysinger <vapier@gentoo.org> wrote:
> From: Jie Zhang <jie.zhang@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@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

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Paul Mundt]

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.

>  /*
> - * 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.

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

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

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Jie Zhang]

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.


Jie

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Geert Uytterhoeven]

On Wed, May 26, 2010 at 04:23, Jie Zhang <jie@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@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

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

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

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Michal Simek]

Mike Frysinger wrote:
> From: Jie Zhang <jie.zhang@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,
> this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.
> 
> But let's ignore that and let arches declare a larger FLAT alignment
> specifically anyways as some arches are OK with the default slab alignment
> but need stricter FLAT alignments for shared FLAT libraries.
> 
> The nommu arches might want to check to see if they need to declare this
> in their headers as well ...

Microblaze noMMU contains this fault too. I registered it yesterday.
I will use the same fix as you when you find out the correct solution. :-)

Michal



> 
> Signed-off-by: Jie Zhang <jie.zhang@analog.com>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  arch/blackfin/include/asm/flat.h |    2 ++
>  fs/binfmt_flat.c                 |   11 ++++++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/blackfin/include/asm/flat.h b/arch/blackfin/include/asm/flat.h
> index c1314c5..cf2a73e 100644
> --- a/arch/blackfin/include/asm/flat.h
> +++ b/arch/blackfin/include/asm/flat.h
> @@ -11,6 +11,8 @@
>  
>  #include <asm/unaligned.h>
>  
> +#define ARCH_FLAT_DATA_ALIGN			0x20
> +
>  #define	flat_argvp_envp_on_stack()		0
>  #define	flat_old_ram_flag(flags)		(flags)
>  
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 49566c1..6906170 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -56,12 +56,17 @@
>  #endif
>  
>  /*
> - * 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.
> + */
> +#ifdef ARCH_FLAT_DATA_ALIGN
> +#define FLAT_DATA_ALIGN	(ARCH_FLAT_DATA_ALIGN)
> +/* Otherwise user data nees to be aligned for the same reasons
> + * as SLAB memory is aligned, and to the same amount.
>   * Avoid duplicating architecture specific code by using the same
>   * macro as with SLAB allocation:
>   */
> -#ifdef ARCH_SLAB_MINALIGN
> +#elif defined(ARCH_SLAB_MINALIGN)
>  #define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
>  #define FLAT_DATA_ALIGN	(sizeof(void *))


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Paul Mundt]

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.

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

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

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

On Tue, May 25, 2010 at 22: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.

ok, i found the reports Jie worked on originally.  the 0x20 value isnt
a hardware restriction or anything.  some gcc tests use the alignment
directive and then double check that it was respected.  the way the
FLAT loader crams things into the start of the data page before
appending the data breaks this.  so 0x20 was selected because, as Jie
said, it seemed to satisfy all of the gcc tests.

presumably this issue could be resolved by changing the FLAT loader to
append this internal state data instead of prepending.  that would fix
FLAT behavior wrt alignment directives (up to a page).
-mike

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Paul Mundt]

On Tue, May 25, 2010 at 07:17:16PM -0400, Mike Frysinger wrote:
> 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.
> 
The comment here is a bit misleading, it's true that SLAB will happily
align to BYTES_PER_WORD as a default if nothing else is specified, but
it's also true that a growing number of structures contain 64-bit values
that need to be accessed on 64-bit boundaries on 32-bit platforms. If
this doesn't apply to blackfin then simply sticking with the default is
fine. SLAB/SLOB will align to 4 bytes while SLUB will presently align to
8.

> 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.

I have no objections to adding a new alignment value for binfmt_flat, but
given the confusion that exists around things like ARCH_SLAB_MINALIGN and
ARCH_KMALLOC_MINALIGN already today it should be quite obvious what
exactly the new value is for and what case it is specifically addressing.
My guess is that the issues you are seeing with the gcc testsuite will
also pop up on other nommu platforms, so it may be something we want to
just deal with generically. At least I suspect you guys are running the
gcc testsuite a lot more frequently than the rest of us!

Re: [PATCH] FLAT: allow arches to declare a larger alignment than the slab

[Mike Frysinger]

On Wed, May 26, 2010 at 03:48, Paul Mundt wrote:
> On Tue, May 25, 2010 at 07:17:16PM -0400, Mike Frysinger wrote:
>> 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.
>
> I have no objections to adding a new alignment value for binfmt_flat, but
> given the confusion that exists around things like ARCH_SLAB_MINALIGN and
> ARCH_KMALLOC_MINALIGN already today it should be quite obvious what
> exactly the new value is for and what case it is specifically addressing.
> My guess is that the issues you are seeing with the gcc testsuite will
> also pop up on other nommu platforms, so it may be something we want to
> just deal with generically. At least I suspect you guys are running the
> gcc testsuite a lot more frequently than the rest of us!

looking at the linker scripts elf2flt uses, i'm wondering if perhaps
we shouldnt use 0x20 for the FLAT data chunk all the time.  it uses
ALIGN(0x20) when packing in the rodata/data/etc... sections, and
obviously this would only work if the starting alignment were 0x20+ to
begin with.

elf2flt.ld.in:
...
    . = ALIGN(0x20) ;
    *(.rodata)
    *(.rodata1)
    *(.rodata.*)
    *(.gnu.linkonce.r*)
    *(.data)
    *(.data1)
    *(.data.*)
    *(.gnu.linkonce.d*)
...

that would address our gcc test issues, but not the current initial
loading crash
-mike

[PATCH 1/2 v2] FLAT: split the stack & data alignments

[Mike Frysinger]

The stack and data have different alignment requirements, so don't force
them to wear the same shoe.  Increase the data alignment to match that
which the elf2flt linker script has always been using: 0x20 bytes.  Not
only does this bring the kernel loader in line with the toolchain, but
it also fixes a swath of gcc tests which try to force larger alignment
values but randomly fail when the FLAT loader fails to deliver.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- split changes & document better

 fs/binfmt_flat.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 49566c1..b865622 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -56,15 +56,22 @@
 #endif
 
 /*
- * User data (stack, data section and bss) needs to be aligned
- * for the same reasons as SLAB memory is, and to the same amount.
- * Avoid duplicating architecture specific code by using the same
- * macro as with SLAB allocation:
+ * User data (data section and bss) needs to be aligned.
+ * We pick 0x20 here because it is the max value elf2flt has always
+ * used in producing FLAT files, and because it seems to be large
+ * enough to make all the gcc alignment related tests happy.
+ */
+#define FLAT_DATA_ALIGN	(0x20)
+
+/*
+ * User data (stack) also needs to be aligned.
+ * Here we can be a bit looser than the data sections since this
+ * needs to only meet arch ABI requirements.
  */
 #ifdef ARCH_SLAB_MINALIGN
-#define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
+#define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
 #else
-#define FLAT_DATA_ALIGN	(sizeof(void *))
+#define FLAT_STACK_ALIGN	(sizeof(void *))
 #endif
 
 #define RELOC_FAILED 0xff00ff01		/* Relocation incorrect somewhere */
@@ -129,7 +136,7 @@ static unsigned long create_flat_tables(
 
 	sp = (unsigned long *)p;
 	sp -= (envc + argc + 2) + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
-	sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN);
+	sp = (unsigned long *) ((unsigned long)sp & -FLAT_STACK_ALIGN);
 	argv = sp + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
 	envp = argv + (argc + 1);
 
@@ -876,7 +883,7 @@ static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs)
 	stack_len = TOP_OF_ARGS - bprm->p;             /* the strings */
 	stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
 	stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
-	stack_len += FLAT_DATA_ALIGN - 1;  /* reserve for upcoming alignment */
+	stack_len += FLAT_STACK_ALIGN - 1;  /* reserve for upcoming alignment */
 	
 	res = load_flat_file(bprm, &libinfo, 0, &stack_len);
 	if (IS_ERR_VALUE(res))
-- 
1.7.1

[PATCH 2/2 v2] FLAT: tweak default stack alignment

[Mike Frysinger]

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,
this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.

But let's ignore that and simply ignore min alignment values of 0.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- split changes & document better

 fs/binfmt_flat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index b865622..4959a0a 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -68,7 +68,7 @@
  * Here we can be a bit looser than the data sections since this
  * needs to only meet arch ABI requirements.
  */
-#ifdef ARCH_SLAB_MINALIGN
+#if defined(ARCH_SLAB_MINALIGN) && ARCH_SLAB_MINALIGN != 0
 #define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
 #else
 #define FLAT_STACK_ALIGN	(sizeof(void *))
-- 
1.7.1

Re: [PATCH 1/2 v2] FLAT: split the stack & data alignments

[Michal Simek]

Mike Frysinger wrote:
> The stack and data have different alignment requirements, so don't force
> them to wear the same shoe.  Increase the data alignment to match that
> which the elf2flt linker script has always been using: 0x20 bytes.  Not
> only does this bring the kernel loader in line with the toolchain, but
> it also fixes a swath of gcc tests which try to force larger alignment
> values but randomly fail when the FLAT loader fails to deliver.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Solve the problem on Microblaze:

Tested-by: Michal Simek <monstr@monstr.eu>

Who will add it to mainline?

Thanks,
Michal

> ---
> v2
> 	- split changes & document better
> 
>  fs/binfmt_flat.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 49566c1..b865622 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -56,15 +56,22 @@
>  #endif
>  
>  /*
> - * User data (stack, data section and bss) needs to be aligned
> - * for the same reasons as SLAB memory is, and to the same amount.
> - * Avoid duplicating architecture specific code by using the same
> - * macro as with SLAB allocation:
> + * User data (data section and bss) needs to be aligned.
> + * We pick 0x20 here because it is the max value elf2flt has always
> + * used in producing FLAT files, and because it seems to be large
> + * enough to make all the gcc alignment related tests happy.
> + */
> +#define FLAT_DATA_ALIGN	(0x20)
> +
> +/*
> + * User data (stack) also needs to be aligned.
> + * Here we can be a bit looser than the data sections since this
> + * needs to only meet arch ABI requirements.
>   */
>  #ifdef ARCH_SLAB_MINALIGN
> -#define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
> +#define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
> -#define FLAT_DATA_ALIGN	(sizeof(void *))
> +#define FLAT_STACK_ALIGN	(sizeof(void *))
>  #endif
>  
>  #define RELOC_FAILED 0xff00ff01		/* Relocation incorrect somewhere */
> @@ -129,7 +136,7 @@ static unsigned long create_flat_tables(
>  
>  	sp = (unsigned long *)p;
>  	sp -= (envc + argc + 2) + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
> -	sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN);
> +	sp = (unsigned long *) ((unsigned long)sp & -FLAT_STACK_ALIGN);
>  	argv = sp + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
>  	envp = argv + (argc + 1);
>  
> @@ -876,7 +883,7 @@ static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs)
>  	stack_len = TOP_OF_ARGS - bprm->p;             /* the strings */
>  	stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
>  	stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
> -	stack_len += FLAT_DATA_ALIGN - 1;  /* reserve for upcoming alignment */
> +	stack_len += FLAT_STACK_ALIGN - 1;  /* reserve for upcoming alignment */
>  	
>  	res = load_flat_file(bprm, &libinfo, 0, &stack_len);
>  	if (IS_ERR_VALUE(res))


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [Uclinux-dist-devel] [PATCH 1/2 v2] FLAT: split the stack & data alignments

[Mike Frysinger]

On Thu, May 27, 2010 at 04:24, Michal Simek wrote:
> Mike Frysinger wrote:
>> The stack and data have different alignment requirements, so don't force
>> them to wear the same shoe.  Increase the data alignment to match that
>> which the elf2flt linker script has always been using: 0x20 bytes.  Not
>> only does this bring the kernel loader in line with the toolchain, but
>> it also fixes a swath of gcc tests which try to force larger alignment
>> values but randomly fail when the FLAT loader fails to deliver.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> Solve the problem on Microblaze:
>
> Tested-by: Michal Simek <monstr@monstr.eu>
>
> Who will add it to mainline?

if we can get the misc nommu peeps to agree on this (looking mostly at
David [McCullough] and Greg), then akpm will most likely expedite the
merge.  i like to think they're the best FLAT experts out there ;).
-mike

Re: [microblaze-uclinux] Re: [Uclinux-dist-devel] [PATCH 1/2 v2] FLAT: split the stack & data alignments

[David McCullough]

Jivin Mike Frysinger lays it down ...
> On Thu, May 27, 2010 at 04:24, Michal Simek wrote:
> > Mike Frysinger wrote:
> >> The stack and data have different alignment requirements, so don't force
> >> them to wear the same shoe. ??Increase the data alignment to match that
> >> which the elf2flt linker script has always been using: 0x20 bytes. ??Not
> >> only does this bring the kernel loader in line with the toolchain, but
> >> it also fixes a swath of gcc tests which try to force larger alignment
> >> values but randomly fail when the FLAT loader fails to deliver.
> >>
> >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >
> > Solve the problem on Microblaze:
> >
> > Tested-by: Michal Simek <monstr@monstr.eu>
> >
> > Who will add it to mainline?
> 
> if we can get the misc nommu peeps to agree on this (looking mostly at
> David [McCullough] and Greg), then akpm will most likely expedite the
> merge.  i like to think they're the best FLAT experts out there ;).

Did the latest patch include the changes Paul requested.

It looks like it to me,  I've just been a bit distracted.  If so I'll send
in an Ack.

Cheers,
Davidm

-- 
David McCullough,      david_mccullough@mcafee.com,  Ph:+61 734352815
McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org

Re: [microblaze-uclinux] Re: [Uclinux-dist-devel] [PATCH 1/2 v2] FLAT: split the stack & data alignments

[Mike Frysinger]

On Thu, May 27, 2010 at 19:15, David McCullough wrote:
> Jivin Mike Frysinger lays it down ...
>> On Thu, May 27, 2010 at 04:24, Michal Simek wrote:
>> > Mike Frysinger wrote:
>> >> The stack and data have different alignment requirements, so don't force
>> >> them to wear the same shoe. ??Increase the data alignment to match that
>> >> which the elf2flt linker script has always been using: 0x20 bytes. ??Not
>> >> only does this bring the kernel loader in line with the toolchain, but
>> >> it also fixes a swath of gcc tests which try to force larger alignment
>> >> values but randomly fail when the FLAT loader fails to deliver.
>> >
>> > Solve the problem on Microblaze:
>> >
>> > Tested-by: Michal Simek <monstr@monstr.eu>
>> >
>> > Who will add it to mainline?
>>
>> if we can get the misc nommu peeps to agree on this (looking mostly at
>> David [McCullough] and Greg), then akpm will most likely expedite the
>> merge.  i like to think they're the best FLAT experts out there ;).
>
> Did the latest patch include the changes Paul requested.
>
> It looks like it to me,  I've just been a bit distracted.  If so I'll send
> in an Ack.

i dont want to rush you or something ... the patches need to be in
before 2.6.35 is released (since trouble started with a patched added
to 2.6.35 merge window), so if you want to review things a bit more,
it should be OK ..
-mike

Re: [PATCH 1/2 v2] FLAT: split the stack & data alignments

[Mike Frysinger]

On Wed, May 26, 2010 at 04:45, Mike Frysinger wrote:
> The stack and data have different alignment requirements, so don't force
> them to wear the same shoe.  Increase the data alignment to match that
> which the elf2flt linker script has always been using: 0x20 bytes.  Not
> only does this bring the kernel loader in line with the toolchain, but
> it also fixes a swath of gcc tests which try to force larger alignment
> values but randomly fail when the FLAT loader fails to deliver.

btw, a follow up patch might be to move the shared lib identifiers
from the start of the data section to the end of it so that the
re-aligning isnt necessary (we'd get a 4k page alignment from mmap and
such).  but i cant seem to figure out how these identifiers are being
read/written.  otherwise, the fact that we're force aligning to 0x20
bytes means that there is always room for 8 identifiers ... no point
in flipping between 1 or 4, at least from this point of view ...
-mike

Re: [PATCH 1/2 v2] FLAT: split the stack & data alignments

[David McCullough]

Jivin Mike Frysinger lays it down ...
> The stack and data have different alignment requirements, so don't force
> them to wear the same shoe.  Increase the data alignment to match that
> which the elf2flt linker script has always been using: 0x20 bytes.  Not
> only does this bring the kernel loader in line with the toolchain, but
> it also fixes a swath of gcc tests which try to force larger alignment
> values but randomly fail when the FLAT loader fails to deliver.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: David McCullough <david_mccullough@mcafee.com>

Cheers,
Davidm

> ---
> v2
> 	- split changes & document better
> 
>  fs/binfmt_flat.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 49566c1..b865622 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -56,15 +56,22 @@
>  #endif
>  
>  /*
> - * User data (stack, data section and bss) needs to be aligned
> - * for the same reasons as SLAB memory is, and to the same amount.
> - * Avoid duplicating architecture specific code by using the same
> - * macro as with SLAB allocation:
> + * User data (data section and bss) needs to be aligned.
> + * We pick 0x20 here because it is the max value elf2flt has always
> + * used in producing FLAT files, and because it seems to be large
> + * enough to make all the gcc alignment related tests happy.
> + */
> +#define FLAT_DATA_ALIGN	(0x20)
> +
> +/*
> + * User data (stack) also needs to be aligned.
> + * Here we can be a bit looser than the data sections since this
> + * needs to only meet arch ABI requirements.
>   */
>  #ifdef ARCH_SLAB_MINALIGN
> -#define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
> +#define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
> -#define FLAT_DATA_ALIGN	(sizeof(void *))
> +#define FLAT_STACK_ALIGN	(sizeof(void *))
>  #endif
>  
>  #define RELOC_FAILED 0xff00ff01		/* Relocation incorrect somewhere */
> @@ -129,7 +136,7 @@ static unsigned long create_flat_tables(
>  
>  	sp = (unsigned long *)p;
>  	sp -= (envc + argc + 2) + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
> -	sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN);
> +	sp = (unsigned long *) ((unsigned long)sp & -FLAT_STACK_ALIGN);
>  	argv = sp + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
>  	envp = argv + (argc + 1);
>  
> @@ -876,7 +883,7 @@ static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs)
>  	stack_len = TOP_OF_ARGS - bprm->p;             /* the strings */
>  	stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
>  	stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
> -	stack_len += FLAT_DATA_ALIGN - 1;  /* reserve for upcoming alignment */
> +	stack_len += FLAT_STACK_ALIGN - 1;  /* reserve for upcoming alignment */
>  	
>  	res = load_flat_file(bprm, &libinfo, 0, &stack_len);
>  	if (IS_ERR_VALUE(res))
> -- 
> 1.7.1
> 
> 
> 

-- 
David McCullough,      david_mccullough@mcafee.com,  Ph:+61 734352815
McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org

Re: [PATCH 2/2 v2] FLAT: tweak default stack alignment

[David McCullough]

Jivin Mike Frysinger lays it down ...
> 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,
> this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.
> 
> But let's ignore that and simply ignore min alignment values of 0.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: David McCullough <david_mccullough@mcafee.com>

Cheers,
Davidm

> ---
> v2
> 	- split changes & document better
> 
>  fs/binfmt_flat.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index b865622..4959a0a 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -68,7 +68,7 @@
>   * Here we can be a bit looser than the data sections since this
>   * needs to only meet arch ABI requirements.
>   */
> -#ifdef ARCH_SLAB_MINALIGN
> +#if defined(ARCH_SLAB_MINALIGN) && ARCH_SLAB_MINALIGN != 0
>  #define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
>  #define FLAT_STACK_ALIGN	(sizeof(void *))
> -- 
> 1.7.1
> 
> 
> 

-- 
David McCullough,      david_mccullough@mcafee.com,  Ph:+61 734352815
McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org

Re: [PATCH 2/2 v2] FLAT: tweak default stack alignment

[Greg Ungerer]

Mike Frysinger wrote:
> 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,
> this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.
> 
> But let's ignore that and simply ignore min alignment values of 0.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: Greg Ungerer <gerg@uclinux.org>



> ---
> v2
> 	- split changes & document better
> 
>  fs/binfmt_flat.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index b865622..4959a0a 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -68,7 +68,7 @@
>   * Here we can be a bit looser than the data sections since this
>   * needs to only meet arch ABI requirements.
>   */
> -#ifdef ARCH_SLAB_MINALIGN
> +#if defined(ARCH_SLAB_MINALIGN) && ARCH_SLAB_MINALIGN != 0
>  #define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
>  #define FLAT_STACK_ALIGN	(sizeof(void *))


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH 1/2 v2] FLAT: split the stack & data alignments

[Greg Ungerer]

Mike Frysinger wrote:
> The stack and data have different alignment requirements, so don't force
> them to wear the same shoe.  Increase the data alignment to match that
> which the elf2flt linker script has always been using: 0x20 bytes.  Not
> only does this bring the kernel loader in line with the toolchain, but
> it also fixes a swath of gcc tests which try to force larger alignment
> values but randomly fail when the FLAT loader fails to deliver.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: Greg Ungerer <gerg@uclinux.org>


> ---
> v2
> 	- split changes & document better
> 
>  fs/binfmt_flat.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 49566c1..b865622 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -56,15 +56,22 @@
>  #endif
>  
>  /*
> - * User data (stack, data section and bss) needs to be aligned
> - * for the same reasons as SLAB memory is, and to the same amount.
> - * Avoid duplicating architecture specific code by using the same
> - * macro as with SLAB allocation:
> + * User data (data section and bss) needs to be aligned.
> + * We pick 0x20 here because it is the max value elf2flt has always
> + * used in producing FLAT files, and because it seems to be large
> + * enough to make all the gcc alignment related tests happy.
> + */
> +#define FLAT_DATA_ALIGN	(0x20)
> +
> +/*
> + * User data (stack) also needs to be aligned.
> + * Here we can be a bit looser than the data sections since this
> + * needs to only meet arch ABI requirements.
>   */
>  #ifdef ARCH_SLAB_MINALIGN
> -#define FLAT_DATA_ALIGN	(ARCH_SLAB_MINALIGN)
> +#define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
>  #else
> -#define FLAT_DATA_ALIGN	(sizeof(void *))
> +#define FLAT_STACK_ALIGN	(sizeof(void *))
>  #endif
>  
>  #define RELOC_FAILED 0xff00ff01		/* Relocation incorrect somewhere */
> @@ -129,7 +136,7 @@ static unsigned long create_flat_tables(
>  
>  	sp = (unsigned long *)p;
>  	sp -= (envc + argc + 2) + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
> -	sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN);
> +	sp = (unsigned long *) ((unsigned long)sp & -FLAT_STACK_ALIGN);
>  	argv = sp + 1 + (flat_argvp_envp_on_stack() ? 2 : 0);
>  	envp = argv + (argc + 1);
>  
> @@ -876,7 +883,7 @@ static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs)
>  	stack_len = TOP_OF_ARGS - bprm->p;             /* the strings */
>  	stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
>  	stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
> -	stack_len += FLAT_DATA_ALIGN - 1;  /* reserve for upcoming alignment */
> +	stack_len += FLAT_STACK_ALIGN - 1;  /* reserve for upcoming alignment */
>  	
>  	res = load_flat_file(bprm, &libinfo, 0, &stack_len);
>  	if (IS_ERR_VALUE(res))


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

[PATCH v3] FLAT: tweak default stack alignment

[Mike Frysinger]

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,
this is kind of hokey that the FLAT is semi-abusing defines it shouldn't.

So let's merge the two alignment requirements so the floor is never 0.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- split changes & document better

v3
	- combine the diff align values via max_t

 fs/binfmt_flat.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index b6ab27c..811384b 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -68,11 +68,7 @@
  * Here we can be a bit looser than the data sections since this
  * needs to only meet arch ABI requirements.
  */
-#ifdef ARCH_SLAB_MINALIGN
-#define FLAT_STACK_ALIGN	(ARCH_SLAB_MINALIGN)
-#else
-#define FLAT_STACK_ALIGN	(sizeof(void *))
-#endif
+#define FLAT_STACK_ALIGN	max_t(unsigned long, sizeof(void *), ARCH_SLAB_MINALIGN)
 
 #define RELOC_FAILED 0xff00ff01		/* Relocation incorrect somewhere */
 #define UNLOADED_LIB 0x7ff000ff		/* Placeholder for unused library */
-- 
1.7.1