WIP inline the fast path of the mutexes

Change-Id: I6e0abac9ab51ec462135d269194cb67df3b41358

kernel/app/tests/thread_tests.c

@@ -40,6 +40,7 @@ static int mutex_thread(void *arg)
 {
     int i;
     const int iterations = 100000;
+    int count = 0;
 
     static volatile int shared = 0;
 
@@ -54,13 +55,19 @@ static int mutex_thread(void *arg)
             panic("someone else has messed with the shared data\n");
 
         shared = (intptr_t)get_current_thread();
-        thread_yield();
+        if (rand() % 1)
+            thread_yield();
+
+        if (++count % 10000 == 0)
+            printf("%p: count %d\n", get_current_thread(), count);
         shared = 0;
 
         mutex_release(m);
         thread_yield();
     }
 
+    printf("mutex tester %p done\n", get_current_thread());
+
     return 0;
 }
 
@@ -84,6 +91,18 @@ static int mutex_test(void)
         thread_join(threads[i], NULL, INFINITE_TIME);
     }
 
+    thread_sleep_relative(LK_MSEC(100));
+
+    static const uint count = 128*1024*1024;
+    uint64_t c = arch_cycle_count();
+    for (uint i = 0; i < count; i++) {
+        mutex_acquire(&m);
+        mutex_release(&m);
+    }
+    c = arch_cycle_count() - c;
+
+    printf("%" PRIu64 " cycles to acquire/release uncontended mutex %u times (%" PRIu64 " cycles per)\n", c, count, c / count);
+
     printf("done with mutex tests\n");
 
     return 0;

kernel/include/kernel/mutex.h

@@ -12,6 +12,7 @@
 #include <magenta/compiler.h>
 #include <magenta/thread_annotations.h>
 #include <debug.h>
+#include <err.h>
 #include <stdint.h>
 #include <kernel/thread.h>
 
@@ -21,16 +22,18 @@ __BEGIN_CDECLS;
 
 typedef struct TA_CAP("mutex") mutex {
     uint32_t magic;
-    thread_t *holder;
-    int count;
+    uintptr_t val;
     wait_queue_t wait;
 } mutex_t;
 
+#define MUTEX_FLAG_QUEUED ((uintptr_t)1)
+
+#define MUTEX_HOLDER(m) ((thread_t *)(((m)->val) & ~MUTEX_FLAG_QUEUED))
+
 #define MUTEX_INITIAL_VALUE(m) \
 { \
     .magic = MUTEX_MAGIC, \
-    .holder = NULL, \
-    .count = 0, \
+    .val = 0, \
     .wait = WAIT_QUEUE_INITIAL_VALUE((m).wait), \
 }
 
@@ -38,20 +41,53 @@ typedef struct TA_CAP("mutex") mutex {
  * - Mutexes are only safe to use from thread context.
  * - Mutexes are non-recursive.
 */
+void mutex_init(mutex_t *m);
+void mutex_destroy(mutex_t *m);
+status_t mutex_acquire_slow(mutex_t *m) TA_ACQ(m);
+void mutex_release_slow(mutex_t *m, bool resched, bool thread_lock_held) TA_REL(m);
 
-void mutex_init(mutex_t *);
-void mutex_destroy(mutex_t *);
-void mutex_acquire(mutex_t *m) TA_ACQ(m);
-void mutex_release(mutex_t *m) TA_REL(m);
+/* fast path for mutexes */
+static inline void mutex_acquire(mutex_t *m) TA_ACQ(m)
+{
+    thread_t *ct = get_current_thread();
 
-/* Internal functions for use by condvar implementation. */
-void mutex_acquire_internal(mutex_t *m) TA_ACQ(m);
-void mutex_release_internal(mutex_t *m, bool reschedule) TA_REL(m);
+    for (;;) {
+        uintptr_t zero = 0;
+        if (likely(atomic_cmpxchg_u64(&m->val, &zero, (uintptr_t)ct))) {
+            // acquired it cleanly
+            return;
+        }
+
+        if (likely(mutex_acquire_slow(m) == NO_ERROR))
+            return;
+    }
+}
+
+/* fast path for release */
+static inline void mutex_release_etc(mutex_t *m, bool resched, bool thread_lock_held) TA_REL(m)
+{
+    thread_t *ct = get_current_thread();
+    uintptr_t oldval;
+
+    // in case there's no contention, try the fast path
+    oldval = (uintptr_t)ct;
+    if (likely(atomic_cmpxchg_u64(&m->val, &oldval, 0))) {
+        // we're done, exit
+        return;
+    }
+
+    mutex_release_slow(m, resched, thread_lock_held);
+}
+
+static inline void mutex_release(mutex_t *m) TA_REL(m)
+{
+    mutex_release_etc(m, true, false);
+}
 
 /* does the current thread hold the mutex? */
-static bool is_mutex_held(const mutex_t *m)
+static inline bool is_mutex_held(const mutex_t *m)
 {
-    return m->holder == get_current_thread();
+    return (MUTEX_HOLDER(m) == get_current_thread());
 }
 
 __END_CDECLS;

kernel/include/kernel/wait.h

@@ -54,6 +54,7 @@ status_t wait_queue_block(wait_queue_t *, lk_time_t deadline);
  */
 int wait_queue_wake_one(wait_queue_t *, bool reschedule, status_t wait_queue_error);
 int wait_queue_wake_all(wait_queue_t *, bool reschedule, status_t wait_queue_error);
+struct thread *wait_queue_dequeue_one(wait_queue_t *wait, status_t wait_queue_error);
 
 /*
  * remove the thread from whatever wait queue it's in.
@@ -61,6 +62,9 @@ int wait_queue_wake_all(wait_queue_t *, bool reschedule, status_t wait_queue_err
  */
 status_t thread_unblock_from_wait_queue(struct thread *t, status_t wait_queue_error);
 
+/* is the wait queue currently empty */
+bool wait_queue_is_empty(wait_queue_t *);
+
 __END_CDECLS;
 
 #endif

kernel/kernel/mutex.c

@@ -16,10 +16,16 @@
  */
 
 #include <kernel/mutex.h>
+
 #include <debug.h>
 #include <assert.h>
 #include <err.h>
+#include <inttypes.h>
 #include <kernel/thread.h>
+#include <kernel/sched.h>
+#include <trace.h>
+
+#define LOCAL_TRACE 0
 
 /**
  * @brief  Initialize a mutex_t
@@ -42,91 +48,113 @@ void mutex_destroy(mutex_t *m)
 
     THREAD_LOCK(state);
 #if LK_DEBUGLEVEL > 0
-    if (unlikely(m->count > 0)) {
+    if (unlikely(m->val != 0)) {
+        thread_t *holder = MUTEX_HOLDER(m);
         panic("mutex_destroy: thread %p (%s) tried to destroy locked mutex %p,"
               " locked by %p (%s)\n",
               get_current_thread(), get_current_thread()->name, m,
-              m->holder, m->holder->name);
+              holder, holder->name);
     }
 #endif
     m->magic = 0;
-    m->count = 0;
+    m->val = 0;
     wait_queue_destroy(&m->wait);
     THREAD_UNLOCK(state);
 }
 
-void mutex_acquire_internal(mutex_t *m) TA_NO_THREAD_SAFETY_ANALYSIS
-{
-    DEBUG_ASSERT(arch_ints_disabled());
-    DEBUG_ASSERT(spin_lock_held(&thread_lock));
-    DEBUG_ASSERT(!arch_in_int_handler());
-
-    if (unlikely(++m->count > 1)) {
-        status_t ret = wait_queue_block(&m->wait, INFINITE_TIME);
-        if (unlikely(ret < NO_ERROR)) {
-            /* mutexes are not interruptable and cannot time out, so it
-             * is illegal to return with any error state.
-             */
-            panic("mutex_acquire_internal: wait_queue_block returns with error %d m %p, thr %p, sp %p\n",
-                   ret, m, get_current_thread(), __GET_FRAME());
-        }
-    }
-
-    m->holder = get_current_thread();
-}
-
-/**
- * @brief  Acquire the mutex
- *
- * @return  NO_ERROR on success, other values on error
- */
-void mutex_acquire(mutex_t *m)
+status_t mutex_acquire_slow(mutex_t *m) TA_NO_THREAD_SAFETY_ANALYSIS
 {
     DEBUG_ASSERT(m->magic == MUTEX_MAGIC);
     DEBUG_ASSERT(!arch_in_int_handler());
 
+    thread_t *ct = get_current_thread();
+    uintptr_t oldval;
+
+    LTRACEF("%p slow path\n", ct);
+
 #if LK_DEBUGLEVEL > 0
-    if (unlikely(get_current_thread() == m->holder))
+    if (unlikely(ct == MUTEX_HOLDER(m)))
         panic("mutex_acquire: thread %p (%s) tried to acquire mutex %p it already owns.\n",
-              get_current_thread(), get_current_thread()->name, m);
+              ct, ct->name, m);
 #endif
 
+    // we contended with someone else, will probably need to block
     THREAD_LOCK(state);
-    mutex_acquire_internal(m);
-    THREAD_UNLOCK(state);
-}
 
-void mutex_release_internal(mutex_t *m, bool reschedule) TA_NO_THREAD_SAFETY_ANALYSIS
-{
-    DEBUG_ASSERT(arch_ints_disabled());
-    DEBUG_ASSERT(spin_lock_held(&thread_lock));
-    DEBUG_ASSERT(!arch_in_int_handler());
-
-    m->holder = 0;
-
-    if (unlikely(--m->count >= 1)) {
-        /* release a thread */
-        wait_queue_wake_one(&m->wait, reschedule, NO_ERROR);
+    // save the current state and check to see if it wasn't released in the interim
+    oldval = m->val;
+    if (unlikely(oldval == 0)) {
+        THREAD_UNLOCK(state);
+        return ERR_BAD_STATE;
     }
+
+    // try to exchange again with a flag indicating that we're blocking is set
+    if (unlikely(!atomic_cmpxchg_u64(&m->val, &oldval, oldval | MUTEX_FLAG_QUEUED))) {
+        // if we fail, just start over from the top
+        THREAD_UNLOCK(state);
+        return ERR_BAD_STATE;
+    }
+
+    status_t ret = wait_queue_block(&m->wait, INFINITE_TIME);
+    if (unlikely(ret < NO_ERROR)) {
+        /* mutexes are not interruptable and cannot time out, so it
+         * is illegal to return with any error state.
+         */
+        panic("mutex_acquire: wait_queue_block returns with error %d m %p, thr %p, sp %p\n",
+               ret, m, ct, __GET_FRAME());
+    }
+
+    LTRACEF("%p woken up, m->val is %#" PRIxPTR "\n", ct, m->val);
+
+    // someone must have woken us up, we should own the mutex now
+    DEBUG_ASSERT(ct == MUTEX_HOLDER(m));
+
+    THREAD_UNLOCK(state);
+
+    return NO_ERROR;
 }
 
-/**
- * @brief  Release mutex
- */
-void mutex_release(mutex_t *m)
+void mutex_release_slow(mutex_t *m, bool resched, bool thread_lock_held) TA_NO_THREAD_SAFETY_ANALYSIS
 {
     DEBUG_ASSERT(m->magic == MUTEX_MAGIC);
     DEBUG_ASSERT(!arch_in_int_handler());
 
+    thread_t *ct = get_current_thread();
+    uintptr_t oldval;
+
+    // slow path from now on out
+    LTRACEF("%p slow path\n", ct);
+
 #if LK_DEBUGLEVEL > 0
-    if (unlikely(get_current_thread() != m->holder)) {
+    if (unlikely(ct != MUTEX_HOLDER(m))) {
+        thread_t *holder = MUTEX_HOLDER(m);
         panic("mutex_release: thread %p (%s) tried to release mutex %p it doesn't own. owned by %p (%s)\n",
-              get_current_thread(), get_current_thread()->name, m, m->holder, m->holder ? m->holder->name : "none");
+              ct, ct->name, m, holder, holder ? holder->name : "none");
     }
 #endif
 
-    THREAD_LOCK(state);
-    mutex_release_internal(m, true);
-    THREAD_UNLOCK(state);
+    // must have been some contention, try the slow release
+    spin_lock_saved_state_t state;
+    if (!thread_lock_held)
+        spin_lock_irqsave(&thread_lock, state);
+
+    oldval = (uintptr_t)ct | MUTEX_FLAG_QUEUED;
+
+    thread_t *t = wait_queue_dequeue_one(&m->wait, NO_ERROR);
+    DEBUG_ASSERT_MSG(t, "mutex_release: wait queue didn't have anything, but m->val = %#" PRIxPTR "\n", m->val);
+
+    // we woke up a thread, mark the mutex owned by that thread
+    uintptr_t newval = (uintptr_t)t | (wait_queue_is_empty(&m->wait) ? 0 : MUTEX_FLAG_QUEUED);
+
+    LTRACEF("%p woke up thread %p, marking it as owner, newval %#" PRIxPTR "\n", ct, t, newval);
+
+    if (!atomic_cmpxchg_u64(&m->val, &oldval, newval)) {
+        panic("bad state in mutex release %p, current thread %p\n", m, ct);
+    }
+
+    sched_unblock(t, resched);
+
+    if (!thread_lock_held)
+        spin_unlock_irqrestore(&thread_lock, state);
 }
 

kernel/kernel/thread.c

@@ -1433,6 +1433,24 @@ int wait_queue_wake_one(wait_queue_t *wait, bool reschedule, status_t wait_queue
     return ret;
 }
 
+thread_t *wait_queue_dequeue_one(wait_queue_t *wait, status_t wait_queue_error)
+{
+    thread_t *t;
+
+    DEBUG_ASSERT(wait->magic == WAIT_QUEUE_MAGIC);
+    DEBUG_ASSERT(arch_ints_disabled());
+    DEBUG_ASSERT(spin_lock_held(&thread_lock));
+
+    t = list_remove_head_type(&wait->list, thread_t, queue_node);
+    if (t) {
+        wait->count--;
+        DEBUG_ASSERT(t->state == THREAD_BLOCKED);
+        t->blocked_status = wait_queue_error;
+        t->blocking_wait_queue = NULL;
+    }
+
+    return t;
+}
 
 /**
  * @brief  Wake all threads sleeping on a wait queue
@@ -1483,6 +1501,15 @@ int wait_queue_wake_all(wait_queue_t *wait, bool reschedule, status_t wait_queue
     return ret;
 }
 
+bool wait_queue_is_empty(wait_queue_t *wait)
+{
+    DEBUG_ASSERT(wait->magic == WAIT_QUEUE_MAGIC);
+    DEBUG_ASSERT(arch_ints_disabled());
+    DEBUG_ASSERT(spin_lock_held(&thread_lock));
+
+    return list_is_empty(&wait->list);
+}
+
 /**
  * @brief  Tear down a wait queue
  *

kernel/lib/magenta/futex_node.cpp

@@ -111,7 +111,7 @@ status_t FutexNode::BlockThread(Mutex* mutex, mx_time_t deadline) TA_NO_THREAD_S
     // We specifically want reschedule=false here, otherwise the
     // combination of releasing the mutex and enqueuing the current thread
     // would not be atomic, which would mean that we could miss wakeups.
-    mutex_release_internal(mutex->GetInternal(), /* reschedule= */ false);
+    mutex_release_etc(mutex->GetInternal(), /* reschedule= */ false, /* thread_lock_held= */ true);
 
     // Check whether a kill has been initiated, and block if not.  This
     // check+wait must be done atomically (with respect to THREAD_LOCK),