From 6a51a10decf92a2a0d585d485aada065877625c8 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Mon, 13 Feb 2023 00:43:47 -0500 Subject: [PATCH] kernel/timer: fix period argument clamp handling Commit 3e729b2b1c52 ("kernel/timer: Correctly clamp period argument") increased the lower limit to 1 so that it wouldn't conflict with a K_NO_WAIT. But in doing so it enforced a minimum period of 2 ticks. And the subtraction must obviously be avoided if the period is zero, etc. Instead of doing this masquerade in k_timer_start(), let's move the subtraction and clamping in z_timer_expiration_handler() right before registering a new timeout. It makes the code cleaner, and then it is possible to have single-tick periods again. Whith this, timer_jitter_drift in tests/kernel/timer/timer_behavior does pass with any CONFIG_SYS_CLOCK_TICKS_PER_SEC value, even when the tick period is equal or larger than the specified timer period for the test which failed the test before. Signed-off-by: Nicolas Pitre --- kernel/timer.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/kernel/timer.c b/kernel/timer.c index 325224d11b..9de9768c36 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -52,6 +52,9 @@ void z_timer_expiration_handler(struct _timeout *t) !K_TIMEOUT_EQ(timer->period, K_FOREVER)) { k_timeout_t next = timer->period; + /* see note about z_add_timeout() in z_impl_k_timer_start() */ + next.ticks = MAX(next.ticks - 1, 0); + #ifdef CONFIG_TIMEOUT_64BIT /* Exploit the fact that uptime during a kernel * timeout handler reflects the time of the scheduled @@ -64,8 +67,7 @@ void z_timer_expiration_handler(struct _timeout *t) * beginning of a tick, so need to defeat the "round * down" behavior on timeout addition). */ - next = K_TIMEOUT_ABS_TICKS(k_uptime_ticks() + 1 - + timer->period.ticks); + next = K_TIMEOUT_ABS_TICKS(k_uptime_ticks() + 1 + next.ticks); #endif z_add_timeout(&timer->timeout, z_timer_expiration_handler, next); @@ -139,8 +141,8 @@ void z_impl_k_timer_start(struct k_timer *timer, k_timeout_t duration, * to round up to the next tick (by convention it waits for * "at least as long as the specified timeout"), but the * period interval is always guaranteed to be reset from - * within the timer ISR, so no round up is desired. Subtract - * one. + * within the timer ISR, so no round up is desired and 1 is + * subtracted in there. * * Note that the duration (!) value gets the same treatment * for backwards compatibility. This is unfortunate @@ -148,10 +150,6 @@ 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 (!K_TIMEOUT_EQ(period, K_FOREVER) && period.ticks != 0 && - Z_TICK_ABS(period.ticks) < 0) { - period.ticks = MAX(period.ticks - 1, 1); - } if (Z_TICK_ABS(duration.ticks) < 0) { duration.ticks = MAX(duration.ticks - 1, 0); }