Thread scheduler in C

Question

I have made a simple round robin scheduler that switches between threads in ptable array. Currently it's only for 1 CPU, so there are no locks at the moment.

But each CPU has its own curproc variable with the help of the GS register.

More details in the comments.

proc.c

#include <stdint.h>
#include "franklin/switch.h"
#include "franklin/apic.h"
#include "franklin/mmu.h"
#include "franklin/proc.h"
#include "asm/x86.h"
#define NPROC 255
struct proc *curproc;
struct proc ptable[NPROC];
struct proc* set_current_proc(struct proc *p) {
 wrmsr((uint64_t)p);
 swapgs();
 return p;
}
struct proc* get_current_proc() {
 struct proc *p;
 swapgs();
 rdmsr(&p);
 if (p == 0) // if p = 0, it means swapgs() swapped the wrong way, so we need to swap again
 swapgs(); rdmsr(&p);
 
 return p;
}
void startproc(struct proc *p) {
 struct proc *current;
 
 curproc = get_current_proc();
 curproc->state = RUNNING;
 // this useless stack needs to be "saved" somewhere
 stack *discard;
 switc(&discard, p->stack);
}
void allocproc(uintptr_t *entry) {
 static uint32_t nextpid = 0;
 struct proc *p;
 
 p = &ptable[nextpid];
 p->pid = nextpid++;
 // palloc(1) allocates 1 page in phys memory and returns a physical addr
 p->stack = (stack*)P2V((uintptr_t)palloc(1));
 p->stack->rip = (uintptr_t)entry;
 
 p->state = RUNNABLE;
}
void scheduler() {
 struct proc *p, *prev;
 static uint8_t i = 1; // note: this is static
 // find next runnable process
 while ((p = &ptable[i++]) && p->state != RUNNABLE) {
 if (i == NPROC)
 i = 0;
 }
 
 prev = get_current_proc();
 curproc = set_current_proc(p);
 curproc->state = RUNNING;
 // save current stack on the previous process's struct
 // and switch to new process
 switc(&prev->stack, curproc->stack); 
}

x86.h

static void swapgs() {
 asm("swapgs":::"memory");
}
// "rdmsr" instruction returns the value in EDX:EAX
static void rdmsr(uint64_t **val) {
 uint64_t rdx, rax;
 asm volatile(
 "mov 0ドルxc0000102, %%rcx;"
 "rdmsr;"
 : "=a"(rax), "=d"(rdx)
 );
 *val = rax | (uint64_t)rdx << 32;
}
// "wrmsr" instruction writes the 64 bit value in EDX:EAX
static void wrmsr(uint64_t val) {
 asm(
 "mov %0, %%edx;"
 "mov %1, %%eax;"
 "mov 0ドルxc0000102, %%rcx;"
 "wrmsr;"
 ::
 "r"((uint32_t)(val >> 32)),
 "r"((uint32_t)val)
 : "rdx", "rax", "rcx"
 );
}

proc.h

#ifndef _PROC_
#define _PROC_
void scheduler(void);
enum procstate {
 UNUSED,
 RUNNABLE,
 RUNNING,
};
struct proc {
 regs_t *regs;
 stack *stack;
 enum procstate state;
 uint32_t pid;
};
#endif

isr.asm

 ; only push registers that belong to the called function, 
 the C function that gets called will save the rest of the registers
 %macro pushregs 0
 push rbp
 push rax
 push rcx
 push rdx
 push rdi
 push rsi
 push r8
 push r9
 push r10
 push r11
 %endmacro
 %macro popregs 0
 pop r11
 pop r10
 pop r9
 pop r8
 pop rsi
 pop rdi
 pop rdx
 pop rcx
 pop rax
 pop rbp
 %endmacro
 
 extern trap
 %macro isr_stub 1
isr_handler_%1: 
 push %1
 jmp alltraps
 %endmacro
global ret
alltraps:
 cld
 pushregs
 mov rbp, rsp
 and rsp, ~0xf ; 16 byte align stack
 mov rdi, rbp ; pass stack as parameter to trap function
 call trap
ret: 
 mov rsp, rbp ; this works even after scheduling, because 'trap'
 ; will save the rbp on entry
 popregs
 add rsp, 8 ; irq number
 iretq

switch.asm

 global switc
switc:
 push rbp
 mov qword [rdi], rsp
 mov rsp, rsi
 pop rbp
 ret

and switch.h

typedef struct {
 uint64_t r11;
 uint64_t r10;
 uint64_t r9;
 uint64_t r8;
 uint64_t rsi;
 uint64_t rdi;
 uint64_t rdx;
 uint64_t rcx;
 uint64_t rax;
 uint64_t rbp;
 uint64_t code;
 /* uint64_t errcode; */
 uint64_t rip;
 uint64_t cs;
 uint64_t eflags;
} __attribute__((packed))regs_t;
typedef struct {
 unsigned long rbp;
 unsigned long rip;
} __attribute__((packed))stack;

apic.c

extern void scheduler(void);
extern struct proc *curproc;
void apic_timer(regs_t *regs) {
 *EOI = 0;
 curproc->state = RUNNABLE;
 scheduler();
}

trap.c

extern uint32_t* EOI;
void trap(regs_t *regs) {
// this function is not really implemented yet
 switch(regs->code) {
 case 32:
 timerh(10);
 break;
 case 33:
 kbd_press();
 break;
 case 34:
 apic_timer(regs);
 break;
 }
};

• mdfst13: It's not clear to me what about this is algorithm. It would make more sense to me to tag this assembly. I would also find x86 more useful than kernel. And this is a task scheduler, not scheduled-tasks.

Answer 1

 if (p == 0) // if p = 0, it means swapgs() swapped the wrong way, so we need to swap again
 swapgs(); rdmsr(&p);

This is a really weird way to write it. It becomes a lot clearer if you write it with curly braces.

 // if p = 0, it means swapgs() swapped the wrong way, so we need to swap again
 if (0 == p) {
 swapgs();
 }
 rdmsr(&p);

Otherwise, perhaps someone might think that it was intended to be

 // if p = 0, it means swapgs() swapped the wrong way, so we need to swap again
 if (0 == p) {
 swapgs();
 rdmsr(&p);
 }

Note: if you were under the impression that the quoted code worked like the last, then that is exactly why some of us recommend never using the statement form of control structures. Because they don't work that way.

I am somewhat of the opinion that the last block is how it should be written. Because I think that if you don't swap again, rdmsr is a no-op. But perhaps I am misunderstanding. I don't actually know x86 assembly.

Regardless, it is confusing to put a statement that will always execute on the same line as a statement that executes conditionally. Even if it were important to save space, then

 if (0 == p) swapgs();
 rdmsr(&p);

is much clearer. But there isn't really a reason to save space.

In my opinion, splitting the statement form across two lines doesn't help it. It would be far superior to use the three or four lines of the block form (with curly braces).

Similarly, in C, it is often helpful to put constants on the left of the ==, because if (p = 0) is valid C. Valid in the sense that it will compile. However, of course it won't ever run the conditional statement and it would overwrite p. Meanwhile, if (0 = p) would give a compiler error. So if (0 == p) is safer, because it is harder to mess it up in a way that won't be noticed immediately. This can be controversial, because some don't like the way that it reads.

I find comments clearer if they are always written on their own lines. And of course that prevents side scroll in the Stack format in this case.