From 1a578eb72c9dffe74aa3bc47b48400525096218e Mon Sep 17 00:00:00 2001 From: Yong Cong Sin Date: Tue, 24 Dec 2024 17:41:31 +0800 Subject: [PATCH] lib: os: mpsc_pbuf: do not wait when spinlock is held Check if the spinlock is held before attempting to wait by taking the semaphore, as that would cause a context switch which isn't allowed and will trigger an assertion error when `CONFIG_SPIN_VALIDATE` is enabled. Logging in spinlock-held context when the log buffer is full can lead to an infinite assertion error loop, as the logging subsys attempts to allocate buffer when there's none available, it will try to wait for one and thus triggers the assertion error, the error message will be printed through the logging sybsys but there's no buffer available, so it will try to wait for one and triggers another assertion error.. This loop just goes on and on forever, and nothing gets printed to the terminal. Added a test to validate the fix. Signed-off-by: Yong Cong Sin Signed-off-by: Yong Cong Sin Signed-off-by: Maxim Adelman --- lib/os/mpsc_pbuf.c | 2 +- tests/lib/mpsc_pbuf/src/main.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/os/mpsc_pbuf.c b/lib/os/mpsc_pbuf.c index eac184f5aa50c1..845c2456cfbd39 100644 --- a/lib/os/mpsc_pbuf.c +++ b/lib/os/mpsc_pbuf.c @@ -373,7 +373,7 @@ union mpsc_pbuf_generic *mpsc_pbuf_alloc(struct mpsc_pbuf_buffer *buffer, add_skip_item(buffer, free_wlen); cont = true; } else if (IS_ENABLED(CONFIG_MULTITHREADING) && !K_TIMEOUT_EQ(timeout, K_NO_WAIT) && - !k_is_in_isr()) { + !k_is_in_isr() && arch_irq_unlocked(key.key)) { int err; k_spin_unlock(&buffer->lock, key); diff --git a/tests/lib/mpsc_pbuf/src/main.c b/tests/lib/mpsc_pbuf/src/main.c index 201eced47f9e76..923050b1b6d42e 100644 --- a/tests/lib/mpsc_pbuf/src/main.c +++ b/tests/lib/mpsc_pbuf/src/main.c @@ -1294,5 +1294,28 @@ ZTEST(log_buffer, test_utilization) zassert_true(packet == NULL); } +/* Make sure that `mpsc_pbuf_alloc()` works in spinlock-held context when buf is not available */ +ZTEST(log_buffer, test_alloc_in_spinlock) +{ + struct mpsc_pbuf_buffer buffer; + struct test_data_var *packet; + struct k_spinlock l = {0}; + + init(&buffer, 32, false); + + /* Allocate all available buffer */ + packet = (struct test_data_var *)mpsc_pbuf_alloc( + &buffer, 32, K_MSEC(10)); + zassert_not_null(packet); + + K_SPINLOCK(&l) { + /* Try to allocate another buf */ + packet = (struct test_data_var *)mpsc_pbuf_alloc( + &buffer, 32, K_MSEC(10)); + /* No buf is available this time */ + zassert_is_null(packet); + } +} + /*test case main entry*/ ZTEST_SUITE(log_buffer, NULL, NULL, NULL, NULL, NULL);