Avoid nested locking where possible; add scheduler preference to fix …

…g_yield_t

Holding locks and calling other code can lead to the situation where processor 0 tries to lock ABBA and processor 1 tries BAAB, which causes a deadlock when the simultaneously reach the second lock. This pattern should avoid this.

kernel/inc/build_config.hpp

@@ -48,7 +48,7 @@
 // version
 #define G_VERSION_MAJOR 0
 #define G_VERSION_MINOR 20
-#define G_VERSION_PATCH 0
+#define G_VERSION_PATCH 1
 
 #define G_LOADER_VERSION_MAJOR 1
 #define G_LOADER_VERSION_MINOR 1

kernel/inc/shared/system/spinlock.hpp

@@ -21,12 +21,13 @@
 #ifndef __SYSTEM_SPINLOCK__
 #define __SYSTEM_SPINLOCK__
 
-typedef volatile int g_spinlock;
+typedef int g_spinlock;
 
-#define G_SPINLOCK_ACQUIRE(lock)                        \
-	while(!__sync_bool_compare_and_swap(&lock, 0, 1)) \
-		asm volatile("pause");
+#define G_SPINLOCK_ACQUIRE(lock)                          \
+      while(!__sync_bool_compare_and_swap(&lock, 0, 1))   \
+      asm volatile("pause");
 
-#define G_SPINLOCK_RELEASE(lock) lock = 0;
+#define G_SPINLOCK_RELEASE(lock)                          \
+      do { __sync_synchronize(); lock = 0; } while(0)
 
 #endif

kernel/src/kernel/calls/syscall_memory.cpp

@@ -143,16 +143,16 @@ void syscallShareMemory(g_task* task, g_syscall_share_mem* data)
 
 	g_virtual_address virtualRangeBase = addressRangePoolAllocate(targetProcess->virtualRangePool, pages,
 	                                                              G_PROC_VIRTUAL_RANGE_FLAG_NONE);
+	mutexRelease(&targetProcess->lock);
+
 	if(virtualRangeBase == 0)
 	{
 		logInfo(
 				"%! task %i was unable to share memory area %h of size %h with task %i because there was no free virtual range",
 				"syscall",
 				task->id, memory, pages * G_PAGE_SIZE, targetProcess->main->id);
-		mutexRelease(&targetProcess->lock);
 		return;
 	}
-	mutexRelease(&targetProcess->lock);
 
 	for(uint32_t i = 0; i < pages; i++)
 	{

kernel/src/kernel/calls/syscall_messaging.cpp

@@ -29,12 +29,14 @@ void syscallMessageSend(g_task* task, g_syscall_send_message* data)
 	      G_MESSAGE_SEND_STATUS_QUEUE_FULL &&
 	      data->mode == G_MESSAGE_SEND_MODE_BLOCKING)
 	{
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&task->lock);
 		task->status = G_TASK_STATUS_WAITING;
 		task->waitsFor = "message-send";
-		messageWaitForSend(task->id, data->receiver);
 		mutexRelease(&task->lock);
+		messageWaitForSend(task->id, data->receiver);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 	messageUnwaitForSend(task->id, data->receiver);
 }
@@ -55,11 +57,13 @@ void syscallMessageReceive(g_task* task, g_syscall_receive_message* data)
 		//	break;
 		// }
 
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&task->lock);
 		task->status = G_TASK_STATUS_WAITING;
 		task->waitsFor = "message-recv";
 		mutexRelease(&task->lock);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 }
 

kernel/src/kernel/calls/syscall_tasking.cpp

@@ -32,12 +32,14 @@
 
 void syscallSleep(g_task* task, g_syscall_sleep* data)
 {
+	INTERRUPTS_PAUSE;
 	mutexAcquire(&task->lock);
 	task->status = G_TASK_STATUS_WAITING;
 	task->waitsFor = "sleeps";
-	clockWaitForTime(task->id, clockGetLocal()->time + data->milliseconds);
 	mutexRelease(&task->lock);
+	clockWaitForTime(task->id, clockGetLocal()->time + data->milliseconds);
 	taskingYield();
+	INTERRUPTS_RESUME;
 }
 
 void syscallYield(g_task* task, g_syscall_yield* data)
@@ -85,12 +87,14 @@ void syscallGetProcessIdForTaskId(g_task* task, g_syscall_get_pid_for_tid* data)
 
 void syscallJoin(g_task* task, g_syscall_join* data)
 {
+	INTERRUPTS_PAUSE;
 	mutexAcquire(&task->lock);
 	task->status = G_TASK_STATUS_WAITING;
 	task->waitsFor = "sleeps";
-	taskingWaitForExit(data->taskId, task->id);
 	mutexRelease(&task->lock);
+	taskingWaitForExit(data->taskId, task->id);
 	taskingYield();
+	INTERRUPTS_RESUME;
 }
 
 void syscallSpawn(g_task* task, g_syscall_spawn* data)
@@ -141,12 +145,14 @@ void syscallKill(g_task* task, g_syscall_kill* data)
 	g_task* target = taskingGetById(data->pid);
 	if(target)
 	{
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&target->lock);
 		data->status = G_KILL_STATUS_SUCCESSFUL;
 		target->process->main->status = G_TASK_STATUS_DEAD;
-		waitQueueWake(&target->waitersJoin);
 		mutexRelease(&target->lock);
+		waitQueueWake(&target->waitersJoin);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 	else
 	{
@@ -156,8 +162,6 @@ void syscallKill(g_task* task, g_syscall_kill* data)
 
 void syscallCreateTask(g_task* task, g_syscall_create_task* data)
 {
-	 mutexAcquire(&task->process->lock);
-
 	g_task* newTask = taskingCreateTask((g_virtual_address) data->initialEntry, task->process,
 	                                    task->process->main->securityLevel);
 	if(newTask)
@@ -172,8 +176,6 @@ void syscallCreateTask(g_task* task, g_syscall_create_task* data)
 	{
 		data->status = G_CREATE_TASK_STATUS_FAILED;
 	}
-
-	mutexRelease(&task->process->lock);
 }
 
 void syscallGetTaskEntry(g_task* task, g_syscall_get_task_entry* data)

kernel/src/kernel/filesystem/filesystem.cpp

@@ -411,12 +411,14 @@ g_fs_read_status filesystemRead(g_task* task, g_fd fd, uint8_t* buffer, uint64_t
 			panic("%! task %i tried to wait for file %i but delegate didn't provide wait-for-read implementation",
 			      "filesytem", task->id, node->id);
 
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&task->lock);
 		task->status = G_TASK_STATUS_WAITING;
 		task->waitsFor = "read";
-		delegate->waitForRead(task->id, node);
 		mutexRelease(&task->lock);
+		delegate->waitForRead(task->id, node);
 		taskingYield();
+		INTERRUPTS_RESUME;
 
 		// TODO
 		if(node->nonInterruptible && clockHasTimedOut(task->id))
@@ -503,12 +505,14 @@ g_fs_write_status filesystemWrite(g_task* task, g_fd fd, uint8_t* buffer, uint64
 			panic("%! task %i tried to wait for file %i but delegate didn't provide wait-for-write implementation",
 			      "filesytem", task->id, node->id);
 
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&task->lock);
 		task->status = G_TASK_STATUS_WAITING;
 		task->waitsFor = "write";
-		delegate->waitForWrite(task->id, node);
 		mutexRelease(&task->lock);
+		delegate->waitForWrite(task->id, node);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 	if(wrote > 0)
 	{

kernel/src/kernel/ipc/message.cpp

@@ -161,17 +161,13 @@ void messageTaskRemoved(g_tid task)
 void messageWaitForSend(g_tid sender, g_tid receiver)
 {
 	g_message_queue* queue = _messageGetQueue(receiver);
-	mutexAcquire(&queue->lock);
 	waitQueueAdd(&queue->waitersSend, sender);
-	mutexRelease(&queue->lock);
 }
 
 void messageUnwaitForSend(g_tid sender, g_tid receiver)
 {
 	g_message_queue* queue = _messageGetQueue(receiver);
-	mutexAcquire(&queue->lock);
 	waitQueueRemove(&queue->waitersSend, sender);
-	mutexRelease(&queue->lock);
 }
 
 void _messageWakeWaitingReceiver(g_message_queue* queue)

kernel/src/kernel/ipc/pipes.cpp

@@ -264,9 +264,7 @@ void pipeWaitForRead(g_tid task, g_fs_phys_id pipeId)
 	if(!pipe)
 		return;
 
-	mutexAcquire(&pipe->lock);
 	waitQueueAdd(&pipe->waitersRead, task);
-	mutexRelease(&pipe->lock);
 }
 
 void pipeWaitForWrite(g_tid task, g_fs_phys_id pipeId)
@@ -275,7 +273,5 @@ void pipeWaitForWrite(g_tid task, g_fs_phys_id pipeId)
 	if(!pipe)
 		return;
 
-	mutexAcquire(&pipe->lock);
 	waitQueueAdd(&pipe->waitersWrite, task);
-	mutexRelease(&pipe->lock);
 }

kernel/src/kernel/system/interrupts/interrupts.cpp

@@ -91,7 +91,7 @@ extern "C" volatile g_processor_state* _interruptHandler(volatile g_processor_st
 		if(irq == 0) // Timer
 		{
 			clockUpdate();
-			taskingSchedule();
+			taskingSchedule(true);
 		}
 		else
 		{

kernel/src/kernel/system/processor/processor.cpp

@@ -313,3 +313,8 @@ const uint8_t* processorGetInitialFpuState()
 {
 	return _processorGetCurrent()->fpu.initialState;
 }
+
+bool processorIsBsp()
+{
+	return processorGetCurrentId() == 0;
+}

kernel/src/kernel/system/processor/processor.hpp

@@ -258,4 +258,9 @@ bool processorHasFeatureReady(g_cpuid_standard_edx_feature feature);
  */
 const uint8_t* processorGetInitialFpuState();
 
+/**
+ * @return whether this code is running on the BSP
+ */
+bool processorIsBsp();
+
 #endif

kernel/src/kernel/tasking/cleanup.cpp

@@ -73,11 +73,13 @@ void taskingCleanupThread()
 		}
 
 		// Sleep for some time
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&self->lock);
 		self->status = G_TASK_STATUS_WAITING;
 		self->waitsFor = "cleanup-sleep";
-		clockWaitForTime(self->id, clockGetLocal()->time + 3000);
 		mutexRelease(&self->lock);
+		clockWaitForTime(self->id, clockGetLocal()->time + 3000);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 }

kernel/src/kernel/tasking/scheduler/scheduler_round_robin.cpp

@@ -50,9 +50,8 @@ g_schedule_entry* schedulerGetNextTask(g_tasking_local* local)
 		return entry;
 	}
 
-	// // Check if there is a global "preferred task" to do next
-	// TODO: This causes rare deadlocks...
-	if(false && preferredTask != G_TID_NONE)
+	// Check if there is a global "preferred task" to do next
+	if(preferredTask != G_TID_NONE)
 	{
 		while(entry)
 		{

kernel/src/kernel/tasking/tasking.cpp

@@ -308,7 +308,12 @@ void taskingRestoreState(g_task* task)
 		processorRestoreFpuState(task->fpu.state);
 }
 
-void taskingSchedule() { schedulerSchedule(taskingGetLocal()); }
+void taskingSchedule(bool resetPreference)
+{
+	if(resetPreference && processorIsBsp())
+		schedulerPrefer(G_TID_NONE);
+	schedulerSchedule(taskingGetLocal());
+}
 
 g_process* taskingCreateProcess(g_security_level securityLevel)
 {
@@ -439,7 +444,7 @@ void taskingDestroyTask(g_task* task)
 
 void _taskingInitializeTask(g_task* task, g_process* process, g_security_level level)
 {
-	if(process->main == 0)
+	if(process->main == nullptr)
 		task->id = process->id;
 	else
 		task->id = taskingGetNextId();
@@ -510,17 +515,19 @@ g_spawn_result taskingSpawn(g_fd fd, g_security_level securityLevel)
 	mutexAcquire(&caller->lock);
 	caller->status = G_TASK_STATUS_WAITING;
 	caller->waitsFor = "spawn";
+	mutexRelease(&caller->lock);
 	waitQueueAdd(&res.process->waitersSpawn, caller->id);
 	taskingAssignBalanced(newTask);
-	mutexRelease(&caller->lock);
 
 	while(!newTask->spawnFinished)
 	{
+		INTERRUPTS_PAUSE;
 		mutexAcquire(&caller->lock);
 		caller->status = G_TASK_STATUS_WAITING;
 		caller->waitsFor = "spawn-rep";
 		mutexRelease(&caller->lock);
 		taskingYield();
+		INTERRUPTS_RESUME;
 	}
 
 	// Take result
@@ -566,12 +573,14 @@ void taskingFinalizeSpawn(g_task* task)
 	taskingStateReset(task, process->spawnArgs->entry, task->securityLevel);
 	waitQueueWake(&process->waitersSpawn);
 
+	INTERRUPTS_PAUSE;
 	mutexAcquire(&task->lock);
 	task->status = G_TASK_STATUS_WAITING;
 	task->waitsFor = "wake-after-spawn";
 	task->spawnFinished = true;
 	mutexRelease(&task->lock);
 	taskingYield();
+	INTERRUPTS_RESUME;
 }
 
 void taskingWaitForExit(g_tid joinedTid, g_tid waiter)