Kernel Threads, take two

[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;
}

[Brendan]

Hi,

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

[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.

[alethiophile]

Do you know where precisely it loops? If so, where? If not, add some printf()'s to find out.

[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;

[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.

[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.

[pcmattman]

Code: Select all

new_stack(current_thread);

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.

[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.