[PATCH] fix strange stack calculation for secondary cpus

[PATCH] fix strange stack calculation for secondary cpus

[Dave Hansen]

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

in arch/i386/kernel/smpboot.c:
stack_start.esp = (void *) (1024 + PAGE_SIZE + (char *)idle);

This causes problems when I switch to 4k stacks?  What is supposed to 
be going on here?  Why point esp into the middle of the stack?  If you 
wanted to do that, why not just use PAGE_SIZE>>2?

In any case, I think THREAD_SIZE needs to be here instead of PAGE_SIZE.
-- 
Dave Hansen
haveblue@us.ibm.com

[-- Attachment #2: fix-esp-2.5.51.patch --]
[-- Type: text/plain, Size: 467 bytes --]

--- linux-2.5.50/arch/i386/kernel/smpboot.c.bad	Tue Dec 10 12:56:10 2002
+++ linux-2.5.50/arch/i386/kernel/smpboot.c	Tue Dec 10 12:56:55 2002
@@ -806,7 +806,7 @@
 
 	/* So we see what's up   */
 	printk("Booting processor %d/%d eip %lx\n", cpu, apicid, start_eip);
-	stack_start.esp = (void *) (1024 + PAGE_SIZE + (char *)idle->thread_info);
+	stack_start.esp = (void *) (THREAD_SIZE + (char *)idle->thread_info);
 
 	/*
 	 * This grunge runs the startup process for

Re: [PATCH] fix strange stack calculation for secondary cpus

[Hugh Dickins]

On Tue, 10 Dec 2002, Dave Hansen wrote:
> in arch/i386/kernel/smpboot.c:
> stack_start.esp = (void *) (1024 + PAGE_SIZE + (char *)idle);
> 
> This causes problems when I switch to 4k stacks?  What is supposed to 
> be going on here?  Why point esp into the middle of the stack?  If you 
> wanted to do that, why not just use PAGE_SIZE>>2?
> 
> In any case, I think THREAD_SIZE needs to be here instead of PAGE_SIZE.

Yes, it is weird: I wondered the same when we did our bigstack patch
for debugging 2.4 stack overflows.

The conclusion I came to was, it was trying to start the stack somewhere
that wouldn't clash with where it's set up for the trampoline at the top
of the stack area, see in particular initialize_secondary(): was choosing
somewhere arbitrarily far below that.

To avoid mysterious magic numbers, I chose instead to start it immediately
below that area i.e. set the top esp here to the bottom esp there.  That
worked fine for 2.4, I don't see why the same shouldn't work for 2.5.

Whereas with your patch, you might be overwriting that area.
So below I've munged your patch into what we found worked back then.

To be honest, I can't quite remember my way around that stuff now,
and my words above may make little sense!

Hugh

--- linux-2.5.50/arch/i386/kernel/smpboot.c.bad	Tue Dec 10 12:56:10 2002
+++ linux-2.5.50/arch/i386/kernel/smpboot.c	Tue Dec 10 12:56:55 2002
@@ -806,7 +806,7 @@
 
 	/* So we see what's up   */
 	printk("Booting processor %d/%d eip %lx\n", cpu, apicid, start_eip);
-	stack_start.esp = (void *) (1024 + PAGE_SIZE + (char *)idle->thread_info);
+	stack_start.esp = (void *) idle->thread.esp;
 
 	/*
 	 * This grunge runs the startup process for

Re: [PATCH] fix strange stack calculation for secondary cpus

[Dave Hansen]

Hugh Dickins wrote:
> On Tue, 10 Dec 2002, Dave Hansen wrote:
> 
>>in arch/i386/kernel/smpboot.c:
>>stack_start.esp = (void *) (1024 + PAGE_SIZE + (char *)idle);
>>
>>This causes problems when I switch to 4k stacks?  What is supposed to 
>>be going on here?  Why point esp into the middle of the stack?  If you 
>>wanted to do that, why not just use PAGE_SIZE>>2?
>>
> To avoid mysterious magic numbers, I chose instead to start it immediately
> below that area i.e. set the top esp here to the bottom esp there.  That
> worked fine for 2.4, I don't see why the same shouldn't work for 2.5.

It should.  I just want to be able to use arbitrary stack sizes.

> Whereas with your patch, you might be overwriting that area.
> So below I've munged your patch into what we found worked back then.

Agreed.  I was really just trying to eliminate the magic number 
without much real knowledge about what was going on.

Would you like to send your patch on to Alan or Linus?
-- 
Dave Hansen
haveblue@us.ibm.com