From 701aab92e21bb7a05145c604025102f002e7e703 Mon Sep 17 00:00:00 2001 From: Peter Mitsis Date: Tue, 25 Feb 2025 16:02:46 -0800 Subject: [PATCH] kernel: Add Z_IS_TIMEOUT_RELATIVE() macro Introduces the Z_IS_TIMEOUT_RELATIVE() macro to help ensure that checking for relative/absolute timeouts is consistent. Using this macro also helps ensure that we get the correct behavior when using 32-bit timeouts (CONFIG_TIMEOUT_64BIT=n). Signed-off-by: Peter Mitsis --- drivers/timer/nrf_grtc_timer.c | 3 +-- drivers/timer/nrf_rtc_timer.c | 3 +-- include/zephyr/sys_clock.h | 7 +++++++ kernel/nothread.c | 2 +- kernel/sched.c | 24 +++++++++++------------- kernel/timeout.c | 13 ++++++------- kernel/timer.c | 9 +++++++-- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/drivers/timer/nrf_grtc_timer.c b/drivers/timer/nrf_grtc_timer.c index c75d9dc80a9..104040455ea 100644 --- a/drivers/timer/nrf_grtc_timer.c +++ b/drivers/timer/nrf_grtc_timer.c @@ -300,8 +300,7 @@ uint64_t z_nrf_grtc_timer_get_ticks(k_timeout_t t) grtc_ticks = t.ticks * CYC_PER_TICK; abs_ticks = Z_TICK_ABS(t.ticks); - if (abs_ticks < 0) { - /* relative timeout */ + if (Z_IS_TIMEOUT_RELATIVE(t)) { return (grtc_ticks > (int64_t)COUNTER_SPAN) ? -EINVAL : (curr_time + grtc_ticks); } diff --git a/drivers/timer/nrf_rtc_timer.c b/drivers/timer/nrf_rtc_timer.c index d45d9229d31..0fdf951f925 100644 --- a/drivers/timer/nrf_rtc_timer.c +++ b/drivers/timer/nrf_rtc_timer.c @@ -208,8 +208,7 @@ uint64_t z_nrf_rtc_timer_get_ticks(k_timeout_t t) } while (curr_time != z_nrf_rtc_timer_read()); abs_ticks = Z_TICK_ABS(t.ticks); - if (abs_ticks < 0) { - /* relative timeout */ + if (Z_IS_TIMEOUT_RELATIVE(t)) { return (t.ticks > COUNTER_SPAN) ? -EINVAL : (curr_time + t.ticks); } diff --git a/include/zephyr/sys_clock.h b/include/zephyr/sys_clock.h index 9a1884ba983..6c3589a5102 100644 --- a/include/zephyr/sys_clock.h +++ b/include/zephyr/sys_clock.h @@ -149,6 +149,13 @@ typedef struct { */ #define Z_TICK_ABS(t) (K_TICKS_FOREVER - 1 - (t)) +/* Test for relative timeout */ +#if CONFIG_TIMEOUT_64BIT +#define Z_IS_TIMEOUT_RELATIVE(timeout) (Z_TICK_ABS((timeout).ticks) < 0) +#else +#define Z_IS_TIMEOUT_RELATIVE(timeout) true +#endif + /* added tick needed to account for tick in progress */ #define _TICK_ALIGN 1 diff --git a/kernel/nothread.c b/kernel/nothread.c index 3e590e714b2..09bf4545961 100644 --- a/kernel/nothread.c +++ b/kernel/nothread.c @@ -36,7 +36,7 @@ int32_t z_impl_k_sleep(k_timeout_t timeout) } ticks = timeout.ticks; - if (Z_TICK_ABS(ticks) <= 0) { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { /* ticks is delta timeout */ ticks_to_wait = ticks; } else { diff --git a/kernel/sched.c b/kernel/sched.c index a612ccb03ff..d517a01787b 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -1067,27 +1067,26 @@ static inline void z_vrfy_k_yield(void) #include #endif /* CONFIG_USERSPACE */ -static int32_t z_tick_sleep(k_ticks_t ticks) +static int32_t z_tick_sleep(k_timeout_t timeout) { uint32_t expected_wakeup_ticks; __ASSERT(!arch_is_in_isr(), ""); - LOG_DBG("thread %p for %lu ticks", _current, (unsigned long)ticks); + LOG_DBG("thread %p for %lu ticks", _current, (unsigned long)timeout.ticks); - /* wait of 0 ms is treated as a 'yield' */ - if (ticks == 0) { + /* K_NO_WAIT is treated as a 'yield' */ + if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) { k_yield(); return 0; } - if (Z_TICK_ABS(ticks) <= 0) { - expected_wakeup_ticks = ticks + sys_clock_tick_get_32(); + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { + expected_wakeup_ticks = timeout.ticks + sys_clock_tick_get_32(); } else { - expected_wakeup_ticks = Z_TICK_ABS(ticks); + expected_wakeup_ticks = Z_TICK_ABS(timeout.ticks); } - k_timeout_t timeout = Z_TIMEOUT_TICKS(ticks); k_spinlock_key_t key = k_spin_lock(&_sched_spinlock); #if defined(CONFIG_TIMESLICING) && defined(CONFIG_SWAP_NONATOMIC) @@ -1103,7 +1102,8 @@ static int32_t z_tick_sleep(k_ticks_t ticks) uint32_t left_ticks = expected_wakeup_ticks - sys_clock_tick_get_32(); /* To handle a negative value correctly, once type-cast it to signed 32 bit */ - ticks = (k_ticks_t)(int32_t)left_ticks; + k_ticks_t ticks = (k_ticks_t)(int32_t)left_ticks; + if (ticks > 0) { return ticks; } @@ -1119,9 +1119,7 @@ int32_t z_impl_k_sleep(k_timeout_t timeout) SYS_PORT_TRACING_FUNC_ENTER(k_thread, sleep, timeout); - ticks = timeout.ticks; - - ticks = z_tick_sleep(ticks); + ticks = z_tick_sleep(timeout); /* k_sleep() still returns 32 bit milliseconds for compatibility */ int64_t ms = K_TIMEOUT_EQ(timeout, K_FOREVER) ? K_TICKS_FOREVER : @@ -1146,7 +1144,7 @@ int32_t z_impl_k_usleep(int32_t us) SYS_PORT_TRACING_FUNC_ENTER(k_thread, usleep, us); ticks = k_us_to_ticks_ceil64(us); - ticks = z_tick_sleep(ticks); + ticks = z_tick_sleep(Z_TIMEOUT_TICKS(ticks)); int32_t ret = k_ticks_to_us_ceil64(ticks); diff --git a/kernel/timeout.c b/kernel/timeout.c index 17f08c9d2e2..468077d3e99 100644 --- a/kernel/timeout.c +++ b/kernel/timeout.c @@ -117,13 +117,12 @@ void z_add_timeout(struct _timeout *to, _timeout_func_t fn, K_SPINLOCK(&timeout_lock) { struct _timeout *t; - if (IS_ENABLED(CONFIG_TIMEOUT_64BIT) && - (Z_TICK_ABS(timeout.ticks) >= 0)) { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { + to->dticks = timeout.ticks + 1 + elapsed(); + } else { k_ticks_t ticks = Z_TICK_ABS(timeout.ticks) - curr_tick; to->dticks = MAX(1, ticks); - } else { - to->dticks = timeout.ticks + 1 + elapsed(); } for (t = first(); t != NULL; t = next(t)) { @@ -310,10 +309,10 @@ k_timepoint_t sys_timepoint_calc(k_timeout_t timeout) } else { k_ticks_t dt = timeout.ticks; - if (IS_ENABLED(CONFIG_TIMEOUT_64BIT) && Z_TICK_ABS(dt) >= 0) { - timepoint.tick = Z_TICK_ABS(dt); - } else { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { timepoint.tick = sys_clock_tick_get() + MAX(1, dt); + } else { + timepoint.tick = Z_TICK_ABS(dt); } } diff --git a/kernel/timer.c b/kernel/timer.c index 466cab6c484..513d676bf0d 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -166,8 +166,13 @@ void z_impl_k_timer_start(struct k_timer *timer, k_timeout_t duration, * argument the same way k_sleep() does), but historical. The * timer_api test relies on this behavior. */ - if (Z_TICK_ABS(duration.ticks) < 0) { - duration.ticks = MAX(duration.ticks - 1, 0); + if (Z_IS_TIMEOUT_RELATIVE(duration)) { + /* For the duration == K_NO_WAIT case, ensure that behaviour + * is consistent for both 32-bit k_ticks_t which are unsigned + * and 64-bit k_ticks_t which are signed. + */ + duration.ticks = MAX(1, duration.ticks); + duration.ticks = duration.ticks - 1; } (void)z_abort_timeout(&timer->timeout);