From 6984eff3c5bc6ff7be62f1cefec5b42bc0b9cf69 Mon Sep 17 00:00:00 2001 From: Amir Haroush Date: Tue, 7 Mar 2023 11:41:19 +0200 Subject: [PATCH] ocf: fix env_ticks_to_{msec,usec,nsec} precision & accuracy - fix precision when one convert to seconds and then multiply we can have precision errors for example if one have 77ms, it will go to 0 when converted to seconds and then multiply that 0 by 1000 will return 0 instead of 77ms. - fix mismatch nsec/usec nsec was multiplied by 1000*1000 while usec by 1000*1000*1000 it should be the opposite. anyway the implementation had changed. - implementation description * env_ticks_to_msec: j / (tick_hz / 1000) this is exactly the same as (j * 1000) / tick_hz (eq #2). but this implementation (eq #2) can only handle 54b in j (before overflowing) because of the multiplication by 1000 (10b). with the correct implementation we use all 64b in j. we assume that tick_hz will be prefectly divisible by 1000 so we are ok. * env_ticks_to_usec: j / (tick_hz / (1000 * 1000)) same as in msec case, we use all 64b in j. here we assume that tick_hz is perfectly divisible by (1000 * 1000) i.e. we assume that CPU frequency is some multiple of 1MHz. * env_ticks_to_nsec: (j * 1000) / (tick_hz / (1000 * 1000)) in this case we can't assume that tick_hz is divisible by 10^9 because there are many CPUs with 2.8GHz or 3.3GHz for example. so we multiply j by 1000 this means that we can only handle correctly j up to 54b. (64b - 10b, 10b for the *1000 operation) Signed-off-by: Amir Haroush Signed-off-by: Shai Fultheim Change-Id: Ia8ea7f88b718df206fa0731e3f39f419ee922aa7 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17078 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/env_ocf/ocf_env.h | 44 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/lib/env_ocf/ocf_env.h b/lib/env_ocf/ocf_env.h index cfd7a0fdc..1d5f312ea 100644 --- a/lib/env_ocf/ocf_env.h +++ b/lib/env_ocf/ocf_env.h @@ -751,22 +751,50 @@ env_ticks_to_secs(uint64_t j) return j / spdk_get_ticks_hz(); } +/** + * @brief Dividing first tick_hz by 1000 is better than multiply j by 1000 + * because if we would multiply j by 1000 we could only handle j + * up to 54b (*1000 is 10b). + * with this implementation we can handle all 64b in j. + * we only assume that ticks_hz is perfectly divisible by 1000 + * which is probably good assumption because CPU frequency is in GHz/MHz scale. + * + * @param[in] j ticks count + */ static inline uint64_t env_ticks_to_msecs(uint64_t j) { - return env_ticks_to_secs(j) * 1000; -} - -static inline uint64_t -env_ticks_to_nsecs(uint64_t j) -{ - return env_ticks_to_secs(j) * 1000 * 1000; + return j / (spdk_get_ticks_hz() / 1000); } +/** + * @brief Same as in msec case + * we divide ticks_hz by 1000 * 1000. + * so we use all 64b in j here as well. + * we assume that ticks_hz is perfectly divisible by 1000 * 1000 + * i.e. CPU frequency is divisible by 1MHz. + * + * @param[in] j ticks count + */ static inline uint64_t env_ticks_to_usecs(uint64_t j) { - return env_ticks_to_secs(j) * 1000 * 1000 * 1000; + return j / (spdk_get_ticks_hz() / (1000 * 1000)); +} + +/** + * @brief We can't divide ticks_hz by 10^9 + * because we can't assume that CPU frequency is prefectly divisible by 10^9. + * for example there are CPUs with 2.8GHz or 3.3GHz. + * so in here we multiply j by 1000 + * which means we can only handle 54b of j correctly. + * + * @param[in] j ticks count + */ +static inline uint64_t +env_ticks_to_nsecs(uint64_t j) +{ + return (j * 1000) / (spdk_get_ticks_hz() / (1000 * 1000)); } static inline uint64_t