V2.7 branch timing related fixes

include/sys/time_units.h

@@ -7,6 +7,9 @@
 #ifndef ZEPHYR_INCLUDE_TIME_UNITS_H_
 #define ZEPHYR_INCLUDE_TIME_UNITS_H_
 
+#include <sys/util.h>
+#include <toolchain.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -56,6 +59,21 @@ static TIME_CONSTEXPR inline int sys_clock_hw_cycles_per_sec(void)
 #endif
 }
 
+/** @internal
+ * Macro determines if fast conversion algorithm can be used. It checks if
+ * maximum timeout represented in source frequency domain and multiplied by
+ * target frequency fits in 64 bits.
+ *
+ * @param from_hz Source frequency.
+ * @param to_hz Target frequency.
+ *
+ * @retval true Use faster algorithm.
+ * @retval false Use algorithm preventing overflow of intermediate value.
+ */
+#define Z_TMCVT_USE_FAST_ALGO(from_hz, to_hz) \
+	((ceiling_fraction(CONFIG_SYS_CLOCK_MAX_TIMEOUT_DAYS * 24ULL * 3600ULL * from_hz, \
+			   UINT32_MAX) * to_hz) <= UINT32_MAX)
+
 /* Time converter generator gadget.  Selects from one of three
  * conversion algorithms: ones that take advantage when the
  * frequencies are an integer ratio (in either direction), or a full
@@ -123,8 +141,18 @@ static TIME_CONSTEXPR ALWAYS_INLINE uint64_t z_tmcvt(uint64_t t, uint32_t from_h
 	} else {
 		if (result32) {
 			return (uint32_t)((t * to_hz + off) / from_hz);
+		} else if (const_hz && Z_TMCVT_USE_FAST_ALGO(from_hz, to_hz)) {
+			/* Faster algorithm but source is first multiplied by target frequency
+			 * and it can overflow even though final result would not overflow.
+			 * Kconfig option shall prevent use of this algorithm when there is a
+			 * risk of overflow.
+			 */
+			return ((t * to_hz + off) / from_hz);
 		} else {
-			return (t * to_hz + off) / from_hz;
+			/* Slower algorithm but input is first divided before being multiplied
+			 * which prevents overflow of intermediate value.
+			 */
+			return (t / from_hz) * to_hz + ((t % from_hz) * to_hz + off) / from_hz;
 		}
 	}
 }

kernel/Kconfig

@@ -613,6 +613,17 @@ config TIMEOUT_64BIT
 	  availability of absolute timeout values (which require the
 	  extra precision).
 
+config SYS_CLOCK_MAX_TIMEOUT_DAYS
+	int "Max timeout (in days) used in conversions"
+	default 365
+	help
+	  Value is used in the time conversion static inline function to determine
+	  at compile time which algorithm to use. One algorithm is faster, takes
+	  less code but may overflow if multiplication of source and target
+	  frequency exceeds 64 bits. Second algorithm prevents that. Faster
+	  algorithm is selected for conversion if maximum timeout represented in
+	  source frequency domain multiplied by target frequency fits in 64 bits.
+
 config XIP
 	bool "Execute in place"
 	help

lib/posix/clock.c

@@ -27,7 +27,6 @@ static struct k_spinlock rt_clock_base_lock;
  */
 int z_impl_clock_gettime(clockid_t clock_id, struct timespec *ts)
 {
-	uint64_t elapsed_nsecs;
 	struct timespec base;
 	k_spinlock_key_t key;
 
@@ -48,9 +47,13 @@ int z_impl_clock_gettime(clockid_t clock_id, struct timespec *ts)
 		return -1;
 	}
 
-	elapsed_nsecs = k_ticks_to_ns_floor64(k_uptime_ticks());
-	ts->tv_sec = (int32_t) (elapsed_nsecs / NSEC_PER_SEC);
-	ts->tv_nsec = (int32_t) (elapsed_nsecs % NSEC_PER_SEC);
+	uint64_t ticks = k_uptime_ticks();
+	uint64_t elapsed_secs = ticks / CONFIG_SYS_CLOCK_TICKS_PER_SEC;
+	uint64_t nremainder = ticks - elapsed_secs * CONFIG_SYS_CLOCK_TICKS_PER_SEC;
+
+	ts->tv_sec = (time_t) elapsed_secs;
+	/* For ns 32 bit conversion can be used since its smaller than 1sec. */
+	ts->tv_nsec = (int32_t) k_ticks_to_ns_floor32(nremainder);
 
 	ts->tv_sec += base.tv_sec;
 	ts->tv_nsec += base.tv_nsec;

subsys/testsuite/ztest/include/ztest.h

@@ -38,6 +38,7 @@
 #define CONFIG_MP_NUM_CPUS 1
 #define CONFIG_SYS_CLOCK_TICKS_PER_SEC 100
 #define CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC 10000000
+#define CONFIG_SYS_CLOCK_MAX_TIMEOUT_DAYS 365
 #define ARCH_STACK_PTR_ALIGN 8
 /* FIXME: Properly integrate with Zephyr's arch specific code */
 #define CONFIG_X86 1

tests/unit/time_units/CMakeLists.txt

@@ -0,0 +1,6 @@
+# Copyright 2022 Meta
+# SPDX-License-Identifier: Apache-2.0
+
+project(time_units)
+set(SOURCES main.c overflow.c)
+find_package(ZephyrUnittest REQUIRED HINTS $ENV{ZEPHYR_BASE})

tests/unit/time_units/main.c

@@ -0,0 +1,15 @@
+/*
+ * Copyright 2022 Meta
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <ztest.h>
+
+extern void test_z_tmcvt_for_overflow(void);
+
+void test_main(void)
+{
+	ztest_test_suite(test_time_units, ztest_unit_test(test_z_tmcvt_for_overflow));
+	ztest_run_test_suite(test_time_units);
+}

tests/unit/time_units/overflow.c

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2022 Meta
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <inttypes.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdint.h>
+
+#include <ztest.h>
+#include <sys/time_units.h>
+
+/**
+ * @brief Test @ref z_tmcvt for robustness against intermediate value overflow.
+ *
+ * With input
+ * ```
+ * [t0, t1, t2] = [
+ *   UINT64_MAX / to_hz - 1,
+ *   UINT64_MAX / to_hz,
+ *   UINT64_MAX / to_hz + 1,
+ * ]
+ * ```
+ *
+ * passed through @ref z_tmcvt, we expect a linear sequence:
+ * ```
+ * [
+ *   562949953369140,
+ *   562949953399658,
+ *   562949953430175,
+ * ]
+ * ```
+ *
+ * If an overflow occurs, we see something like the following:
+ * ```
+ * [
+ *   562949953369140,
+ *   562949953399658,
+ *   8863,
+ * ]
+ * ```
+ */
+void test_z_tmcvt_for_overflow(void)
+{
+	const uint32_t from_hz = 32768UL;
+	const uint32_t to_hz = 1000000000UL;
+
+	zassert_equal(562949953369140ULL,
+		      z_tmcvt(UINT64_MAX / to_hz - 1, from_hz, to_hz, true, false, false, false),
+			  NULL);
+	zassert_equal(562949953399658ULL,
+		      z_tmcvt(UINT64_MAX / to_hz, from_hz, to_hz, true, false, false, false),
+			  NULL);
+	zassert_equal(562949953430175ULL,
+		      z_tmcvt(UINT64_MAX / to_hz + 1, from_hz, to_hz, true, false, false, false),
+			  NULL);
+}

tests/unit/time_units/prj.conf

@@ -0,0 +1 @@
+CONFIG_ZTEST=y

tests/unit/time_units/testcase.yaml

@@ -0,0 +1,5 @@
+common:
+  tags: time_units
+  type: unit
+tests:
+  utilities.time_units.z_tmcvt: {}