From 1f2ad4d239e80ceb5e8e14d7b815114aff66d863 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Fri, 26 Jul 2019 09:47:35 -0700 Subject: [PATCH 1/3] tests/kernel/{fifo,lifo}: Correct tick slop These tests had what appeared to be a hard-coded one tick slop added to the timeout for a fifo to handle the addition of a full tick (vs. proper rounding) in k_sleep() (which is a separate bug). But the timeout side has been tightened with the recent change and doesn't wait as long as an equivalent sleep anymore (which is a feature), so the sides don't match. And it's wrong anyway, as there are two sleeps being accounted for (the fifo timeout and the sleep in the other thread doing the put), both of which can misalign. Do this with the proper tick conversion API, and use two ticks, not one. Signed-off-by: Andy Ross --- tests/kernel/fifo/fifo_timeout/src/main.c | 4 ++-- tests/kernel/lifo/lifo_usage/src/main.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/kernel/fifo/fifo_timeout/src/main.c b/tests/kernel/fifo/fifo_timeout/src/main.c index d8ec1e1318cb6..aa56d2c220291 100644 --- a/tests/kernel/fifo/fifo_timeout/src/main.c +++ b/tests/kernel/fifo/fifo_timeout/src/main.c @@ -367,7 +367,8 @@ static void test_timeout_fifo_thread(void) &timeout, NULL, FIFO_THREAD_PRIO, K_INHERIT_PERMS, 0); - packet = k_fifo_get(&fifo_timeout[0], timeout + 10); + packet = k_fifo_get(&fifo_timeout[0], timeout + + k_ticks_to_ms_ceil32(2)); zassert_true(packet != NULL, NULL); zassert_true(is_timeout_in_range(start_time, timeout), NULL); put_scratch_packet(packet); @@ -382,7 +383,6 @@ static void test_timeout_fifo_thread(void) test_thread_timeout_reply_values, &reply_packet, NULL, NULL, FIFO_THREAD_PRIO, K_INHERIT_PERMS, 0); - k_yield(); packet = k_fifo_get(&timeout_order_fifo, K_NO_WAIT); zassert_true(packet != NULL, NULL); diff --git a/tests/kernel/lifo/lifo_usage/src/main.c b/tests/kernel/lifo/lifo_usage/src/main.c index ac685e3be70c2..430fbd58a6b6e 100644 --- a/tests/kernel/lifo/lifo_usage/src/main.c +++ b/tests/kernel/lifo/lifo_usage/src/main.c @@ -324,7 +324,8 @@ static void test_timeout_lifo_thread(void) &timeout, NULL, LIFO_THREAD_PRIO, K_INHERIT_PERMS, 0); - packet = k_lifo_get(&lifo_timeout[0], timeout + 10); + packet = k_lifo_get(&lifo_timeout[0], timeout + + k_ticks_to_ms_ceil32(2)); zassert_true(packet != NULL, NULL); zassert_true(is_timeout_in_range(start_time, timeout), NULL); put_scratch_packet(packet); From c602f4b87caf492919caa590c1dc56361f4bd1d5 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Fri, 26 Jul 2019 10:19:50 -0700 Subject: [PATCH 2/3] kernel/queue: Fix incorrect spinning when in poll mode In the "POLL" implementation of k_queue_get(), if it ever happened that the elapsed time exactly equalled the time we were supposed to wait, the k_poll() call would be called (correctly) with a timeout of zero, and return synchronously. But the "done" flag would not be set because of an off by one error, so we would have to spin for the next tick needlessly. Even worse: on native_posix, nothing in this loop returns control to the main loop so in fact there we spin forever! (Which is how this got discovered.) Signed-off-by: Andy Ross --- kernel/queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/queue.c b/kernel/queue.c index 0b384d96b0d18..1b0cc034025cb 100644 --- a/kernel/queue.c +++ b/kernel/queue.c @@ -307,7 +307,7 @@ static void *k_queue_poll(struct k_queue *queue, s32_t timeout) if ((val == NULL) && (timeout != K_FOREVER)) { elapsed = k_uptime_get_32() - start; - done = elapsed > timeout; + done = elapsed >= timeout; } } while (!val && !done); From 2ab98557b7256eeb21f329ea8da06932088666d6 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Sun, 28 Jul 2019 06:34:02 -0700 Subject: [PATCH 3/3] tests/kernel/interrupt: Correct timing usage This tests hard codes a 100 Hz tick rate, but it sets a k_timer for a DURATION value of 5ms, which cannot be done with a precision, then checks it against an unrelated k_busy_wait() time of exactly 10ms, which is right at the boundary of what is possible. Playing with timer stuff can push this over the edge easily. Correct DURATION to reflect "one tick", and spin for that value, plus exactly one tick of slop before testing to see whether the timer ISR ran. Signed-off-by: Andy Ross --- tests/kernel/interrupt/src/nested_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/kernel/interrupt/src/nested_irq.c b/tests/kernel/interrupt/src/nested_irq.c index 00af536293b04..3b0a9340e372b 100644 --- a/tests/kernel/interrupt/src/nested_irq.c +++ b/tests/kernel/interrupt/src/nested_irq.c @@ -5,7 +5,7 @@ */ #include "interrupt.h" -#define DURATION 5 +#define DURATION 10 struct k_timer timer; /* This tests uses two IRQ lines, selected within the range of IRQ lines @@ -85,7 +85,7 @@ void isr0(void *param) */ k_busy_wait(MS_TO_US(1000)); #else - k_busy_wait(MS_TO_US(10)); + k_busy_wait(DURATION * 1000 + (int)k_ticks_to_us_ceil64(1)); #endif printk("%s execution completed !!\n", __func__); zassert_equal(new_val, old_val, "Nested interrupt is not working\n");