diff --git a/include/sys/time_units.h b/include/sys/time_units.h index 0963a6ba4275b..bbea29c1359b2 100644 --- a/include/sys/time_units.h +++ b/include/sys/time_units.h @@ -7,6 +7,9 @@ #ifndef ZEPHYR_INCLUDE_TIME_UNITS_H_ #define ZEPHYR_INCLUDE_TIME_UNITS_H_ +#include +#include + #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; } } } diff --git a/kernel/Kconfig b/kernel/Kconfig index c2575c77a50eb..8e5b2a308b4cd 100644 --- a/kernel/Kconfig +++ b/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 diff --git a/lib/posix/clock.c b/lib/posix/clock.c index c36f2401cc4c5..eea05d8b2c3e7 100644 --- a/lib/posix/clock.c +++ b/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; diff --git a/subsys/testsuite/ztest/include/ztest.h b/subsys/testsuite/ztest/include/ztest.h index c0b5fcc92cfee..6aa4306948b2e 100644 --- a/subsys/testsuite/ztest/include/ztest.h +++ b/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 diff --git a/tests/unit/time_units/CMakeLists.txt b/tests/unit/time_units/CMakeLists.txt new file mode 100644 index 0000000000000..34534ebca59fe --- /dev/null +++ b/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}) diff --git a/tests/unit/time_units/main.c b/tests/unit/time_units/main.c new file mode 100644 index 0000000000000..51ea1f620c6c8 --- /dev/null +++ b/tests/unit/time_units/main.c @@ -0,0 +1,15 @@ +/* + * Copyright 2022 Meta + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +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); +} diff --git a/tests/unit/time_units/overflow.c b/tests/unit/time_units/overflow.c new file mode 100644 index 0000000000000..ad58db7ca0947 --- /dev/null +++ b/tests/unit/time_units/overflow.c @@ -0,0 +1,59 @@ +/* + * Copyright 2022 Meta + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include + +#include +#include + +/** + * @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); +} diff --git a/tests/unit/time_units/prj.conf b/tests/unit/time_units/prj.conf new file mode 100644 index 0000000000000..9467c2926896d --- /dev/null +++ b/tests/unit/time_units/prj.conf @@ -0,0 +1 @@ +CONFIG_ZTEST=y diff --git a/tests/unit/time_units/testcase.yaml b/tests/unit/time_units/testcase.yaml new file mode 100644 index 0000000000000..4624ce7143b8c --- /dev/null +++ b/tests/unit/time_units/testcase.yaml @@ -0,0 +1,5 @@ +common: + tags: time_units + type: unit +tests: + utilities.time_units.z_tmcvt: {}