diff mbox

[temp] linux-generic: odp_time: don't use cpu cycle API to get time

Message ID 1446804969-24647-1-git-send-email-ivan.khoronzhuk@linaro.org
State New
Headers show

Commit Message

Ivan Khoronzhuk Nov. 6, 2015, 10:16 a.m. UTC
This will be looking like this patch. It allows to get rid of cpu_freq_max ... a
little. But temp freq actually breaks CPU cycle API that emulates number of cpu
cycles are base on cpu_hz_max....but any way it's not correct...

The linux-generic time API implementation shouldn't depend on cpu
cycle API wich is not stable enough to measure time period.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 platform/linux-generic/Makefile.am                |  1 -
 platform/linux-generic/include/odp_cpu_internal.h | 29 ----------
 platform/linux-generic/odp_cpu.c                  |  6 ++-
 platform/linux-generic/odp_time.c                 | 66 +++++++++++++++++------
 4 files changed, 54 insertions(+), 48 deletions(-)
 delete mode 100644 platform/linux-generic/include/odp_cpu_internal.h

Comments

Ivan Khoronzhuk Nov. 6, 2015, 10:21 a.m. UTC | #1
I will send this patch when series with cycles -> time API will be applied.
So, only dependency for freq_max is cpu cycles emulation when arch is not added.
Any way cpu_max_freq is not correct it doesn't correspond to the maximum freq of CPU.
It's got once at init as current freq and then represented as max )))). So your temp fix
is very close to this...approach..I'm not talking about freq deviations while work.

On 06.11.15 12:16, Ivan Khoronzhuk wrote:
> This will be looking like this patch. It allows to get rid of cpu_freq_max ... a
> little. But temp freq actually breaks CPU cycle API that emulates number of cpu
> cycles are base on cpu_hz_max....but any way it's not correct...
>
> The linux-generic time API implementation shouldn't depend on cpu
> cycle API wich is not stable enough to measure time period.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   platform/linux-generic/Makefile.am                |  1 -
>   platform/linux-generic/include/odp_cpu_internal.h | 29 ----------
>   platform/linux-generic/odp_cpu.c                  |  6 ++-
>   platform/linux-generic/odp_time.c                 | 66 +++++++++++++++++------
>   4 files changed, 54 insertions(+), 48 deletions(-)
>   delete mode 100644 platform/linux-generic/include/odp_cpu_internal.h
>
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am
> index a6b6029..3a2603f 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -147,7 +147,6 @@ noinst_HEADERS = \
>   		  ${srcdir}/include/odp_schedule_internal.h \
>   		  ${srcdir}/include/odp_spin_internal.h \
>   		  ${srcdir}/include/odp_timer_internal.h \
> -		  ${srcdir}/include/odp_cpu_internal.h \
>   		  ${srcdir}/Makefile.inc
>
>   __LIB__libodp_la_SOURCES = \
> diff --git a/platform/linux-generic/include/odp_cpu_internal.h b/platform/linux-generic/include/odp_cpu_internal.h
> deleted file mode 100644
> index 664e2df..0000000
> --- a/platform/linux-generic/include/odp_cpu_internal.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/* Copyright (c) 2015, Linaro Limited
> - * All rights reserved.
> - *
> - * SPDX-License-Identifier:     BSD-3-Clause
> - */
> -
> -#ifndef ODP_CPU_INTERNAL_H_
> -#define ODP_CPU_INTERNAL_H_
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <odp/cpu.h>
> -
> -static inline
> -uint64_t _odp_cpu_cycles_diff(uint64_t c2, uint64_t c1)
> -{
> -	if (odp_likely(c2 >= c1))
> -		return c2 - c1;
> -
> -	return c2 + (odp_cpu_cycles_max() - c1) + 1;
> -}
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> diff --git a/platform/linux-generic/odp_cpu.c b/platform/linux-generic/odp_cpu.c
> index 636f811..4bd6a9a 100644
> --- a/platform/linux-generic/odp_cpu.c
> +++ b/platform/linux-generic/odp_cpu.c
> @@ -6,9 +6,11 @@
>
>   #include <odp/cpu.h>
>   #include <odp/hints.h>
> -#include <odp_cpu_internal.h>
>
>   uint64_t odp_cpu_cycles_diff(uint64_t c2, uint64_t c1)
>   {
> -	return _odp_cpu_cycles_diff(c2, c1);
> +	if (odp_likely(c2 >= c1))
> +		return c2 - c1;
> +
> +	return c2 + (odp_cpu_cycles_max() - c1) + 1;
>   }
> diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c
> index a2e4046..486996e 100644
> --- a/platform/linux-generic/odp_time.c
> +++ b/platform/linux-generic/odp_time.c
> @@ -6,13 +6,10 @@
>
>   #define _POSIX_C_SOURCE 200809L
>
> +#include <time.h>
>   #include <odp/time.h>
>   #include <odp/hints.h>
> -#include <odp/system_info.h>
> -#include <odp/cpu.h>
> -#include <odp_cpu_internal.h>
> -
> -#define GIGA 1000000000
> +#include <odp_debug_internal.h>
>
>   static inline
>   uint64_t time_to_tick(odp_time_t time)
> @@ -28,34 +25,71 @@ odp_time_t tick_to_time(uint64_t tick)
>
>   odp_time_t odp_time_local(void)
>   {
> -	return tick_to_time(odp_cpu_cycles());
> +	int ret;
> +	struct timespec time, tres;
> +	uint64_t sec, nsec, ticks, resolution;
> +
> +	ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
> +	if (odp_unlikely(ret != 0))
> +		ODP_ABORT("clock_gettime failed\n");
> +
> +	sec = (uint64_t)time.tv_sec;
> +	nsec  = (uint64_t)time.tv_nsec;
> +
> +	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> +	if (odp_unlikely(ret != 0))
> +		ODP_ABORT("clock_getres failed\n");
> +
> +	resolution = (uint64_t)tres.tv_nsec;
> +	ticks = sec * ODP_TIME_SEC_IN_NS + nsec;
> +
> +	return ticks / resolution;
>   }
>
>   odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
>   {
> -	return tick_to_time(_odp_cpu_cycles_diff(t2, t1));
> +	uint64_t tick1 = time_to_tick(t1);
> +	uint64_t tick2 = time_to_tick(t2);
> +
> +	/*
> +	 * It's supposed that CLOCK_MONOTONIC_RAW never overflows,
> +	 * and t2 is got after t1
> +	 */
> +	if (odp_unlikely(tick2 < tick1))
> +		ODP_ABORT("incorrect time order\n");
> +
> +	return tick_to_time(tick2 - tick1);
>   }
>
>   uint64_t odp_time_to_ns(odp_time_t time)
>   {
> -	uint64_t hz = odp_cpu_hz_max();
> +	int ret;
> +	uint64_t resolution;
> +	struct timespec tres;
>   	uint64_t tick = time_to_tick(time);
>
> -	if (tick > (UINT64_MAX / GIGA))
> -		return (tick / hz) * GIGA;
> +	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> +	if (odp_unlikely(ret != 0))
> +		ODP_ABORT("clock_getres failed\n");
>
> -	return (tick * GIGA) / hz;
> -}
> +	resolution = (uint64_t)tres.tv_nsec;
>
> +	return tick * resolution;
> +}
>
>   odp_time_t odp_time_local_from_ns(uint64_t ns)
>   {
> -	uint64_t hz = odp_cpu_hz_max();
> +	int ret;
> +	struct timespec tres;
> +	uint64_t resolution;
> +
> +	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> +	if (odp_unlikely(ret != 0))
> +		ODP_ABORT("clock_getres failed\n");
>
> -	if (ns > (UINT64_MAX / hz))
> -		return tick_to_time((ns / GIGA) * hz);
> +	resolution = (uint64_t)tres.tv_nsec;
>
> -	return tick_to_time((ns * hz) / GIGA);
> +	return tick_to_time(ns / resolution);
>   }
>
>   int odp_time_cmp(odp_time_t t2, odp_time_t t1)
>
diff mbox

Patch

diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am
index a6b6029..3a2603f 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -147,7 +147,6 @@  noinst_HEADERS = \
 		  ${srcdir}/include/odp_schedule_internal.h \
 		  ${srcdir}/include/odp_spin_internal.h \
 		  ${srcdir}/include/odp_timer_internal.h \
-		  ${srcdir}/include/odp_cpu_internal.h \
 		  ${srcdir}/Makefile.inc
 
 __LIB__libodp_la_SOURCES = \
diff --git a/platform/linux-generic/include/odp_cpu_internal.h b/platform/linux-generic/include/odp_cpu_internal.h
deleted file mode 100644
index 664e2df..0000000
--- a/platform/linux-generic/include/odp_cpu_internal.h
+++ /dev/null
@@ -1,29 +0,0 @@ 
-/* Copyright (c) 2015, Linaro Limited
- * All rights reserved.
- *
- * SPDX-License-Identifier:     BSD-3-Clause
- */
-
-#ifndef ODP_CPU_INTERNAL_H_
-#define ODP_CPU_INTERNAL_H_
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#include <odp/cpu.h>
-
-static inline
-uint64_t _odp_cpu_cycles_diff(uint64_t c2, uint64_t c1)
-{
-	if (odp_likely(c2 >= c1))
-		return c2 - c1;
-
-	return c2 + (odp_cpu_cycles_max() - c1) + 1;
-}
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif
diff --git a/platform/linux-generic/odp_cpu.c b/platform/linux-generic/odp_cpu.c
index 636f811..4bd6a9a 100644
--- a/platform/linux-generic/odp_cpu.c
+++ b/platform/linux-generic/odp_cpu.c
@@ -6,9 +6,11 @@ 
 
 #include <odp/cpu.h>
 #include <odp/hints.h>
-#include <odp_cpu_internal.h>
 
 uint64_t odp_cpu_cycles_diff(uint64_t c2, uint64_t c1)
 {
-	return _odp_cpu_cycles_diff(c2, c1);
+	if (odp_likely(c2 >= c1))
+		return c2 - c1;
+
+	return c2 + (odp_cpu_cycles_max() - c1) + 1;
 }
diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c
index a2e4046..486996e 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -6,13 +6,10 @@ 
 
 #define _POSIX_C_SOURCE 200809L
 
+#include <time.h>
 #include <odp/time.h>
 #include <odp/hints.h>
-#include <odp/system_info.h>
-#include <odp/cpu.h>
-#include <odp_cpu_internal.h>
-
-#define GIGA 1000000000
+#include <odp_debug_internal.h>
 
 static inline
 uint64_t time_to_tick(odp_time_t time)
@@ -28,34 +25,71 @@  odp_time_t tick_to_time(uint64_t tick)
 
 odp_time_t odp_time_local(void)
 {
-	return tick_to_time(odp_cpu_cycles());
+	int ret;
+	struct timespec time, tres;
+	uint64_t sec, nsec, ticks, resolution;
+
+	ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
+	if (odp_unlikely(ret != 0))
+		ODP_ABORT("clock_gettime failed\n");
+
+	sec = (uint64_t)time.tv_sec;
+	nsec  = (uint64_t)time.tv_nsec;
+
+	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
+	if (odp_unlikely(ret != 0))
+		ODP_ABORT("clock_getres failed\n");
+
+	resolution = (uint64_t)tres.tv_nsec;
+	ticks = sec * ODP_TIME_SEC_IN_NS + nsec;
+
+	return ticks / resolution;
 }
 
 odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
 {
-	return tick_to_time(_odp_cpu_cycles_diff(t2, t1));
+	uint64_t tick1 = time_to_tick(t1);
+	uint64_t tick2 = time_to_tick(t2);
+
+	/*
+	 * It's supposed that CLOCK_MONOTONIC_RAW never overflows,
+	 * and t2 is got after t1
+	 */
+	if (odp_unlikely(tick2 < tick1))
+		ODP_ABORT("incorrect time order\n");
+
+	return tick_to_time(tick2 - tick1);
 }
 
 uint64_t odp_time_to_ns(odp_time_t time)
 {
-	uint64_t hz = odp_cpu_hz_max();
+	int ret;
+	uint64_t resolution;
+	struct timespec tres;
 	uint64_t tick = time_to_tick(time);
 
-	if (tick > (UINT64_MAX / GIGA))
-		return (tick / hz) * GIGA;
+	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
+	if (odp_unlikely(ret != 0))
+		ODP_ABORT("clock_getres failed\n");
 
-	return (tick * GIGA) / hz;
-}
+	resolution = (uint64_t)tres.tv_nsec;
 
+	return tick * resolution;
+}
 
 odp_time_t odp_time_local_from_ns(uint64_t ns)
 {
-	uint64_t hz = odp_cpu_hz_max();
+	int ret;
+	struct timespec tres;
+	uint64_t resolution;
+
+	ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
+	if (odp_unlikely(ret != 0))
+		ODP_ABORT("clock_getres failed\n");
 
-	if (ns > (UINT64_MAX / hz))
-		return tick_to_time((ns / GIGA) * hz);
+	resolution = (uint64_t)tres.tv_nsec;
 
-	return tick_to_time((ns * hz) / GIGA);
+	return tick_to_time(ns / resolution);
 }
 
 int odp_time_cmp(odp_time_t t2, odp_time_t t1)