From d5bfc4f28cef1aec13d3d0bf8f6de16281d3b4f7 Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Mon, 1 Aug 2011 22:01:39 +0200 Subject: [PATCH] avoid races on SMP systems => signalizes with a new flag, that a task switch is finished --- arch/x86/include/asm/tasks.h | 2 +- arch/x86/kernel/entry.asm | 4 +- arch/x86/kernel/gdt.c | 19 +----- arch/x86/kernel/isrs.c | 4 +- include/metalsvm/stddef.h | 11 ++++ include/metalsvm/tasks_types.h | 14 ++-- kernel/tasks.c | 117 +++++++++++++++++++++++++++++---- kernel/tests.c | 14 ++-- 8 files changed, 136 insertions(+), 49 deletions(-) diff --git a/arch/x86/include/asm/tasks.h b/arch/x86/include/asm/tasks.h index 0ee4f9dd..c12f946a 100644 --- a/arch/x86/include/asm/tasks.h +++ b/arch/x86/include/asm/tasks.h @@ -59,7 +59,7 @@ void switch_task(uint32_t id); * - 0 on success * - -EINVAL (-22) on failure */ -int create_default_frame(task_t* task, entry_point_t ep, void* arg); +int create_default_frame(task_t* task, internal_entry_point_t ep, void* arg); /** @brief Register a task's TSS at GDT * diff --git a/arch/x86/kernel/entry.asm b/arch/x86/kernel/entry.asm index 852af114..e2e2d84a 100644 --- a/arch/x86/kernel/entry.asm +++ b/arch/x86/kernel/entry.asm @@ -496,8 +496,8 @@ global apic_lint1 global apic_error global apic_svr -global tss_switch -tss_switch: +global switch_task +switch_task: mov eax, [esp+4] add ax, WORD 5 mov bx, WORD 8 diff --git a/arch/x86/kernel/gdt.c b/arch/x86/kernel/gdt.c index f15057f7..9441410a 100644 --- a/arch/x86/kernel/gdt.c +++ b/arch/x86/kernel/gdt.c @@ -52,23 +52,6 @@ size_t get_stack(uint32_t id) return (size_t) kstacks[id] + KERNEL_STACK_SIZE - sizeof(size_t); } -void switch_task(uint32_t id) -{ -#if MAX_CORES > 1 - /* - * On SMP systems, the task switch on an other could be in progress... - * => we need to check, if the task is still busy - */ - volatile gdt_entry_t* newgdt = (volatile gdt_entry_t*) gdt+id-5; - - while (newgdt->access & GDT_FLAG_TSS_BUSY) { - NOP1; - } -#endif - - tss_switch(id); -} - int register_task(task_t* task) { uint16_t sel; uint32_t id = task->id; @@ -142,7 +125,7 @@ int arch_fork(task_t* task) return 0; } -int create_default_frame(task_t* task, entry_point_t ep, void* arg) +int create_default_frame(task_t* task, internal_entry_point_t ep, void* arg) { uint16_t cs = 0x08; uint16_t ds = 0x10; diff --git a/arch/x86/kernel/isrs.c b/arch/x86/kernel/isrs.c index c37f2689..f1e2d9cc 100644 --- a/arch/x86/kernel/isrs.c +++ b/arch/x86/kernel/isrs.c @@ -229,8 +229,8 @@ static void fault_handler(struct state *s) { if (s->int_no < 32) { kputs(exception_messages[s->int_no]); - kprintf(" Exception (%d) at 0x%x:0x%x on core %d, error code 0x%x, eflags 0x%x\n", - s->int_no, s->cs, CORE_ID, s->eip, s->error, s->eflags); + kprintf(" Exception (%d) at 0x%x:0x%x on core %u, error code 0x%x, eflags 0x%x\n", + s->int_no, s->cs, s->eip, CORE_ID, s->error, s->eflags); /* Now, we signalize that we have handled the interrupt */ if (apic_is_enabled()) diff --git a/include/metalsvm/stddef.h b/include/metalsvm/stddef.h index a34ccf10..725365e2 100644 --- a/include/metalsvm/stddef.h +++ b/include/metalsvm/stddef.h @@ -36,6 +36,7 @@ typedef unsigned int tid_t; #define per_core(name) name #define DECLARE_PER_CORE(type, name) extern type name; #define DEFINE_PER_CORE(type, name, def_value) type name = def_value; +#define DEFINE_PER_CORE_STATIC(type, name, def_value) static type name = def_value; #define CORE_ID 0 #else #define per_core(name) (*__get_percore_##name()) @@ -51,6 +52,16 @@ typedef unsigned int tid_t; } #define DEFINE_PER_CORE(type, name, def_value) \ aligned_##name name[MAX_CORES] = {[0 ... MAX_CORES-1] = {def_value}}; +#define DEFINE_PER_CORE_STATIC(type, name, def_value) \ + typedef struct { type var __attribute__ ((aligned (CACHE_LINE))); } aligned_##name;\ + static aligned_##name name[MAX_CORES] = {[0 ... MAX_CORES-1] = {def_value}}; \ + inline static type* __get_percore_##name(void) {\ + type* ret; \ + uint32_t flags = irq_nested_disable(); \ + ret = &(name[smp_id()].var); \ + irq_nested_enable(flags);\ + return ret; \ + } #define CORE_ID smp_id() #endif diff --git a/include/metalsvm/tasks_types.h b/include/metalsvm/tasks_types.h index b22ca7ce..c6e835e5 100644 --- a/include/metalsvm/tasks_types.h +++ b/include/metalsvm/tasks_types.h @@ -41,17 +41,19 @@ extern "C" { #endif #define TASK_INVALID 0 -#define TASK_READY 1 +#define TASK_READY 1 #define TASK_RUNNING 2 #define TASK_BLOCKED 3 #define TASK_FINISHED 4 -#define TASK_IDLE 5 +#define TASK_IDLE 5 -#define TASK_DEFAULT_FLAGS 0 -#define TASK_FPU_INIT (1 << 0) -#define TASK_FPU_USED (1 << 1) +#define TASK_DEFAULT_FLAGS 0 +#define TASK_FPU_INIT (1 << 0) +#define TASK_FPU_USED (1 << 1) +#define TASK_SWITCH_IN_PROGESS (1 << 2) -typedef int (STDCALL *entry_point_t)(void*); +typedef int (*entry_point_t)(void*); +typedef int (STDCALL *internal_entry_point_t)(void*); struct page_dir; /* @brief The task_t structure */ diff --git a/kernel/tasks.c b/kernel/tasks.c index 37856b18..48182a9b 100644 --- a/kernel/tasks.c +++ b/kernel/tasks.c @@ -52,6 +52,7 @@ static task_t task_table[MAX_TASKS] = { \ static spinlock_irqsave_t table_lock = SPINLOCK_IRQSAVE_INIT; DEFINE_PER_CORE(task_t*, current_task, task_table+0); +DEFINE_PER_CORE_STATIC(task_t*, old_task, NULL); /** @brief helper function for the assembly code to determine the current task * @return Pointer to the task_t structure of current task @@ -170,6 +171,24 @@ void NORETURN abort(void) { do_exit(-1); } +/* + * @brief: if the task switch is finished, we reset + * the TASK_SWITCH_IN_PROGRESS bit + */ +inline static void task_switch_finished(void) +{ + uint32_t flags = irq_nested_disable(); + + // do we already reset the TASK_SWITCH_IN_PROGRESS bit? + task_t* old = per_core(old_task); + if (old) { + old->flags &= ~TASK_SWITCH_IN_PROGESS; + per_core(old_task) = NULL; + } + + irq_nested_enable(flags); +} + /** @brief Create a task with a specific entry point * * @param id Pointer to a tid_t struct were the id shall be set @@ -179,7 +198,7 @@ void NORETURN abort(void) { * - 0 on success * - -ENOMEM (-12) or -EINVAL (-22) on failure */ -static int create_task(tid_t* id, entry_point_t ep, void* arg) +static int create_task(tid_t* id, internal_entry_point_t ep, void* arg) { task_t* curr_task; int ret = -ENOMEM; @@ -229,7 +248,6 @@ create_task_out: return ret; } - int sys_fork(void) { int ret = -ENOMEM; @@ -292,6 +310,10 @@ int sys_fork(void) // Leave the function without releasing the locks // because the locks are already released // by the parent task! + + // first switch to the new current task + // => signalizes a successful task switch + task_switch_finished(); return 0; } @@ -310,15 +332,54 @@ create_task_out: return ret; } -int create_kernel_task(tid_t* id, entry_point_t ep, void* arg) +/** @brief Structure which keeps all + * relevant data for a new kernel task to start */ +typedef struct { + /// entry point of the kernel task + entry_point_t func; + /// arguments + void* args; +} kernel_args_t; + +/** @brief This call is used to adapt create_task calls + * which want to have a start function and argument list */ +static int STDCALL kernel_entry(void* args) { - return create_task(id, ep, arg); + int ret; + kernel_args_t* kernel_args = (kernel_args_t*) args; + + // first switch to the new current task + // => signalizes a successful task switch + task_switch_finished(); + + if (BUILTIN_EXPECT(!kernel_args, 0)) + return -EINVAL; + + ret = kernel_args->func(kernel_args->args); + + kfree(kernel_args, sizeof(kernel_args_t)); + + return ret; +} + +int create_kernel_task(tid_t* id, entry_point_t ep, void* args) +{ + kernel_args_t* kernel_args; + + kernel_args = kmalloc(sizeof(kernel_args_t)); + if (BUILTIN_EXPECT(!kernel_args, 0)) + return -ENOMEM; + + kernel_args->func = ep; + kernel_args->args = args; + + return create_task(id, kernel_entry, kernel_args); } #define MAX_ARGS (PAGE_SIZE - 2*sizeof(int) - sizeof(vfs_node_t*)) /** @brief Structure which keeps all - * relevant data for a new task to start */ + * relevant data for a new user task to start */ typedef struct { /// Points to the node with the executable in the file system vfs_node_t* node; @@ -528,7 +589,20 @@ invalid: * which want to have a start function and argument list */ static int STDCALL user_entry(void* arg) { - return load_task((load_args_t*) arg); + int ret; + + // first switch to the new current task + // => signalizes a successful task switch + task_switch_finished(); + + if (BUILTIN_EXPECT(!arg, 0)) + return -EINVAL; + + ret = load_task((load_args_t*) arg); + + kfree(arg, sizeof(load_args_t)); + + return ret; } /** @brief Luxus-edition of create_user_task functions. Just call with an exe name @@ -717,14 +791,14 @@ int block_task(tid_t id) spinlock_irqsave_lock(&table_lock); - if ((task_table[id].status == TASK_RUNNING) || (task_table[id].status == TASK_READY)) { + if ((task_table[id].status == TASK_RUNNING) || (task_table[id].status == TASK_READY)) { task_table[id].status = TASK_BLOCKED; ret = 0; } else kprintf("Unable to block task %d!\n", id); - spinlock_irqsave_unlock(&table_lock); + spinlock_irqsave_unlock(&table_lock); - return ret; + return ret; } /** @brief _The_ scheduler procedure @@ -738,13 +812,18 @@ void scheduler(void) unsigned int i; unsigned int new_id; + // let's play it save + // => check if we already signalizes that the previous switch + // is finished + task_switch_finished(); + #if MAX_CORES > 1 spinlock_irqsave_lock(&table_lock); #endif orig_task = curr_task = per_core(current_task); - /* signalize that this task could be reused */ + /* signalizes that this task could be reused */ if (curr_task->status == TASK_FINISHED) curr_task->status = TASK_INVALID; @@ -757,16 +836,26 @@ void scheduler(void) for(i=1, new_id=(curr_task->id + 1) % MAX_TASKS; istatus == TASK_RUNNING) + if ((task_table[new_id].status == TASK_READY) && !(task_table[new_id].flags & TASK_SWITCH_IN_PROGESS)) { + if (curr_task->status == TASK_RUNNING) { curr_task->status = TASK_READY; + curr_task->flags |= TASK_SWITCH_IN_PROGESS; + per_core(old_task) = curr_task; + } else per_core(old_task) = NULL; + task_table[new_id].status = TASK_RUNNING; curr_task = per_core(current_task) = task_table+new_id; goto get_task_out; } + + //if ((task_table[new_id].status == TASK_READY) && (task_table[new_id].flags & TASK_SWITCH_IN_PROGESS)) + // kprintf("task switch %d is in progress\n", new_id); } + // old task will never rescheduled + per_core(old_task) = NULL; + if ((curr_task->status == TASK_RUNNING) || (curr_task->status == TASK_IDLE)) goto get_task_out; @@ -783,8 +872,10 @@ get_task_out: spinlock_irqsave_unlock(&table_lock); #endif - if (curr_task != orig_task) + if (curr_task != orig_task) { switch_task(new_id); + task_switch_finished(); + } } void reschedule(void) diff --git a/kernel/tests.c b/kernel/tests.c index d3f8a68a..a39f55a0 100644 --- a/kernel/tests.c +++ b/kernel/tests.c @@ -39,7 +39,7 @@ static sem_t consuming, producing; static mailbox_int32_t mbox; static int val = 0; -static int STDCALL consumer(void* arg) +static int consumer(void* arg) { int i, m = 0; @@ -58,7 +58,7 @@ static int STDCALL consumer(void* arg) return 0; } -static int STDCALL producer(void* arg) +static int producer(void* arg) { int i; int mail[5] = {1, 2, 3, 4, 5}; @@ -78,7 +78,7 @@ static int STDCALL producer(void* arg) return 0; } -static int STDCALL foo(void* arg) +static int foo(void* arg) { int i; @@ -94,7 +94,7 @@ static int STDCALL foo(void* arg) } #ifdef CONFIG_ROCKCREEK -int STDCALL mail_ping(void* arg) { +int mail_ping(void* arg) { int i; for(i=0; i<20; i++) { @@ -107,7 +107,7 @@ int STDCALL mail_ping(void* arg) { } #endif -static int STDCALL join_test(void* arg) +static int join_test(void* arg) { tid_t id, ret; int result = -1234; @@ -125,7 +125,7 @@ static int STDCALL join_test(void* arg) } #if defined(CONFIG_LWIP) && defined(CONFIG_ROCKCREEK) -static int STDCALL server_task(void* e) +static int server_task(void* e) { int sockfd, newsockfd, portno, clilen; char buffer[256]; @@ -193,7 +193,7 @@ static int STDCALL server_task(void* e) return 0; } -static int STDCALL client_task(void* e) +static int client_task(void* e) { char dir[256]; int sd;