From 3d13afcce2e5d09a8d8f3050bd990b9018a7f26f Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 17 Apr 2025 00:24:45 -0400 Subject: [PATCH 1/2] tests: poll: add pipe coverage Add pipe test coverage. Signed-off-by: Nicolas Pitre --- tests/kernel/poll/src/test_poll.c | 62 +++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/tests/kernel/poll/src/test_poll.c b/tests/kernel/poll/src/test_poll.c index ac2131fcbf9d..dc910d76e9fc 100644 --- a/tests/kernel/poll/src/test_poll.c +++ b/tests/kernel/poll/src/test_poll.c @@ -18,12 +18,14 @@ struct fifo_msg { #define MSGQ_MSG_SIZE 4 #define MSGQ_MAX_MSGS 16 #define MSGQ_MSG_VALUE {'a', 'b', 'c', 'd'} +#define PIPE_DATA "atad_epip" #define STACK_SIZE (1024 + CONFIG_TEST_EXTRA_STACK_SIZE) /* verify k_poll() without waiting */ static struct k_sem no_wait_sem; static struct k_fifo no_wait_fifo; static struct k_poll_signal no_wait_signal; +K_PIPE_DEFINE(no_wait_pipe, 32, 1); static struct k_poll_signal test_signal; #ifndef CONFIG_USERSPACE static struct k_msgq no_wait_msgq; @@ -63,6 +65,7 @@ ZTEST_USER(poll_api_1cpu, test_poll_no_wait) unsigned int signaled; char msgq_recv_buf[MSGQ_MSG_SIZE] = {0}; char msgq_msg[MSGQ_MSG_SIZE] = MSGQ_MSG_VALUE; + char pipe_recv_buf[sizeof(PIPE_DATA) + 4]; int result; struct k_msgq *mq; #ifdef CONFIG_USERSPACE @@ -78,6 +81,8 @@ ZTEST_USER(poll_api_1cpu, test_poll_no_wait) k_msgq_alloc_init(mq, MSGQ_MSG_SIZE, MSGQ_MAX_MSGS); + k_pipe_write(&no_wait_pipe, PIPE_DATA, sizeof(PIPE_DATA), K_NO_WAIT); + struct k_poll_event events[] = { K_POLL_EVENT_INITIALIZER(K_POLL_TYPE_SEM_AVAILABLE, K_POLL_MODE_NOTIFY_ONLY, @@ -94,6 +99,9 @@ ZTEST_USER(poll_api_1cpu, test_poll_no_wait) K_POLL_EVENT_INITIALIZER(K_POLL_TYPE_MSGQ_DATA_AVAILABLE, K_POLL_MODE_NOTIFY_ONLY, mq), + K_POLL_EVENT_INITIALIZER(K_POLL_TYPE_PIPE_DATA_AVAILABLE, + K_POLL_MODE_NOTIFY_ONLY, + &no_wait_pipe), }; #ifdef CONFIG_USERSPACE @@ -159,12 +167,18 @@ ZTEST_USER(poll_api_1cpu, test_poll_no_wait) zassert_false(k_msgq_get(mq, msgq_recv_buf, K_NO_WAIT)); zassert_false(memcmp(msgq_msg, msgq_recv_buf, MSGQ_MSG_SIZE), ""); + zassert_equal(events[5].state, K_POLL_STATE_PIPE_DATA_AVAILABLE); + result = k_pipe_read(&no_wait_pipe, pipe_recv_buf, sizeof(pipe_recv_buf), K_NO_WAIT); + zassert_equal(result, sizeof(PIPE_DATA)); + zassert_str_equal(pipe_recv_buf, PIPE_DATA); + /* verify events are not ready anymore (user has to clear them first) */ events[0].state = K_POLL_STATE_NOT_READY; events[1].state = K_POLL_STATE_NOT_READY; events[2].state = K_POLL_STATE_NOT_READY; events[3].state = K_POLL_STATE_NOT_READY; events[4].state = K_POLL_STATE_NOT_READY; + events[5].state = K_POLL_STATE_NOT_READY; k_poll_signal_reset(&no_wait_signal); zassert_equal(k_poll(events, ARRAY_SIZE(events), K_NO_WAIT), -EAGAIN, @@ -174,11 +188,11 @@ ZTEST_USER(poll_api_1cpu, test_poll_no_wait) zassert_equal(events[2].state, K_POLL_STATE_NOT_READY, ""); zassert_equal(events[3].state, K_POLL_STATE_NOT_READY, ""); zassert_equal(events[4].state, K_POLL_STATE_NOT_READY, ""); + zassert_equal(events[5].state, K_POLL_STATE_NOT_READY, ""); zassert_not_equal(k_sem_take(&no_wait_sem, K_NO_WAIT), 0, ""); zassert_is_null(k_fifo_get(&no_wait_fifo, K_NO_WAIT), ""); - zassert_not_equal(k_msgq_get(mq, msgq_recv_buf, K_NO_WAIT), 0, - ""); + zassert_not_equal(k_msgq_get(mq, msgq_recv_buf, K_NO_WAIT), 0, ""); } /* verify k_poll() that has to wait */ @@ -192,10 +206,13 @@ static struct k_poll_signal wait_signal = struct fifo_msg wait_msg = { NULL, FIFO_MSG_VALUE }; +K_PIPE_DEFINE(wait_pipe, 32, 1); + #define TAG_0 10 #define TAG_1 11 #define TAG_2 12 #define TAG_3 13 +#define TAG_4 14 struct k_poll_event wait_events[] = { K_POLL_EVENT_STATIC_INITIALIZER(K_POLL_TYPE_SEM_AVAILABLE, @@ -213,10 +230,14 @@ struct k_poll_event wait_events[] = { K_POLL_EVENT_STATIC_INITIALIZER(K_POLL_TYPE_MSGQ_DATA_AVAILABLE, K_POLL_MODE_NOTIFY_ONLY, &wait_msgq, TAG_3), + K_POLL_EVENT_STATIC_INITIALIZER(K_POLL_TYPE_PIPE_DATA_AVAILABLE, + K_POLL_MODE_NOTIFY_ONLY, + &wait_pipe, TAG_4), }; #define USE_FIFO (1 << 0) #define USE_MSGQ (1 << 1) +#define USE_PIPE (1 << 2) static void poll_wait_helper(void *use_queuelike, void *msgq, void *p3) { @@ -239,6 +260,10 @@ static void poll_wait_helper(void *use_queuelike, void *msgq, void *p3) k_msgq_put(msgq, &m[0], K_FOREVER); } + + if (flags & USE_PIPE) { + k_pipe_write(&wait_pipe, PIPE_DATA, sizeof(PIPE_DATA), K_NO_WAIT); + } } /* check results for multiple events */ @@ -248,6 +273,8 @@ void check_results(struct k_poll_event *events, uint32_t event_type, struct fifo_msg *msg_ptr; char msgq_recv_buf[MSGQ_MSG_SIZE] = {0}; char msg[] = MSGQ_MSG_VALUE; + char pipe_recv_buf[sizeof(PIPE_DATA) + 4]; + int result; switch (event_type) { case K_POLL_TYPE_SEM_AVAILABLE: @@ -317,7 +344,20 @@ void check_results(struct k_poll_event *events, uint32_t event_type, ""); } break; - + case K_POLL_TYPE_PIPE_DATA_AVAILABLE: + if (is_available) { + zassert_equal(events->state, K_POLL_STATE_PIPE_DATA_AVAILABLE); + result = k_pipe_read(&wait_pipe, pipe_recv_buf, + sizeof(pipe_recv_buf), K_NO_WAIT); + zassert_equal(result, sizeof(PIPE_DATA)); + zassert_str_equal(pipe_recv_buf, PIPE_DATA); + zassert_equal(events->tag, TAG_4); + /* reset to not ready */ + events->state = K_POLL_STATE_NOT_READY; + } else { + zassert_equal(events->state, K_POLL_STATE_NOT_READY); + } + break; default: __ASSERT(false, "invalid event type (0x%x)\n", event_type); break; @@ -403,9 +443,9 @@ ZTEST(poll_api_1cpu, test_poll_wait) k_tid_t tid1 = k_thread_create(&test_thread, test_stack, K_THREAD_STACK_SIZEOF(test_stack), - poll_wait_helper, (void *)(USE_FIFO | USE_MSGQ), wait_msgq_ptr, 0, - main_low_prio - 1, K_USER | K_INHERIT_PERMS, - K_NO_WAIT); + poll_wait_helper, (void *)(USE_FIFO | USE_MSGQ | USE_PIPE), + wait_msgq_ptr, 0, main_low_prio - 1, + K_USER | K_INHERIT_PERMS, K_NO_WAIT); rc = k_poll(wait_events, ARRAY_SIZE(wait_events), K_NO_WAIT); zassert_equal(rc, -EAGAIN, "should return EAGAIN with K_NO_WAIT"); @@ -422,6 +462,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, true); check_results(&wait_events[3], K_POLL_TYPE_IGNORE, true); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, true); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, true); /* verify events are not ready anymore */ zassert_equal(k_poll(wait_events, ARRAY_SIZE(wait_events), @@ -433,6 +474,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, false); check_results(&wait_events[3], K_POLL_TYPE_IGNORE, false); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, false); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); /* * Wait for 2 out of 4 non-ready events to become ready from a higher @@ -455,6 +497,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[1], K_POLL_TYPE_DATA_AVAILABLE, false); check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, true); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, false); + check_results(&wait_events[4], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); /* * Wait for each event to be ready from a lower priority thread, one at @@ -473,6 +516,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[1], K_POLL_TYPE_DATA_AVAILABLE, false); check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, false); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, false); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); /* fifo */ rc = k_poll(wait_events, ARRAY_SIZE(wait_events), K_SECONDS(1)); @@ -483,6 +527,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[1], K_POLL_TYPE_DATA_AVAILABLE, true); check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, false); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, false); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); /* poll signal */ rc = k_poll(wait_events, ARRAY_SIZE(wait_events), K_SECONDS(1)); @@ -493,6 +538,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[1], K_POLL_TYPE_DATA_AVAILABLE, false); check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, true); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, false); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); /* message queue */ rc = k_poll(wait_events, ARRAY_SIZE(wait_events), K_SECONDS(1)); @@ -503,6 +549,7 @@ ZTEST(poll_api_1cpu, test_poll_wait) check_results(&wait_events[1], K_POLL_TYPE_DATA_AVAILABLE, false); check_results(&wait_events[2], K_POLL_TYPE_SIGNAL, false); check_results(&wait_events[4], K_POLL_TYPE_MSGQ_DATA_AVAILABLE, true); + check_results(&wait_events[5], K_POLL_TYPE_PIPE_DATA_AVAILABLE, false); k_thread_abort(tid1); k_thread_abort(tid2); @@ -780,7 +827,8 @@ void poll_test_grant_access(void) &no_wait_signal, &wait_sem, &wait_fifo, &cancel_fifo, &non_cancel_fifo, &wait_signal, &test_thread, &test_signal, - &test_stack, &multi_sem, &multi_reply); + &test_stack, &multi_sem, &multi_reply, + &no_wait_pipe, &wait_pipe); } From d048755869000461c2d1756e6452050ea2882693 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 17 Apr 2025 00:27:30 -0400 Subject: [PATCH 2/2] kernel/pipe: fix poll support Two issues: - is_condition_met() was missing proper code for The K_POLL_TYPE_PIPE_DATA_AVAILABLE case - z_handle_obj_poll_events() was misplaced in z_impl_k_pipe_write() Note: I added support for the deprecated pipe implementation to is_condition_met() but that is untested. Signed-off-by: Nicolas Pitre --- kernel/pipe.c | 7 ++++--- kernel/poll.c | 11 +++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/kernel/pipe.c b/kernel/pipe.c index 09582260ab51..c5157ea6921e 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -186,11 +186,12 @@ int z_impl_k_pipe_write(struct k_pipe *pipe, const uint8_t *data, size_t len, k_ break; } } + } + #ifdef CONFIG_POLL - z_handle_obj_poll_events(&pipe->poll_events, - K_POLL_STATE_PIPE_DATA_AVAILABLE); + need_resched |= z_handle_obj_poll_events(&pipe->poll_events, + K_POLL_STATE_PIPE_DATA_AVAILABLE); #endif /* CONFIG_POLL */ - } written += ring_buf_put(&pipe->buf, &data[written], len - written); if (likely(written == len)) { diff --git a/kernel/poll.c b/kernel/poll.c index a1930a6292fc..b6e20b24c3a2 100644 --- a/kernel/poll.c +++ b/kernel/poll.c @@ -88,8 +88,15 @@ static inline bool is_condition_met(struct k_poll_event *event, uint32_t *state) } break; case K_POLL_TYPE_PIPE_DATA_AVAILABLE: - *state = K_POLL_STATE_PIPE_DATA_AVAILABLE; - return true; +#ifdef CONFIG_PIPES + if (event->pipe->bytes_used != 0) { +#else + if (!ring_buf_is_empty(&event->pipe->buf)) { +#endif + *state = K_POLL_STATE_PIPE_DATA_AVAILABLE; + return true; + } + break; case K_POLL_TYPE_IGNORE: break; default: