Commit 6984eff3 authored Mar 07, 2023 by Amir Haroush Committed by Ben Walker Mar 21, 2023

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 <amir.haroush@huawei.com>
Signed-off-by: Shai Fultheim <shai.fultheim@huawei.com>
Change-Id: Ia8ea7f88b718df206fa0731e3f39f419ee922aa7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17078


Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

parent 7c7267e9

lib/env_ocf/ocf_env.h

+33 −5

Original line number	Diff line number	Diff line
		@@ -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;
		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_nsecs(uint64_t j)
		env_ticks_to_usecs(uint64_t j)
		{
		return env_ticks_to_secs(j) * 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_usecs(uint64_t j)
		env_ticks_to_nsecs(uint64_t j)
		{
		return env_ticks_to_secs(j) * 1000 * 1000 * 1000;
		return (j * 1000) / (spdk_get_ticks_hz() / (1000 * 1000));
		}

		static inline uint64_t