breakage: flex mmap patch for x86-64

breakage: flex mmap patch for x86-64

[Ricky Beam]

ChangeSet Key: 4188122b4-5vHFkvfD5Pc0Egjyaz8w

======== ChangeSet 1.2424.7.12 ========
D 1.2424.7.12 04/11/02 15:03:07-08:00 ak@suse.de[torvalds] 53848 53847 3/0/1
P ChangeSet
C [PATCH] x86_64: flex mmap patch for x86-64
C
C From: Arjan van de Ven <arjanv@redhat.com>
C
C Do flex mmap for x86-64.  mmaps will grow down in the address space now
C instead of up.
C
C Patch has 2 parts; the generic strategy selection, and code to make a correct
C TASK_SIZE .  the later may be fixed in your tree already in which case it's
C redundant.
C
C Modified by AK to apply to 64bit processes too.
C
C Signed-off-by: Andrew Morton <akpm@osdl.org>
C Signed-off-by: Linus Torvalds <torvalds@osdl.org>
------------------------------------------------

This prevents 32bit apps from running on x86_64.  Backing out the Makefile
and processor.h changes has everything working again.  Perhaps something
needs to check for a 32bit environment?  I don't know if it's the change
to TASK_SIZE or the "backwards" mmaps that's the real breakage.  And at this
point, I don't have time to test.

(64bit apps work just fine.)

--Ricky

Re: breakage: flex mmap patch for x86-64

[Rafael J. Wysocki]

On Friday 05 of November 2004 23:24, Ricky Beam wrote:
> ChangeSet Key: 4188122b4-5vHFkvfD5Pc0Egjyaz8w
> 
> ======== ChangeSet 1.2424.7.12 ========
> D 1.2424.7.12 04/11/02 15:03:07-08:00 ak@suse.de[torvalds] 53848 53847 3/0/1
> P ChangeSet
> C [PATCH] x86_64: flex mmap patch for x86-64
> C
> C From: Arjan van de Ven <arjanv@redhat.com>
> C
> C Do flex mmap for x86-64.  mmaps will grow down in the address space now
> C instead of up.
> C
> C Patch has 2 parts; the generic strategy selection, and code to make a 
correct
> C TASK_SIZE .  the later may be fixed in your tree already in which case 
it's
> C redundant.
> C
> C Modified by AK to apply to 64bit processes too.
> C
> C Signed-off-by: Andrew Morton <akpm@osdl.org>
> C Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> ------------------------------------------------
> 
> This prevents 32bit apps from running on x86_64.  Backing out the Makefile
> and processor.h changes has everything working again.  Perhaps something
> needs to check for a 32bit environment?  I don't know if it's the change
> to TASK_SIZE or the "backwards" mmaps that's the real breakage.  And at this
> point, I don't have time to test.
> 
> (64bit apps work just fine.)

Confirmed, and apparently it is not sifficient to change the TASK_SIZE 
definition in include/asm-x86_64/processor.h to make the 32-bit userland 
work.  Hence, it seems that the "backwards" mmaps break things.

Greets,
RJW

-- 
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
		-- Lewis Carroll "Alice's Adventures in Wonderland"

Re: breakage: flex mmap patch for x86-64

[Ricky Beam]

On Fri, 5 Nov 2004, Rafael J. Wysocki wrote:
>> This prevents 32bit apps from running on x86_64.  Backing out the Makefile
>> and processor.h changes has everything working again.  Perhaps something
>> needs to check for a 32bit environment?  I don't know if it's the change
>> to TASK_SIZE or the "backwards" mmaps that's the real breakage.  And at this
>> point, I don't have time to test.
>>
>> (64bit apps work just fine.)
>
>Confirmed, and apparently it is not sifficient to change the TASK_SIZE
>definition in include/asm-x86_64/processor.h to make the 32-bit userland
>work.  Hence, it seems that the "backwards" mmaps break things.

Looks like checking for PER_LINUX32 might fix it...

>>>      if (current->personality & (ADDR_COMPAT_LAYOUT|PER_LINUX32))

--Ricky

Re: breakage: flex mmap patch for x86-64

[Rafael J. Wysocki]

On Friday 05 of November 2004 23:54, Ricky Beam wrote:
> On Fri, 5 Nov 2004, Rafael J. Wysocki wrote:
> >> This prevents 32bit apps from running on x86_64.  Backing out the 
Makefile
> >> and processor.h changes has everything working again.  Perhaps something
> >> needs to check for a 32bit environment?  I don't know if it's the change
> >> to TASK_SIZE or the "backwards" mmaps that's the real breakage.  And at 
this
> >> point, I don't have time to test.
> >>
> >> (64bit apps work just fine.)
> >
> >Confirmed, and apparently it is not sifficient to change the TASK_SIZE
> >definition in include/asm-x86_64/processor.h to make the 32-bit userland
> >work.  Hence, it seems that the "backwards" mmaps break things.
> 
> Looks like checking for PER_LINUX32 might fix it...
> 
> >>>      if (current->personality & (ADDR_COMPAT_LAYOUT|PER_LINUX32))

It does not seem to work either.

Greets,
RJW

-- 
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
		-- Lewis Carroll "Alice's Adventures in Wonderland"

Re: breakage: flex mmap patch for x86-64

[Ricky Beam]

On Sat, 6 Nov 2004, Rafael J. Wysocki wrote:
>> Looks like checking for PER_LINUX32 might fix it...
>>
>> >>>      if (current->personality & (ADDR_COMPAT_LAYOUT|PER_LINUX32))
>
>It does not seem to work either.

... because the personality isn't set (as one might expect.)  TIF_IA32 is
set, however.

The second diff (processor.h) isn't necessary to fix the bug, but does
remove some duplication that I don't trust gcc to do on it's own.

--Ricky

===== arch/x86_64/mm/mmap.c 1.1 vs edited =====
--- 1.1/arch/x86_64/mm/mmap.c   2004-11-02 09:40:35 -05:00
+++ edited/arch/x86_64/mm/mmap.c        2004-11-05 23:31:40 -05:00
@@ -49,6 +49,9 @@

 static inline int mmap_is_legacy(void)
 {
+       if (test_thread_flag(TIF_IA32))
+               return 1;
+
        if (current->personality & ADDR_COMPAT_LAYOUT)
                return 1;

===== include/asm-x86_64/processor.h 1.43 vs edited =====
--- 1.43/include/asm-x86_64/processor.h 2004-11-02 09:40:35 -05:00
+++ edited/include/asm-x86_64/processor.h       2004-11-05 22:13:24 -05:00
@@ -170,7 +170,7 @@
  */
 #define IA32_PAGE_OFFSET ((current->personality & ADDR_LIMIT_3GB) ? 0xc0000000 : 0xFFFFe000)
 #define TASK_UNMAPPED_32 PAGE_ALIGN(IA32_PAGE_OFFSET/3)
-#define TASK_UNMAPPED_64 PAGE_ALIGN(TASK_SIZE/3)
+#define TASK_UNMAPPED_64 PAGE_ALIGN(TASK_SIZE_64/3)
 #define TASK_UNMAPPED_BASE     \
        (test_thread_flag(TIF_IA32) ? TASK_UNMAPPED_32 : TASK_UNMAPPED_64)

Re: breakage: flex mmap patch for x86-64

[Andi Kleen]

>  static inline int mmap_is_legacy(void)
>  {
> +       if (test_thread_flag(TIF_IA32))
> +               return 1;

That's definitely not the right fix because for 32bit you need flexmmap
more than for 64bit because it gives you more address space.

-Andi

Re: breakage: flex mmap patch for x86-64

[Rafael J. Wysocki]

On Saturday 06 of November 2004 10:12, Andi Kleen wrote:
> >  static inline int mmap_is_legacy(void)
> >  {
> > +       if (test_thread_flag(TIF_IA32))
> > +               return 1;
> 
> That's definitely not the right fix because for 32bit you need flexmmap
> more than for 64bit because it gives you more address space.

So let's call it temporary, but I like 32-bit apps having less address space 
rather than segfaulting.

Greets,
RJW

-- 
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
		-- Lewis Carroll "Alice's Adventures in Wonderland"

Re: breakage: flex mmap patch for x86-64

[Andi Kleen]

On Sat, Nov 06, 2004 at 10:50:27AM +0100, Rafael J. Wysocki wrote:
> On Saturday 06 of November 2004 10:12, Andi Kleen wrote:
> > >  static inline int mmap_is_legacy(void)
> > >  {
> > > +       if (test_thread_flag(TIF_IA32))
> > > +               return 1;
> > 
> > That's definitely not the right fix because for 32bit you need flexmmap
> > more than for 64bit because it gives you more address space.
> 
> So let's call it temporary, but I like 32-bit apps having less address space 
> rather than segfaulting.

If you want a temporary fix use the appended one.  But I think Linus pulled it anyways.

-Andi


diff -u linux-2.6.10rc1-mm3/kernel/sysctl.c-o linux-2.6.10rc1-mm3/kernel/sysctl.c
--- linux-2.6.10rc1-mm3/kernel/sysctl.c-o	2004-11-05 11:42:00.000000000 +0100
+++ linux-2.6.10rc1-mm3/kernel/sysctl.c	2004-11-06 13:50:22.000000000 +0100
@@ -147,7 +147,7 @@
 #endif
 
 #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
-int sysctl_legacy_va_layout;
+int sysctl_legacy_va_layout = 1;
 #endif
 
 /* /proc declarations: */

Re: breakage: flex mmap patch for x86-64

[Ricky Beam]

On Sat, 6 Nov 2004, Andi Kleen wrote:
>On Sat, Nov 06, 2004 at 10:50:27AM +0100, Rafael J. Wysocki wrote:
>> On Saturday 06 of November 2004 10:12, Andi Kleen wrote:
>> > >  static inline int mmap_is_legacy(void)
>> > >  {
>> > > +       if (test_thread_flag(TIF_IA32))
>> > > +               return 1;
>> >
>> > That's definitely not the right fix because for 32bit you need flexmmap
>> > more than for 64bit because it gives you more address space.
>>
>> So let's call it temporary, but I like 32-bit apps having less address space
>> rather than segfaulting.
>
>If you want a temporary fix use the appended one.  But I think Linus pulled it anyways.

Right, wrong, whatever.  Using the legacy mmap for IA32 works.  And it works
exactly as before flexmmap was added.  Setting "vm.legacy_va_layout" will
work, but disables flexmmap across the board.

What is it with flexmmap that's causing 32bit apps to fail?

--Ricky