Change of odp_time_local implementation in odp-dpdk

Message ID CAOdUko0anTLGErNCxqjUu9gyYuAkbJJ=n0yuBcxxH_5uvCchBQ@mail.gmail.com
State New
Headers show

Commit Message

Wenxian Li April 20, 2016, 4:29 p.m.
Hi Zoltan & Petri,

I update the diff.
What I changed includes:
i)   Copy odp_time.c from linux_generic to odp_dpdk. Makefile is updated.
ii)  Change odp_time.c file to leveraging dpdk api.
iii) The priority of timer source is: TSC (if invariant TSC is supported) >
HPET (If enabled) > syscall clockgettime

Could you help review the diff?
Any comments are welcomed.

Thanks,
Wenxian

  return -1;
@@ -67,7 +108,13 @@ static inline int time_cmp(odp_time_t t2, odp_time_t t1)
  return t2.tv_nsec - t1.tv_nsec;
 }

-static inline odp_time_t time_sum(odp_time_t t1, odp_time_t t2)
+static inline int time_cmp_dpdk(odp_time_t t2, odp_time_t t1)
+{
+ return t2.ticks - t1.ticks;
+}
+
+
+static inline odp_time_t time_sum_linux(odp_time_t t1, odp_time_t t2)
 {
  odp_time_t time;

@@ -82,7 +129,15 @@ static inline odp_time_t time_sum(odp_time_t t1,
odp_time_t t2)
  return time;
 }

-static inline odp_time_t time_local_from_ns(uint64_t ns)
+static inline odp_time_t time_sum_dpdk(odp_time_t t1, odp_time_t t2)
+{
+ odp_time_t time;
+ time.ticks = t2.ticks + t1.ticks;
+ return time;
+}
+
+
+static inline odp_time_t time_local_from_ns_linux(uint64_t ns)
 {
  odp_time_t time;

@@ -92,16 +147,23 @@ static inline odp_time_t time_local_from_ns(uint64_t
ns)
  return time;
 }

+static inline odp_time_t time_local_from_ns_dpdk(uint64_t ns)
+{
+ odp_time_t time;
+ time.ticks = ns * time_local_res_dpdk() / ODP_TIME_SEC_IN_NS;
+ return time;
+}
+
 static inline void time_wait_until(odp_time_t time)
 {
  odp_time_t cur;

  do {
  cur = time_local();
- } while (time_cmp(time, cur) > 0);
+ } while (time_handler.time_cmp(time, cur) > 0);
 }

-static inline uint64_t time_local_res(void)
+static inline uint64_t time_local_res_linux(void)
 {
  int ret;
  struct timespec tres;
@@ -125,49 +187,49 @@ odp_time_t odp_time_global(void)

 odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
 {
- return time_diff(t2, t1);
+ return time_handler.time_diff(t2, t1);
 }

 uint64_t odp_time_to_ns(odp_time_t time)
 {
- return time_to_ns(time);
+ return time_handler.time_to_ns(time);
 }

 odp_time_t odp_time_local_from_ns(uint64_t ns)
 {
- return time_local_from_ns(ns);
+ return time_handler.time_local_from_ns(ns);
 }

 odp_time_t odp_time_global_from_ns(uint64_t ns)
 {
- return time_local_from_ns(ns);
+ return time_handler.time_local_from_ns(ns);
 }

 int odp_time_cmp(odp_time_t t2, odp_time_t t1)
 {
- return time_cmp(t2, t1);
+ return time_handler.time_cmp(t2, t1);
 }

 odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
 {
- return time_sum(t1, t2);
+ return time_handler.time_sum(t1, t2);
 }

 uint64_t odp_time_local_res(void)
 {
- return time_local_res();
+ return time_handler.time_local_res();
 }

 uint64_t odp_time_global_res(void)
 {
- return time_local_res();
+ return time_handler.time_local_res();
 }

 void odp_time_wait_ns(uint64_t ns)
 {
  odp_time_t cur = time_local();
- odp_time_t wait = time_local_from_ns(ns);
- odp_time_t end_time = time_sum(cur, wait);
+ odp_time_t wait = time_handler.time_local_from_ns(ns);
+ odp_time_t end_time = time_handler.time_sum(cur, wait);

  time_wait_until(end_time);
 }
@@ -177,7 +239,7 @@ void odp_time_wait_until(odp_time_t time)
  return time_wait_until(time);
 }

-uint64_t odp_time_to_u64(odp_time_t time)
+static uint64_t time_to_u64_linux(odp_time_t time)
 {
  int ret;
  struct timespec tres;
@@ -189,18 +251,99 @@ uint64_t odp_time_to_u64(odp_time_t time)

  resolution = (uint64_t)tres.tv_nsec;

- return time_to_ns(time) / resolution;
+ return time_handler.time_to_ns(time) / resolution;
 }

-int odp_time_init_global(void)
+static uint64_t time_to_u64_dpdk(odp_time_t time)
 {
- int ret;
- _odp_time_t time;
+ return time.ticks;
+}

- ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
- start_time = ret ? ODP_TIME_NULL : time.ex;
+uint64_t odp_time_to_u64(odp_time_t time)
+{
+ return time_handler.time_to_u64(time);
+}
+
+
+static bool
+is_invariant_tsc_supported(void)
+{
+ FILE  *file;
+ char  *line         = NULL;
+ size_t len          = 0;
+ bool   nonstop_tsc  = false;
+ bool   constant_tsc = false;
+ bool   ret = false;
+
+ file = fopen("/proc/cpuinfo", "rt");
+ while (getline(&line, &len, file) != -1) {
+ if (strstr(line, "flags") !=NULL) {
+ if (strstr(line, "constant_tsc") !=NULL) {
+ constant_tsc = true;
+ }
+ if (strstr(line, "nonstop_tsc") != NULL) {
+ nonstop_tsc = true;
+ }
+ if (constant_tsc && nonstop_tsc) {
+ ret = true;
+ } else {
+ ret = false;
+ }
+ free(line);
+ fclose(file);
+ return ret;
+ }
+ }
+ free(line);
+ fclose(file);
+ return false;
+}
+
+static inline bool
+is_dpdk_timer_cycles_support(void)
+{
+ if (is_invariant_tsc_supported()==true) {
+ return true;
+ }
+ #ifdef RTE_LIBEAL_USE_HPET
+ if (rte_eal_hpet_init(1) == 0) {
+ return true;
+ }
+ #endif
+ return false;
+}

- return ret;
+int odp_time_init_global(void)
+{
+ if (is_dpdk_timer_cycles_support()) {
+ time_handler.time_to_ns         = time_to_ns_dpdk;
+ time_handler.time_diff          = time_diff_dpdk;
+ time_handler.time_curr          = time_curr_dpdk;
+ time_handler.time_cmp           = time_cmp_dpdk;
+ time_handler.time_sum           = time_sum_dpdk;
+ time_handler.time_local_from_ns = time_local_from_ns_dpdk;
+ time_handler.time_local_res     = time_local_res_dpdk;
+ time_handler.time_to_u64        = time_to_u64_dpdk;
+ } else {
+ time_handler.time_to_ns         = time_to_ns_linux;
+ time_handler.time_diff          = time_diff_linux;
+ time_handler.time_curr          = time_curr_linux;
+ time_handler.time_cmp           = time_cmp_linux;
+ time_handler.time_sum           = time_sum_linux;
+ time_handler.time_local_from_ns = time_local_from_ns_linux;
+ time_handler.time_local_res     = time_local_res_linux;
+ time_handler.time_to_u64        = time_to_u64_linux;
+ }
+
+ start_time = time_handler.time_curr();
+ odp_time_t zero_time;
+ zero_time = ODP_TIME_NULL;
+ if (time_handler.time_cmp(start_time, ODP_) == 0) {
+ ODP_ABORT("initiate odp time failed\n");
+ return -1;
+ } else {
+   return 0;
+ }
 }

 int odp_time_term_global(void)

On 11 April 2016 at 20:58, Wenxian Li <wenxian.li@linaro.org> wrote:

> Thanks Petri & Zoltan.

> Let me update the diff.

>

> Thanks,

> Wenxian

>

> On 11 April 2016 at 20:19, Savolainen, Petri (Nokia - FI/Espoo) <

> petri.savolainen@nokia.com> wrote:

>

>> RDTSC counter may not be monotonic in all systems. Maybe some extra CPU

>> flag check could be done to make sure that it is not affected by CPU

>> frequency scaling. When RDTSC is not “invariant”,  DPDK seems to just warn

>> and sample RDTSC for 1 sec get some estimate of the counter frequency. See

>> more comments under.

>>

>>

>>

>>

>>

>>

>>

>> *From:* lng-odp-dpdk [mailto:lng-odp-dpdk-bounces@lists.linaro.org] *On

>> Behalf Of *EXT Wenxian Li

>> *Sent:* Thursday, April 07, 2016 2:41 PM

>> *To:* Zoltan Kiss <zoltan.kiss@linaro.org>

>> *Cc:* lng-odp-dpdk@lists.linaro.org

>> *Subject:* [lng-odp-dpdk] Change of odp_time_local implementation in

>> odp-dpdk

>>

>>

>>

>> Hi Zoltan,

>>

>>

>>

>> Regarding the odp_time_local api, I made some change leveraging some DPDK

>> api to avoiding system call.

>>

>>

>>

>> I have use my change in my project, which benefits the performance.

>>

>> Below is my diff.

>>

>>

>>

>> Could you help review it? Any comments would be appreciated.

>>

>>

>>

>> Thanks,

>>

>> Wenxian

>>

>>

>>

>> diff --git a/platform/linux-generic/odp_time.c

>> b/platform/linux-generic/odp_time.c

>>

>> index ca8a955..565ce93 100644

>>

>> --- a/platform/linux-generic/odp_time.c

>>

>> +++ b/platform/linux-generic/odp_time.c

>>

>> @@ -10,6 +10,7 @@

>>

>>  #include <odp/time.h>

>>

>>  #include <odp/hints.h>

>>

>>  #include <odp_debug_internal.h>

>>

>> +#include <rte_cycles.h>

>>

>>

>>

>>  typedef union {

>>

>>           odp_time_t      ex;

>>

>> @@ -17,6 +18,8 @@ typedef union {

>>

>>  } _odp_time_t;

>>

>>

>>

>>  static odp_time_t start_time;

>>

>> +static inline odp_time_t time_local_from_ns(uint64_t ns);

>>

>> +static inline uint64_t time_local_res(void);

>>

>>

>>

>>  static inline

>>

>>  uint64_t time_to_ns(odp_time_t time)

>>

>> @@ -46,14 +49,10 @@ static inline odp_time_t time_diff(odp_time_t t2,

>> odp_time_t t1)

>>

>>

>>

>>  static inline odp_time_t time_local(void)

>>

>>  {

>>

>> -         int ret;

>>

>> -         _odp_time_t time;

>>

>> -

>>

>> -         ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);

>>

>> -         if (odp_unlikely(ret != 0))

>>

>> -                     ODP_ABORT("clock_gettime failed\n");

>>

>> -

>>

>> -         return time_diff(time.ex, start_time);

>>

>> +    uint64_t ns = ((double)rte_rdtsc()/(double)time_local_res())

>>

>> +                  *ODP_TIME_SEC_IN_NS;

>>

>> +    odp_time_t time = time_local_from_ns(ns);

>>

>> +    return time_diff(time, start_time);

>>

>>

>>

>> ODP API is defined so that implementation can avoid these kind of (float)

>> division operations per timestamp operation. This function should just read

>> time stamp and store it into _*odp_time*_t (e.g. add a u64 bit field for

>> that). Conversion to nsec should be done only when needed (in from/to_ns

>> functions).

>>

>>

>>

>> So for best performance, all functions should be re-written to use RDTSC

>> values, instead of conversions between timespec and RDTSC counter value.

>>

>>

>>

>>  }

>>

>>

>>

>>  static inline int time_cmp(odp_time_t t2, odp_time_t t1)

>>

>> @@ -103,14 +102,7 @@ static inline void time_wait_until(odp_time_t time)

>>

>>

>>

>>  static inline uint64_t time_local_res(void)

>>

>>  {

>>

>> -         int ret;

>>

>> -         struct timespec tres;

>>

>> -

>>

>> -         ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);

>>

>> -         if (odp_unlikely(ret != 0))

>>

>> -                     ODP_ABORT("clock_getres failed\n");

>>

>> -

>>

>> -         return ODP_TIME_SEC_IN_NS / (uint64_t)tres.tv_nsec;

>>

>> +    return rte_get_tsc_hz();

>>

>>  }

>>

>>

>>

>>  odp_time_t odp_time_local(void)

>>

>> @@ -179,28 +171,14 @@ void odp_time_wait_until(odp_time_t time)

>>

>>

>>

>>  uint64_t odp_time_to_u64(odp_time_t time)

>>

>>  {

>>

>> -         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");

>>

>> -

>>

>> -         resolution = (uint64_t)tres.tv_nsec;

>>

>> -

>>

>> -         return time_to_ns(time) / resolution;

>>

>> +    return time_to_ns(time) * time_local_res() / ODP_TIME_SEC_IN_NS;

>>

>>

>>

>> When u64 rdtsc value is stored into _*odp_time*_t, this would be simply

>>

>> return time.u64;

>>

>>

>>

>>  }

>>

>>

>>

>>  int odp_time_init_global(void)

>>

>>  {

>>

>> -         int ret;

>>

>> -         _odp_time_t time;

>>

>> -

>>

>> -         ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);

>>

>> -         start_time = ret ? ODP_TIME_NULL : time.ex;

>>

>> -

>>

>> -         return ret;

>>

>> +    start_time = ODP_TIME_NULL;

>>

>> +    start_time = odp_time_local();

>>

>> +    return 0;

>>

>>

>>

>> Since RDTSC counters of various cores may run out of sync, some check

>> would be needed (in init phase) if RDTSC can be used for global time stamp.

>> For example, if  frequency scaling affects the value, counters will soon be

>> out of sync after boot.

>>

>>

>>

>>

>>

>>

>>

>> -Petri

>>

>>

>>

>>

>>

>>

>>

>>  }

>>

>>

>>

>>  int odp_time_term_global(void)

>>

>

>

Patch hide | download patch | download mbox

diff --git a/platform/linux-dpdk/include/odp/plat/time_types.h
b/platform/linux-dpdk/include/odp/plat/time_types.h
deleted file mode 120000
index 31d16ad..0000000
--- a/platform/linux-dpdk/include/odp/plat/time_types.h
+++ /dev/null
@@ -1 +0,0 @@ 
-../../../../linux-generic/include/odp/plat/time_types.h
\ No newline at end of file
diff --git a/platform/linux-dpdk/include/odp/plat/time_types.h
b/platform/linux-dpdk/include/odp/plat/time_types.h
new file mode 100644
index 0000000..00a28e5
--- /dev/null
+++ b/platform/linux-dpdk/include/odp/plat/time_types.h
@@ -0,0 +1,69 @@ 
+/* Copyright (c) 2015, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP time service
+ */
+
+#ifndef ODP_TIME_TYPES_H_
+#define ODP_TIME_TYPES_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @addtogroup odp_time
+ *  @{
+ **/
+
+/**
+ * @internal Time structure used to isolate linux-dpdk implementation from
+ * the linux timespec structure, which is dependent on POSIX extension
level.
+ */
+typedef union {
+    struct {
+        uint64_t ticks;  /**< @internal ticks */
+        uint64_t pad;
+    };
+    struct {
+        int64_t tv_sec;      /**< @internal Seconds */
+        int64_t tv_nsec;     /**< @internal Nanoseconds */
+    };
+} odp_time_t;
+
+#define ODP_TIME_NULL ((odp_time_t){{0}})
+
+typedef uint64_t (*time_to_ns_fn) (odp_time_t time);
+typedef odp_time_t (*time_diff_fn) (odp_time_t t2, odp_time_t t1);
+typedef odp_time_t (*time_curr_fn)(void);
+typedef int (*time_cmp_fn) (odp_time_t t2, odp_time_t t1);
+typedef odp_time_t (*time_sum_fn) (odp_time_t t1, odp_time_t t2);
+typedef odp_time_t (*time_local_from_ns_fn) (uint64_t ns);
+typedef uint64_t (*time_local_res_fn)(void);
+typedef uint64_t (*time_to_u64_fn) (odp_time_t time);
+
+typedef struct time_handler_ {
+    time_to_ns_fn         time_to_ns;
+    time_diff_fn          time_diff;
+    time_curr_fn          time_curr;
+    time_cmp_fn           time_cmp;
+    time_sum_fn           time_sum;
+    time_local_from_ns_fn time_local_from_ns;
+    time_local_res_fn     time_local_res;
+    time_to_u64_fn        time_to_u64;
+} time_handler_t;
+
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux-dpdk/odp_init.c
index 833621d..1d01aea 100644
--- a/platform/linux-dpdk/odp_init.c
+++ b/platform/linux-dpdk/odp_init.c
@@ -246,12 +246,6 @@  int odp_init_global(const odp_init_t *params,
  odp_global_data.abort_fn = params->abort_fn;
  }

- if (odp_time_init_global()) {
- ODP_ERR("ODP time init failed.\n");
- goto init_failed;
- }
- stage = TIME_INIT;
-
  if (odp_system_info_init()) {
  ODP_ERR("ODP system_info init failed.\n");
  goto init_failed;
@@ -263,6 +257,12 @@  int odp_init_global(const odp_init_t *params,
  return -1;
  }

+ if (odp_time_init_global()) {
+ ODP_ERR("ODP time init failed.\n");
+ goto init_failed;
+ }
+ stage = TIME_INIT;
+
  if (odp_shm_init_global()) {
  ODP_ERR("ODP shm init failed.\n");
  goto init_failed;
diff --git a/platform/linux-dpdk/odp_time.c b/platform/linux-dpdk/odp_time.c
index ca8a955..a7f885b 100644
--- a/platform/linux-dpdk/odp_time.c
+++ b/platform/linux-dpdk/odp_time.c
@@ -10,6 +10,8 @@ 
 #include <odp/time.h>
 #include <odp/hints.h>
 #include <odp_debug_internal.h>
+#include <rte_cycles.h>
+#include <string.h>

 typedef union {
  odp_time_t      ex;
@@ -17,9 +19,15 @@  typedef union {
 } _odp_time_t;

 static odp_time_t start_time;
+static time_handler_t time_handler;
+
+static inline uint64_t time_local_res_dpdk(void)
+{
+ return rte_get_timer_hz();
+}

 static inline
-uint64_t time_to_ns(odp_time_t time)
+uint64_t time_to_ns_linux(odp_time_t time)
 {
  uint64_t ns;

@@ -29,7 +37,14 @@  uint64_t time_to_ns(odp_time_t time)
  return ns;
 }

-static inline odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
+static inline
+uint64_t time_to_ns_dpdk(odp_time_t time)
+{
+ return (time.ticks*ODP_TIME_SEC_IN_NS/time_local_res_dpdk());
+}
+
+
+static inline odp_time_t time_diff_linux(odp_time_t t2, odp_time_t t1)
 {
  odp_time_t time;

@@ -44,19 +59,45 @@  static inline odp_time_t time_diff(odp_time_t t2,
odp_time_t t1)
  return time;
 }

-static inline odp_time_t time_local(void)
+static inline odp_time_t time_diff_dpdk(odp_time_t t2, odp_time_t t1)
+{
+ odp_time_t time;
+ time.ticks = t2.ticks - t1.ticks;
+ return time;
+}
+
+
+static inline odp_time_t time_curr_linux(void)
 {
  int ret;
  _odp_time_t time;

  ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
- if (odp_unlikely(ret != 0))
+ if (odp_unlikely(ret != 0)) {
  ODP_ABORT("clock_gettime failed\n");
+ time.ex.tv_sec = 0;
+ time.ex.tv_nsec = 0;
+ }
+ return time.ex;
+}
+
+static inline odp_time_t time_curr_dpdk(void)
+{
+ odp_time_t time;
+ time.ticks = rte_get_timer_cycles();
+ return time;
+}

- return time_diff(time.ex, start_time);
+static inline odp_time_t time_local(void)
+{
+ odp_time_t time;
+ time = time_handler.time_curr();
+ return time_handler.time_diff(time, start_time);
 }

-static inline int time_cmp(odp_time_t t2, odp_time_t t1)
+
+
+static inline int time_cmp_linux(odp_time_t t2, odp_time_t t1)
 {
  if (t2.tv_sec < t1.tv_sec)