Page 1 of 1
Kernel Threads, take two
Posted: Wed Jul 29, 2009 10:38 pm
by xvedejas
My code is as follows. When I try to make a new stack in threads_install(), for some reason it keeps looping and never exiting the function threads_install(). I'm not sure what I'm doing wrong here, *in theory* this code should run just fine, but I'm afraid I'm not setting the stack up right.
The problem is in the first couple of functions, but I added the rest to give anyone an idea of the bigger picture, if they want it.
Code: Select all
#include <system.h>
extern u32int read_eip();
u32int next_thread_id = 1;
volatile thread_t *current_thread = 0;
void threads_install()
{
current_thread = (thread_t*)kmalloc(sizeof(thread_t));
current_thread->stack_start = initial_esp;
new_stack(current_thread);
asm volatile("mov %0, %%esp" :: "r"(current_thread->esp));
asm volatile("mov %0, %%ebp" :: "r"(current_thread->ebp));
current_thread->id = next_thread_id++;
current_thread->status = 0;
current_thread->next = current_thread;
current_thread->previous = current_thread;
current_thread->priority = 20;
current_thread->counter = 20;
// loops from here back up to the asm part, infinitely
}
void new_stack(thread_t *thread)
{
u32int current_esp, current_ebp, size, stack, offset, new_stack_start, i;
asm volatile("mov %%esp, %0" : "=r"(current_esp));
asm volatile("mov %%ebp, %0" : "=r"(current_ebp));
size = current_thread->stack_start - current_esp;
stack = (u32int)kmalloc(0x2000); // reserve 8192 bytes for stack
// start of stack is at _end_ of allocation
new_stack_start = stack + 0x2000;
offset = new_stack_start - current_thread->stack_start;
// find the esp and ebp for the new stack
thread->stack_start = new_stack_start;
thread->esp = current_esp + offset;
thread->ebp = current_ebp + offset;
// copy over the old stack to the new one
memcpyd((u32int*)thread->esp, (u32int*)current_esp, size);
u32int *tmp;
for (tmp = thread->esp; tmp <= thread->stack_start; tmp++)
{
if (*tmp < current_thread->stack_start && *tmp > current_esp)
{
*tmp += offset;
}
}
thread->stack_start = new_stack_start;
thread->stack_size = size;
}
u32int fork()
{ // make new thread
thread_t *parent_thread = (thread_t*)current_thread;
thread_t *new_thread = (thread_t*)kmalloc(sizeof(thread_t));
new_thread-> id = next_thread_id++;
new_thread->next = parent_thread->next;
new_thread->next->previous = new_thread;
parent_thread->next = new_thread;
new_thread->previous = parent_thread;
new_thread->priority = parent_thread->priority;
new_thread->counter = new_thread->priority;
new_thread->status = 1; // halted
// entry point
u32int eip = read_eip();
if (current_thread == parent_thread)
{
asm volatile("cli");
new_stack(new_thread);
new_thread->status = 0; // running
asm volatile("sti");
return new_thread->id;
}
else
{
return 0;
}
}
void switch_thread()
{
// reset counter
current_thread->counter = current_thread->priority;
// get stack
u32int esp, ebp, eip;
asm volatile("mov %%esp, %0" : "=r"(esp));
asm volatile("mov %%ebp, %0" : "=r"(ebp));
// get the eip
eip = read_eip();
if (eip == 0x12345) return;
// save stack
current_thread->esp = esp;
current_thread->ebp = ebp;
// advance current_thread
do current_thread = current_thread->next;
while (current_thread->status);
esp = current_thread->esp;
ebp = current_thread->ebp;
__asm__ __volatile__("\
mov %0, %%ecx; \
mov %1, %%esp; \
mov %2, %%ebp; \
mov $0x12345, %%eax; \
jmp *%%ecx" :: "r"(eip), "r"(esp), "r"(ebp) : "%eax", "%ecx");
}
u32int get_pid()
{
return current_thread->id;
}
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 1:35 am
by Brendan
Hi,
xvedejas wrote:Code: Select all
void threads_install()
{
current_thread = (thread_t*)kmalloc(sizeof(thread_t));
current_thread->stack_start = initial_esp;
new_stack(current_thread);
asm volatile("mov %0, %%esp" :: "r"(current_thread->esp));
asm volatile("mov %0, %%ebp" :: "r"(current_thread->ebp));
Why do you assume it's a sane idea to mess with ESP and EBP without the compiler's knowledge while the compiler's code may be relying on both of these registers (e.g. to access local variables)?
Cheers,
Brendan
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 11:50 am
by xvedejas
The following code:
Code: Select all
for (tmp = thread->esp; tmp <= thread->stack_start; tmp++)
{
if (*tmp < current_thread->stack_start && *tmp > current_esp)
{
*tmp += offset;
}
}
updates all pointers to positions in the new stack. The compiler shouldn't notice a difference.
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 1:06 pm
by alethiophile
Do you know where precisely it loops? If so, where? If not, add some printf()'s to find out.
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 1:19 pm
by xvedejas
I do know precisely where it loops, perhaps I didn't explain well enough.
Code: Select all
void threads_install()
{
current_thread = (thread_t*)kmalloc(sizeof(thread_t));
current_thread->stack_start = initial_esp;
new_stack(current_thread);
asm volatile("mov %0, %%esp" :: "r"(current_thread->esp));
asm volatile("mov %0, %%ebp" :: "r"(current_thread->ebp));
current_thread->id = next_thread_id++;
current_thread->status = 0;
current_thread->next = current_thread;
current_thread->previous = current_thread;
current_thread->priority = 20;
current_thread->counter = 20;
// loops from here back up to the asm part, infinitely
}
the part that loops is this:
Code: Select all
asm volatile("mov %0, %%esp" :: "r"(current_thread->esp));
asm volatile("mov %0, %%ebp" :: "r"(current_thread->ebp));
current_thread->id = next_thread_id++;
current_thread->status = 0;
current_thread->next = current_thread;
current_thread->previous = current_thread;
current_thread->priority = 20;
current_thread->counter = 20;
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 3:36 pm
by alethiophile
Huh? Try putting a printf() between every statement there and tell us which one doesn't run. None of those statements should hang; if anything, they should fault.
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 4:44 pm
by xvedejas
I have, none of them hang, none of them fault. When I get to the end of the function, it loops back to the asm part.
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 5:54 pm
by pcmattman
I have a feeling you're not pushing the correct return address. I'll edit after I've had a proper look at the function.
EDIT:
Scrap that thought...
Code: Select all
// copy over the old stack to the new one
memcpyd((u32int*)thread->esp, (u32int*)current_esp, size);
Does your memcpyd copy backwards? If I'm not mistaken, thread->esp and current_esp are both nearer the top of the stack than the bottom (because the stacks grow down).
You might want to make sure that this line is actually doing what you think it is.
EDIT 2: To extend Brendan's comment earlier... It should also be noted that you're definitely better off doing stack switches in a dedicated stack switch assembly function, which is basically (in psuedocode):
Code: Select all
stack_switch:
push registers
mov esp, new_esp
pop registers
ret
There's room for improvement (as there always is in code examples) but I just want to get the basic idea across.
Re: Kernel Threads, take two
Posted: Thu Jul 30, 2009 7:26 pm
by xvedejas
All registers besides ebp/esp are already saved since switch_thread() is called by an interrupt handler. So all I should have to do is copy the stack to a new position and update those registers.
Does your memcpyd copy backwards? If I'm not mistaken, thread->esp and current_esp are both nearer the top of the stack than the bottom (because the stacks grow down).
No, it doesn't copy backwards, if it did it would copy the memory that is allocated for the stack (but the stack isn't large enough to use yet). It copies forward from esp until the bottom of the stack.
I could try to recode it in assembly but I really don't see why it won't work in C.