Changeset f6372be9 in mainline

Timestamp:

2018-06-26T17:34:23Z (6 years ago)

Author:

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

Branches:

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

Children:

b59318e

Parents:

38e3427

git-author:

Jiří Zárevúcky <jiri.zarevucky@…> (2018-06-25 20:50:53)

git-committer:

Jiří Zárevúcky <jiri.zarevucky@…> (2018-06-26 17:34:23)

Message:

Improve the debugging options for futexes.

Files:

• HelenOS.config (modified) (1 diff)
• uspace/lib/c/generic/fibril.c (modified) (4 diffs)
• uspace/lib/c/generic/futex.c (modified) (2 diffs)
• uspace/lib/c/include/futex.h (modified) (2 diffs)

HelenOS.config

@@ -385 +385 @@
 ! CONFIG_UBSAN_KERNEL (n/y)
 
+% Track owner for futexes in userspace.
+! CONFIG_DEBUG_FUTEX (y/n)
+
 % Deadlock detection support for spinlocks
 ! [CONFIG_DEBUG=y&CONFIG_SMP=y] CONFIG_DEBUG_SPINLOCK (y/n)

uspace/lib/c/generic/fibril.c

@@ -150 +150 @@
         case FIBRIL_FROM_DEAD:
                 /* Make sure the async_futex is held. */
-                assert((atomic_signed_t) async_futex.val.count <= 0);
+                futex_assert_is_locked(&async_futex);
 
                 /* If we are going to manager and none exists, create it */
@@ -162 +162 @@
                     fibril_t, link);
 
+                /* Bookkeeping. */
+                futex_give_to(&async_futex, dstf);
+
                 if (stype == FIBRIL_FROM_DEAD)
                         dstf->clean_after_me = srcf;
@@ -167 +170 @@
         case FIBRIL_PREEMPT:
         case FIBRIL_FROM_MANAGER:
+                futex_assert_is_not_locked(&async_futex);
+
                 if (list_empty(&ready_list)) {
                         futex_unlock(&fibril_futex);
@@ -196 +201 @@
                 break;
         }
+
+        /* Bookkeeping. */
+        futex_give_to(&fibril_futex, dstf);
 
         /* Swap to the next fibril. */

uspace/lib/c/generic/futex.c

@@ -34 +34 @@
 
 #include <futex.h>
+
+#include <assert.h>
 #include <atomic.h>
+#include <fibril.h>
+#include <io/kio.h>
+
+#include "private/fibril.h"
+
+//#define DPRINTF(...) kio_printf(__VA_ARGS__)
+#define DPRINTF(...) ((void)0)
 
 /** Initialize futex counter.
@@ -47 +56 @@
 }
 
+#ifdef CONFIG_DEBUG_FUTEX
+
+void __futex_assert_is_locked(futex_t *futex, const char *name)
+{
+        void *owner = __atomic_load_n(&futex->owner, __ATOMIC_RELAXED);
+        fibril_t *self = (fibril_t *) fibril_get_id();
+        if (owner != self) {
+                DPRINTF("Assertion failed: %s (%p) is not locked by fibril %p (instead locked by fibril %p).\n", name, futex, self, owner);
+        }
+        assert(owner == self);
+}
+
+void __futex_assert_is_not_locked(futex_t *futex, const char *name)
+{
+        void *owner = __atomic_load_n(&futex->owner, __ATOMIC_RELAXED);
+        fibril_t *self = (fibril_t *) fibril_get_id();
+        if (owner == self) {
+                DPRINTF("Assertion failed: %s (%p) is already locked by fibril %p.\n", name, futex, self);
+        }
+        assert(owner != self);
+}
+
+void __futex_lock(futex_t *futex, const char *name)
+{
+        /* We use relaxed atomics to avoid violating C11 memory model.
+         * They should compile to regular load/stores, but simple assignments
+         * would be UB by definition.
+         */
+
+        fibril_t *self = (fibril_t *) fibril_get_id();
+        DPRINTF("Locking futex %s (%p) by fibril %p.\n", name, futex, self);
+        __futex_assert_is_not_locked(futex, name);
+        futex_down(futex);
+
+        void *prev_owner = __atomic_exchange_n(&futex->owner, self, __ATOMIC_RELAXED);
+        assert(prev_owner == NULL);
+
+        atomic_inc(&self->futex_locks);
+}
+
+void __futex_unlock(futex_t *futex, const char *name)
+{
+        fibril_t *self = (fibril_t *) fibril_get_id();
+        DPRINTF("Unlocking futex %s (%p) by fibril %p.\n", name, futex, self);
+        __futex_assert_is_locked(futex, name);
+        __atomic_store_n(&futex->owner, NULL, __ATOMIC_RELAXED);
+        atomic_dec(&self->futex_locks);
+        futex_up(futex);
+}
+
+bool __futex_trylock(futex_t *futex, const char *name)
+{
+        fibril_t *self = (fibril_t *) fibril_get_id();
+        bool success = futex_trydown(futex);
+        if (success) {
+                void *owner = __atomic_load_n(&futex->owner, __ATOMIC_RELAXED);
+                assert(owner == NULL);
+
+                __atomic_store_n(&futex->owner, self, __ATOMIC_RELAXED);
+
+                atomic_inc(&self->futex_locks);
+
+                DPRINTF("Trylock on futex %s (%p) by fibril %p succeeded.\n", name, futex, self);
+        } else {
+                DPRINTF("Trylock on futex %s (%p) by fibril %p failed.\n", name, futex, self);
+        }
+
+        return success;
+}
+
+void __futex_give_to(futex_t *futex, void *new_owner, const char *name)
+{
+        fibril_t *self = fibril_self();
+        fibril_t *no = new_owner;
+        DPRINTF("Passing futex %s (%p) from fibril %p to fibril %p.\n", name, futex, self, no);
+
+        __futex_assert_is_locked(futex, name);
+        atomic_dec(&self->futex_locks);
+        atomic_inc(&no->futex_locks);
+        __atomic_store_n(&futex->owner, new_owner, __ATOMIC_RELAXED);
+}
+
+#endif
+
 /** @}
  */

uspace/lib/c/include/futex.h

@@ -39 +39 @@
 #include <errno.h>
 #include <libc.h>
+#include <time.h>
 
 typedef struct futex {
         atomic_t val;
+#ifdef CONFIG_DEBUG_FUTEX
+        _Atomic void *owner;
+#endif
 } futex_t;
 
 extern void futex_initialize(futex_t *futex, int value);
+
+#ifdef CONFIG_DEBUG_FUTEX
+
+#define FUTEX_INITIALIZE(val) {{ (val) }, NULL }
+#define FUTEX_INITIALIZER     FUTEX_INITIALIZE(1)
+
+void __futex_assert_is_locked(futex_t *, const char *);
+void __futex_assert_is_not_locked(futex_t *, const char *);
+void __futex_lock(futex_t *, const char *);
+void __futex_unlock(futex_t *, const char *);
+bool __futex_trylock(futex_t *, const char *);
+void __futex_give_to(futex_t *, void *, const char *);
+
+#define futex_lock(futex) __futex_lock((futex), #futex)
+#define futex_unlock(futex) __futex_unlock((futex), #futex)
+#define futex_trylock(futex) __futex_trylock((futex), #futex)
+
+#define futex_give_to(futex, new_owner) __futex_give_to((futex), (new_owner), #futex)
+#define futex_assert_is_locked(futex) __futex_assert_is_locked((futex), #futex)
+#define futex_assert_is_not_locked(futex) __futex_assert_is_not_locked((futex), #futex)
+
+#else
 
 #define FUTEX_INITIALIZE(val) {{ (val) }}
@@ -52 +78 @@
 #define futex_trylock(fut)  futex_trydown((fut))
 #define futex_unlock(fut)   (void) futex_up((fut))
+
+#define futex_give_to(fut, owner) ((void)0)
+#define futex_assert_is_locked(fut) assert((atomic_signed_t) (fut)->val.count <= 0)
+#define futex_assert_is_not_locked(fut) ((void)0)
+
+#endif
 
 /** Try to down the futex.