lib: os: timespec_util: reimplement and test with broad inputs

Originally, the timespec_to_timeout() and timespec_from_timeout() tests
assumed that tv_sec and tv_nsec values divided evenly with the number of
ticks per second (CONFIG_SYS_CLOCK_TICKS_PER_SEC). However, those
assumptions broke down when testing with 32768 ticks/s .

Upon further investigation, there were additional corner cases
discovered that were not handled in an ideal or safe way.

Part of this fix involved identifying several "domains" and special
values that have semantic meaning that must be handled particularly
carefully.

Additional hidden-API constants were made to simplify the solution.

K_TICK_MIN (1)
K_TICK_MAX (UINT32_MAX-1 in 32-bit, INT64_MAX in 64-bit)

K_TS_NO_WAIT (like K_NO_WAIT)
K_TS_FOREVER (like K_FOREVER)

1. Converting a negative k_timeout_t

These only exist in 64-bit representation and actually encode absolute
timepoints via ticks. Since the stated purpose of the conversion
functions is to convert between durations, we must reject absolute
time points in timespec_from_timeout().

2. Converting a negative timespec

We assume that this duration means a timeout has already expired, and
round these up to K_NO_WAIT in timespec_to_timeout().

3. Due to the larger numeric space in the timespec representation,
the reverse mapping of timespec to k_timeout_t is "fuzzy". However,
K_NO_WAIT (K_TS_NO_WAIT) and K_FOREVER (K_TS_FOREVER) must remain
semantically equivalent. The previous implementation also held this
to be true, but the test cases are a bit clearer about it now.

4. Also, due to the larger numeric space in timespec representation,
there was a special requirement to round up to the nearest tick
boundary for any timespec in the strictly exclusive range
(K_TS_NO_WAIT, K_TS_MAX). We must round up, since
a) rounding down to K_NO_WAIT is most certainly not representative
   of a non-zero finite duration delay
b) the kernel operates on tick boundaries

However, since it's possible that rounding up might not _always_ be
the right thing to do, in order to allow the application to make
more informed decisions, we created a modified
timespec_to_timeout_rem() that also returns the remainder (or
difference) between the requested time to convert and resulting
k_timeout_t. The difference is expressed as a timespec object.

5. Above the K_TS_MAX boundary, which is the highest possible
timespec that can be represented by a tick, and specifically in the
64-bit timeout representation, there is a domain that cannot be
rounded up to K_TS_FOREVER and should always be rounded down to
K_TS_MAX.

This is to ensure that finite durations remain finite (even if
they are beyond the heat death of the universe).

Signed-off-by: Chris Friedt <cfriedt@tenstorrent.com>

Author: cfriedt

include/zephyr/sys/clock.h

@@ -166,6 +166,44 @@ typedef struct {
 /* added tick needed to account for tick in progress */
 #define _TICK_ALIGN 1
 
+/* Converts ticks to nanoseconds with minimal rounding error */
+#define K_TICKS_TO_NSECS(ticks)                                                                    \
+	MULDIV((uint64_t)(ticks), NSEC_PER_SEC, CONFIG_SYS_CLOCK_TICKS_PER_SEC)
+
+/* The minimum duration in ticks strictly greater than that of K_NO_WAIT */
+#define K_TICK_MIN ((k_ticks_t)1)
+
+/* The maximum duration in ticks strictly and semantically "less than" K_FOREVER */
+#define K_TICK_MAX ((k_ticks_t)(IS_ENABLED(CONFIG_TIMEOUT_64BIT) ? INT64_MAX : UINT32_MAX - 1))
+
+/* The semantic equivalent of K_NO_WAIT but expressed as a timespec object*/
+#define K_TS_NO_WAIT                                                                               \
+	((struct timespec){                                                                        \
+		.tv_sec = 0,                                                                       \
+		.tv_nsec = 0,                                                                      \
+	})
+
+/* The semantic equivalent of K_FOREVER but expressed as a timespec object*/
+#define K_TS_FOREVER                                                                               \
+	((struct timespec){                                                                        \
+		.tv_sec = (time_t)INT64_MAX,                                                       \
+		.tv_nsec = (long)(NSEC_PER_SEC - 1),                                               \
+	})
+
+/* The semantic equivalent of K_TICK_MIN but expressed as a timespec object */
+#define K_TS_MIN                                                                                   \
+	((struct timespec){                                                                        \
+		.tv_sec = 0,                                                                       \
+		.tv_nsec = (long)K_TICKS_TO_NSECS(K_TICK_MIN),                                     \
+	})
+
+/* The semantic equivalent of K_TICK_MAX but expressed as a timespec object */
+#define K_TS_MAX                                                                                   \
+	((struct timespec){                                                                        \
+		.tv_sec = (time_t)((uint64_t)K_TICK_MAX / CONFIG_SYS_CLOCK_TICKS_PER_SEC),         \
+		.tv_nsec = (long)(K_TICKS_TO_NSECS(K_TICK_MAX) % NSEC_PER_SEC),                    \
+	})
+
 /** @endcond */
 
 #ifndef CONFIG_TIMER_READS_ITS_FREQUENCY_AT_RUNTIME

include/zephyr/sys/timeutil.h

@@ -381,25 +381,6 @@ static inline bool timespec_normalize(struct timespec *ts)
 {
 	__ASSERT_NO_MSG(ts != NULL);
 
-#if defined(CONFIG_SPEED_OPTIMIZATIONS) && HAS_BUILTIN(__builtin_add_overflow)
-
-	int sign = (ts->tv_nsec >= 0) - (ts->tv_nsec < 0);
-	int64_t sec = (ts->tv_nsec >= (long)NSEC_PER_SEC) * (ts->tv_nsec / (long)NSEC_PER_SEC) +
-		      ((ts->tv_nsec < 0) && (ts->tv_nsec != LONG_MIN)) *
-			      DIV_ROUND_UP((unsigned long)-ts->tv_nsec, (long)NSEC_PER_SEC) +
-		      (ts->tv_nsec == LONG_MIN) * ((LONG_MAX / NSEC_PER_SEC) + 1);
-	bool overflow = __builtin_add_overflow(ts->tv_sec, sign * sec, &ts->tv_sec);
-
-	ts->tv_nsec -= sign * (long)NSEC_PER_SEC * sec;
-
-	if (!overflow) {
-		__ASSERT_NO_MSG(timespec_is_valid(ts));
-	}
-
-	return !overflow;
-
-#else
-
 	long sec;
 
 	if (ts->tv_nsec >= (long)NSEC_PER_SEC) {
@@ -441,11 +422,10 @@ static inline bool timespec_normalize(struct timespec *ts)
 		/* no change: SonarQube was complaining */
 	}
 
-	__ASSERT_NO_MSG(timespec_is_valid(ts));
+	__ASSERT(timespec_is_valid(ts), "{%lld, %ld} is not a valid timespec",
+		 (long long)ts->tv_sec, (long)ts->tv_nsec);
 
 	return true;
-
-#endif
 }
 
 /**
@@ -614,123 +594,60 @@ static inline bool timespec_equal(const struct timespec *a, const struct timespe
  * This function converts time durations expressed as Zephyr @ref k_timeout_t
  * objects to `struct timespec` objects.
  *
+ * @note This function will assert if assertions are enabled and @p timeout is not relative,
+ * (i.e. a timeout generated by `K_TIMEOUT_ABS_TICKS` or similar is used).
+ *
  * @param timeout the kernel timeout to convert
  * @param[out] ts the timespec to store the result
  */
-static inline void timespec_from_timeout(k_timeout_t timeout, struct timespec *ts)
-{
-	__ASSERT_NO_MSG(ts != NULL);
-
-#if defined(CONFIG_SPEED_OPTIMIZATIONS)
-
-	uint64_t ns = k_ticks_to_ns_ceil64(timeout.ticks);
-
-	*ts = (struct timespec){
-		.tv_sec = (timeout.ticks == K_TICKS_FOREVER) * INT64_MAX +
-			  (timeout.ticks != K_TICKS_FOREVER) * (ns / NSEC_PER_SEC),
-		.tv_nsec = (timeout.ticks == K_TICKS_FOREVER) * (NSEC_PER_SEC - 1) +
-			   (timeout.ticks != K_TICKS_FOREVER) * (ns % NSEC_PER_SEC),
-	};
-
-#else
-
-	if (timeout.ticks == 0) {
-		/* This is equivalent to K_NO_WAIT, but without including <zephyr/kernel.h> */
-		ts->tv_sec = 0;
-		ts->tv_nsec = 0;
-	} else if (timeout.ticks == K_TICKS_FOREVER) {
-		/* This is roughly equivalent to K_FOREVER, but not including <zephyr/kernel.h> */
-		ts->tv_sec = (time_t)INT64_MAX;
-		ts->tv_nsec = NSEC_PER_SEC - 1;
-	} else {
-		uint64_t ns = k_ticks_to_ns_ceil64(timeout.ticks);
-
-		ts->tv_sec = ns / NSEC_PER_SEC;
-		ts->tv_nsec = ns - ts->tv_sec * NSEC_PER_SEC;
-	}
-
-#endif
-
-	__ASSERT_NO_MSG(timespec_is_valid(ts));
-}
+void timespec_from_timeout(k_timeout_t timeout, struct timespec *ts);
 
 /**
  * @brief Convert a timespec to a kernel timeout
  *
- * This function converts durations expressed as a `struct timespec` to Zephyr @ref k_timeout_t
- * objects.
+ * This function converts a time duration, @p req, expressed as a `timespec` object, to a Zephyr
+ * @ref k_timeout_t object.
+ *
+ * If @p req contains a negative duration or if both `tv_sec` and `tv_nsec` fields are zero, this
+ * function will return @ref K_NO_WAIT.
+ *
+ * If @p req contains the maximum representable `timespec`, `{max(time_t), 999999999}`, then this
+ * function will return @ref K_FOREVER.
+ *
+ * If @p req contains a value that is greater than the maximum equivalent tick duration, then this
+ * function will return the maximum representable tick duration (i.e. @p req will be rounded-down).
  *
- * Given that the range of a `struct timespec` is much larger than the range of @ref k_timeout_t,
- * and also given that the functions are only intended to be used to convert time durations
- * (which are always positive), the function will saturate to @ref K_NO_WAIT if the `tv_sec` field
- * of @a ts is negative.
+ * Otherwise, this function will return the `k_timeout_t` that is rounded-up to a tick boundary.
+ *
+ * If @p rem is not `NULL`, it will be set to the remainder of the conversion, i.e. the difference
+ * between the requested duration and the converted duration as a `timespec` object, approximately
+ * as shown below.
+ *
+ * ```python
+ * rem = requested_duration - converted_duration
+ * ```
+ *
+ * @param req the requested `timespec` to convert
+ * @param[out] rem optional pointer to a `timespec` to store the remainder
+ * @return the corresponding kernel timeout
+ */
+k_timeout_t timespec_to_timeout_rem(const struct timespec *req, struct timespec *rem);
+
+/**
+ * @brief Convert a timespec to a kernel timeout
  *
- * Similarly, if the duration is too large to fit in @ref k_timeout_t, the function will
- * saturate to @ref K_FOREVER.
+ * This function converts a time duration, @p req, expressed as a `timespec` object, to a Zephyr
+ * @ref k_timeout_t object exactly as @ref timespec_to_timeout_rem, but does not provide a
+ * remainder.
  *
  * @param ts the timespec to convert
  * @return the kernel timeout
+ *
+ * @see timespec_to_timeout_rem()
  */
 static inline k_timeout_t timespec_to_timeout(const struct timespec *ts)
 {
-	__ASSERT_NO_MSG((ts != NULL) && timespec_is_valid(ts));
-
-#if defined(CONFIG_SPEED_OPTIMIZATIONS)
-
-	return (k_timeout_t){
-		/* note: must check for 32-bit size here until #90029 is resolved */
-		.ticks = ((sizeof(ts->tv_sec) == sizeof(int32_t) && (ts->tv_sec == INT32_MAX) &&
-			   (ts->tv_nsec == NSEC_PER_SEC - 1)) ||
-			  ((sizeof(ts->tv_sec) == sizeof(int64_t)) && (ts->tv_sec == INT64_MAX) &&
-			   (ts->tv_nsec == NSEC_PER_SEC - 1))) *
-				 K_TICKS_FOREVER +
-			 ((sizeof(ts->tv_sec) == sizeof(int32_t) && (ts->tv_sec == INT32_MAX) &&
-			   (ts->tv_nsec == NSEC_PER_SEC - 1)) ||
-			  ((sizeof(ts->tv_sec) == sizeof(int64_t)) && (ts->tv_sec != INT64_MAX) &&
-			   (ts->tv_sec >= 0))) *
-				 (IS_ENABLED(CONFIG_TIMEOUT_64BIT)
-					  ? (int64_t)(CLAMP(
-						    k_sec_to_ticks_floor64(ts->tv_sec) +
-							    k_ns_to_ticks_floor64(ts->tv_nsec),
-						    0, (uint64_t)INT64_MAX))
-					  : (uint32_t)(CLAMP(
-						    k_sec_to_ticks_floor64(ts->tv_sec) +
-							    k_ns_to_ticks_floor64(ts->tv_nsec),
-						    0, (uint64_t)UINT32_MAX)))};
-
-#else
-
-	if ((ts->tv_sec < 0) || (ts->tv_sec == 0 && ts->tv_nsec == 0)) {
-		/* This is equivalent to K_NO_WAIT, but without including <zephyr/kernel.h> */
-		return (k_timeout_t){
-			.ticks = 0,
-		};
-		/* note: must check for 32-bit size here until #90029 is resolved */
-	} else if (((sizeof(ts->tv_sec) == sizeof(int32_t)) && (ts->tv_sec == INT32_MAX) &&
-		    (ts->tv_nsec == NSEC_PER_SEC - 1)) ||
-		   ((sizeof(ts->tv_sec) == sizeof(int64_t)) && (ts->tv_sec == INT64_MAX) &&
-		    (ts->tv_nsec == NSEC_PER_SEC - 1))) {
-		/* This is equivalent to K_FOREVER, but not including <zephyr/kernel.h> */
-		return (k_timeout_t){
-			.ticks = K_TICKS_FOREVER,
-		};
-	} else {
-		if (IS_ENABLED(CONFIG_TIMEOUT_64BIT)) {
-			return (k_timeout_t){
-				.ticks = (int64_t)CLAMP(k_sec_to_ticks_floor64(ts->tv_sec) +
-								k_ns_to_ticks_floor64(ts->tv_nsec),
-							0, (uint64_t)INT64_MAX),
-			};
-		} else {
-			return (k_timeout_t){
-				.ticks = (uint32_t)CLAMP(k_sec_to_ticks_floor64(ts->tv_sec) +
-								 k_ns_to_ticks_floor64(ts->tv_nsec),
-							 0, (uint64_t)UINT32_MAX),
-			};
-		}
-	}
-
-#endif
+	return timespec_to_timeout_rem(ts, NULL);
 }
 
 /**

include/zephyr/sys/util.h

@@ -372,6 +372,13 @@ extern "C" {
 		 ? ((n) - ((d) / 2)) / (d)                                                         \
 		 : ((n) + ((d) / 2)) / (d))
 
+/**
+ * @brief Calculate `a * (b / c)` as `(a * b) / c` to minimize rounding errors
+ *
+ * @return The result  of @p a * @p b / @p c ordered to minimize rounding errors.
+ */
+#define MULDIV(a, b, c) (((a) * (b)) / (c))
+
 #ifndef MAX
 /**
  * @brief Obtain the maximum of two values.
@@ -846,6 +853,8 @@ static inline bool util_eq(const void *m1, size_t len1, const void *m2, size_t l
 #define KHZ(x) ((x) * 1000)
 /** @brief Number of Hz in @p x MHz */
 #define MHZ(x) (KHZ(x) * 1000)
+/** @brief Number of Hz in @p x GHz */
+#define GHZ(x) (MHZ(x) * 1000ULL)
 
 /**
  * @brief For the POSIX architecture add a minimal delay in a busy wait loop.

lib/os/CMakeLists.txt

@@ -11,6 +11,7 @@ zephyr_sources(
   printk.c
   sem.c
   thread_entry.c
+  timeutil.c
   )
 
 zephyr_sources_ifdef(CONFIG_FDTABLE fdtable.c)

lib/os/timeutil.c

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2025 Tenstorrent AI ULC
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <time.h>
+
+#include <zephyr/kernel.h>
+#include <zephyr/sys/__assert.h>
+#include <zephyr/sys/timeutil.h>
+#include <zephyr/sys/time_units.h>
+#include <zephyr/sys/util.h>
+
+void timespec_from_timeout(k_timeout_t timeout, struct timespec *ts)
+{
+	__ASSERT_NO_MSG(ts != NULL);
+	__ASSERT_NO_MSG(Z_IS_TIMEOUT_RELATIVE(timeout) ||
+			(IS_ENABLED(CONFIG_TIMEOUT_64BIT) && K_TIMEOUT_EQ(timeout, K_FOREVER)));
+
+	if (K_TIMEOUT_EQ(timeout, K_FOREVER)) {
+		/* duration == K_TICKS_FOREVER ticks */
+		*ts = K_TS_FOREVER;
+	} else if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) {
+		/* duration <= 0 ticks */
+		*ts = K_TS_NO_WAIT;
+	} else {
+		*ts = (struct timespec){
+			.tv_sec =
+				(time_t)((uint64_t)timeout.ticks / CONFIG_SYS_CLOCK_TICKS_PER_SEC),
+			.tv_nsec = (long)(K_TICKS_TO_NSECS((uint64_t)timeout.ticks) % NSEC_PER_SEC),
+		};
+	}
+
+	__ASSERT_NO_MSG(timespec_is_valid(ts));
+}
+
+static k_timeout_t timespec_timeout_rem(const struct timespec *ts, struct timespec *rem,
+					k_timeout_t timeout)
+{
+	if (rem != NULL) {
+		timespec_from_timeout(timeout, rem);
+		timespec_sub(rem, ts);
+		timespec_negate(rem);
+	}
+
+	return timeout;
+}
+
+k_timeout_t timespec_to_timeout_rem(const struct timespec *ts, struct timespec *rem)
+{
+	__ASSERT_NO_MSG((ts != NULL) && timespec_is_valid(ts));
+
+	if (timespec_compare(ts, &K_TS_NO_WAIT) <= 0) {
+		if (rem != NULL) {
+			*rem = *ts;
+		}
+		return K_NO_WAIT;
+	}
+
+	if (timespec_compare(ts, &K_TS_FOREVER) == 0) {
+		if (rem != NULL) {
+			*rem = K_TS_NO_WAIT;
+		}
+		return K_FOREVER;
+	}
+
+	if (timespec_compare(ts, &K_TS_MIN) <= 0) {
+		/* round up to align to min ticks */
+		return timespec_timeout_rem(ts, rem, K_TICKS(K_TICK_MIN));
+	}
+
+	if (timespec_compare(ts, &K_TS_MAX) >= 0) {
+		/* round down to align to max ticks */
+		return timespec_timeout_rem(ts, rem, K_TICKS(K_TICK_MAX));
+	}
+
+	uint64_t ticks_s = k_sec_to_ticks_ceil64(ts->tv_sec);
+	uint64_t ticks_ns = k_ns_to_ticks_ceil64(ts->tv_nsec);
+
+	return timespec_timeout_rem(ts, rem, K_TICKS(ticks_s + ticks_ns));
+}

tests/lib/timespec_util/prj.conf

@@ -1 +1,4 @@
 CONFIG_ZTEST=y
+
+# required to use native_sim for testing and have consistent time_t size across all platforms
+CONFIG_PICOLIBC=y