wip fixes

8 files changed, 136 insertions, 157 deletions

diff --git a/libcr_ipc/CMakeLists.txt b/libcr_ipc/CMakeLists.txt
index 957b638..509e5a6 100644
--- a/libcr_ipc/CMakeLists.txt
+++ b/libcr_ipc/CMakeLists.txt
@@ -5,10 +5,6 @@
 
 add_library(libcr_ipc INTERFACE)
 target_include_directories(libcr_ipc SYSTEM INTERFACE ${CMAKE_CURRENT_LIST_DIR}/include)
-target_sources(libcr_ipc INTERFACE
-	sema.c
-	mutex.c
-)
 target_link_libraries(libcr_ipc INTERFACE
 	libcr
 )
diff --git a/libcr_ipc/include/libcr_ipc/_common.h b/libcr_ipc/include/libcr_ipc/_common.h
index 5efe964..547ab81 100644
--- a/libcr_ipc/include/libcr_ipc/_common.h
+++ b/libcr_ipc/include/libcr_ipc/_common.h
@@ -7,13 +7,46 @@
 #ifndef _COROUTINE__COMMON_H_
 #define _COROUTINE__COMMON_H_
 
-struct _cr_ipc_cid_list {
-	cid_t                            val;
-	struct _cr_ipc_cid_list         *next;
-};
+#include <assert.h>
+
+#include <libcr/coroutine.h>
 
 #define _cr_ipc_static_assert_sametype(a, b, msg) \
 	static_assert(_Generic(a, typeof(b): 1, default: 0), msg)
 #define _cr_ipc_str(a) #a
 
+struct _cr_ipc_cid_fifo_node {
+	cid_t                            val;
+	struct _cr_ipc_cid_fifo_node     *next;
+};
+
+struct _cr_ipc_cid_fifo {
+	struct _cr_ipc_cid_fifo_node     *head, **tail;
+};
+
+
+#define _cr_ipc_cid_fifo_self(name)           \
+	struct _cr_ipc_cid_fifo_node name = { \
+		.val = cr_getcid(),           \
+		.next = NULL,                 \
+	}
+
+static inline void _cr_ipc_cid_fifo_push(struct _cr_ipc_cid_fifo *fifo, struct _cr_ipc_cid_fifo_node *self) {
+	if (!fifo->tail)
+		fifo->tail = &(fifo->head);
+	*(fifo->tail) = self;
+}
+
+static inline cid_t _cr_ipc_cid_fifo_pop(struct _cr_ipc_cid_fifo *fifo) {
+	if (!fifo->tail)
+		fifo->tail = &(fifo->head);
+	cid_t cid = 0;
+	if (fifo->head) {
+		cid = fifo->head->val;
+		if (!(( fifo->head = fifo->head->next )) )
+			fifo->tail = &(fifo->head);
+	}
+	return cid;
+}
+
 #endif /* _COROUTINE__COMMON_H_ */
diff --git a/libcr_ipc/include/libcr_ipc/chan.h b/libcr_ipc/include/libcr_ipc/chan.h
index ca53fc2..4bb8f66 100644
--- a/libcr_ipc/include/libcr_ipc/chan.h
+++ b/libcr_ipc/include/libcr_ipc/chan.h
@@ -13,7 +13,6 @@
 #define _COROUTINE_CHAN_H_
 
 #include <assert.h>
-
 #include <libcr/coroutine.h>
 
 /**
diff --git a/libcr_ipc/include/libcr_ipc/mutex.h b/libcr_ipc/include/libcr_ipc/mutex.h
index 1a82423..79424e3 100644
--- a/libcr_ipc/include/libcr_ipc/mutex.h
+++ b/libcr_ipc/include/libcr_ipc/mutex.h
@@ -1,4 +1,4 @@
-/* libcr_ipc/mutex.h - Simple mutexes for libcr (header file)
+/* libcr_ipc/mutex.h - Simple mutexes for libcr
  *
  * Copyright (C) 2024  Luke T. Shumaker <lukeshu@lukeshu.com>
  * SPDX-Licence-Identifier: AGPL-3.0-or-later
@@ -7,18 +7,15 @@
 #ifndef _COROUTINE_MUTEX_H_
 #define _COROUTINE_MUTEX_H_
 
-#include <stdbool.h> /* for bool */
-
-#include <libcr/coroutine.h> /* for cid_t */
-
-#include <libcr_ipc/_common.h> /* for struct _cr_ipc_list_t */
+#include <stdbool.h>
+#include <libcr_ipc/_common.h>
 
 /**
  * A cr_mutex_t is a fair mutex.
  */
 typedef struct {
 	bool                             locked;
-	struct _cr_ipc_cid_list         *head, **tail;
+	struct _cr_ipc_cid_fifo          waiters;
 } cr_mutex_t;
 
 /**
@@ -28,7 +25,16 @@ typedef struct {
  * @yields maybe
  * @run_in coroutine
  */
-void cr_mutex_lock(cr_mutex_t *);
+static inline void cr_mutex_lock(cr_mutex_t *mu) {
+	assert(mu);
+	if (!mu->locked) {
+		mu->locked = true;
+		return;
+	}
+	_cr_ipc_cid_fifo_self(self);
+	_cr_ipc_cid_fifo_push(&mu->waiters, &self);
+	cr_pause_and_yield();
+}
 
 /**
  * Unlock the mutex.  Unblocks a coroutine that is blocked on
@@ -38,6 +44,14 @@ void cr_mutex_lock(cr_mutex_t *);
  * @yields never
  * @may_run_in coroutine
  */
-void cr_mutex_unlock(cr_mutex_t *);
+static inline void cr_mutex_unlock(cr_mutex_t *mu) {
+	assert(mu);
+	assert(mu->locked);
+	cid_t next = _cr_ipc_cid_fifo_pop(&mu->waiters);
+	if (next)
+		cr_unpause(next);
+	else
+		mu->locked = false;
+}
 
 #endif /* _COROUTINE_MUTEX_H_ */
diff --git a/libcr_ipc/include/libcr_ipc/rpc.h b/libcr_ipc/include/libcr_ipc/rpc.h
index 2525ba8..abdb3b6 100644
--- a/libcr_ipc/include/libcr_ipc/rpc.h
+++ b/libcr_ipc/include/libcr_ipc/rpc.h
@@ -11,11 +11,7 @@
 #ifndef _COROUTINE_RPC_H_
 #define _COROUTINE_RPC_H_
 
-#include <assert.h>
-
-#include <libcr/coroutine.h>
-
-#include <libcr_ipc/_common.h> /* for _cr_ipc_* */
+#include <libcr_ipc/_common.h>
 
 /**
  * cr_rpc_t(req_t, resp_t) returns the type definition for a
diff --git a/libcr_ipc/include/libcr_ipc/sema.h b/libcr_ipc/include/libcr_ipc/sema.h
index 236e7e5..8d71c69 100644
--- a/libcr_ipc/include/libcr_ipc/sema.h
+++ b/libcr_ipc/include/libcr_ipc/sema.h
@@ -1,4 +1,4 @@
-/* libcr_ipc/sema.h - Simple semaphores for libcr (header file)
+/* libcr_ipc/sema.h - Simple semaphores for libcr
  *
  * Copyright (C) 2024  Luke T. Shumaker <lukeshu@lukeshu.com>
  * SPDX-Licence-Identifier: AGPL-3.0-or-later
@@ -7,35 +7,81 @@
 #ifndef _COROUTINE_SEMA_H_
 #define _COROUTINE_SEMA_H_
 
-#include <stdbool.h> /* for bool */
-
-#include <libcr/coroutine.h> /* for cid_t */
-
-#include <libcr_ipc/_common.h> /* for struct _cr_ipc_list_t */
+#include <stdbool.h>
+#include <libcr_ipc/_common.h>
 
 /**
  * A cr_sema_t is a fair unbounded[1] counting semaphore.
  *
  * [1]: Well, INT_MAX
  */
-typedef volatile struct {
-	int                              cnt;
+typedef struct {
+	volatile int                     cnt;
 
-	struct _cr_ipc_cid_list         *head, **tail;
+	struct _cr_ipc_cid_fifo          waiters;
 	/* locked indicates that a call from within a coroutine is is
-	 * messing with ->{head,tail}, so a signal handler can't read
-	 * it.  */
-	bool                             locked;
+	 * messing with ->waiters, so a signal handler can't read it.
+	 * Use `asm volatile ("":::"memory")` after setting =true and
+	 * before setting =false to make sure the compiler doesn't
+	 * re-order writes!  */
+	volatile bool                    locked;
 } cr_sema_t;
 
 /**
+ * Drain a maximum of sema->cnt coroutines from the sema->waiters
+ * list.  Returns true if cr_getcid() was drained.
+ */
+static inline bool _cr_sema_drain(cr_sema_t *sema) {
+	assert(!sema->locked);
+	cid_t self = cr_getcid();
+
+	enum drain_result {
+		DRAINING,
+		DRAINED_SELF, /* stopped because drained `self` */
+		DRAINED_ALL,  /* stopped because sema->waiters is empty */
+		DRAINED_SOME, /* stopped because sema->cnt == 0 */
+	} state = DRAINING;
+	do { 
+		sema->locked = true;
+		asm volatile ("":::"memory");
+		while (state == DRAINING) {
+			if (!sema->waiters.head) {
+				state = DRAINED_ALL;
+			} else if (!sema->cnt) {
+				state = DRAINED_SOME;
+			} else {
+				sema->cnt--;
+				cid_t cid = _cr_ipc_cid_fifo_pop(&sema->waiters);
+				if (cid == self)
+					state = DRAINED_SELF;
+				else
+					cr_unpause_from_sighandler(cid);
+			}
+		}
+		asm volatile ("":::"memory");
+		sema->locked = false;
+		/* If there are still coroutines in sema->waiters,
+		 * check that sema->cnt wasn't incremented between `if
+		 * (!sema->cnt)` and `sema->locked = false`.  */
+	} while (state == DRAINED_SOME && sema->cnt);
+	/* If state == DRAINED_SELF, then we better have been the last
+	 * item in the list!  */
+	assert(state != DRAINED_SELF || !sema->waiters.head);
+	return state == DRAINED_SELF;
+}
+
+/**
  * Increment the semaphore,
  *
  * @blocks never
  * @yields never
  * @run_in anywhere (coroutine, sighandler)
  */
-void cr_sema_signal(cr_sema_t *sema);
+static inline void cr_sema_signal(cr_sema_t *sema)  {
+	sema->cnt++;
+	if (!sema->locked)
+		_cr_sema_drain(sema);
+}
 
 /**
  * Wait until the semaphore is >0, then decrement it.
@@ -44,6 +90,21 @@ void cr_sema_signal(cr_sema_t *sema);
  * @yields maybe
  * @may_run_in coroutine
  */
-void cr_sema_wait(cr_sema_t *sema);
+static inline void cr_sema_wait(cr_sema_t *sema) {
+	_cr_ipc_cid_fifo_self(self);
+
+	sema->locked = true;
+	asm volatile ("":::"memory");
+	_cr_ipc_cid_fifo_push(&sema->waiters, &self);
+	asm volatile ("":::"memory");
+	sema->locked = false;
+
+	if (_cr_sema_drain(sema))
+		/* DRAINED_SELF: (1) No need to pause+yield, (2) we
+		 * better have been the last item in the list!  */
+		assert(!self.next);
+	else
+		cr_pause_and_yield();
+}
 
 #endif /* _COROUTINE_SEMA_H_ */
diff --git a/libcr_ipc/mutex.c b/libcr_ipc/mutex.c
deleted file mode 100644
index 198c738..0000000
--- a/libcr_ipc/mutex.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/* libcr_ipc/mutex.c - Simple mutexes for libcr (implementation file)
- *
- * Copyright (C) 2024  Luke T. Shumaker <lukeshu@lukeshu.com>
- * SPDX-Licence-Identifier: AGPL-3.0-or-later
- */
-
-#include <assert.h>
-
-#include <libcr_ipc/mutex.h>
-
-void cr_mutex_lock(cr_mutex_t *mu) {
-	assert(mu);
-	if (!mu->tail)
-		mu->tail = &mu->head;
-	if (!mu->locked) {
-		mu->locked = true;
-		return;
-	}
-	struct _cr_ipc_cid_list self = {
-		.val = cr_getcid(),
-		.next = NULL,
-	};
-	*(mu->tail) = &self;
-	mu->tail = &(self.next);
-	cr_pause_and_yield();
-}
-
-void cr_mutex_unlock(cr_mutex_t *mu) {
-	assert(mu);
-	assert(mu->tail);
-	assert(mu->locked);
-	if (mu->head) {
-		cr_unpause(mu->head->val);
-		mu->head = mu->head->next;
-		if (!mu->head)
-			mu->tail = &mu->head;
-	} else
-		mu->locked = false;
-}
diff --git a/libcr_ipc/sema.c b/libcr_ipc/sema.c
deleted file mode 100644
index a3efb57..0000000
--- a/libcr_ipc/sema.c
+++ /dev/null
@@ -1,81 +0,0 @@
-/* libcr_ipc/sema.c - Simple semaphores for libcr (implementation file)
- *
- * Copyright (C) 2024  Luke T. Shumaker <lukeshu@lukeshu.com>
- * SPDX-Licence-Identifier: AGPL-3.0-or-later
- */
-
-#include <assert.h>
-
-#include <libcr_ipc/sema.h>
-
-/** Drain the sema->{head,tail} list.  Returns true if cr_getcid() was drained.  */
-static inline bool drain(cr_sema_t *sema) {
-	assert(!sema->locked);
-	cid_t self = cr_getcid();
-
-	enum drain_result {
-		DRAINING,
-		DRAINED_SELF, /* stopped because drained `self` */
-		DRAINED_ALL,  /* stopped because sema->head == NULL */
-		DRAINED_SOME, /* stopped because sema->cnt == 0 */
-	} state = DRAINING;
-	do { 
-		sema->locked = true;
-		while (state == DRAINING) {
-			if (!sema->head) {
-				state = DRAINED_ALL;
-			} else if (!sema->cnt) {
-				state = DRAINED_SOME;
-			} else {
-				sema->cnt--;
-				cid_t cid = sema->head->val;
-				if (cid == self)
-					state = DRAINED_SELF;
-				else
-					cr_unpause(sema->head->val);
-				sema->head = sema->head->next;
-				if (!sema->head)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdiscarded-qualifiers"
-					sema->tail = &sema->head;
-#pragma GCC diagnostic pop
-			}
-		}
-		sema->locked = false;
-		/* If there are still coroutines in sema->head, check
-		 * that sema->cnt wasn't incremented between `if
-		 * (!sema->cnt)` and `sema->locked = false`.  */
-	} while (state == DRAINED_SOME && sema->cnt);
-	/* If state == DRAINED_SELF, then we better have been the last
-	 * item in the list!  */
-	assert(state != DRAINED_SELF || !sema->head);
-	return state == DRAINED_SELF;
-}
-
-void cr_sema_signal(cr_sema_t *sema) {
-	sema->cnt++;
-	if (!sema->locked)
-		drain(sema);
-}
-
-void cr_sema_wait(cr_sema_t *sema) {
-	struct _cr_ipc_cid_list self = {
-		.val = cr_getcid(),
-		.next = NULL,
-	};
-
-	sema->locked = true;
-	if (!sema->tail)
-		sema->head = &self;
-	else
-		*(sema->tail) = &self;
-	sema->tail = &(self.next);
-	sema->locked = false;
-
-	if (drain(sema))
-		/* DRAINED_SELF: (1) No need to pause+yield, (2) we
-		 * better have been the last item in the list!  */
-		assert(!self.next);
-	else
-		cr_pause_and_yield();
-}