Changeset 1871118 in mainline for kernel/generic/src/proc

Timestamp:

2023-02-10T22:59:11Z (3 years ago)

Author:

Jiří Zárevúcky <zarevucky.jiri@…>

Branches:

master, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

11d2c983

Parents:

daadfa6

git-author:

Jiří Zárevúcky <zarevucky.jiri@…> (2023-02-10 22:53:12)

git-committer:

Jiří Zárevúcky <zarevucky.jiri@…> (2023-02-10 22:59:11)

Message:

Make thread_t reference counted

This simplifies interaction between various locks and thread
lifespan, which simplifies things. For example, threads_lock can
now simply be a mutex protecting the global it was made for, and
nothing more.

Location:

kernel/generic/src/proc

Files:

• program.c (modified) (1 diff)
• scheduler.c (modified) (3 diffs)
• task.c (modified) (3 diffs)
• thread.c (modified) (19 diffs)

kernel/generic/src/proc/program.c

@@ -204 +204 @@
 {
         thread_ready(prg->main_thread);
+        prg->main_thread = NULL;
 }
 

kernel/generic/src/proc/scheduler.c

@@ -389 +389 @@
 void scheduler_separated_stack(void)
 {
-        DEADLOCK_PROBE_INIT(p_joinwq);
         task_t *old_task = TASK;
         as_t *old_as = AS;
@@ -419 +418 @@
 
                 case Exiting:
-                repeat:
-                        if (THREAD->detached) {
-                                thread_destroy(THREAD, false);
-                        } else {
-                                /*
-                                 * The thread structure is kept allocated until
-                                 * somebody calls thread_detach() on it.
-                                 */
-                                if (!irq_spinlock_trylock(&THREAD->join_wq.lock)) {
-                                        /*
-                                         * Avoid deadlock.
-                                         */
-                                        irq_spinlock_unlock(&THREAD->lock, false);
-                                        delay(HZ);
-                                        irq_spinlock_lock(&THREAD->lock, false);
-                                        DEADLOCK_PROBE(p_joinwq,
-                                            DEADLOCK_THRESHOLD);
-                                        goto repeat;
-                                }
-                                _waitq_wakeup_unsafe(&THREAD->join_wq,
-                                    WAKEUP_FIRST);
-                                irq_spinlock_unlock(&THREAD->join_wq.lock, false);
-
-                                THREAD->state = Lingering;
-                                irq_spinlock_unlock(&THREAD->lock, false);
-                        }
+                        irq_spinlock_unlock(&THREAD->lock, false);
+                        waitq_wakeup(&THREAD->join_wq, WAKEUP_CLOSE);
+
+                        /*
+                         * Release the reference CPU has for the thread.
+                         * If there are no other references (e.g. threads calling join),
+                         * the thread structure is deallocated.
+                         */
+                        thread_put(THREAD);
                         break;
 
@@ -556 +538 @@
         size_t average;
         size_t rdy;
-
-        /*
-         * Detach kcpulb as nobody will call thread_join_timeout() on it.
-         */
-        thread_detach(THREAD);
 
 loop:

kernel/generic/src/proc/task.c

@@ -385 +385 @@
         irq_spinlock_lock(&tasks_lock, true);
         irq_spinlock_lock(&TASK->lock, false);
-        irq_spinlock_lock(&threads_lock, false);
 
         /* Set task name */
         str_cpy(TASK->name, TASK_NAME_BUFLEN, namebuf);
 
-        irq_spinlock_unlock(&threads_lock, false);
         irq_spinlock_unlock(&TASK->lock, false);
         irq_spinlock_unlock(&tasks_lock, true);
@@ -529 +527 @@
 {
         irq_spinlock_lock(&task->lock, false);
-        irq_spinlock_lock(&threads_lock, false);
 
         /*
@@ -536 +533 @@
 
         list_foreach(task->threads, th_link, thread_t, thread) {
-                thread_interrupt(thread, false);
+                thread_t *thr = thread_try_ref(thread);
+                if (thr)
+                        thread_interrupt(thr, false);
+
+                // If NULL, the thread is already getting destroyed concurrently with this.
         }
 
-        irq_spinlock_unlock(&threads_lock, false);
         irq_spinlock_unlock(&task->lock, false);
 }

kernel/generic/src/proc/thread.c

@@ -94 +94 @@
  *
  * Members are of type thread_t.
+ *
+ * This structure contains weak references. Any reference from it must not leave
+ * threads_lock critical section unless strengthened via thread_try_ref().
  */
 odict_t threads;
@@ -249 +252 @@
 /** Make thread ready
  *
- * Switch thread to the ready state.
+ * Switch thread to the ready state. Consumes reference passed by the caller.
  *
  * @param thread Thread to make ready.
@@ -320 +323 @@
                 return NULL;
 
+        refcount_init(&thread->refcount);
+
         if (thread_create_arch(thread, flags) != EOK) {
                 slab_free(thread_cache, thread);
@@ -368 +373 @@
 
         thread->interrupted = false;
-        thread->detached = false;
         waitq_initialize(&thread->join_wq);
 
@@ -399 +403 @@
  *
  */
-void thread_destroy(thread_t *thread, bool irq_res)
-{
-        assert(irq_spinlock_locked(&thread->lock));
+static void thread_destroy(void *obj)
+{
+        thread_t *thread = (thread_t *) obj;
+
+        irq_spinlock_lock(&thread->lock, true);
         assert((thread->state == Exiting) || (thread->state == Lingering));
         assert(thread->task);
@@ -421 +427 @@
          */
         list_remove(&thread->th_link);
-        irq_spinlock_unlock(&thread->task->lock, irq_res);
+        irq_spinlock_unlock(&thread->task->lock, true);
 
         /*
@@ -430 +436 @@
 }
 
+void thread_put(thread_t *thread)
+{
+        if (refcount_down(&thread->refcount)) {
+                thread_destroy(thread);
+        }
+}
+
 /** Make the thread visible to the system.
  *
@@ -441 +454 @@
 void thread_attach(thread_t *thread, task_t *task)
 {
+        ipl_t ipl = interrupts_disable();
+
         /*
          * Attach to the specified task.
          */
-        irq_spinlock_lock(&task->lock, true);
+        irq_spinlock_lock(&task->lock, false);
 
         /* Hold a reference to the task. */
@@ -455 +470 @@
         list_append(&thread->th_link, &task->threads);
 
-        irq_spinlock_pass(&task->lock, &threads_lock);
+        irq_spinlock_unlock(&task->lock, false);
 
         /*
          * Register this thread in the system-wide dictionary.
          */
+        irq_spinlock_lock(&threads_lock, false);
         odict_insert(&thread->lthreads, &threads, NULL);
-        irq_spinlock_unlock(&threads_lock, true);
+        irq_spinlock_unlock(&threads_lock, false);
+
+        interrupts_restore(ipl);
 }
 
@@ -513 +531 @@
  * blocking call was interruptable. See waitq_sleep_timeout().
  *
- * The caller must guarantee the thread object is valid during the entire
- * function, eg by holding the threads_lock lock.
- *
  * Interrupted threads automatically exit when returning back to user space.
  *
- * @param thread A valid thread object. The caller must guarantee it
- *               will remain valid until thread_interrupt() exits.
+ * @param thread A valid thread object.
  */
 void thread_interrupt(thread_t *thread, bool irq_dis)
@@ -534 +548 @@
         if (sleeping)
                 waitq_interrupt_sleep(thread);
+
+        thread_put(thread);
 }
 
@@ -581 +597 @@
 
 /** Wait for another thread to exit.
+ * This function does not destroy the thread. Reference counting handles that.
  *
  * @param thread Thread to join on exit.
@@ -594 +611 @@
                 return EINVAL;
 
-        /*
-         * Since thread join can only be called once on an undetached thread,
-         * the thread pointer is guaranteed to be still valid.
-         */
-
         irq_spinlock_lock(&thread->lock, true);
-        assert(!thread->detached);
+        state_t state = thread->state;
         irq_spinlock_unlock(&thread->lock, true);
 
-        return waitq_sleep_timeout(&thread->join_wq, usec, flags, NULL);
-
-        // FIXME: join should deallocate the thread.
-        //        Current code calls detach after join, that's contrary to how
-        //        join is used in other threading APIs.
-}
-
-/** Detach thread.
- *
- * Mark the thread as detached. If the thread is already
- * in the Lingering state, deallocate its resources.
- *
- * @param thread Thread to be detached.
- *
- */
-void thread_detach(thread_t *thread)
-{
-        /*
-         * Since the thread is expected not to be already detached,
-         * pointer to it must be still valid.
-         */
-        irq_spinlock_lock(&thread->lock, true);
-        assert(!thread->detached);
-
-        if (thread->state == Lingering) {
-                /*
-                 * Unlock &thread->lock and restore
-                 * interrupts in thread_destroy().
-                 */
-                thread_destroy(thread, true);
-                return;
+        if (state == Exiting) {
+                return EOK;
         } else {
-                thread->detached = true;
-        }
-
-        irq_spinlock_unlock(&thread->lock, true);
+                return waitq_sleep_timeout(&thread->join_wq, usec,
+                    SYNCH_FLAGS_NON_BLOCKING, NULL);
+        }
 }
 
@@ -702 +684 @@
         thread_t *thread;
 
-        /* Messing with thread structures, avoid deadlock */
+        /* Accessing system-wide threads list through thread_first()/thread_next(). */
         irq_spinlock_lock(&threads_lock, true);
 
@@ -730 +712 @@
 }
 
-/** Check whether thread exists.
- *
- * Note that threads_lock must be already held and
- * interrupts must be already disabled.
- *
- * @param thread Pointer to thread.
- *
- * @return True if thread t is known to the system, false otherwise.
- *
- */
-bool thread_exists(thread_t *thread)
-{
-        assert(interrupts_disabled());
-        assert(irq_spinlock_locked(&threads_lock));
-
+static bool thread_exists(thread_t *thread)
+{
         odlink_t *odlink = odict_find_eq(&threads, thread, NULL);
         return odlink != NULL;
+}
+
+/** Check whether the thread exists, and if so, return a reference to it.
+ */
+thread_t *thread_try_get(thread_t *thread)
+{
+        irq_spinlock_lock(&threads_lock, true);
+
+        if (thread_exists(thread)) {
+                /* Try to strengthen the reference. */
+                thread = thread_try_ref(thread);
+        } else {
+                thread = NULL;
+        }
+
+        irq_spinlock_unlock(&threads_lock, true);
+
+        return thread;
 }
 
@@ -777 +764 @@
  * interrupts must be disabled.
  *
+ * The returned reference is weak.
+ * If the caller needs to keep it, thread_try_ref() must be used to upgrade
+ * to a strong reference _before_ threads_lock is released.
+ *
  * @param id Thread ID.
  *
@@ -854 +845 @@
 {
         irq_spinlock_lock(&threads_lock, true);
-
-        thread_t *thread = thread_find_by_id(thread_id);
+        thread_t *thread = thread_try_ref(thread_find_by_id(thread_id));
+        irq_spinlock_unlock(&threads_lock, true);
+
         if (thread == NULL) {
                 printf("No such thread.\n");
-                irq_spinlock_unlock(&threads_lock, true);
                 return;
         }
-
-        irq_spinlock_lock(&thread->lock, false);
 
         /*
@@ -876 +865 @@
          */
 
+        irq_spinlock_lock(&thread->lock, true);
+
         bool sleeping = false;
         istate_t *istate = thread->udebug.uspace_state;
@@ -886 +877 @@
                 printf("Thread interrupt state not available.\n");
 
-        irq_spinlock_unlock(&thread->lock, false);
+        irq_spinlock_unlock(&thread->lock, true);
 
         if (sleeping)
                 waitq_interrupt_sleep(thread);
 
-        irq_spinlock_unlock(&threads_lock, true);
+        thread_put(thread);
 }