wait: Final implementation

nx/include/switch.h

@@ -35,6 +35,9 @@ extern "C" {
 #include "switch/kernel/jit.h"
 #include "switch/kernel/ipc.h"
 #include "switch/kernel/barrier.h"
+#include "switch/kernel/uevent.h"
+#include "switch/kernel/utimer.h"
+#include "switch/kernel/wait.h"
 
 #include "switch/services/sm.h"
 #include "switch/services/smm.h"

nx/include/switch/kernel/uevent.h

@@ -1,14 +1,13 @@
 // Copyright 2018 plutoo
 #pragma once
-#include "kernel/mutex.h"
-#include "kernel/waiter.h"
+#include "../kernel/mutex.h"
+#include "../kernel/wait.h"
 
 typedef struct UsermodeEvent UsermodeEvent;
 
 struct UsermodeEvent
 {
-    Mutex mutex;
-    WaiterNode waiter_list;
+    Waitable waitable;
     bool signal;
     bool auto_clear;
 };

nx/include/switch/kernel/utimer.h

@@ -1,12 +1,14 @@
 // Copyright 2018 plutoo
 #pragma once
-#include "kernel/svc.h"
+#include "../kernel/mutex.h"
+#include "../kernel/wait.h"
 
 typedef struct UsermodeTimer UsermodeTimer;
 
 struct UsermodeTimer
 {
-    u64 next_time;
+    Waitable waitable;
+    u64 next_tick;
     u64 interval;
 };
 

nx/include/switch/kernel/wait.h

@@ -1,24 +1,97 @@
 // Copyright 2018 plutoo
 #pragma once
-#include "kernel/mutex.h"
+#include "../kernel/mutex.h"
+#include "../kernel/event.h"
+#include "../kernel/thread.h"
+
+// Implementation details.
+typedef struct UsermodeEvent UsermodeEvent;
+typedef struct UsermodeTimer UsermodeTimer;
 
 typedef enum {
-    WaitObjectType_Handle,
-    WaitObjectType_UsermodeTimer,
-    WaitObjectType_UsermodeEvent,
-} WaitObjectType;
+    WaiterNodeType_Event,
+    WaiterNodeType_Timer
+} WaiterNodeType;
 
-typedef struct UsermodeTimer UsermodeTimer;
-typedef struct UsermodeEvent UsermodeEvent;
+typedef struct Waitable Waitable;
+typedef struct WaitableNode WaitableNode;
+
+struct WaitableNode
+{
+    WaitableNode* prev;
+    WaitableNode* next;
+};
 
 typedef struct {
-    WaitObjectType type;
+    WaitableNode node;
+    WaiterNodeType type;
+    Handle thread;
+    union {
+        Waitable* parent;
+        UsermodeEvent* parent_event;
+        UsermodeTimer* parent_timer;
+    };
+    size_t idx;
+    size_t* idx_out;
+} WaiterNode;
+
+struct Waitable
+{
+    WaitableNode list;
+    Mutex mutex;
+};
+
+// User-facing API starts here.
+typedef enum {
+    WaiterType_Handle,
+    WaiterType_UsermodeTimer,
+    WaiterType_UsermodeEvent,
+} WaiterType;
+
+typedef struct {
+    WaiterType type;
 
     union {
         Handle handle;
         UsermodeTimer* timer;
         UsermodeEvent* event;
     };
-} WaitObject;
+} Waiter;
 
-Result waitN(s32* idx_out, WaitObject* objects, size_t num_objects, u64 timeout);
+static inline Waiter waiterForHandle(Handle h)
+{
+    Waiter wait_obj;
+    wait_obj.type = WaiterType_Handle;
+    wait_obj.handle = h;
+    return wait_obj;
+}
+
+static inline Waiter waiterForUtimer(UsermodeTimer* t)
+{
+    Waiter wait_obj;
+    wait_obj.type = WaiterType_UsermodeTimer;
+    wait_obj.timer = t;
+    return wait_obj;
+}
+
+static inline Waiter waiterForUevent(UsermodeEvent* e)
+{
+    Waiter wait_obj;
+    wait_obj.type = WaiterType_UsermodeEvent;
+    wait_obj.event = e;
+    return wait_obj;
+}
+
+static inline Waiter waiterForEvent(Event* e) {
+    return waiterForHandle(e->revent);
+}
+
+static inline Waiter waiterForThreadExit(Thread* t) {
+    return waiterForHandle(t->handle);
+}
+
+#define waitMulti(idx_out, timeout, ...) \
+    waitN((idx_out), (timeout), (Waiter[]) { __VA_ARGS__ }, sizeof((Waiter[]) { __VA_ARGS__ }) / sizeof(Waiter))
+
+
+Result waitN(s32* idx_out, u64 timeout, Waiter* objects, size_t num_objects);

nx/include/switch/kernel/waiter.h

@@ -1,28 +0,0 @@
-// Copyright 2018 plutoo
-#pragma once
-#include "kernel/svc.h"
-#include "kernel/mutex.h"
-#include "kernel/wait.h"
-
-#define MAX_WAIT 0x40
-
-typedef struct WaiterNode WaiterNode;
-typedef struct Waiter Waiter;
-
-struct WaiterNode
-{
-    Waiter* owner;
-    s32 idx;
-    WaiterNode* prev;
-    WaiterNode* next;
-};
-
-struct Waiter
-{
-    Mutex mutex;
-    Handle thread;
-    s32 signalled_idx;
-
-    WaiterNode nodes[MAX_WAIT];
-    size_t num_nodes;
-};

nx/source/kernel/uevent.c

@@ -1,75 +1,64 @@
 // Copyright 2018 plutoo
 #include "kernel/svc.h"
 #include "kernel/mutex.h"
-#include "kernel/waiter.h"
 #include "kernel/uevent.h"
-#include "waiter.h"
+#include "wait.h"
+#include "uevent.h"
 
 void ueventCreate(UsermodeEvent* e, bool auto_clear)
 {
-    mutexInit(&e->mutex);
-    e->waiter_list.prev = &e->waiter_list;
-    e->waiter_list.next = &e->waiter_list;
+    _waitableInitialize(&e->waitable);
+
     e->signal = false;
     e->auto_clear = auto_clear;
 }
 
 void ueventClear(UsermodeEvent* e)
 {
-    mutexLock(&e->mutex);
+    mutexLock(&e->waitable.mutex);
     e->signal = false;
-    mutexUnlock(&e->mutex);
+    mutexUnlock(&e->waitable.mutex);
 }
 
 void ueventSignal(UsermodeEvent* e)
 {
-    mutexLock(&e->mutex);
+    mutexLock(&e->waitable.mutex);
     e->signal = true;
+    _waitableSignalAllListeners(&e->waitable);
+    mutexUnlock(&e->waitable.mutex);
+}
 
-    WaiterNode* end = &e->waiter_list;
-    WaiterNode* w = end;
+void _ueventTryAutoClear(UsermodeEvent* e)
+{
+    mutexLock(&e->waitable.mutex);
+    if (e->auto_clear) {
+        e->signal = true;
+    }
+    mutexUnlock(&e->waitable.mutex);
+}
 
-    while (w->next != end)
+bool _ueventAddListener(UsermodeEvent* e, WaiterNode* w, size_t idx, size_t* idx_out, Handle thread)
+{
+    _waiterNodeInitialize(w, WaiterNodeType_Event, &e->waitable, thread, idx, idx_out);
+
+    mutexLock(&e->waitable.mutex);
+
+    bool signalled = e->signal;
+    bool ret;
+
+    if (signalled)
     {
-        w = w->next;
-
-        bool signalled = _waiterSignal(w->owner, w->idx);
-
-        if (signalled && e->auto_clear)
+        if (e->auto_clear) {
             e->signal = false;
+        }
+        ret = false;
+    }
+    else
+    {
+        _waiterNodeAddToWaitable(w, &e->waitable);
+        ret = true;
     }
 
-    mutexUnlock(&e->mutex);
-}
-
-bool _ueventConsumeIfSignalled(UsermodeEvent* e)
-{
-    bool ret;
-    mutexLock(&e->mutex);
-
-    ret = e->signal;
-
-    if (ret && e->auto_clear)
-        e->signal = false;
-
-    mutexUnlock(&e->mutex);
+    mutexUnlock(&e->waitable.mutex);
     return ret;
 }
-
-void _ueventAddListener(UsermodeEvent* e, WaiterNode* w)
-{
-    mutexLock(&e->mutex);
-    e->waiter_list.next->prev = w;
-    w->next = e->waiter_list.next;
-    w->prev = &e->waiter_list;
-    e->waiter_list.next = w;
-    mutexUnlock(&e->mutex);
-}
-
-void _ueventRemoveListener(UsermodeEvent* e, WaiterNode* w)
-{
-    mutexLock(&e->mutex);
-    w->prev->next = w->next;
-    w->next->prev = w->prev;
-    mutexUnlock(&e->mutex);
-}

nx/source/kernel/uevent.h

@@ -2,6 +2,5 @@
 #pragma once
 #include "kernel/uevent.h"
 
-bool _ueventConsumeIfSignalled(UsermodeEvent* e);
-void _ueventAddListener(UsermodeEvent* e, WaiterNode* w);
-void _ueventRemoveListener(UsermodeEvent* e, WaiterNode* w);
+void _ueventTryAutoClear(UsermodeEvent* e);
+bool _ueventAddListener(UsermodeEvent* e, WaiterNode* w, size_t idx, size_t* idx_out, Handle thread);

nx/source/kernel/utimer.c

@@ -2,10 +2,16 @@
 #include "kernel/svc.h"
 #include "kernel/utimer.h"
 #include "arm/counter.h"
+#include "utimer.h"
+#include "wait.h"
+
+#define STOPPED 0
 
 void utimerCreate(UsermodeTimer* t, u64 interval, bool start)
 {
-    t->next_time = 0;
+    _waitableInitialize(&t->waitable);
+
+    t->next_tick = STOPPED;
     t->interval = armNsToTicks(interval);
 
     if (start)
@@ -14,23 +20,62 @@ void utimerCreate(UsermodeTimer* t, u64 interval, bool start)
 
 void utimerStart(UsermodeTimer* t)
 {
-    u64 zero = 0;
-    __atomic_compare_exchange_n(&t->next_time, &zero, armGetSystemTick() + armNsToTicks(t->interval), false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
+    mutexLock(&t->waitable.mutex);
+
+    if (t->next_tick == STOPPED)
+    {
+        u64 new_tick = armGetSystemTick() + t->interval;
+        t->next_tick = new_tick;
+        _waitableSignalAllListeners(&t->waitable);
+    }
+
+    mutexUnlock(&t->waitable.mutex);
 }
 
 void utimerStop(UsermodeTimer* t)
 {
-    while (!__atomic_compare_exchange_n(&t->next_time, &t->next_time, 0, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST));
+    mutexLock(&t->waitable.mutex);
+
+    if (t->next_tick != STOPPED)
+    {
+        t->next_tick = STOPPED;
+        _waitableSignalAllListeners(&t->waitable);
+    }
+
+    mutexUnlock(&t->waitable.mutex);
 }
 
-void _utimerRecalculate(UsermodeTimer* t, u64 old_time)
+void _utimerRecalculate(UsermodeTimer* t, u64 old_tick)
 {
-    s64 interval = t->interval;
-    s64 new_time = ((armGetSystemTick() - old_time + interval - 1) / interval) * interval;
-    __atomic_compare_exchange_n(&t->next_time, &old_time, new_time, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
+    mutexLock(&t->waitable.mutex);
+
+    if (t->next_tick == old_tick)
+    {
+        u64 interval = t->interval;
+        u64 new_tick = ((armGetSystemTick() + interval - 1) / interval) * interval;
+
+        t->next_tick = new_tick;
+    }
+
+    mutexUnlock(&t->waitable.mutex);
 }
 
-u64 _utimerGetNextTime(UsermodeTimer* t)
+u64 _utimerGetNextTick(UsermodeTimer* t)
 {
-    return __atomic_load_n(&t->next_time, __ATOMIC_SEQ_CST);
+    u64 ret;
+
+    mutexLock(&t->waitable.mutex);
+    ret = t->next_tick;
+    mutexUnlock(&t->waitable.mutex);
+
+    return ret;
+}
+
+void _utimerAddListener(UsermodeTimer* t, WaiterNode* w, size_t idx, size_t* idx_out, Handle thread)
+{
+    _waiterNodeInitialize(w, WaiterNodeType_Timer, &t->waitable, thread, idx, idx_out);
+
+    mutexLock(&t->waitable.mutex);
+    _waiterNodeAddToWaitable(w, &t->waitable);
+    mutexUnlock(&t->waitable.mutex);
 }

nx/source/kernel/utimer.h

@@ -2,5 +2,6 @@
 #pragma once
 #include "kernel/utimer.h"
 
-void _utimerRecalculate(UsermodeTimer* t, u64 old_time);
-u64  _utimerGetNextTime(UsermodeTimer* t);
+void _utimerRecalculate(UsermodeTimer* t, u64 old_tick);
+u64  _utimerGetNextTick(UsermodeTimer* t);
+void _utimerAddListener(UsermodeTimer* t, WaiterNode* w, size_t idx, size_t* idx_out, Handle thread);

nx/source/kernel/wait.c

@@ -2,114 +2,183 @@
 #include "result.h"
 #include "kernel/svc.h"
 #include "kernel/wait.h"
-#include "kernel/waiter.h"
 #include "kernel/utimer.h"
 #include "kernel/uevent.h"
 #include "arm/counter.h"
-#include "waiter.h"
 #include "utimer.h"
 #include "uevent.h"
+#include "wait.h"
 #include "../internal.h"
 
-Result waitN(s32* idx_out, WaitObject* objects, size_t num_objects, u64 timeout)
+#define MAX_WAIT 0x40
+
+#define KernelError_Timeout 0xEA01
+#define KernelError_Interrupt 0xEC01
+
+static Result waitImpl(s32* idx_out, u64 timeout, Waiter* objects, size_t num_objects)
 {
     if (num_objects > MAX_WAIT)
         return MAKERESULT(Module_Libnx, LibnxError_TooManyWaitables);
 
-    Handle dummy_handle = getThreadVars()->handle;
+    Handle own_thread_handle = getThreadVars()->handle;
+    Handle dummy_handle = own_thread_handle;
+    Result rc;
 
-    Waiter waiter;
-    _waiterCreate(&waiter);
-
-    Handle handles[MAX_WAIT];
+    Handle handles[num_objects];
     u64 cur_tick = armGetSystemTick();
 
-    u64 end_time = timeout;
-    s32 end_time_idx = -1;
+    size_t triggered_idx = -1;
+    size_t num_waiters = 0;
+    WaiterNode waiters[num_objects];
+
+    u64 end_tick = armNsToTicks(timeout);
+    s32 end_tick_idx = -1;
     size_t i;
 
     for (i=0; i<num_objects; i++)
     {
-        WaitObject* obj = &objects[i];
+        Waiter* obj = &objects[i];
         u64 timer_tick;
+        bool added;
 
         switch (obj->type)
         {
-        case WaitObjectType_UsermodeTimer:
-            timer_tick = _utimerGetNextTime(obj->timer);
+        case WaiterType_UsermodeTimer:
 
-            // Skip timer if disabled.
-            if (timer_tick == 0)
-                break;
+            timer_tick = _utimerGetNextTick(obj->timer);
 
-            // If the timer already signalled, we're done.
-            if (timer_tick < cur_tick)
+            // Skip timer if stopped.
+            if (timer_tick != 0)
             {
-                *idx_out = i;
-                _utimerRecalculate(obj->timer, timer_tick);
-                _waiterFree(&waiter, objects);
-                return 0;
+                // If the timer already signalled, we're done.
+                if (timer_tick < cur_tick)
+                {
+                    _utimerRecalculate(obj->timer, timer_tick);
+
+                    *idx_out = i;
+                    rc = 0;
+                    goto clean_up;
+                }
+
+                // Override the user-supplied timeout if timer would fire before that.
+                if ((timer_tick - cur_tick) < end_tick)
+                {
+                    end_tick = timer_tick - cur_tick;
+                    end_tick_idx = i;
+                }
             }
 
-            // Override the user-supplied timeout if timer would fire before that.
-            if ((timer_tick - cur_tick) < end_time)
-            {
-                end_time = timer_tick - cur_tick;
-                end_time_idx = i;
-            }
+            // Always add a listener on the timer,
+            // So that we can detect another thread were to stopping/starting it during our waiting.
+            _utimerAddListener(
+                obj->timer, &waiters[num_waiters], num_waiters, &triggered_idx,
+                own_thread_handle);
+
+            num_waiters++;
             break;
 
-        case WaitObjectType_UsermodeEvent:
+        case WaiterType_UsermodeEvent:
+
+            // Try to add a listener to the event, if it hasn't already signalled.
+            added = _ueventAddListener(
+                obj->event, &waiters[num_waiters], num_waiters, &triggered_idx,
+                own_thread_handle);
+
             // If the event already happened, we're done.
-            if (_ueventConsumeIfSignalled(obj->event))
+            if (!added)
             {
                 *idx_out = i;
-                _waiterFree(&waiter, objects);
-                return 0;
+                rc = 0;
+                goto clean_up;
             }
 
-            // If not, add a listener.
-            _waiterSubscribe(&waiter, obj->event);
+            // If the event hasn't signalled, we added a listener.
+            num_waiters++;
             break;
 
-        case WaitObjectType_Handle:
+        case WaiterType_Handle:
             break;
         }
 
-        handles[i] = (obj->type == WaitObjectType_Handle) ? obj->handle : dummy_handle;
+        // Add handle for i:th object.
+        // If that object has no handle, add a dummy handle.
+        handles[i] = (obj->type == WaiterType_Handle) ? obj->handle : dummy_handle;
     }
 
-
     // Do the actual syscall.
-    Result rc;
-    rc = svcWaitSynchronization(idx_out, handles, num_objects, end_time);
+    rc = svcWaitSynchronization(idx_out, handles, num_objects, armTicksToNs(end_tick));
 
-    // Timeout-error?
-    if (rc == 0xEA01)
+    if (rc == KernelError_Timeout)
     {
-        // If the user-supplied timeout, we return the error back to them.
-        if (end_time_idx == -1)
+        // If we hit the user-supplied timeout, we return the timeout error back to caller.
+        if (end_tick_idx == -1)
+            goto clean_up;
+
+        // If not, it means a timer triggered the timeout.
+        _utimerRecalculate(objects[end_tick_idx].timer, end_tick + cur_tick);
+
+        *idx_out = end_tick_idx;
+        rc = 0;
+    }
+    else if (rc == KernelError_Interrupt)
+    {
+        // If no listener filled in its own index, we return the interrupt error back to caller.
+        // This only happens if user for some reason manually does a svcCancelSynchronization.
+        // Check just in case.
+        if (triggered_idx == -1)
+            goto clean_up;
+
+        // An event was signalled, or a timer was updated.
+        // So.. which is it?
+        switch (waiters[triggered_idx].type)
         {
-            _waiterFree(&waiter, objects);
-            return rc;
+        case WaiterNodeType_Event:
+            _ueventTryAutoClear(waiters[triggered_idx].parent_event);
+            *idx_out = triggered_idx;
+            rc = 0;
+            break;
+
+        case WaiterNodeType_Timer:
+            rc = KernelError_Interrupt;
+            break;
         }
-
-        // If not, it means a timer was triggered.
-        *idx_out = end_time_idx;
-        _utimerRecalculate(objects[end_time_idx].timer, end_time + cur_tick);
-        _waiterFree(&waiter, objects);
-        return 0;
     }
 
-    // Interrupted-error?
-    if (rc == 0xEC01)
-    {
-        // An event was signalled.
-        *idx_out = _waiterGetSignalledIndex(&waiter);
-        _waiterFree(&waiter, objects);
-        return 0;
+clean_up:
+
+    // Remove listeners.
+    for (i=0; i<num_waiters; i++) {
+        _waiterNodeFree(&waiters[i]);
     }
 
-    _waiterFree(&waiter, objects);
     return rc;
 }
+
+Result waitN(s32* idx_out, u64 timeout, Waiter* objects, size_t num_objects)
+{
+    while (1)
+    {
+        u64 cur_tick = armGetSystemTick();
+        Result rc = waitImpl(idx_out, timeout, objects, num_objects);
+
+        if (rc == KernelError_Interrupt)
+        {
+            // On timer stop/start an interrupt is sent to listeners.
+            // It means the timer state has changed, and we should restart the wait.
+
+            // Adjust timeout..
+            if (timeout != -1)
+            {
+                u64 time_spent = armTicksToNs(armGetSystemTick() - cur_tick);
+
+                if (time_spent >= timeout)
+                    return KernelError_Timeout;
+
+                timeout -= time_spent;
+            }
+        }
+        else {
+            return rc;
+        }
+    }
+}

nx/source/kernel/wait.h

@@ -0,0 +1,52 @@
+// Copyright 2018 plutoo
+#pragma once
+#include "kernel/mutex.h"
+#include "kernel/wait.h"
+
+static inline void _waitableInitialize(Waitable* ww)
+{
+    mutexInit(&ww->mutex);
+    ww->list.next = &ww->list;
+    ww->list.prev = &ww->list;
+}
+
+static inline void _waitableSignalAllListeners(Waitable* ww)
+{
+    WaitableNode* node = &ww->list;
+    WaitableNode* end = node;
+
+    while (node->next != end)
+    {
+        node = node->next;
+        WaiterNode* w = (WaiterNode*) node;
+
+        *w->idx_out = w->idx;
+        svcCancelSynchronization(w->thread);
+    }
+}
+
+static inline void _waiterNodeInitialize(
+    WaiterNode* w, WaiterNodeType type, Waitable* parent, Handle thread,
+    size_t idx, size_t* idx_out)
+{
+    w->type = type;
+    w->parent = parent;
+    w->thread = thread;
+    w->idx = idx;
+    w->idx_out = idx_out;
+}
+
+static inline void _waiterNodeAddToWaitable(WaiterNode* w, Waitable* ww)
+{
+    w->node.next = ww->list.next;
+    ww->list.next = &w->node;
+    w->node.prev = &ww->list;
+}
+
+static inline void _waiterNodeFree(WaiterNode* w)
+{
+    mutexLock(&w->parent->mutex);
+    w->node.prev->next = w->node.next;
+    w->node.next->prev = w->node.prev;
+    mutexUnlock(&w->parent->mutex);
+}

nx/source/kernel/waiter.c

@@ -1,49 +0,0 @@
-// Copyright 2018 plutoo
-#include "kernel/waiter.h"
-#include "kernel/uevent.h"
-#include "waiter.h"
-#include "uevent.h"
-#include "../internal.h"
-
-#define NOT_YET_SIGNALLED (-1)
-
-void _waiterCreate(Waiter* w)
-{
-    mutexInit(&w->mutex);
-    w->thread = getThreadVars()->handle;
-    w->signalled_idx = NOT_YET_SIGNALLED;
-    w->num_nodes = 0;
-}
-
-void _waiterFree(Waiter* w, WaitObject* objects)
-{
-    size_t i;
-    for (i=0; i<w->num_nodes; i++)
-        _ueventRemoveListener(objects[w->nodes[i].idx].event, &w->nodes[i]);
-}
-
-bool _waiterSignal(Waiter* w, s32 idx)
-{
-    bool ret = false;
-    mutexLock(&w->mutex);
-
-    if (w->signalled_idx == NOT_YET_SIGNALLED)
-    {
-        w->signalled_idx = idx;
-        svcCancelSynchronization(w->thread);
-        ret = true;
-    }
-
-    mutexUnlock(&w->mutex);
-    return ret;
-}
-
-void _waiterSubscribe(Waiter* w, UsermodeEvent* e)
-{
-    _ueventAddListener(e, &w->nodes[w->num_nodes++]);
-}
-
-s32 _waiterGetSignalledIndex(Waiter* w)
-{
-    return w->signalled_idx;
-}

nx/source/kernel/waiter.h

@@ -1,10 +0,0 @@
-// Copyright 2018 plutoo
-#pragma once
-#include "kernel/waiter.h"
-
-void _waiterCreate(Waiter* w);
-void _waiterFree(Waiter* w, WaitObject* objects);
-void _waiterSubscribe(Waiter* w, UsermodeEvent* e);
-bool _waiterSignal(Waiter* w, s32 idx);
-s32  _waiterGetSignalledIndex(Waiter* w);
-