[parisc-linux] [PATCH] fix arbitrary limits on stack size

[parisc-linux] [PATCH] fix arbitrary limits on stack size

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

One of the problems with the PA linux implementation is the upward
growing stack.  On most architectures, the virtual process space is
divided into


| unmapped | exec and heap |  mappings and stack |
                   TASK_UNMAPPED_BASE        TASK_SIZE


The mappings grow up from the start and the stack grows down from the
end of the area. If they ever meet, the process segfaults.

On palinux, we have an upward growing stack, so we have to size the
stack so when it reaches its ulimit, it hits the top of the area.  This
means that large stack limits cut into the mappable area (and also that
we have to impose a hard limit of 1GB on the stack size otherwise the
user could remove the ability to map processes entirely by setting the
stack limit too high).

The proposal to fix this is to start the stack growing upwards from
TASK_UNMAPPED_BASE and have the mappings grow downwards from TASK_SIZE. 
This should allow us to behave in exactly the same manner as x86 and not
have an arbitrary limit on the stack size.

The attached patch (against 2.5.70-pa1) does this, if you'd like to
comment on it or try it out.

James



[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 7909 bytes --]

===== arch/parisc/kernel/sys_parisc.c 1.8 vs edited =====
--- 1.8/arch/parisc/kernel/sys_parisc.c	Mon Mar 17 19:15:27 2003
+++ edited/arch/parisc/kernel/sys_parisc.c	Sat Jul 12 10:51:09 2003
@@ -26,22 +26,59 @@
 	return error;
 }
 
+/* Routine to find unshared mappings.  If we cannot satisfy the
+ * address and length exactly, we start at the top of memory and check
+ * down.  Since the vm_area_struct is designed for a forward search,
+ * not a backward one, we cache the address of the lowest assigned
+ * mapping in current->mm->free_area_cache.
+ */
 static unsigned long get_unshared_area(unsigned long addr, unsigned long len)
 {
 	struct vm_area_struct *vma;
+	struct vm_area_struct *prev_vma;
 
-	if (!addr)
-		addr = TASK_UNMAPPED_BASE;
-	addr = PAGE_ALIGN(addr);
+	if (len > TASK_FREE_AREA_CACHE)
+		return -ENOMEM;
 
-	for (vma = find_vma(current->mm, addr); ; vma = vma->vm_next) {
-		/* At this point:  (!vma || addr < vma->vm_end). */
-		if (TASK_SIZE - len < addr)
-			return -ENOMEM;
-		if (!vma || addr + len <= vma->vm_start)
+	/* quick check to see if we can satisfy the fixed address */
+	if (addr && (addr &= PAGE_MASK), TASK_SIZE - len <= addr) {
+		vma = find_vma_prev(current->mm, addr, &prev_vma);
+		if ((!vma || addr + len <= vma->vm_start)
+		    && (!prev_vma || addr >= prev_vma->vm_end)) {
+			printk("FIXED AREA MAP AT %lx-%lx\n",
+			       addr, addr+len);
 			return addr;
-		addr = vma->vm_end;
+		}
+	}
+
+	/* start from the lowest cached address */
+	addr = (current->mm->free_area_cache - len) & PAGE_MASK;
+	
+	for(;;) {
+		if (addr < TASK_UNMAPPED_BASE)
+			goto err;
+
+		vma = find_vma_prev(current->mm, addr, &prev_vma);
+			
+
+		/* At this point:  (!vma || addr < vma->vm_end). */
+		if ((!vma || addr + len <= vma->vm_start)
+		    && (!prev_vma || addr >= prev_vma->vm_end))
+				goto found;
+
+		/* OK, go backwards one mapping and try again */
+		if(vma)
+			addr = (vma->vm_start - len) & PAGE_MASK;
+		else
+			addr = (prev_vma->vm_start - len) & PAGE_MASK;
+
 	}
+ found:
+	current->mm->free_area_cache = addr;
+	return addr;
+ err:
+	printk("RETURNING -ENOMEM\n");
+	return -ENOMEM;
 }
 
 #define DCACHE_ALIGN(addr) (((addr) + (SHMLBA - 1)) &~ (SHMLBA - 1))
@@ -49,26 +86,50 @@
 static unsigned long get_shared_area(struct inode *inode, unsigned long addr,
 		unsigned long len, unsigned long pgoff)
 {
-	struct vm_area_struct *vma, *first_vma;
+	struct vm_area_struct *vma, *first_vma, *prev_vma;
 	int offset;
 
 	first_vma = list_entry(inode->i_mapping->i_mmap_shared.next, struct vm_area_struct, shared);
 	offset = (first_vma->vm_start + ((pgoff - first_vma->vm_pgoff) << PAGE_SHIFT)) & (SHMLBA - 1);
 
-	if (!addr)
-		addr = TASK_UNMAPPED_BASE;
+	if (addr && (addr = DCACHE_ALIGN(addr - offset) + offset),
+	    TASK_SIZE - len <= addr) {
+		vma = find_vma_prev(current->mm, addr, &prev_vma);
+		if ((!vma || addr + len <= vma->vm_start)
+		    && (!prev_vma || addr >= prev_vma->vm_end)) {
+			printk("SHARED FIXED AREA MAP AT %lx-%lx\n",
+			       addr, addr+len);
+			return addr;
+		}
+	}
+
+	addr = (current->mm->free_area_cache - len) & PAGE_MASK;
 	addr = DCACHE_ALIGN(addr - offset) + offset;
 
-	for (vma = find_vma(current->mm, addr); ; vma = vma->vm_next) {
+	for(;;) {
+		if (addr < TASK_UNMAPPED_BASE)
+			goto err;
+
+		vma = find_vma_prev(current->mm, addr, &prev_vma);
+
 		/* At this point:  (!vma || addr < vma->vm_end). */
-		if (TASK_SIZE - len < addr)
-			return -ENOMEM;
-		if (!vma || addr + len <= vma->vm_start)
-			return addr;
-		addr = DCACHE_ALIGN(vma->vm_end - offset) + offset;
-		if (addr < vma->vm_end) /* handle wraparound */
-			return -ENOMEM;
+		if ((!vma || addr + len <= vma->vm_start)
+		    && (!prev_vma || addr >= prev_vma->vm_end))
+			goto found;
+
+		if(vma)
+			addr = (vma->vm_start - len) & PAGE_MASK;
+		else
+			addr = (prev_vma->vm_start - len) & PAGE_MASK;
+
+		addr = DCACHE_ALIGN(addr - offset) + offset;
 	}
+ found:
+	printk("SHARED RETURNING ADDR %lx\n", addr);
+	return addr;
+ err:
+	printk("SHARED RETURNING -ENOMEM\n");
+	return -ENOMEM;
 }
 
 unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
===== fs/binfmt_aout.c 1.16 vs edited =====
--- 1.16/fs/binfmt_aout.c	Sat Feb 15 21:30:17 2003
+++ edited/fs/binfmt_aout.c	Fri Jul 11 18:17:28 2003
@@ -308,7 +308,7 @@
 		(current->mm->start_data = N_DATADDR(ex));
 	current->mm->brk = ex.a_bss +
 		(current->mm->start_brk = N_BSSADDR(ex));
-	current->mm->free_area_cache = TASK_UNMAPPED_BASE;
+	current->mm->free_area_cache = TASK_FREE_AREA_CACHE;
 
 	current->mm->rss = 0;
 	current->mm->mmap = NULL;
===== fs/binfmt_elf.c 1.49 vs edited =====
--- 1.49/fs/binfmt_elf.c	Wed Jul  2 11:08:30 2003
+++ edited/fs/binfmt_elf.c	Fri Jul 11 18:17:15 2003
@@ -635,7 +635,7 @@
 	/* Do this so that we can load the interpreter, if need be.  We will
 	   change some of these later */
 	current->mm->rss = 0;
-	current->mm->free_area_cache = TASK_UNMAPPED_BASE;
+	current->mm->free_area_cache = TASK_FREE_AREA_CACHE;
 	retval = setup_arg_pages(bprm);
 	if (retval < 0) {
 		send_sig(SIGKILL, current, 0);
===== fs/exec.c 1.92 vs edited =====
--- 1.92/fs/exec.c	Thu Jul 10 10:56:33 2003
+++ edited/fs/exec.c	Fri Jul 11 20:30:50 2003
@@ -375,11 +375,8 @@
 	/* Adjust bprm->p to point to the end of the strings. */
 	bprm->p = PAGE_SIZE * i - offset;
 
-	/* Limit stack size to 1GB */
-	stack_base = current->rlim[RLIMIT_STACK].rlim_max;
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
-	stack_base = PAGE_ALIGN(STACK_TOP - stack_base);
+
+	stack_base = PAGE_ALIGN(TASK_UNMAPPED_BASE);
 
 	mm->arg_start = stack_base;
 	arg_size = i << PAGE_SHIFT;
===== include/asm-parisc/processor.h 1.11 vs edited =====
--- 1.11/include/asm-parisc/processor.h	Mon Jun 23 10:25:25 2003
+++ edited/include/asm-parisc/processor.h	Sat Jul 12 11:00:45 2003
@@ -41,6 +41,13 @@
 #define TASK_UNMAPPED_BASE      (current->thread.map_base)
 #define DEFAULT_MAP_BASE        (0x40000000UL)
 
+/* This defines the start of the cached area for the downward growing
+ * maps.  NOTE: The elf interpreter seems to insist on having a fixed
+ * mapping address just above the text for its data and other
+ * allocations, so space must be left for that between this and the
+ * end of usable memory (TASK_SIZE) */
+#define TASK_FREE_AREA_CACHE	(TASK_SIZE-0x800000)
+
 #ifndef __ASSEMBLY__
 
 /*
===== include/linux/mm.h 1.124 vs edited =====
--- 1.124/include/linux/mm.h	Sun Jul  6 15:23:51 2003
+++ edited/include/linux/mm.h	Sat Jul 12 11:11:39 2003
@@ -30,6 +30,14 @@
 #define MM_VM_SIZE(mm)	TASK_SIZE
 #endif
 
+/* This defines the value current->mm->free_area_cache will be set to
+ * for every new process.  If you define your own
+ * arch_get_unmapped_area() you may override this in
+ * asm/processor.h */
+#ifndef TASK_FREE_AREA_CACHE
+#define TASK_FREE_AREA_CACHE	TASK_UNMAPPED_BASE
+#endif
+
 /*
  * Linux kernel virtual memory manager primitives.
  * The idea being to have a "virtual" mm in the same way
===== kernel/fork.c 1.130 vs edited =====
--- 1.130/kernel/fork.c	Sat Jul  5 01:52:49 2003
+++ edited/kernel/fork.c	Fri Jul 11 18:16:41 2003
@@ -262,7 +262,7 @@
 	mm->locked_vm = 0;
 	mm->mmap = NULL;
 	mm->mmap_cache = NULL;
-	mm->free_area_cache = TASK_UNMAPPED_BASE;
+	mm->free_area_cache = TASK_FREE_AREA_CACHE;
 	mm->map_count = 0;
 	mm->rss = 0;
 	mm->cpu_vm_mask = 0;
@@ -376,7 +376,7 @@
 	mm->page_table_lock = SPIN_LOCK_UNLOCKED;
 	mm->ioctx_list_lock = RW_LOCK_UNLOCKED;
 	mm->default_kioctx = (struct kioctx)INIT_KIOCTX(mm->default_kioctx, *mm);
-	mm->free_area_cache = TASK_UNMAPPED_BASE;
+	mm->free_area_cache = TASK_FREE_AREA_CACHE;
 
 	if (likely(!mm_alloc_pgd(mm))) {
 		mm->def_flags = 0;

RE: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[Joel Soete]

>One of the problems with the PA linux implementation is the upward
>growing stack.  On most architectures, the virtual process space is
>divided into
>
>
>| unmapped | exec and heap |  mappings and stack | TASK_UNMAPPED_BASE  
     
>TASK_SIZE
>
>
>The mappings grow up from the start and the stack grows down from the
>end of the area. If they ever meet, the process segfaults.
>
>On palinux, we have an upward growing stack, so we have to size the
>stack so when it reaches its ulimit, it hits the top of the area.  This
>means that large stack limits cut into the mappable area (and also that
>we have to impose a hard limit of 1GB on the stack size otherwise the
>user could remove the ability to map processes entirely by setting the
>stack limit too high).
>
>The proposal to fix this is to start the stack growing upwards from
>TASK_UNMAPPED_BASE and have the mappings grow downwards from TASK_SIZE.

>This should allow us to behave in exactly the same manner as x86 and not
>have an arbitrary limit on the stack size.
>
>The attached patch (against 2.5.70-pa1) does this, if you'd like to
>comment on it or try it out.

I don' t remember the historical reason of this choice but do you think it
is possible to back-port it into 2.4 (it will facilitate so much the merge
with upstream?)

Thanks in advance for advise,
    Joel

------------------------------------------------------
Soldes Tiscali ADSL : 27,50 euros/mois jusque fin 2003.
On s'habitue vite à payer son ADSL moins cher!
Plus d'info? Cliquez ici... http://reg.tiscali.be/default.asp?lg=fr 

Re: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[Matthew Wilcox]

On Mon, Jul 14, 2003 at 12:48:56PM +0200, Joel Soete wrote:
> I don' t remember the historical reason of this choice but do you think it
> is possible to back-port it into 2.4 (it will facilitate so much the merge
> with upstream?)

2.6.0-test1 is out.  IMO, all work on 2.4 should cease.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

RE: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[James Bottomley]

On Mon, 2003-07-14 at 05:48, Joel Soete wrote:
> I don' t remember the historical reason of this choice but do you think it
> is possible to back-port it into 2.4 (it will facilitate so much the merge
> with upstream?)

Well, I suppose, theoretically.

However, before we consider backporting, what I really need is for
people to test it.  Downward growing mappings may cause application
issues:  If you map to an unspecified address, you may be surprised to
discover that you can't expand the mapping upwards.  Of course, no
application should ever depend on being able to expand unspecified maps
in either direction, but you never know...

James

RE: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[Joel Soete]

>
>On Mon, 2003-07-14 at 05:48, Joel Soete wrote:
> I don' t remember the historical reason of this choice but do you think
it
> is possible to back-port it into 2.4 (it will facilitate so much the merge
> with upstream?)

>Well, I suppose, theoretically.

> However, before we consider backporting, what I really need is for
> people to test it. 

Ok Jame, Now I have a 32bits 2.6.0-test1-pa1 running so I come back to you.

No problem to apply it :) , so I can launch the compile.

Just a question in the mean time: it doesn't seens that your patch take care
of CONFIG_STACK_GROWSUP in related files?

Thanks,
    Joel


------------------------------------------------------
Soldes Tiscali ADSL : 27,50 euros/mois jusque fin 2003.
On s'habitue vite à payer son ADSL moins cher!
Plus d'info? Cliquez ici... http://reg.tiscali.be/default.asp?lg=fr 

Re: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[Matthew Wilcox]

On Wed, Jul 16, 2003 at 07:30:29PM +0200, Joel Soete wrote:
> Ok Jame, Now I have a 32bits 2.6.0-test1-pa1 running so I come back to you.
> 
> No problem to apply it :) , so I can launch the compile.
> 
> Just a question in the mean time: it doesn't seens that your patch take care
> of CONFIG_STACK_GROWSUP in related files?

um, the stack continues to grow up.  it's just a question of where the
stack starts at.  before, it was at top of memory - stack limit, now it's
at task_unmapped_base and mmaps grow down from the top of memory.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [parisc-linux] [PATCH] fix arbitrary limits on stack size

[Joel Soete]

> um, the stack continues to grow up.  it's just a question of where the
> stack starts at.  before, it was at top of memory - stack limit, now it's
> at task_unmapped_base and mmaps grow down from the top of memory.

Ok I missunderstood.

This said, I applied against 2.6.0-test1-pa1. It compiled well and also boot
well on my b180 of test (still have a pb with b2k and N; another story).

I also try to re-compile kernel 2.6 reboot with this last one: all seems
ok.

hth,
    Joel


------------------------------------------------------
Soldes Tiscali ADSL : 27,50 euros/mois jusque fin 2003.
On s'habitue vite à payer son ADSL moins cher!
Plus d'info? Cliquez ici... http://reg.tiscali.be/default.asp?lg=fr