Re: [PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

Re: [PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

[William Lee Irwin III]

On Thu, Aug 12, 2004 at 12:15:32AM -0400, Jeff Dike wrote:
> The patch below fixes a few UML-specific bugs not related to the rest of the
> kernel
> 	a bogus error return and some formatting in the fork code
> 	correct calculation of task.thread.kernel_stack
> 	remove a bogus panic
> 	a couple of fixes to allow UML to boot in the presence of exec-shield
[...]
>  	p->thread.kernel_stack = 
> -		(unsigned long) p->thread_info + THREAD_SIZE;
> +		(unsigned long) p->thread_info + 2 * PAGE_SIZE;
>  	return(CHOOSE_MODE_PROC(copy_thread_tt, copy_thread_skas, nr, 
>  				clone_flags, sp, stack_top, p, regs));

Out of curiosity, why are you allocating 4*PAGE_SIZE for the stack if
you're only going to use 2*PAGE_SIZE of it? I saw no other users for
the rest of ->thread_info offhand.


-- wli

[PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

[Jeff Dike]

The patch below fixes a few UML-specific bugs not related to the rest of the
kernel
	a bogus error return and some formatting in the fork code
	correct calculation of task.thread.kernel_stack
	remove a bogus panic
	a couple of fixes to allow UML to boot in the presence of exec-shield

				Jeff

Index: 2.6.8-rc4-mm1/arch/um/kernel/process.c
===================================================================
--- 2.6.8-rc4-mm1.orig/arch/um/kernel/process.c	2004-08-11 22:44:42.000000000 -0400
+++ 2.6.8-rc4-mm1/arch/um/kernel/process.c	2004-08-11 23:09:44.000000000 -0400
@@ -124,10 +124,14 @@
 
 	/* Start the process and wait for it to kill itself */
 	new_pid = clone(outer_tramp, (void *) sp, clone_flags, &arg);
-	if(new_pid < 0) return(-errno);
+	if(new_pid < 0) 
+		return(new_pid);
+
 	CATCH_EINTR(err = waitpid(new_pid, &status, 0));
-	if(err < 0) panic("Waiting for outer trampoline failed - errno = %d", 
-			  errno);
+	if(err < 0) 
+		panic("Waiting for outer trampoline failed - errno = %d", 
+		      errno);
+
 	if(!WIFSIGNALED(status) || (WTERMSIG(status) != SIGKILL))
 		panic("outer trampoline didn't exit with SIGKILL, "
 		      "status = %d", status);
Index: 2.6.8-rc4-mm1/arch/um/kernel/process_kern.c
===================================================================
--- 2.6.8-rc4-mm1.orig/arch/um/kernel/process_kern.c	2004-08-11 23:03:14.000000000 -0400
+++ 2.6.8-rc4-mm1/arch/um/kernel/process_kern.c	2004-08-11 23:09:44.000000000 -0400
@@ -167,7 +167,7 @@
 {
 	p->thread = (struct thread_struct) INIT_THREAD;
 	p->thread.kernel_stack = 
-		(unsigned long) p->thread_info + THREAD_SIZE;
+		(unsigned long) p->thread_info + 2 * PAGE_SIZE;
 	return(CHOOSE_MODE_PROC(copy_thread_tt, copy_thread_skas, nr, 
 				clone_flags, sp, stack_top, p, regs));
 }
Index: 2.6.8-rc4-mm1/arch/um/kernel/trap_kern.c
===================================================================
--- 2.6.8-rc4-mm1.orig/arch/um/kernel/trap_kern.c	2004-08-11 22:44:42.000000000 -0400
+++ 2.6.8-rc4-mm1/arch/um/kernel/trap_kern.c	2004-08-11 23:09:44.000000000 -0400
@@ -54,8 +54,6 @@
 	if(is_write && !(vma->vm_flags & VM_WRITE)) 
 		goto out;
 	page = address & PAGE_MASK;
-	if(address < (unsigned long) current_thread + 1024 && !is_user)
-		panic("Kernel stack overflow");
 	pgd = pgd_offset(mm, page);
 	pmd = pmd_offset(pgd, page);
 	do {
Index: 2.6.8-rc4-mm1/arch/um/kernel/tt/mem.c
===================================================================
--- 2.6.8-rc4-mm1.orig/arch/um/kernel/tt/mem.c	2004-08-11 22:44:42.000000000 -0400
+++ 2.6.8-rc4-mm1/arch/um/kernel/tt/mem.c	2004-08-11 23:09:44.000000000 -0400
@@ -18,7 +18,7 @@
 	if(!jail || debug)
 		remap_data(UML_ROUND_DOWN(&_stext), UML_ROUND_UP(&_etext), 1);
 	remap_data(UML_ROUND_DOWN(&_sdata), UML_ROUND_UP(&_edata), 1);
-	remap_data(UML_ROUND_DOWN(&__bss_start), UML_ROUND_UP(brk_start), 1);
+	remap_data(UML_ROUND_DOWN(&__bss_start), UML_ROUND_UP(&_end), 1);
 }
 
 #ifdef CONFIG_HOST_2G_2G
Index: 2.6.8-rc4-mm1/arch/um/kernel/tt/process_kern.c
===================================================================
--- 2.6.8-rc4-mm1.orig/arch/um/kernel/tt/process_kern.c	2004-08-11 22:44:42.000000000 -0400
+++ 2.6.8-rc4-mm1/arch/um/kernel/tt/process_kern.c	2004-08-11 23:09:44.000000000 -0400
@@ -412,7 +412,7 @@
 	protect_memory(start, end - start, 1, w, 1, 1);
 
 	start = (unsigned long) UML_ROUND_DOWN(&__bss_start);
-	end = (unsigned long) UML_ROUND_UP(brk_start);
+	end = (unsigned long) UML_ROUND_UP(&_end);
 	protect_memory(start, end - start, 1, w, 1, 1);
 
 	mprotect_kernel_vm(w);

Re: [PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

[Jeff Dike]

wli@holomorphy.com said:
> Out of curiosity, why are you allocating 4*PAGE_SIZE for the stack if
> you're only going to use 2*PAGE_SIZE of it? I saw no other users for
> the rest of ->thread_info offhand. 

Well, that's slightly misleading.  The other two pages (minus the thread_info)
are available for stack if needed.  UML stacks are somewhat larger than the
native kernel stacks because of the userspace signal frames, so I allocate
4 pages for now to be safe.

				Jeff

Re: [PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

[William Lee Irwin III]

wli@holomorphy.com said:
>> Out of curiosity, why are you allocating 4*PAGE_SIZE for the stack if
>> you're only going to use 2*PAGE_SIZE of it? I saw no other users for
>> the rest of ->thread_info offhand. 

On Thu, Aug 12, 2004 at 01:41:18AM -0400, Jeff Dike wrote:
> Well, that's slightly misleading.  The other two pages (minus the thread_info)
> are available for stack if needed.  UML stacks are somewhat larger than the
> native kernel stacks because of the userspace signal frames, so I allocate
> 4 pages for now to be safe.

This might confuse CONFIG_DEBUG_PAGEALLOC, which uses THREAD_SIZE to
detect the end of the kernel stack in store_stackinfo() in mm/slab.c
and kstack_end() in include/linux/sched.h, and the sizing heuristic
for max_threads in fork_init().

Also, how is this meant to interoperate with CONFIG_KERNEL_STACK_ORDER?
It seems to ignore the setting from the config option.


-- wli

Re: [PATCH 3/3] 2.6.8-rc4-mm1 - UML fixes

[Jeff Dike]

wli@holomorphy.com said:
> This might confuse CONFIG_DEBUG_PAGEALLOC, which uses THREAD_SIZE to
> detect the end of the kernel stack in store_stackinfo() in mm/slab.c
> and kstack_end() in include/linux/sched.h, and the sizing heuristic
> for max_threads in fork_init().

I think it's OK.  UML isn't lying about its stack size, just that snippet of
code is misleading.

> Also, how is this meant to interoperate with CONFIG_KERNEL_STACK_ORDER?
>  It seems to ignore the setting from the config option. 

I'm going to fix that.  For this patch I just did the quickest thing and 
reverted back to what was there before.

				Jeff