kevent: refactor callback system

arch/at91sam3x8e/serial.c

@@ -134,11 +134,8 @@ void irq_uart(void)
 		tmp = REG_UART_RHR;
 		ringbuf_write(arch_serial_default_device.device.rx, &tmp, sizeof(tmp));
 
-		/* TODO: we need some error handling mechanism for event creation */
-		struct device_kevent *event = device_kevent_create(&serial_default_device->device,
-								   DEVICE_CHANNEL_IN);
-		if (event != NULL)
-			kevent_dispatch(&event->event);
+		device_kevent_create_and_dispatch(&serial_default_device->device,
+						  DEVICE_CHANNEL_IN);
 	}
 
 	/* REG_UART_PDC_TCR has reached zero */
@@ -153,11 +150,8 @@ void irq_uart(void)
 		if (arch_serial_default_device.tx_current == NULL)
 			REG_UART_IDR = REG_UART_IDR_ENDTX_MASK;
 
-		/* TODO: we need some error handling mechanism for event creation */
-		struct device_kevent *event = device_kevent_create(&serial_default_device->device,
-								   DEVICE_CHANNEL_OUT);
-		if (event != NULL)
-			kevent_dispatch(&event->event);
+		device_kevent_create_and_dispatch(&serial_default_device->device,
+						  DEVICE_CHANNEL_OUT);
 	}
 
 	/* check for error conditions */

include/ardix/device.h

@@ -50,6 +50,14 @@ __always_inline struct device *kevent_to_device(struct kevent *event)
  */
 struct device_kevent *device_kevent_create(struct device *device, enum device_channel channel);
 
+/**
+ * @brief Convenience wrapper for creating and immediately dispatching a device kevent.
+ *
+ * @param device Device the event refers to
+ * @param channel Which channel (in or out) the event applies to
+ */
+void device_kevent_create_and_dispatch(struct device *device, enum device_channel channel);
+
 /** Initialize the devices subsystem. */
 int devices_init(void);
 

include/ardix/kevent.h

@@ -36,6 +36,31 @@ struct kevent {
 	enum kevent_kind kind;
 };
 
+/**
+ * @brief Flags for kevent callback return values.
+ */
+enum kevent_cb_flags {
+	/** @brief No particular action is taken. */
+	KEVENT_CB_NONE			= 0,
+	/** @brief Stop processing the other callbacks in the queue after the callback. */
+	KEVENT_CB_STOP			= (1 << 0),
+	/** @brief Call `kevent_listener_del()` after the callback. */
+	KEVENT_CB_LISTENER_DEL		= (1 << 1),
+};
+
+/**
+ * @brief Identifies a single kevent listener.
+ *
+ * This is returned by `kevent_listener_add()` and must be passed to
+ * `kevent_listener_del()` when the listener is no longer needed.
+ * You shouldn't modify the members yourself.
+ */
+struct kevent_listener {
+	struct list_head link;
+	int (*cb)(struct kevent *event, void *extra);
+	void *extra;
+};
+
 /** @brief Initialize the kevent subsystem. */
 void kevents_init(void);
 
@@ -55,27 +80,24 @@ void kevent_dispatch(struct kevent *event);
 /**
  * @brief Add an event listener to the end of the listener queue.
  * The callback will be invoked for every event that is dispatched and matches
- * the event kind.  If its return value is nonzero, event handler processing
- * stops after that callback.  This is useful if you know for sure that you are
- * the only one interested in the event, for example when waiting for I/O.
+ * the event kind.  The return value of this callback is a set of flags, see
+ * `enum kevent_cb_flags` for details.
  *
  * @param kind Kind of kevent to listen for
  * @param cb Callback that will be invoked for every kevent that is dispatched
  * @param extra An optional extra pointer that will be passed to the callback
- * @returns 0 on success, or a negtive error number on failure
+ * @returns The listener (pass to `kevent_listener_del()` when no longer needed)
  */
-int kevent_add_listener(enum kevent_kind kind,
-			int (*cb)(struct kevent *event, void *extra),
-			void *extra);
+struct kevent_listener *kevent_listener_add(enum kevent_kind kind,
+					    int (*cb)(struct kevent *event, void *extra),
+					    void *extra);
 
 /**
  * @brief Remove an event listener.
  *
- * @param kind Kind that was passed to `kevent_add_listener()`
- * @param cb Callback that was passed to `kevent_add_listener()`
+ * @param listener The listener returned by `kevent_listener_add()`
  */
-void kevent_remove_listener(enum kevent_kind kind,
-			    int (*cb)(struct kevent *event, void *extra));
+void kevent_listener_del(struct kevent_listener *listener);
 
 __always_inline void kevent_get(struct kevent *event)
 {

kernel/device.c

@@ -77,6 +77,13 @@ struct device_kevent *device_kevent_create(struct device *device, enum device_ch
 	return event;
 }
 
+void device_kevent_create_and_dispatch(struct device *device, enum device_channel channel)
+{
+	struct device_kevent *event = device_kevent_create(device, channel);
+	if (event != NULL)
+		kevent_dispatch(&event->event);
+}
+
 /*
  * This file is part of Ardix.
  * Copyright (c) 2020, 2021 Felix Kopp <owo@fef.moe>.

kernel/kevent.c

@@ -31,20 +31,13 @@ static struct kevent_queue kev_queues[KEVENT_KIND_COUNT];
 
 /**
  * irqs cannot sleep or block, so we can only *try* to claim the lock on
- * an event queue when dispatching an event.  If this fails (which shouldn't
+ * an event queue when dispatching an event.  If this fails (which should
  * happen only rarely), we store the event in this (unsorted) pile and sort them
- * out before processing the event queue.  We technically need a lock for this
- * queue as well, but that's okay because we don't
+ * out before processing the event queue.
  */
 LIST_HEAD(kev_cache);
 MUTEX(kev_cache_lock);
 
-struct kevent_listener {
-	struct list_head link;	/* -> kevent_queue::list */
-	void *extra;
-	int (*cb)(struct kevent *event, void *extra);
-};
-
 void kevents_init(void)
 {
 	for (int i = 0; i < KEVENT_KIND_COUNT; i++) {
@@ -71,7 +64,13 @@ static inline void process_single_queue(struct kevent_queue *queue, struct list_
 
 			list_for_each_entry(listeners, listener, link) {
 				int cb_ret = listener->cb(event, listener->extra);
-				if (cb_ret != 0)
+
+				if (cb_ret & KEVENT_CB_LISTENER_DEL) {
+					list_delete(&listener->link);
+					free(listener);
+				}
+
+				if (cb_ret & KEVENT_CB_STOP)
 					break;
 			}
 
@@ -149,36 +148,31 @@ void kevent_dispatch(struct kevent *event)
 	}
 }
 
-int kevent_add_listener(enum kevent_kind kind, int (*cb)(struct kevent *, void *), void *extra)
+struct kevent_listener *kevent_listener_add(enum kevent_kind kind,
+					    int (*cb)(struct kevent *, void *),
+					    void *extra)
 {
 	struct kevent_listener *listener = malloc(sizeof(*listener));
-	if (listener == NULL)
-		return -ENOMEM;
 
-	listener->cb = cb;
-	listener->extra = extra;
+	if (listener != NULL) {
+		listener->cb = cb;
+		listener->extra = extra;
 
-	mutex_lock(&kev_listeners_lock);
-	list_insert(&kev_listeners[kind], &listener->link);
-	mutex_unlock(&kev_listeners_lock);
+		mutex_lock(&kev_listeners_lock);
+		list_insert(&kev_listeners[kind], &listener->link);
+		mutex_unlock(&kev_listeners_lock);
+	}
 
-	return 0;
+	return listener;
 }
 
-void kevent_remove_listener(enum kevent_kind kind, int (*cb)(struct kevent *, void *))
+void kevent_listener_del(struct kevent_listener *listener)
 {
-	struct kevent_listener *listener, *tmp;
+	mutex_lock(&kev_listeners_lock);
+	list_delete(&listener->link);
+	mutex_unlock(&kev_listeners_lock);
 
-	list_for_each_entry_safe(&kev_listeners[kind], listener, tmp, link) {
-		if (listener->cb == cb) {
-			mutex_lock(&kev_listeners_lock);
-			list_delete(&listener->link);
-			mutex_unlock(&kev_listeners_lock);
-
-			free(listener);
-			break;
-		}
-	}
+	free(listener);
 }
 
 /*