Page 1 of 1

Is my multitasking code here correct?

Posted: Tue Oct 10, 2017 9:46 pm
by danaprakoso
Hello, I have been implementing multitasking system and I am confused why the output on the screen is not like what I expect.
If I test my multitasking system by outputting to Text Mode screen memory (0xB8000), the output is like what I expect.
For example: when I output "Hello, world" to the screen, than what appear on screen is "Hello, world".
But when I test it by drawing pixels in Graphics Mode, the screen gone weird.
What I expect:
I draw a rectangle that moving from left to right (from X position 0, to X position 800)
The result:
The rectangle is lagging while moving, and stops moving at X position 400.

What do I do wrong? Am I missing something? Or I lose my contents on some registers?

I guess there are some registers that I do not save.

multitask.cpp:

Code: Select all

#include <system.h>

Multitask multitask1;
Multitask multitask2;
Multitask multitask3;
Multitask* currentTask;

extern "C" void _switch_task(Registers* regs1, Registers* regs2);
extern "C" void switchTask(Registers* regs1, Registers* regs2);
extern "C" void _test_assembly(uint32_t param1, uint32_t param2);

void switch_task(Multitask* task1, Multitask* task2) {
	switchTask(&task1->regs, &task2->regs);
}

extern "C" void __pit_handler();

extern "C" Multitask* getLastTask() {
	return &multitask2;
}

extern "C" Multitask* getNextTask() {
	return &multitask2;
	//flush();
	Multitask* last = currentTask;
    currentTask = currentTask->next;
    /*if (currentTask == &multitask1) {
    	ps("Task1");
    } else if (currentTask == &multitask2) {
    	ps("Task2");
    } else if (currentTask == &multitask3) {
    	ps("Task3");
    } else {
    	ps("Unknown task");
    }*/
    if (currentTask == &multitask1) {
    	currentTask = currentTask->next;
    }
	return currentTask;
}

void create_task(Multitask* task, void (*main)(), uint32_t flags, uint32_t* pagedir, uint32_t cs, uint32_t ss) {
	task->regs.eax = 0;
    task->regs.ebx = 0;
    task->regs.ecx = 0;
    task->regs.edx = 0;
    task->regs.esi = 0;
    task->regs.edi = 0;
    task->regs.eflags = flags;
    task->regs.eip = (uint32_t)main;
    task->regs.cr3 = (uint32_t)pagedir;
    task->regs.esp = (uint32_t)malloc_align(4096, 4096)+0x1000;
    task->regs.ss = ss;
    task->regs.cs = cs;
    task->next = 0;
}

void yield() {
	Multitask* last = currentTask;
    currentTask = currentTask->next;
    if (currentTask == &multitask1) {
    	currentTask = currentTask->next;
    }
    _switch_task(&last->regs, &currentTask->regs);
}

extern "C" void main1() {
	while (1) {
		fill_rect(0, 0, get_width(), get_height(), 0xFFFFFF);
		//flush();
		//printf("Task 1, ");
		//yield();
	}
	stop();
}

extern "C" void main2() {
	int x = 0;
	while (1) {
		fill_rect(x, 0, 100, 100, 0xFF0000);
		flush();
		//printf("Task 2, ");
		//yield();
		x++;
	}
	stop();
}

void main3() {
	int x = 700;
	while (1) {
		//fill_rect(x, 100, 100, 100, 0x0000FF);
		printf("Task 3, ");
		//x--;
		//yield();
	}
	stop();
}

void test_multitasking() {
	uint32_t cr3 = read_cr(3);
	uint32_t eflags;
	uint32_t cs, ss;
	asm volatile("pushf; mov eax, [esp]; mov %0, eax; popf;":"=m"(eflags)::"eax");
	asm volatile("mov eax, cs; mov %0, eax;":"=m"(cs)::"eax");
	asm volatile("mov eax, ss; mov %0, eax;":"=m"(ss)::"eax");
	create_task((Multitask*)&multitask1, main1, eflags, cr3, cs, ss);
	create_task((Multitask*)&multitask2, main2, eflags, cr3, cs, ss);
	create_task((Multitask*)&multitask3, main3, eflags, cr3, cs, ss);
	multitask1.regs.esp = malloc_align(4096, 4096)+4096;
	multitask1.next = &multitask2;
	multitask2.regs.esp = malloc_align(4096, 4096)+4096;
	multitask2.next = &multitask3;
	multitask3.regs.esp = malloc_align(4096, 4096)+4096;
	multitask3.next = &multitask1;
	//multitask1 = 0x86E060
	currentTask = (Multitask*)&multitask1;
	idt_set_gate(32, (unsigned)__pit_handler, 0x08, 0x8E);
	//irq_set_handler(0, _pit_handler);
	//yield();
	//switchTask(&multitask1.regs, &multitask2.regs);
}

void init_multitasking() {
	//_test_assembly('A', 'B');
	test_multitasking();
}
multitask.h:

Code: Select all

#ifndef MULTITASK_H
#define MULTITASK_H

typedef struct {
    uint32_t eax, ebx, ecx, edx, esi, edi, esp, ebp, eip, eflags, cr3, cs, ss;
} Registers;

typedef struct Multitask {
    Registers regs;
    struct Multitask *next;
} Multitask;

void init_multitasking();
extern "C" void yield();

#endif
multitask_asm.asm:

Code: Select all

global __pit_handler
__pit_handler:
	extern pnh
	extern getLastTask
	extern getNextTask
	extern main1
	extern main2
	pushad
	call getLastTask
	mov [last_task], eax
	call getNextTask
	mov [next_task], eax
	popad
	push esp
	call pnh
	add esp, 4
	push eax
	push esp
	push ebp
	mov eax, [last_task]
	mov [eax+4], ebx
	mov [eax+8], ecx
	mov [eax+12], edx
	mov [eax+16], esi
	mov [eax+20], edi
	mov ebx, [esp+8] ;EAX
	mov ecx, [esp+12] ;EIP
	mov edx, [esp+4] ;ESP
	add edx, 4
	mov esi, [esp] ;EBP
	mov edi, [esp+20] ;EFLAGS
	mov [eax], ebx
	mov [eax+24], edx
	mov [eax+28], esi
	mov [eax+32], ecx
	mov [eax+36], edi
	pop dword [.tmp]
	pop dword [.tmp]
	pop dword [.tmp]
	mov eax, [next_task]
	mov ebx, [eax+4]
	mov ecx, [eax+8]
	mov edx, [eax+12]
	mov esi, [eax+16]
	mov edi, [eax+20]
	push eax
	mov eax, [eax+24]
	push eax
	call pnh
	add esp, 4
	pop eax
	mov ebp, [eax+28]
	mov esp, [eax+24]
	push eax
	mov eax, [eax+44] ;CS
	mov [esp+8], eax
	pop eax
	push eax
	mov eax, [eax+48] ;SS
	mov [esp+20], eax
	pop eax
	push eax
	mov eax, [eax+36] ;EFLAGS
	mov [esp+12], eax
	push eax
	popfd
	pop eax
	push eax
	mov eax, [eax+32] ;EIP
	mov eax, main2
	mov [esp+4], eax
	xchg eax, [esp]
	pop dword [.tmp]
	mov eax, [eax]
	; Send End Of Interrupt
	push eax
	mov al, 0x20
	out 0x20, al
	mov eax, cr0
	or eax, (1<<3)
	mov cr0, eax
	pop eax
	iret
	
	.eflags dd 0
	.esp dd 0
	.ss dd 0
	.tmp dd 0

last_task dq 0
next_task dq 0

Re: Is my multitasking code here correct?

Posted: Wed Oct 11, 2017 1:17 am
by Korona
Your code is incomplete, so we can't comment on it well. Where is _switch_task?

That being said, I think your PIT handler trashes registers after the first popad.

Your assembler code is a good example of how assembly should not be written. It has random instructions all over the place (pop [.tmp]?), no formatting into blocks, no comments on what the purpose of the code is. And I don't think that xchg does what you think it does: It performs an atomic exchange and forces the processor to lock the cache line.

Re: Is my multitasking code here correct?

Posted: Wed Oct 11, 2017 2:51 am
by Brendan
Hi,

Some notes about multitasking...

There are 3 different reasons for task switches:
  • The currently running task can't continue running (it terminated itself or crashed, or has to wait for data to arrive from disk or from network or from another task or from somewhere else, or has to wait for the user to press a key or move the mouse, or...). In this case the scheduler has to immediately find a different task to switch to and then switch to that task (and should not waste CPU time for no sane reason while it waits for the next timer IRQ to occur).
  • A new higher priority task was created; or something that a higher priority task was waiting for happened (e.g. a task wasn't able to run because it had to wait for the user to press a key, but the user pressed a key so it can run again now) causing the scheduler to switch to the higher priority task immediately (so that the task can respond to whatever happened immediately, and not later on after less important/lower priority tasks have wasted ages). In this case the scheduler knows exactly which task to switch to (and doesn't need to figure out which task to switch to at all). Note: In theory this "higher priority tasks preempt lower priority tasks" behaviour is optional; but in practice if an OS doesn't do it the OS feels slow/sluggish (especially on computers with only one CPU) because it's wasting CPU time on things that don't matter when important things that do matter are waiting for CPU time.
  • A task used all of the CPU time that the scheduler gave it. This is the least common cause of a task switch (most tasks block before they consume all the time the scheduler gave them) and is relatively unimportant (e.g. you can have a perfectly good scheduler that doesn't bother doing this). In this case the scheduler has to immediately find a different task to switch to and then switch to that task.
Building the scheduler directly into the timer's IRQ handler only works for the last case (and doesn't work for anything that actually matters in practice).

Instead, you should have a low-level "switch_to_task(task)" function. This function needs to be written in assembly, and (initially - you might add more to it later) the only thing it really has to do is push registers on the kernel stack, save the old task's kernel stack top in the old task's data, change ESP to the next task's kernel stack, then load registers from the next task's kernel stack. For cases involving pre-emption (where the scheduler knows exactly which task to switch to, and doesn't need to figure it out) it can call this "switch_to_task(task)" function directly.

The scheduler should also have a "find_task_then_switch_to_it()" function for the other cases; which would decide which task to switch to and then call the low-level "switch_to_task(task)" function to do the actual task switch.

The timer's IRQ would only call the "find_task_then_switch_to_it()" function if/when necessary (which doesn't include "no other task is waiting for CPU time" where calling the "find_task_then_switch_to_it()" function is not necessary).

For implementing it; you can/should write the low-level "switch_to_task(task)" function and then test it by using kernel threads that switch directly to each other (e.g. "main1()" calls "switch_to_task(2)" and "main2()" calls "switch_to_task(1)"). Once you know that works properly, then implement the "find_task_then_switch_to_it()" function and test that in a similar manner (kernel threads that call "find_task_then_switch_to_it()").

After that's done, implement blocking (which is mostly just some way to mark a thread as "blocked" so that the scheduler won't give it any more CPU time before you call the "find_task_then_switch_to_it()" function), and implement unblocking (which is mostly just clearing/removing the mark that was used when it was blocked, and then optionally calling "switch_to_task(task)" if the unblocked task is higher priority than the currently running task). Test this in the same way (kernel threads that block themselves, and unblock each other).

Once all that's done, you can benchmark various cases (how long does a task switch take under various conditions?) and add various features (changing CR3 if/when the virtual address space needs to change, keeping track of how much CPU time each task has consumed, ....) and change the "find_task_then_switch_to_it()" function's code so it's much much smarter (e.g. maybe several different "scheduling policies", maybe with support for real-time tasks, etc) and maybe add some basic power management (use the "HLT" instruction when there isn't any useful work for the CPU to do).


Cheers,

Brendan

Re: Is my multitasking code here correct?

Posted: Wed Oct 11, 2017 11:53 am
by Octocontrabass
Your use of inline assembly is both pointless (you could set EFLAGS, CS, and SS to constant values instead of loading the current values) and broken. It's also Intel syntax, but I'll assume you've told GCC to use Intel syntax instead of the default AT&T syntax.
danaprakoso wrote:

Code: Select all

	asm volatile("pushf; mov eax, [esp]; mov %0, eax; popf;":"=m"(eflags)::"eax");
	asm volatile("mov eax, cs; mov %0, eax;":"=m"(cs)::"eax");
	asm volatile("mov eax, ss; mov %0, eax;":"=m"(ss)::"eax");
Your first line violates GCC's assumptions about ESP-relative memory addressing, unnecessarily clobbers EAX, and unnecessarily requires a memory operand. You can fix all of those things like this:

Code: Select all

asm volatile( "pushf; pop %0;" : "=rm"(eflags) );
Your second and third lines don't violate GCC's assumptions, but they still unnecessarily clobber EAX and unnecessarily require memory operands. You can fix those things like this:

Code: Select all

asm volatile( "mov %0, cs;" : "=rm"(cs) );
asm volatile( "mov %0, ss;" : "=rm"(ss) );