From 135746903711724c932d104c90a26352dd795b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Ansgariusson?= Date: Fri, 20 Dec 2024 15:46:36 +0100 Subject: [PATCH] kernel: Rewrite k_pipe_* API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `k_pipe_*` API has been reworked to provide a more consistent and intuitive interface. The new API aims to provide a simple to use byte stream interface that is more in line with the POSIX pipe API. The previous API has been deprecated and will be removed in a future release. Signed-off-by: Måns Ansgariusson --- include/zephyr/kernel.h | 131 +++++++++++++++++++++++++++- include/zephyr/sys/ring_buffer.h | 19 ++-- kernel/CMakeLists.txt | 4 + kernel/pipe.c | 145 +++++++++++++++++++++++++++++++ kernel/pipes.c | 19 ++-- 5 files changed, 301 insertions(+), 17 deletions(-) create mode 100644 kernel/pipe.c diff --git a/include/zephyr/kernel.h b/include/zephyr/kernel.h index 24f88da2885db9..f9aa6ba89a57f0 100644 --- a/include/zephyr/kernel.h +++ b/include/zephyr/kernel.h @@ -22,6 +22,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -4971,6 +4972,7 @@ void k_mbox_data_get(struct k_mbox_msg *rx_msg, void *buffer); * @{ */ +#ifdef CONFIG_PIPES /** Pipe Structure */ struct k_pipe { unsigned char *buffer; /**< Pipe buffer: may be NULL */ @@ -5041,6 +5043,7 @@ struct k_pipe { Z_PIPE_INITIALIZER(name, _k_pipe_buf_##name, pipe_buffer_size) /** + * @deprecated * @brief Initialize a pipe. * * This routine initializes a pipe object, prior to its first use. @@ -5054,6 +5057,7 @@ struct k_pipe { void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size); /** + * @deprecated * @brief Release a pipe's allocated buffer * * If a pipe object was given a dynamically allocated buffer via @@ -5067,6 +5071,7 @@ void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size); int k_pipe_cleanup(struct k_pipe *pipe); /** + * @deprecated * @brief Initialize a pipe and allocate a buffer for it * * Storage for the buffer region will be allocated from the calling thread's @@ -5084,6 +5089,7 @@ int k_pipe_cleanup(struct k_pipe *pipe); __syscall int k_pipe_alloc_init(struct k_pipe *pipe, size_t size); /** + * @deprecated * @brief Write data to a pipe. * * This routine writes up to @a bytes_to_write bytes of data to @a pipe. @@ -5106,6 +5112,7 @@ __syscall int k_pipe_put(struct k_pipe *pipe, const void *data, size_t min_xfer, k_timeout_t timeout); /** + * @deprecated * @brief Read data from a pipe. * * This routine reads up to @a bytes_to_read bytes of data from @a pipe. @@ -5129,6 +5136,7 @@ __syscall int k_pipe_get(struct k_pipe *pipe, void *data, size_t min_xfer, k_timeout_t timeout); /** + * @deprecated * @brief Query the number of bytes that may be read from @a pipe. * * @param pipe Address of the pipe. @@ -5139,6 +5147,7 @@ __syscall int k_pipe_get(struct k_pipe *pipe, void *data, __syscall size_t k_pipe_read_avail(struct k_pipe *pipe); /** + * @deprecated * @brief Query the number of bytes that may be written to @a pipe * * @param pipe Address of the pipe. @@ -5149,6 +5158,7 @@ __syscall size_t k_pipe_read_avail(struct k_pipe *pipe); __syscall size_t k_pipe_write_avail(struct k_pipe *pipe); /** + * @deprecated * @brief Flush the pipe of write data * * This routine flushes the pipe. Flushing the pipe is equivalent to reading @@ -5161,6 +5171,7 @@ __syscall size_t k_pipe_write_avail(struct k_pipe *pipe); __syscall void k_pipe_flush(struct k_pipe *pipe); /** + * @deprecated * @brief Flush the pipe's internal buffer * * This routine flushes the pipe's internal buffer. This is equivalent to @@ -5175,10 +5186,128 @@ __syscall void k_pipe_buffer_flush(struct k_pipe *pipe); /** @} */ +#else /* CONFIG_PIPES */ + +enum pipe_flags { + PIPE_FLAG_OPEN = BIT(0), + PIPE_FLAG_RESET = BIT(1), + PIPE_FLAG_FULL = BIT(2), +}; + +struct k_pipe { + size_t waiting; + struct ring_buf buf; + struct k_spinlock lock; + _wait_q_t data; + _wait_q_t space; + uint8_t flags; +}; + +#define Z_PIPE_INITIALIZER(obj, pipe_buffer, pipe_buffer_size) \ +{ \ + .flags = 0, \ + .waiting = 0, \ + .buf = RING_BUF_INIT(pipe_buffer, pipe_buffer_size), \ + .lock = {}, \ + .data = Z_WAIT_Q_INIT(&obj.data), \ + .space = Z_WAIT_Q_INIT(&obj.space), \ +} + /** - * @cond INTERNAL_HIDDEN + * INTERNAL_HIDDEN @endcond + */ + +/** + * @brief Statically define and initialize a pipe. + * + * The pipe can be accessed outside the module where it is defined using: + * + * @code extern struct k_pipe ; @endcode + * + * @param name Name of the pipe. + * @param pipe_buffer_size Size of the pipe's ring buffer (in bytes). + * @param pipe_align Alignment of the pipe's ring buffer (power of 2). + * */ +#define K_PIPE_DEFINE(name, pipe_buffer_size, pipe_align) \ + static unsigned char __noinit __aligned(pipe_align) \ + _k_pipe_buf_##name[pipe_buffer_size]; \ + STRUCT_SECTION_ITERABLE(k_pipe, name) = \ + Z_PIPE_INITIALIZER(name, _k_pipe_buf_##name, pipe_buffer_size) +/** + * @brief initialize a pipe + * + * This routine initializes a pipe object, prior to its first use. + * + * @param pipe Address of the pipe. + * @param buffer Address of the pipe's buffer. + * @param buffer_size Size of the pipe's buffer. + */ +__syscall void k_pipe_init(struct k_pipe *pipe, uint8_t *buffer, size_t buffer_size); + +/** + * @brief Write data to a pipe + * + * This routine writes up to @a len bytes of data to @a pipe. + * If the pipe is full, the routine will block until the data can be written or the timeout expires. + * + * @param pipe Address of the pipe. + * @param data Address of data to write. + * @param len Size of data (in bytes). + * @param timeout Waiting period to wait for the data to be written. + * + * @retval 0 on success + * @retval -EAGAIN if the data could not be written before the timeout expired + * @retval -ECANCELED if the write was interrupted by eg. k_pipe_flush(..) + * @retval -EPIPE if the pipe was closed + */ +__syscall int k_pipe_write(struct k_pipe *pipe, const uint8_t *data, size_t len, + k_timeout_t timeout); + +/** + * @brief Read data from a pipe + * This routine reads up to @a len bytes of data from @a pipe. + * If the pipe is empty, the routine will block until the data can be read or the timeout expires. + * + * @param pipe Address of the pipe. + * @param data Address to place the data read from pipe. + * @param len Requested number of bytes to read. + * @param timeout Waiting period to wait for the data to be read. + * + * @retval number of bytes read on success + * @retval -EAGAIN if the data could not be read before the timeout expired + * @retval -ECANCELED if the read was interrupted by eg. k_pipe_flush(..) + * @retval -EPIPE if the pipe was closed + */ +__syscall int k_pipe_read(struct k_pipe *pipe, uint8_t *data, size_t len, + k_timeout_t timeout); + +/** + * @brief Reset the pipe + * + * This routine resets the internal state of the pipe and any + * writers or readers that were previously pended become unpended. + * + * @param pipe Address of the pipe. + */ +__syscall int k_pipe_reset(struct k_pipe *pipe); + +/** + * @brief Close a pipe + * + * This routine closes a pipe. Any threads that were blocked on the pipe + * will be unblocked and receive an error code. + * + * @param pipe Address of the pipe. + * @retval 0 on success + */ +__syscall int k_pipe_close(struct k_pipe *pipe); +#endif /* CONFIG_PIPES */ + +/** + * @cond INTERNAL_HIDDEN + */ struct k_mem_slab_info { uint32_t num_blocks; size_t block_size; diff --git a/include/zephyr/sys/ring_buffer.h b/include/zephyr/sys/ring_buffer.h index 6bec33e9e3a904..a7ca154bb5bb18 100644 --- a/include/zephyr/sys/ring_buffer.h +++ b/include/zephyr/sys/ring_buffer.h @@ -6,8 +6,6 @@ #ifndef ZEPHYR_INCLUDE_SYS_RING_BUFFER_H_ #define ZEPHYR_INCLUDE_SYS_RING_BUFFER_H_ - -#include #include #include @@ -61,7 +59,17 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, int32_t value) buf->put_head = buf->put_tail = buf->put_base = value; buf->get_head = buf->get_tail = buf->get_base = value; } - +#define RING_BUF_INIT(buf, size8) \ + { \ + .buffer = buf, \ + .size = size8, \ + .put_head = 0, \ + .put_tail = 0, \ + .put_base = 0, \ + .get_head = 0, \ + .get_tail = 0, \ + .get_base = 0 \ + } /** * @brief Define and initialize a ring buffer for byte data. * @@ -80,10 +88,7 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, int32_t value) BUILD_ASSERT(size8 < RING_BUFFER_MAX_SIZE,\ RING_BUFFER_SIZE_ASSERT_MSG); \ static uint8_t __noinit _ring_buffer_data_##name[size8]; \ - struct ring_buf name = { \ - .buffer = _ring_buffer_data_##name, \ - .size = size8 \ - } + struct ring_buf name = RING_BUF_INIT(_ring_buffer_data_##name, size8) /** * @brief Define and initialize an "item based" ring buffer. diff --git a/kernel/CMakeLists.txt b/kernel/CMakeLists.txt index e5db39713b6699..8ba95f6c5708cf 100644 --- a/kernel/CMakeLists.txt +++ b/kernel/CMakeLists.txt @@ -84,6 +84,10 @@ list(APPEND kernel_files thread.c sched.c ) +# FIXME: Once the prior pipe implementation is removed, this should be included in the above list +if(NOT CONFIG_PIPES) +list(APPEND kernel_files pipe.c) +endif() # NOT CONFIG_PIPES if(CONFIG_SMP) list(APPEND kernel_files smp.c diff --git a/kernel/pipe.c b/kernel/pipe.c new file mode 100644 index 00000000000000..72e1b4737302ce --- /dev/null +++ b/kernel/pipe.c @@ -0,0 +1,145 @@ +#include +#include +#include +#include +#include + +typedef bool (*wait_cond_t)(struct k_pipe *pipe); +static bool pipe_full(struct k_pipe *pipe) +{ + return ring_buf_space_get(&pipe->buf) == 0; +} + +static bool pipe_empty(struct k_pipe *pipe) +{ + return ring_buf_capacity_get(&pipe->buf) - ring_buf_space_get(&pipe->buf) == 0; +} + +static int wait_for(_wait_q_t *waitq, struct k_pipe *pipe, k_spinlock_key_t *key, + wait_cond_t cond, k_timeout_t timeout) +{ + if (K_TIMEOUT_EQ(timeout, K_NO_WAIT) || pipe->flags & PIPE_FLAG_RESET) { + k_spin_unlock(&pipe->lock, *key); + return -EAGAIN; + } + pipe->waiting++; + z_pend_curr(&pipe->lock, *key, waitq, timeout); + *key = k_spin_lock(&pipe->lock); + pipe->waiting--; + if (unlikely(!(pipe->flags & PIPE_FLAG_OPEN))) { + return -EPIPE; + } else if (unlikely(pipe->flags & PIPE_FLAG_RESET)) { + if (!pipe->waiting) { + pipe->flags &= ~PIPE_FLAG_RESET; + } + return -ECANCELED; + } else if (!cond(pipe)) { + return 0; + } + return -EAGAIN; +} + +static void notify_waiter(_wait_q_t *waitq) +{ + struct k_thread *thread_to_unblock = z_unpend_first_thread(waitq); + if (likely(thread_to_unblock)) { + z_ready_thread(thread_to_unblock); + } +} + +void z_impl_k_pipe_init(struct k_pipe *pipe, uint8_t *buffer, size_t buffer_size) +{ + ring_buf_init(&pipe->buf, buffer_size, buffer); + pipe->flags = PIPE_FLAG_OPEN; + + pipe->lock = (struct k_spinlock){0}; + z_waitq_init(&pipe->data); + z_waitq_init(&pipe->space); +} + +int z_impl_k_pipe_write(struct k_pipe *pipe, const uint8_t *data, size_t len, k_timeout_t timeout) +{ + int rc; + + __ASSERT_NO_MSG(pipe != NULL); + __ASSERT_NO_MSG(data != NULL); + + k_spinlock_key_t key = k_spin_lock(&pipe->lock); + if (unlikely(pipe_full(pipe))) { + rc = wait_for(&pipe->space, pipe, &key, pipe_full, timeout); + if (rc) { + k_spin_unlock(&pipe->lock, key); + return rc; + } + } + + if (!(pipe->flags & PIPE_FLAG_OPEN)) { + k_spin_unlock(&pipe->lock, key); + return -EPIPE; + } + + rc = ring_buf_put(&pipe->buf, data, len); + if (rc) { + notify_waiter(&pipe->data); + } + k_spin_unlock(&pipe->lock, key); + return rc; +} + +int z_impl_k_pipe_read(struct k_pipe *pipe, uint8_t *data, size_t len, k_timeout_t timeout) +{ + int rc; + + __ASSERT_NO_MSG(pipe != NULL); + __ASSERT_NO_MSG(data != NULL); + + k_spinlock_key_t key = k_spin_lock(&pipe->lock); + if (pipe_empty(pipe) && pipe->flags & PIPE_FLAG_OPEN) { + rc = wait_for(&pipe->data, pipe, &key, pipe_empty, timeout); + if (rc && rc != -EPIPE) { + k_spin_unlock(&pipe->lock, key); + return rc; + } + } + if (pipe_empty(pipe) && !(pipe->flags & PIPE_FLAG_OPEN)) { + k_spin_unlock(&pipe->lock, key); + return -EPIPE; + } + + rc = ring_buf_get(&pipe->buf, data, len); + if (rc) { + notify_waiter(&pipe->space); + } + k_spin_unlock(&pipe->lock, key); + return rc; +} + +int z_impl_k_pipe_reset(struct k_pipe *pipe) +{ + __ASSERT_NO_MSG(pipe != NULL); + K_SPINLOCK(&pipe->lock) { + ring_buf_reset(&pipe->buf); + pipe->flags |= PIPE_FLAG_RESET; + z_unpend_all(&pipe->data); + z_unpend_all(&pipe->space); + } + + return 0; +} + +int z_impl_k_pipe_close(struct k_pipe *pipe) +{ + int rc = 0; + + __ASSERT_NO_MSG(pipe != NULL); + K_SPINLOCK(&pipe->lock) { + if (!(pipe->flags & PIPE_FLAG_OPEN)) { + rc = -EALREADY; + K_SPINLOCK_BREAK; + } + pipe->flags = 0; + z_unpend_all(&pipe->data); + z_unpend_all(&pipe->space); + } + return rc; +} diff --git a/kernel/pipes.c b/kernel/pipes.c index a9eef5a4f368cb..97adc5556b8962 100644 --- a/kernel/pipes.c +++ b/kernel/pipes.c @@ -21,6 +21,7 @@ #include #include +#warning "This k_pipe API is deprecated and will be removed in future releases. Please update your configuration." struct waitq_walk_data { sys_dlist_t *list; size_t bytes_requested; @@ -36,7 +37,7 @@ static struct k_obj_type obj_type_pipe; #endif /* CONFIG_OBJ_CORE_PIPE */ -void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size) +__deprecated void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size) { pipe->buffer = buffer; pipe->size = size; @@ -60,7 +61,7 @@ void k_pipe_init(struct k_pipe *pipe, unsigned char *buffer, size_t size) #endif /* CONFIG_OBJ_CORE_PIPE */ } -int z_impl_k_pipe_alloc_init(struct k_pipe *pipe, size_t size) +__deprecated int z_impl_k_pipe_alloc_init(struct k_pipe *pipe, size_t size) { void *buffer; int ret; @@ -148,7 +149,7 @@ void z_impl_k_pipe_buffer_flush(struct k_pipe *pipe) } #ifdef CONFIG_USERSPACE -void z_vrfy_k_pipe_buffer_flush(struct k_pipe *pipe) +__deprecated void z_vrfy_k_pipe_buffer_flush(struct k_pipe *pipe) { K_OOPS(K_SYSCALL_OBJ(pipe, K_OBJ_PIPE)); @@ -156,7 +157,7 @@ void z_vrfy_k_pipe_buffer_flush(struct k_pipe *pipe) } #endif /* CONFIG_USERSPACE */ -int k_pipe_cleanup(struct k_pipe *pipe) +__deprecated int k_pipe_cleanup(struct k_pipe *pipe) { SYS_PORT_TRACING_OBJ_FUNC_ENTER(k_pipe, cleanup, pipe); @@ -377,7 +378,7 @@ static size_t pipe_write(struct k_pipe *pipe, sys_dlist_t *src_list, return num_bytes_written; } -int z_impl_k_pipe_put(struct k_pipe *pipe, const void *data, +__deprecated int z_impl_k_pipe_put(struct k_pipe *pipe, const void *data, size_t bytes_to_write, size_t *bytes_written, size_t min_xfer, k_timeout_t timeout) { @@ -513,7 +514,7 @@ int z_impl_k_pipe_put(struct k_pipe *pipe, const void *data, } #ifdef CONFIG_USERSPACE -int z_vrfy_k_pipe_put(struct k_pipe *pipe, const void *data, +__deprecated int z_vrfy_k_pipe_put(struct k_pipe *pipe, const void *data, size_t bytes_to_write, size_t *bytes_written, size_t min_xfer, k_timeout_t timeout) { @@ -696,7 +697,7 @@ static int pipe_get_internal(k_spinlock_key_t key, struct k_pipe *pipe, return ret; } -int z_impl_k_pipe_get(struct k_pipe *pipe, void *data, size_t bytes_to_read, +__deprecated int z_impl_k_pipe_get(struct k_pipe *pipe, void *data, size_t bytes_to_read, size_t *bytes_read, size_t min_xfer, k_timeout_t timeout) { __ASSERT(((arch_is_in_isr() == false) || @@ -736,7 +737,7 @@ int z_vrfy_k_pipe_get(struct k_pipe *pipe, void *data, size_t bytes_to_read, #include #endif /* CONFIG_USERSPACE */ -size_t z_impl_k_pipe_read_avail(struct k_pipe *pipe) +__deprecated size_t z_impl_k_pipe_read_avail(struct k_pipe *pipe) { size_t res; k_spinlock_key_t key; @@ -773,7 +774,7 @@ size_t z_vrfy_k_pipe_read_avail(struct k_pipe *pipe) #include #endif /* CONFIG_USERSPACE */ -size_t z_impl_k_pipe_write_avail(struct k_pipe *pipe) +__deprecated size_t z_impl_k_pipe_write_avail(struct k_pipe *pipe) { size_t res; k_spinlock_key_t key;