Changeset 5663872 in mainline

Timestamp:

2024-01-14T18:24:05Z (4 months ago)

Author:

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

Branches:

master

Children:

23f36a3

Parents:

4760793

git-author:

Jiří Zárevúcky <zarevucky.jiri@…> (2023-02-18 13:37:30)

git-committer:

Jiří Zárevúcky <zarevucky.jiri@…> (2024-01-14 18:24:05)

Message:

Move stuff around for thread sleep

Only mark the thread as ready for wakeup after we switch to
another context. This way, soundness of the sychronization
does not depend on thread lock being held across the context
switch, which gives us more freedom.

Location:

kernel/generic

Files:

• include/proc/thread.h (modified) (1 diff)
• src/proc/scheduler.c (modified) (2 diffs)
• src/proc/thread.c (modified) (5 diffs)

kernel/generic/include/proc/thread.h

@@ -190 +190 @@
 extern void thread_interrupt(thread_t *);
 
+enum sleep_state {
+        SLEEP_INITIAL,
+        SLEEP_ASLEEP,
+        SLEEP_WOKE,
+};
+
 typedef enum {
         THREAD_OK,

kernel/generic/src/proc/scheduler.c

@@ -443 +443 @@
                 after_thread_ran();
 
+                int expected;
+
                 switch (THREAD->state) {
                 case Running:
@@ -462 +464 @@
 
                 case Sleeping:
-                        /*
-                         * Prefer the thread after it's woken up.
-                         */
-                        THREAD->priority = -1;
-                        irq_spinlock_unlock(&THREAD->lock, false);
+                        expected = SLEEP_INITIAL;
+
+                        /* Only set SLEEP_ASLEEP in sleep pad if it's still in initial state */
+                        if (atomic_compare_exchange_strong_explicit(&THREAD->sleep_state,
+                            &expected, SLEEP_ASLEEP,
+                            memory_order_acq_rel, memory_order_acquire)) {
+
+                                /* Prefer the thread after it's woken up. */
+                                THREAD->priority = -1;
+                                irq_spinlock_unlock(&THREAD->lock, false);
+                        } else {
+                                assert(expected == SLEEP_WOKE);
+                                /* The thread has already been woken up, requeue immediately. */
+                                irq_spinlock_unlock(&THREAD->lock, false);
+                                thread_ready(THREAD);
+                        }
+
                         break;
 

kernel/generic/src/proc/thread.c

@@ -82 +82 @@
 };
 
-enum sleep_state {
-        SLEEP_INITIAL,
-        SLEEP_ASLEEP,
-        SLEEP_WOKE,
-};
-
 /** Lock protecting the @c threads ordered dictionary .
  *
@@ -579 +573 @@
 }
 
-static void thread_wait_internal(void)
-{
-        assert(THREAD != NULL);
-
-        ipl_t ipl = interrupts_disable();
-
-        if (atomic_load(&haltstate))
-                halt();
-
-        /*
-         * Lock here to prevent a race between entering the scheduler and another
-         * thread rescheduling this thread.
-         */
-        irq_spinlock_lock(&THREAD->lock, false);
-
-        int expected = SLEEP_INITIAL;
-
-        /* Only set SLEEP_ASLEEP in sleep pad if it's still in initial state */
-        if (atomic_compare_exchange_strong_explicit(&THREAD->sleep_state, &expected,
-            SLEEP_ASLEEP, memory_order_acq_rel, memory_order_acquire)) {
-                THREAD->state = Sleeping;
-                scheduler_locked(ipl);
-        } else {
-                assert(expected == SLEEP_WOKE);
-                /* Return immediately. */
-                irq_spinlock_unlock(&THREAD->lock, false);
-                interrupts_restore(ipl);
-        }
-}
-
 static void thread_wait_timeout_callback(void *arg)
 {
@@ -649 +613 @@
         timeout_t timeout;
 
+        /* Extra check to avoid going to scheduler if we don't need to. */
+        if (atomic_load_explicit(&THREAD->sleep_state, memory_order_acquire) !=
+            SLEEP_INITIAL)
+                return THREAD_WAIT_SUCCESS;
+
         if (deadline != DEADLINE_NEVER) {
-                /* Extra check to avoid setting up a deadline if we don't need to. */
-                if (atomic_load_explicit(&THREAD->sleep_state, memory_order_acquire) !=
-                    SLEEP_INITIAL)
-                        return THREAD_WAIT_SUCCESS;
-
                 timeout_initialize(&timeout);
                 timeout_register_deadline(&timeout, deadline,
@@ -660 +624 @@
         }
 
-        thread_wait_internal();
+        ipl_t ipl = interrupts_disable();
+        irq_spinlock_lock(&THREAD->lock, false);
+        THREAD->state = Sleeping;
+        scheduler_locked(ipl);
 
         if (deadline != DEADLINE_NEVER && !timeout_unregister(&timeout)) {
@@ -674 +641 @@
 
         int state = atomic_exchange_explicit(&thread->sleep_state, SLEEP_WOKE,
-            memory_order_release);
+            memory_order_acq_rel);
 
         if (state == SLEEP_ASLEEP) {