From 46617644e7118a173cd014cc19a22b7b844b9a92 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Fri, 10 Jan 2025 17:45:44 -0500 Subject: [PATCH] kernel/pipe: code flow cleanup Simplify the logic, avoid repeated conditionals, avoid superfluous scheduler calls, make the code more efficient and easier to read. Signed-off-by: Nicolas Pitre --- kernel/pipe.c | 138 ++++++++++++++++++++------------------------------ 1 file changed, 56 insertions(+), 82 deletions(-) diff --git a/kernel/pipe.c b/kernel/pipe.c index d707dc1d6d3..89f59f20216 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -34,8 +34,8 @@ static inline bool pipe_empty(struct k_pipe *pipe) return ring_buf_is_empty(&pipe->buf); } -static inline int wait_for(_wait_q_t *waitq, struct k_pipe *pipe, k_spinlock_key_t *key, - k_timepoint_t time_limit) +static int wait_for(_wait_q_t *waitq, struct k_pipe *pipe, k_spinlock_key_t *key, + k_timepoint_t time_limit) { k_timeout_t timeout = sys_timepoint_timeout(time_limit); @@ -48,9 +48,7 @@ static inline int wait_for(_wait_q_t *waitq, struct k_pipe *pipe, k_spinlock_key z_pend_curr(&pipe->lock, *key, waitq, timeout); *key = k_spin_lock(&pipe->lock); pipe->waiting--; - if (unlikely(pipe_closed(pipe))) { - return -EPIPE; - } else if (unlikely(pipe_resetting(pipe))) { + if (unlikely(pipe_resetting(pipe))) { if (pipe->waiting == 0) { pipe->flags &= ~PIPE_FLAG_RESET; } @@ -95,58 +93,47 @@ int z_impl_k_pipe_write(struct k_pipe *pipe, const uint8_t *data, size_t len, k_ { int rc; size_t written = 0; - k_spinlock_key_t key; k_timepoint_t end = sys_timepoint_calc(timeout); + k_spinlock_key_t key = k_spin_lock(&pipe->lock); SYS_PORT_TRACING_OBJ_FUNC_ENTER(k_pipe, write, pipe, data, len, timeout); - while (written < len) { - key = k_spin_lock(&pipe->lock); + + if (unlikely(pipe_resetting(pipe))) { + rc = -ECANCELED; + goto exit; + } + + for (;;) { if (unlikely(pipe_closed(pipe))) { - k_spin_unlock(&pipe->lock, key); rc = -EPIPE; - goto exit; - } else if (unlikely(pipe_resetting(pipe))) { - k_spin_unlock(&pipe->lock, key); - rc = -ECANCELED; - goto exit; - } else if (pipe_full(pipe)) { - rc = wait_for(&pipe->space, pipe, &key, end); - if (rc == -EAGAIN) { - /* the timeout expired */ - k_spin_unlock(&pipe->lock, key); - rc = written ? written : -EAGAIN; - goto exit; - } else if (unlikely(rc != 0)) { - /* The pipe was closed or reseted while waiting for space */ - k_spin_unlock(&pipe->lock, key); - goto exit; - } else if (unlikely(pipe_full(pipe))) { - /* Timeout has not elapsed, the pipe is open and not resetting, - * we've been notified of available space, but the notified space - * was consumed by another thread before the calling thread. - */ - k_spin_unlock(&pipe->lock, key); - continue; - } - /* The timeout has not elapsed, we've been notified of - * available space, and the pipe is not full. Continue writing. - */ + break; } - rc = ring_buf_put(&pipe->buf, &data[written], len - written); - if (likely(rc != 0)) { + if (pipe_empty(pipe)) { notify_waiter(&pipe->data); #ifdef CONFIG_POLL z_handle_obj_poll_events(&pipe->poll_events, - K_POLL_STATE_PIPE_DATA_AVAILABLE); + K_POLL_STATE_PIPE_DATA_AVAILABLE); #endif /* CONFIG_POLL */ } - k_spin_unlock(&pipe->lock, key); - written += rc; + + written += ring_buf_put(&pipe->buf, &data[written], len - written); + if (likely(written == len)) { + rc = written; + break; + } + + rc = wait_for(&pipe->space, pipe, &key, end); + if (rc != 0) { + if (rc == -EAGAIN) { + rc = written ? written : -EAGAIN; + } + break; + } } - rc = written; exit: SYS_PORT_TRACING_OBJ_FUNC_EXIT(k_pipe, write, pipe, rc); + k_spin_unlock(&pipe->lock, key); return rc; } @@ -154,56 +141,43 @@ int z_impl_k_pipe_read(struct k_pipe *pipe, uint8_t *data, size_t len, k_timeout { int rc; size_t read = 0; - k_spinlock_key_t key; k_timepoint_t end = sys_timepoint_calc(timeout); + k_spinlock_key_t key = k_spin_lock(&pipe->lock); SYS_PORT_TRACING_OBJ_FUNC_ENTER(k_pipe, read, pipe, data, len, timeout); - while (read < len) { - key = k_spin_lock(&pipe->lock); - if (unlikely(pipe_resetting(pipe))) { - k_spin_unlock(&pipe->lock, key); - rc = -ECANCELED; - goto exit; - } else if (pipe_empty(pipe) && !pipe_closed(pipe)) { - rc = wait_for(&pipe->data, pipe, &key, end); - if (rc == -EAGAIN) { - /* The timeout elapsed */ - k_spin_unlock(&pipe->lock, key); - rc = read ? read : -EAGAIN; - goto exit; - } else if (unlikely(rc == -ECANCELED)) { - /* The pipe is being rested. */ - k_spin_unlock(&pipe->lock, key); - goto exit; - } else if (unlikely(rc == 0 && pipe_empty(pipe))) { - /* Timeout has not elapsed, we've been notified of available bytes - * but they have been consumed by another thread before the calling - * thread. - */ - k_spin_unlock(&pipe->lock, key); - continue; - } - /* The timeout has not elapsed, we've been notified of - * available bytes, and the pipe is not empty. Continue reading. - */ - } - if (unlikely(pipe_closed(pipe) && pipe_empty(pipe))) { - k_spin_unlock(&pipe->lock, key); - rc = read ? read : -EPIPE; - goto exit; - } + if (unlikely(pipe_resetting(pipe))) { + rc = -ECANCELED; + goto exit; + } - rc = ring_buf_get(&pipe->buf, &data[read], len - read); - if (likely(rc != 0)) { + for (;;) { + if (pipe_full(pipe)) { notify_waiter(&pipe->space); } - read += rc; - k_spin_unlock(&pipe->lock, key); + + read += ring_buf_get(&pipe->buf, &data[read], len - read); + if (likely(read == len)) { + rc = read; + break; + } + + if (unlikely(pipe_closed(pipe))) { + rc = read ? read : -EPIPE; + break; + } + + rc = wait_for(&pipe->data, pipe, &key, end); + if (rc != 0) { + if (rc == -EAGAIN) { + rc = read ? read : -EAGAIN; + } + break; + } } - rc = read; exit: SYS_PORT_TRACING_OBJ_FUNC_EXIT(k_pipe, read, pipe, rc); + k_spin_unlock(&pipe->lock, key); return rc; }