poll: Enable multiple threads to use k_poll in the same object

This is necessary in order for k_queue_get to work properly since that
is used with buffer pools which might be used by multiple threads asking
for buffers.

Jira: ZEP-2553

Signed-off-by: Luiz Augusto von Dentz <[email protected]>

Author: Vudentz

include/kernel.h

@@ -98,11 +98,11 @@ typedef sys_dlist_t _wait_q_t;
 #endif
 
 #ifdef CONFIG_POLL
-#define _POLL_EVENT_OBJ_INIT \
-	.poll_event = NULL,
-#define _POLL_EVENT struct k_poll_event *poll_event
+#define _POLL_EVENT_OBJ_INIT(obj) \
+	.poll_events = SYS_DLIST_STATIC_INIT(&obj.poll_events),
+#define _POLL_EVENT sys_dlist_t poll_events
 #else
-#define _POLL_EVENT_OBJ_INIT
+#define _POLL_EVENT_OBJ_INIT(obj)
 #define _POLL_EVENT
 #endif
 
@@ -1327,7 +1327,7 @@ struct k_queue {
 	{ \
 	.wait_q = SYS_DLIST_STATIC_INIT(&obj.wait_q), \
 	.data_q = SYS_SLIST_STATIC_INIT(&obj.data_q), \
-	_POLL_EVENT_OBJ_INIT \
+	_POLL_EVENT_OBJ_INIT(obj) \
 	_OBJECT_TRACING_INIT \
 	}
 
@@ -2360,7 +2360,7 @@ struct k_sem {
 	.wait_q = SYS_DLIST_STATIC_INIT(&obj.wait_q), \
 	.count = initial_count, \
 	.limit = count_limit, \
-	_POLL_EVENT_OBJ_INIT \
+	_POLL_EVENT_OBJ_INIT(obj) \
 	_OBJECT_TRACING_INIT \
 	}
 
@@ -3544,9 +3544,6 @@ enum _poll_states_bits {
 	/* default state when creating event */
 	_POLL_STATE_NOT_READY,
 
-	/* there was another poller already on the object */
-	_POLL_STATE_EADDRINUSE,
-
 	/* signaled by k_poll_signal() */
 	_POLL_STATE_SIGNALED,
 
@@ -3601,7 +3598,6 @@ enum k_poll_modes {
 
 /* public - values for k_poll_event.state bitfield */
 #define K_POLL_STATE_NOT_READY 0
-#define K_POLL_STATE_EADDRINUSE _POLL_STATE_BIT(_POLL_STATE_EADDRINUSE)
 #define K_POLL_STATE_SIGNALED _POLL_STATE_BIT(_POLL_STATE_SIGNALED)
 #define K_POLL_STATE_SEM_AVAILABLE _POLL_STATE_BIT(_POLL_STATE_SEM_AVAILABLE)
 #define K_POLL_STATE_DATA_AVAILABLE _POLL_STATE_BIT(_POLL_STATE_DATA_AVAILABLE)
@@ -3610,7 +3606,7 @@ enum k_poll_modes {
 /* public - poll signal object */
 struct k_poll_signal {
 	/* PRIVATE - DO NOT TOUCH */
-	struct k_poll_event *poll_event;
+	sys_dlist_t poll_events;
 
 	/*
 	 * 1 if the event has been signaled, 0 otherwise. Stays set to 1 until
@@ -3622,14 +3618,17 @@ struct k_poll_signal {
 	int result;
 };
 
-#define K_POLL_SIGNAL_INITIALIZER() \
+#define K_POLL_SIGNAL_INITIALIZER(obj) \
 	{ \
-	.poll_event = NULL, \
+	.poll_events = SYS_DLIST_STATIC_INIT(&obj.poll_events), \
 	.signaled = 0, \
 	.result = 0, \
 	}
 
 struct k_poll_event {
+	/* PRIVATE - DO NOT TOUCH */
+	sys_dnode_t _node;
+
 	/* PRIVATE - DO NOT TOUCH */
 	struct _poller *poller;
 
@@ -3716,16 +3715,9 @@ extern void k_poll_event_init(struct k_poll_event *event, u32_t type,
  * reason, the k_poll() call is more effective when the objects being polled
  * only have one thread, the polling thread, trying to acquire them.
  *
- * Only one thread can be polling for a particular object at a given time. If
- * another thread tries to poll on it, the k_poll() call returns -EADDRINUSE
- * and returns as soon as it has finished handling the other events. This means
- * that k_poll() can return -EADDRINUSE and have the state value of some events
- * be non-K_POLL_STATE_NOT_READY. When this condition occurs, the @a timeout
- * parameter is ignored.
- *
- * When k_poll() returns 0 or -EADDRINUSE, the caller should loop on all the
- * events that were passed to k_poll() and check the state field for the values
- * that were expected and take the associated actions.
+ * When k_poll() returns 0, the caller should loop on all the events that were
+ * passed to k_poll() and check the state field for the values that were
+ * expected and take the associated actions.
  *
  * Before being reused for another call to k_poll(), the user has to reset the
  * state field to K_POLL_STATE_NOT_READY.
@@ -3736,7 +3728,6 @@ extern void k_poll_event_init(struct k_poll_event *event, u32_t type,
  *                or one of the special values K_NO_WAIT and K_FOREVER.
  *
  * @retval 0 One or more events are ready.
- * @retval -EADDRINUSE One or more objects already had a poller.
  * @retval -EAGAIN Waiting period timed out.
  */
 
@@ -3777,8 +3768,7 @@ extern void k_poll_signal_init(struct k_poll_signal *signal);
 extern int k_poll_signal(struct k_poll_signal *signal, int result);
 
 /* private internal function */
-extern int _handle_obj_poll_event(struct k_poll_event **obj_poll_event,
-				  u32_t state);
+extern int _handle_obj_poll_events(sys_dlist_t *events, u32_t state);
 
 /**
  * @} end defgroup poll_apis

kernel/poll.c

@@ -89,30 +89,46 @@ static inline int is_condition_met(struct k_poll_event *event, u32_t *state)
 	return 0;
 }
 
+static inline void add_event(sys_dlist_t *events, struct k_poll_event *event,
+			     struct _poller *poller)
+{
+	struct k_poll_event *pending;
+
+	pending = (struct k_poll_event *)sys_dlist_peek_tail(events);
+	if (!pending || _is_t1_higher_prio_than_t2(pending->poller->thread,
+						   poller->thread)) {
+		sys_dlist_append(events, &event->_node);
+		return;
+	}
+
+	SYS_DLIST_FOR_EACH_CONTAINER(events, pending, _node) {
+		if (_is_t1_higher_prio_than_t2(poller->thread,
+					       pending->poller->thread)) {
+			sys_dlist_insert_before(events, &pending->_node,
+						&event->_node);
+			return;
+		}
+	}
+
+	sys_dlist_append(events, &event->_node);
+}
+
 /* must be called with interrupts locked */
-static inline int register_event(struct k_poll_event *event)
+static inline int register_event(struct k_poll_event *event,
+				 struct _poller *poller)
 {
 	switch (event->type) {
 	case K_POLL_TYPE_SEM_AVAILABLE:
 		__ASSERT(event->sem, "invalid semaphore\n");
-		if (event->sem->poll_event) {
-			return -EADDRINUSE;
-		}
-		event->sem->poll_event = event;
+		add_event(&event->sem->poll_events, event, poller);
 		break;
 	case K_POLL_TYPE_DATA_AVAILABLE:
 		__ASSERT(event->queue, "invalid queue\n");
-		if (event->queue->poll_event) {
-			return -EADDRINUSE;
-		}
-		event->queue->poll_event = event;
+		add_event(&event->queue->poll_events, event, poller);
 		break;
 	case K_POLL_TYPE_SIGNAL:
-		__ASSERT(event->queue, "invalid poll signal\n");
-		if (event->signal->poll_event) {
-			return -EADDRINUSE;
-		}
-		event->signal->poll_event = event;
+		__ASSERT(event->signal, "invalid poll signal\n");
+		add_event(&event->signal->poll_events, event, poller);
 		break;
 	case K_POLL_TYPE_IGNORE:
 		/* nothing to do */
@@ -122,6 +138,8 @@ static inline int register_event(struct k_poll_event *event)
 		break;
 	}
 
+	event->poller = poller;
+
 	return 0;
 }
 
@@ -133,15 +151,15 @@ static inline void clear_event_registration(struct k_poll_event *event)
 	switch (event->type) {
 	case K_POLL_TYPE_SEM_AVAILABLE:
 		__ASSERT(event->sem, "invalid semaphore\n");
-		event->sem->poll_event = NULL;
+		sys_dlist_remove(&event->_node);
 		break;
 	case K_POLL_TYPE_DATA_AVAILABLE:
 		__ASSERT(event->queue, "invalid queue\n");
-		event->queue->poll_event = NULL;
+		sys_dlist_remove(&event->_node);
 		break;
 	case K_POLL_TYPE_SIGNAL:
 		__ASSERT(event->signal, "invalid poll signal\n");
-		event->signal->poll_event = NULL;
+		sys_dlist_remove(&event->_node);
 		break;
 	case K_POLL_TYPE_IGNORE:
 		/* nothing to do */
@@ -176,18 +194,13 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout)
 	__ASSERT(events, "NULL events\n");
 	__ASSERT(num_events > 0, "zero events\n");
 
-	int last_registered = -1, in_use = 0, rc;
+	int last_registered = -1, rc;
 	unsigned int key;
 
 	key = irq_lock();
 	set_polling_state(_current);
 	irq_unlock(key);
 
-	/*
-	 * We can get by with one poller structure for all events for now:
-	 * if/when we allow multiple threads to poll on the same object, we
-	 * will need one per poll event associated with an object.
-	 */
 	struct _poller poller = { .thread = _current };
 
 	/* find events whose condition is already fulfilled */
@@ -198,18 +211,10 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout)
 		if (is_condition_met(&events[ii], &state)) {
 			set_event_ready(&events[ii], state);
 			clear_polling_state(_current);
-		} else if (timeout != K_NO_WAIT && is_polling() && !in_use) {
-			rc = register_event(&events[ii]);
+		} else if (timeout != K_NO_WAIT && is_polling()) {
+			rc = register_event(&events[ii], &poller);
 			if (rc == 0) {
-				events[ii].poller = &poller;
 				++last_registered;
-			} else if (rc == -EADDRINUSE) {
-				/* setting in_use also prevents any further
-				 * registrations by the current thread
-				 */
-				in_use = -EADDRINUSE;
-				events[ii].state = K_POLL_STATE_EADDRINUSE;
-				clear_polling_state(_current);
 			} else {
 				__ASSERT(0, "unexpected return code\n");
 			}
@@ -224,16 +229,12 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout)
 	 * condition is met, either when looping through the events here or
 	 * because one of the events registered has had its state changed, or
 	 * that one of the objects we wanted to poll on already had a thread
-	 * polling on it. We can remove all registrations and return either
-	 * success or a -EADDRINUSE error. In the case of a -EADDRINUSE error,
-	 * the events that were available are still flagged as such, and it is
-	 * valid for the caller to consider them available, as if this function
-	 * returned success.
+	 * polling on it.
 	 */
 	if (!is_polling()) {
 		clear_event_registrations(events, last_registered, key);
 		irq_unlock(key);
-		return in_use;
+		return 0;
 	}
 
 	clear_polling_state(_current);
@@ -306,38 +307,43 @@ static int _signal_poll_event(struct k_poll_event *event, u32_t state,
 }
 
 /* returns 1 if a reschedule must take place, 0 otherwise */
-/* *obj_poll_event is guaranteed to not be NULL */
-int _handle_obj_poll_event(struct k_poll_event **obj_poll_event, u32_t state)
+int _handle_obj_poll_events(sys_dlist_t *events, u32_t state)
 {
-	struct k_poll_event *poll_event = *obj_poll_event;
+	struct k_poll_event *poll_event;
 	int must_reschedule;
 
-	*obj_poll_event = NULL;
+	poll_event = (struct k_poll_event *)sys_dlist_get(events);
+	if (!poll_event) {
+		return 0;
+	}
+
 	(void)_signal_poll_event(poll_event, state, &must_reschedule);
 	return must_reschedule;
 }
 
 void k_poll_signal_init(struct k_poll_signal *signal)
 {
-	signal->poll_event = NULL;
+	sys_dlist_init(&signal->poll_events);
 	signal->signaled = 0;
 	/* signal->result is left unitialized */
 }
 
 int k_poll_signal(struct k_poll_signal *signal, int result)
 {
 	unsigned int key = irq_lock();
+	struct k_poll_event *poll_event;
 	int must_reschedule;
 
 	signal->result = result;
 	signal->signaled = 1;
 
-	if (!signal->poll_event) {
+	poll_event = (struct k_poll_event *)sys_dlist_get(&signal->poll_events);
+	if (!poll_event) {
 		irq_unlock(key);
 		return 0;
 	}
 
-	int rc = _signal_poll_event(signal->poll_event, K_POLL_STATE_SIGNALED,
+	int rc = _signal_poll_event(poll_event, K_POLL_STATE_SIGNALED,
 				    &must_reschedule);
 
 	if (must_reschedule) {

kernel/queue.c

@@ -51,8 +51,9 @@ void k_queue_init(struct k_queue *queue)
 {
 	sys_slist_init(&queue->data_q);
 	sys_dlist_init(&queue->wait_q);
-
-	_INIT_OBJ_POLL_EVENT(queue);
+#if defined(CONFIG_POLL)
+	sys_dlist_init(&queue->poll_events);
+#endif
 
 	SYS_TRACING_OBJ_INIT(k_queue, queue);
 }
@@ -67,13 +68,12 @@ static void prepare_thread_to_run(struct k_thread *thread, void *data)
 #endif /* CONFIG_POLL */
 
 /* returns 1 if a reschedule must take place, 0 otherwise */
-static inline int handle_poll_event(struct k_queue *queue)
+static inline int handle_poll_events(struct k_queue *queue)
 {
 #ifdef CONFIG_POLL
 	u32_t state = K_POLL_STATE_DATA_AVAILABLE;
 
-	return queue->poll_event ?
-	       _handle_obj_poll_event(&queue->poll_event, state) : 0;
+	return _handle_obj_poll_events(&queue->poll_events, state);
 #else
 	return 0;
 #endif
@@ -95,7 +95,7 @@ void k_queue_cancel_wait(struct k_queue *queue)
 		}
 	}
 #else
-	if (handle_poll_event(queue)) {
+	if (handle_poll_events(queue)) {
 		(void)_Swap(key);
 		return;
 	}
@@ -126,7 +126,7 @@ void k_queue_insert(struct k_queue *queue, void *prev, void *data)
 	sys_slist_insert(&queue->data_q, prev, data);
 
 #if defined(CONFIG_POLL)
-	if (handle_poll_event(queue)) {
+	if (handle_poll_events(queue)) {
 		(void)_Swap(key);
 		return;
 	}
@@ -171,7 +171,7 @@ void k_queue_append_list(struct k_queue *queue, void *head, void *tail)
 	}
 #else
 	sys_slist_append_list(&queue->data_q, head, tail);
-	if (handle_poll_event(queue)) {
+	if (handle_poll_events(queue)) {
 		(void)_Swap(key);
 		return;
 	}
@@ -206,11 +206,10 @@ static void *k_queue_poll(struct k_queue *queue, s32_t timeout)
 	event.state = K_POLL_STATE_NOT_READY;
 
 	err = k_poll(&event, 1, timeout);
-	if (err == -EAGAIN) {
+	if (err) {
 		return NULL;
 	}
 
-	__ASSERT_NO_MSG(err == 0);
 	__ASSERT_NO_MSG(event.state == K_POLL_STATE_FIFO_DATA_AVAILABLE);
 
 	return sys_slist_get(&queue->data_q);