Help with very persistent scheduler bug

[prasoc]

OK so I have created an ASM function "set_stack_ptr", (2 parameters: new stack ptr, and new instruction pointer) which is defined like this (nasm format, with Intel syntax):

Code: Select all

global set_stack_ptr
set_stack_ptr:
	mov eax, [esp + 4]
	mov ebx, esp
	mov esp, eax

	push ebp	; continue the linked list of ebp's

	mov eax, [esp + 8]	
	push eax		; push second param for "ret" to work

	mov ecx, 4
loop_stack:
	mov edx, [ebx + 4*ecx]
	push edx
	dec ecx
loop_bottom:
	cmp ecx, 1
	jne loop_stack

	ret

the function changes esp, then pushes the old ebp to the new stack (which corresponds to the new ebp of this particular function), and then pushes eip to the new stack. After doing this, it copies the previous stack's values and pushes them into the new thread stack.

it is called from the scheduler function at the end (after the register restoring memcpy), like this:

Code: Select all

	// set the registers from the current thread's saved state
	memcpy(r, &(thread_running->state_reg), sizeof(registers));	

	set_stack_ptr(r->esp, r->eip);
}

It "kind of" works, but gets stuck on the first thread. So my question is, what values do I need to push to the new stack so that the program can continue seamlessly through the esp transition?

[dozniak]

Well the idea of the "run" variable in each thread is that when the scheduler starts (loops around the first time), it doesn't actually SAVE the registers until it's ran at least once - otherwise you would overwrite the thread's state with nonsense.

But the current thread's state is not nonsense - since it's already running it has some state that HAS to be saved to later return to.

[Brendan]

Hi,

It "kind of" works, but gets stuck on the first thread. So my question is, what values do I need to push to the new stack so that the program can continue seamlessly through the esp transition?

From the Wikipedia page; for the "cdecl" calling convention:

"Registers EAX, ECX, and EDX are caller-saved, and the rest are callee-saved. The x87 floating point registers ST0 to ST7 must be empty (popped or freed) when calling a new function, and ST1 to ST7 must be empty on exiting a function. ST0 must also be empty when not used for returning a value."

This translates to something like:

Code: Select all

switch_to_task:
    mov eax, [esp + 4*1]      ;eax = address to store this task's ESP
    mov ecx, [esp + 4*2]      ;ecx = address to get next task's ESP

    ;Save general purpose registers for previous task
    ; Note: EAX, ECX, EDX and "return EIP" are saved by caller, so don't need to be saved again

    push ebx
    push esi
    push edi
    push ebp

    ;Save previous task's ESP

    mov [eax],esp

    ;Load next task's ESP

    mov esp,[ecx]

    ;Load general purpose registers for next task

    pop ebp
    pop edi
    pop esi
    pop ebx

    ;Return to next task's EIP

    ret

Of course this is just "bare minimum" - you'd add more later (e.g. worrying about FPU/MMX/SSE/AVX state, checking if CR3 needs to be changed and doing that, setting FS for thread local storage, etc).


Cheers,

Brendan

[prasoc]

Well the idea of the "run" variable in each thread is that when the scheduler starts (loops around the first time), it doesn't actually SAVE the registers until it's ran at least once - otherwise you would overwrite the thread's state with nonsense.

But the current thread's state is not nonsense - since it's already running it has some state that HAS to be saved to later return to.

At the beginning it is, each thread needs to run atleast once before it has a proper state, so the scheduler goes like this:

>>>>> run 1: (do not save the current registers into task 0, as it hasnt ran yet)
(increment the task counter)
set task 1 "ran" = true
load task 1 into the register and run
-
>>>>> run 2: save task 1, because "ran = true"
(increment the task counter)
set task 2 "ran" = true
load task 2 into the register and run
-
>>>>> run 3: save task 2, because "ran = true"
(increment the task counter)
set task 3 "ran" = true
load task 3 into the register and run
-
etc. etc.

it pins down the edge case of the first loop not having a "previous state".

This translates to something like:

Thank you for the example task switching code, Brendan. I will try and incorporate it into my scheduler and report back my success / failures

[dozniak]

But the current thread's state is not nonsense - since it's already running it has some state that HAS to be saved to later return to.

At the beginning it is, each thread needs to run atleast once before it has a proper state, so the scheduler goes like this:

I tell you, it is not; because each thread must start the execution somewhere and this is it's initial state (stack and program counter at least).

[prasoc]

But the current thread's state is not nonsense - since it's already running it has some state that HAS to be saved to later return to.

At the beginning it is, each thread needs to run atleast once before it has a proper state, so the scheduler goes like this:

I tell you, it is not; because each thread must start the execution somewhere and this is it's initial state (stack and program counter at least).

Sorry, I misrepresented what my code was doing - I do indeed setup the "beginning state" in the Thread constructor.

What I meant is, the first time "scheduler_next" is called, the previous state of execution corresponds to the kernel's setup function (my kmain function). This doesn't have a thread associated with it. So I need to wait for ONE task switch before I can save the registers.

-


I am getting close to solving the problems: ESP gets changed (thanks Brendan) and some values get pushed, just got to make sure the values needed upon the jmp command are actually on the stack!

[prasoc]

Finally got it!

Brendan, your switch_to_task function gave me enough of the picture to understand how to switch the ESP (with the correct stack values) and finally crack the problem. The code I was using originally was waaaay too complicated for what I needed; I'm shocked at how simple the finished scheduler function is!

Thank you all for the help
prasoc