Implement proper mutex cleanup

init/init.c

@@ -6,7 +6,7 @@
 #include <stdint.h>
 #include <string/string.h>
 
-#define EXAMPLE 2
+#define EXAMPLE 3
 
 #if EXAMPLE == 1
 
@@ -47,8 +47,6 @@ void app_main (void) {
 
   spawn (&app_thread1);
 
-  /* for (volatile int i = 0; i < 1000*1000; i++) */
-  /*   ; */
   for (;;) {
     lock_mutex (MUTEX, RV_PRIVATE);
 
@@ -56,9 +54,6 @@ void app_main (void) {
       test ('a');
 
     unlock_mutex (MUTEX, RV_PRIVATE);
-
-    /* for (volatile int i = 0; i < 1000*1000; i++) */
-    /*   ; */
   }
 }
 
@@ -70,9 +65,49 @@ void app_thread1 (void) {
       test ('b');
 
     unlock_mutex (MUTEX, RV_PRIVATE);
+  }
 
-    /* for (volatile int i = 0; i < 1000*1000; i++) */
-    /*   ; */
+  quit ();
+}
+#elif EXAMPLE == 3
+
+#define MUTEX 2000
+
+void app_thread1 (void);
+
+int spawn (void (*fn) (void)) {
+  size_t stack_size = 256 * PAGE_SIZE;
+  void* stack = malloc (stack_size);
+  if (stack == NULL)
+    return -ST_OOM_ERROR;
+
+  uintptr_t stack_top = (uintptr_t)stack + stack_size;
+  return clone (stack_top, stack_size, fn);
+}
+
+void app_main (void) {
+  create_mutex (MUTEX, RV_PRIVATE);
+
+  spawn (&app_thread1);
+
+  for (;;) {
+    lock_mutex (MUTEX, RV_PRIVATE);
+
+    for (int i = 0; i < 3; i++)
+      test ('a');
+
+    quit ();
+  }
+}
+
+void app_thread1 (void) {
+  for (;;) {
+    lock_mutex (MUTEX, RV_PRIVATE);
+
+    for (int i = 0; i < 3; i++)
+      test ('b');
+
+    unlock_mutex (MUTEX, RV_PRIVATE);
   }
 
   quit ();

kernel/amd64/proc.c

@@ -184,10 +184,35 @@ struct proc* proc_clone (struct proc* proto, uintptr_t vstack_top, size_t stack_
 
 void proc_cleanup (struct proc* proc) {
   struct limine_hhdm_response* hhdm = limine_hhdm_request.response;
-  spin_lock_ctx_t ctxprpd;
+  spin_lock_ctx_t ctxprpd, ctxsq, ctxpr;
 
+  spin_lock (&proc->lock, &ctxpr);
+
+  /* clean suspension queue entries */
+  struct list_node_link *sq_link, *sq_link_tmp;
+  list_foreach (proc->sq_entries, sq_link, sq_link_tmp) {
+    struct proc_sq_entry* sq_entry = list_entry (sq_link, struct proc_sq_entry, proc_link);
+    struct proc_suspension_q* sq = sq_entry->sq;
+
+    spin_lock (&sq->lock, &ctxsq);
+
+    /* remove from sq's list */
+    list_remove (sq->proc_list, &sq_entry->sq_link);
+
+    /* remove from proc's list */
+    list_remove (proc->sq_entries, &sq_entry->proc_link);
+
+    spin_unlock (&sq->lock, &ctxsq);
+
+    free (sq_entry);
+  }
+
+  spin_unlock (&proc->lock, &ctxpr);
+
+  /* clean resources */
   proc_cleanup_resources (proc);
 
+  /* clean virtual address space */
   if (atomic_fetch_sub (&proc->pd->refs, 1) == 1) {
     DEBUG ("PID %d Free virtual address space\n", proc->pid);
     struct list_node_link *mapping_link, *mapping_link_tmp;
@@ -206,13 +231,16 @@ void proc_cleanup (struct proc* proc) {
     free (proc->pd);
   }
 
+  /* clean kstack */
   pmm_free (proc->pdata.kernel_stack - (uintptr_t)hhdm->offset - KSTACK_SIZE,
             KSTACK_SIZE / PAGE_SIZE);
 
+  /* clean ustack */
   if ((proc->flags & PROC_USTK_PREALLOC))
     pmm_free (proc->pdata.user_stack, USTACK_SIZE / PAGE_SIZE);
 
   DEBUG ("PID %d Free stacks\n", proc->pid);
 
+  /* clean the process */
   free (proc);
 }

kernel/proc/mutex.c

@@ -2,6 +2,7 @@
 #include <libk/rbtree.h>
 #include <libk/std.h>
 #include <libk/string.h>
+#include <mm/liballoc.h>
 #include <proc/mutex.h>
 #include <proc/proc.h>
 #include <sync/spin_lock.h>
@@ -9,23 +10,20 @@
 #include <sys/smp.h>
 #include <sys/spin_lock.h>
 
-bool proc_create_resource_mutex (struct proc_mutex* mutex) {
-  memset (mutex, 0, sizeof (*mutex));
-
-  return true;
-}
-
-void proc_cleanup_resource_mutex (struct proc* proc, struct proc_resource* resource) {
-  struct proc_mutex* mutex = &resource->u.mutex;
-
-  /* proc_mutex_unlock (proc, mutex); */
-}
-
 static void proc_mutex_suspend (struct proc* proc, struct proc_suspension_q* sq,
                                 spin_lock_t* resource_lock, spin_lock_ctx_t* ctxrl) {
   spin_lock_ctx_t ctxpr, ctxcpu, ctxsq;
   struct cpu* cpu = proc->cpu;
 
+  struct proc_sq_entry* sq_entry = malloc (sizeof (*sq_entry));
+  if (!sq_entry) {
+    spin_unlock (resource_lock, ctxrl);
+    return;
+  }
+
+  sq_entry->proc = proc;
+  sq_entry->sq = sq;
+
   spin_lock (&cpu->lock, &ctxcpu);
   spin_lock (&proc->lock, &ctxpr);
   spin_lock (&sq->lock, &ctxsq);
@@ -33,7 +31,12 @@ static void proc_mutex_suspend (struct proc* proc, struct proc_suspension_q* sq,
   spin_unlock (resource_lock, ctxrl);
 
   atomic_store (&proc->state, PROC_SUSPENDED);
-  proc->suspension_q = sq;
+
+  /* append to sq's list */
+  list_append (sq->proc_list, &sq_entry->sq_link);
+
+  /* append to proc's list */
+  list_append (proc->sq_entries, &sq_entry->proc_link);
 
   list_remove (cpu->proc_run_q, &proc->cpu_run_q_link);
   atomic_fetch_sub (&cpu->proc_run_q_count, 1);
@@ -41,7 +44,6 @@ static void proc_mutex_suspend (struct proc* proc, struct proc_suspension_q* sq,
   if (cpu->proc_current == proc)
     cpu->proc_current = NULL;
 
-  list_append (sq->proc_list, &proc->suspension_link);
   proc->cpu = NULL;
 
   spin_unlock (&sq->lock, &ctxsq);
@@ -51,34 +53,74 @@ static void proc_mutex_suspend (struct proc* proc, struct proc_suspension_q* sq,
   cpu_request_sched (cpu);
 }
 
-static void proc_mutex_resume (struct proc* proc) {
+static void proc_mutex_resume (struct proc* proc, struct proc_sq_entry* sq_entry) {
   spin_lock_ctx_t ctxsq, ctxpr, ctxcpu;
   struct cpu* cpu = cpu_find_lightest ();
+  struct proc_suspension_q* sq = sq_entry->sq;
 
   spin_lock (&cpu->lock, &ctxcpu);
   spin_lock (&proc->lock, &ctxpr);
-
-  proc->cpu = cpu;
-  struct proc_suspension_q* sq = proc->suspension_q;
-
   spin_lock (&sq->lock, &ctxsq);
 
-  list_remove (sq->proc_list, &proc->suspension_link);
+  /* remove from sq's list */
+  list_remove (sq->proc_list, &sq_entry->sq_link);
 
-  proc->suspension_q = NULL;
-  atomic_store (&proc->state, PROC_READY);
+  /* remove from proc's list */
+  list_remove (proc->sq_entries, &sq_entry->proc_link);
+
+  proc->cpu = cpu;
+
+  if (proc->sq_entries == NULL)
+    atomic_store (&proc->state, PROC_READY);
 
   list_append (cpu->proc_run_q, &proc->cpu_run_q_link);
   atomic_fetch_add (&cpu->proc_run_q_count, 1);
 
   spin_unlock (&sq->lock, &ctxsq);
-
   spin_unlock (&proc->lock, &ctxpr);
   spin_unlock (&cpu->lock, &ctxcpu);
 
+  free (sq_entry);
+
   cpu_request_sched (cpu);
 }
 
+bool proc_create_resource_mutex (struct proc_mutex* mutex) {
+  memset (mutex, 0, sizeof (*mutex));
+
+  return true;
+}
+
+void proc_cleanup_resource_mutex (struct proc* proc, struct proc_resource* resource) {
+  struct proc_mutex* mutex = &resource->u.mutex;
+  spin_lock_ctx_t ctxmt, ctxsq;
+
+  spin_lock (&mutex->resource->lock, &ctxmt);
+  spin_lock (&mutex->suspension_q.lock, &ctxsq);
+
+  while (mutex->suspension_q.proc_list != NULL) {
+    struct list_node_link* node = mutex->suspension_q.proc_list;
+    struct proc_sq_entry* sq_entry = list_entry (node, struct proc_sq_entry, sq_link);
+    struct proc* suspended_proc = sq_entry->proc;
+
+    /* we will relock during resume */
+    spin_unlock (&mutex->suspension_q.lock, &ctxsq);
+    spin_unlock (&mutex->resource->lock, &ctxmt);
+
+    proc_mutex_resume (suspended_proc, sq_entry);
+
+    /* reacquire */
+    spin_lock (&mutex->resource->lock, &ctxmt);
+    spin_lock (&mutex->suspension_q.lock, &ctxsq);
+  }
+
+  mutex->locked = false;
+  mutex->owner = NULL;
+
+  spin_unlock (&mutex->suspension_q.lock, &ctxsq);
+  spin_unlock (&mutex->resource->lock, &ctxmt);
+}
+
 void proc_mutex_lock (struct proc* proc, struct proc_mutex* mutex) {
   spin_lock_ctx_t ctxmt;
 
@@ -108,18 +150,19 @@ bool proc_mutex_unlock (struct proc* proc, struct proc_mutex* mutex) {
 
   spin_lock (&mutex->suspension_q.lock, &ctxsq);
 
-  struct proc* resumed_proc = NULL;
   struct list_node_link* node = mutex->suspension_q.proc_list;
 
   if (node) {
-    resumed_proc = list_entry (node, struct proc, suspension_link);
+    struct proc_sq_entry* sq_entry = list_entry (node, struct proc_sq_entry, sq_link);
+    struct proc* resumed_proc = sq_entry->proc;
+
     mutex->owner = resumed_proc;
     mutex->locked = true;
 
     spin_unlock (&mutex->suspension_q.lock, &ctxsq);
     spin_unlock (&mutex->resource->lock, &ctxmt);
 
-    proc_mutex_resume (resumed_proc);
+    proc_mutex_resume (resumed_proc, sq_entry);
 
     return true;
   }

kernel/proc/proc.h

@@ -42,12 +42,19 @@ struct proc_resources {
   rw_spin_lock_t lock;
 };
 
+struct proc_sq_entry {
+  struct list_node_link sq_link;
+  struct list_node_link proc_link;
+  struct proc* proc;
+  struct proc_suspension_q* sq;
+};
+
 struct proc {
   int pid;
   struct rb_node_link proc_tree_link;
   struct list_node_link cpu_run_q_link;
-  struct list_node_link suspension_link;
   struct list_node_link reap_link;
+  struct list_node_link* sq_entries;
 
   struct list_node_link* mappings; /* pd.lock implicitly protects this field */
   struct proc_platformdata pdata;
@@ -56,7 +63,6 @@ struct proc {
   spin_lock_t lock;
   struct cpu* cpu;
   atomic_int state;
-  struct proc_suspension_q* suspension_q;
   struct proc_resources* resources;
 };