diff mbox

[API-NEXT,v3,6/7] api: time: make odp_local_time to be monotonic wall time

Message ID 1448313655-7949-7-git-send-email-ivan.khoronzhuk@linaro.org
State New
Headers show

Commit Message

Ivan Khoronzhuk Nov. 23, 2015, 9:20 p.m. UTC 
It's more convenient the local time to be a monotonic wall time.
That means time starts from 0 and not wraps. It allows to use local
time in similar manner as it's supposed to be used with global time
and the 64-bit timer is enough to guarantee it.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 example/generator/odp_generator.c             | 10 ++---
 include/odp/api/time.h                        |  3 +-
 platform/linux-generic/include/odp_internal.h |  2 +
 platform/linux-generic/odp_schedule.c         |  9 ++--
 platform/linux-generic/odp_time.c             | 61 +++++++++++++++++++--------
 test/performance/odp_pktio_perf.c             | 26 ++++++------
 test/validation/pktio/pktio.c                 | 21 ++++-----
 7 files changed, 75 insertions(+), 57 deletions(-)

Comments

Ivan Khoronzhuk Nov. 30, 2015, 2:18 p.m. UTC | #1 
On 30.11.15 16:07, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>> Ivan Khoronzhuk
>> Sent: Monday, November 23, 2015 11:21 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [API-NEXT PATCH v3 6/7] api: time: make odp_local_time
>> to be monotonic wall time
>>
>> It's more convenient the local time to be a monotonic wall time.
>> That means time starts from 0 and not wraps. It allows to use local
>> time in similar manner as it's supposed to be used with global time
>> and the 64-bit timer is enough to guarantee it.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>
>
>
>
>> diff --git a/include/odp/api/time.h b/include/odp/api/time.h
>> index 50a0bf5..9865d81 100644
>> --- a/include/odp/api/time.h
>> +++ b/include/odp/api/time.h
>> @@ -45,7 +45,8 @@ extern "C" {
>>    * Current local time
>>    *
>>    * Returns current local time stamp value. The local time source provides
>> high
>> - * resolution time.
>> + * resolution time, it is initialized to zero during ODP startup and will
>> not
>> + * wrap around in at least 10 years.
>>    *
>>    * @return Local time stamp.
>>    */
>
>
> I think it's better to separate this the API spec into its own patch. All other changes here may or may not depend on it, but those can follow in the next patch. It's significant spec change and an implementer may miss it now.

Ok. I will split it on linux-generic + api and examples/testes patches.
But I think this simplification is not very eficient as api change patch will contain:
  include/odp/api/time.h                        |  3 +-
  platform/linux-generic/include/odp_internal.h |  2 +
  platform/linux-generic/odp_schedule.c         |  9 ++--
  platform/linux-generic/odp_time.c             | 61 +++++++++++++++++++--------

and example/test patch:
  example/generator/odp_generator.c             | 10 ++---
  test/performance/odp_pktio_perf.c             | 26 ++++++------
  test/validation/pktio/pktio.c                 | 21 ++++-----

>
> -Petri
>
>
>
>
>
>
Ivan Khoronzhuk Dec. 1, 2015, 8:17 a.m. UTC | #2 
On 01.12.15 09:38, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> Why not just have one patch for API ...
>
> include/odp/api/time.h                        |  3 +-
>
>
> ... and move everything else in another patch(es). The old implementation and usage of the API is still functionally correct after API change. It may not be optimal (does not exploit the knowledge that local time cannot wrap), but it's functionally correct. Or do you expect that CLOCK_MONOTONIC_RAW wraps?
>
>
> -Petri
Ok.

>
>
>> -----Original Message-----
>> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org]
>> Sent: Monday, November 30, 2015 4:19 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [API-NEXT PATCH v3 6/7] api: time: make
>> odp_local_time to be monotonic wall time
>>
>>
>>
>> On 30.11.15 16:07, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
>> EXT
>>>> Ivan Khoronzhuk
>>>> Sent: Monday, November 23, 2015 11:21 PM
>>>> To: lng-odp@lists.linaro.org
>>>> Subject: [lng-odp] [API-NEXT PATCH v3 6/7] api: time: make
>> odp_local_time
>>>> to be monotonic wall time
>>>>
>>>> It's more convenient the local time to be a monotonic wall time.
>>>> That means time starts from 0 and not wraps. It allows to use local
>>>> time in similar manner as it's supposed to be used with global time
>>>> and the 64-bit timer is enough to guarantee it.
>>>>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>>
>>>
>>>
>>>
>>>> diff --git a/include/odp/api/time.h b/include/odp/api/time.h
>>>> index 50a0bf5..9865d81 100644
>>>> --- a/include/odp/api/time.h
>>>> +++ b/include/odp/api/time.h
>>>> @@ -45,7 +45,8 @@ extern "C" {
>>>>     * Current local time
>>>>     *
>>>>     * Returns current local time stamp value. The local time source
>> provides
>>>> high
>>>> - * resolution time.
>>>> + * resolution time, it is initialized to zero during ODP startup and
>> will
>>>> not
>>>> + * wrap around in at least 10 years.
>>>>     *
>>>>     * @return Local time stamp.
>>>>     */
>>>
>>>
>>> I think it's better to separate this the API spec into its own patch.
>> All other changes here may or may not depend on it, but those can follow
>> in the next patch. It's significant spec change and an implementer may
>> miss it now.
>>
>> Ok. I will split it on linux-generic + api and examples/testes patches.
>> But I think this simplification is not very eficient as api change patch
>> will contain:
>>    include/odp/api/time.h                        |  3 +-
>>    platform/linux-generic/include/odp_internal.h |  2 +
>>    platform/linux-generic/odp_schedule.c         |  9 ++--
>>    platform/linux-generic/odp_time.c             | 61 +++++++++++++++++++--
>> ------
>>
>> and example/test patch:
>>    example/generator/odp_generator.c             | 10 ++---
>>    test/performance/odp_pktio_perf.c             | 26 ++++++------
>>    test/validation/pktio/pktio.c                 | 21 ++++-----
>>
>>>
>>> -Petri
>>>
>>>
>>>
>>>
>>>
>>>
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
diff mbox

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index 9d79f89..2de530d 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -586,7 +586,7 @@  static void *gen_recv_thread(void *arg)
  */
 static void print_global_stats(int num_workers)
 {
-	odp_time_t start, wait, diff;
+	odp_time_t cur, wait, next;
 	uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
 	int verbose_interval = 20;
 	odp_thrmask_t thrd_mask;
@@ -595,7 +595,7 @@  static void print_global_stats(int num_workers)
 		continue;
 
 	wait = odp_time_local_from_ns(verbose_interval * ODP_TIME_SEC_IN_NS);
-	start = odp_time_local();
+	next = odp_time_sum(odp_time_local(), wait);
 
 	while (odp_thrmask_worker(&thrd_mask) == num_workers) {
 		if (args->appl.number != -1 &&
@@ -604,11 +604,11 @@  static void print_global_stats(int num_workers)
 			break;
 		}
 
-		diff = odp_time_diff(odp_time_local(), start);
-		if (odp_time_cmp(wait, diff) > 0)
+		cur = odp_time_local();
+		if (odp_time_cmp(next, cur) > 0)
 			continue;
 
-		start = odp_time_local();
+		next = odp_time_sum(cur, wait);
 
 		if (args->appl.mode == APPL_MODE_RCV) {
 			pkts = odp_atomic_load_u64(&counters.udp);
diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index 50a0bf5..9865d81 100644
--- a/include/odp/api/time.h
+++ b/include/odp/api/time.h
@@ -45,7 +45,8 @@  extern "C" {
  * Current local time
  *
  * Returns current local time stamp value. The local time source provides high
- * resolution time.
+ * resolution time, it is initialized to zero during ODP startup and will not
+ * wrap around in at least 10 years.
  *
  * @return Local time stamp.
  */
diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h
index 14ba159..baff135 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -79,6 +79,8 @@  int odp_schedule_term_local(void);
 int odp_timer_init_global(void);
 int odp_timer_disarm_all(void);
 
+int odp_time_local_init(void);
+
 void _odp_flush_caches(void);
 
 #ifdef __cplusplus
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index 96b3ac5..3017876 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -586,7 +586,7 @@  static int schedule_loop(odp_queue_t *out_queue, uint64_t wait,
 			 odp_event_t out_ev[],
 			 unsigned int max_num, unsigned int max_deq)
 {
-	odp_time_t start_time, time, diff, wtime;
+	odp_time_t next, wtime;
 	int first = 1;
 	int ret;
 
@@ -604,15 +604,12 @@  static int schedule_loop(odp_queue_t *out_queue, uint64_t wait,
 
 		if (first) {
 			wtime = odp_time_local_from_ns(wait);
-			start_time = odp_time_local();
+			next = odp_time_sum(odp_time_local(), wtime);
 			first = 0;
 			continue;
 		}
 
-		time = odp_time_local();
-		diff = odp_time_diff(time, start_time);
-
-		if (odp_time_cmp(wtime, diff) < 0)
+		if (odp_time_cmp(next, odp_time_local()) < 0)
 			break;
 	}
 
diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c
index b725086..1aae88c 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -11,19 +11,21 @@ 
 #include <odp/hints.h>
 #include <odp_debug_internal.h>
 
-odp_time_t odp_time_local(void)
+static __thread struct timespec start_local;
+
+static inline
+uint64_t _odp_time_to_ns(odp_time_t val)
 {
-	int ret;
-	struct timespec time;
+	uint64_t ns;
 
-	ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
-	if (odp_unlikely(ret != 0))
-		ODP_ABORT("clock_gettime failed\n");
+	ns = val.tv_sec * ODP_TIME_SEC_IN_NS;
+	ns += val.tv_nsec;
 
-	return time;
+	return ns;
 }
 
-odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
+static inline
+odp_time_t _odp_time_diff(odp_time_t t2, odp_time_t t1)
 {
 	struct timespec time;
 
@@ -38,24 +40,36 @@  odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
 	return time;
 }
 
-uint64_t odp_time_to_ns(odp_time_t time)
+odp_time_t odp_time_local(void)
 {
-	uint64_t ns;
+	int ret;
+	struct timespec time;
+
+	ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
+	if (odp_unlikely(ret != 0))
+		ODP_ABORT("clock_gettime failed\n");
 
-	ns = time.tv_sec * ODP_TIME_SEC_IN_NS;
-	ns += time.tv_nsec;
+	return _odp_time_diff(time, start_local);
+}
 
-	return ns;
+odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
+{
+	return _odp_time_diff(t2, t1);
+}
+
+uint64_t odp_time_to_ns(odp_time_t time)
+{
+	return _odp_time_to_ns(time);
 }
 
 odp_time_t odp_time_local_from_ns(uint64_t ns)
 {
-	struct timespec time;
+	struct timespec val;
 
-	time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
-	time.tv_nsec = ns % ODP_TIME_SEC_IN_NS;
+	val.tv_sec = ns / ODP_TIME_SEC_IN_NS;
+	val.tv_nsec = ns % ODP_TIME_SEC_IN_NS;
 
-	return time;
+	return val;
 }
 
 int odp_time_cmp(odp_time_t t2, odp_time_t t1)
@@ -96,10 +110,21 @@  uint64_t odp_time_to_u64(odp_time_t time)
 
 	resolution = (uint64_t)tres.tv_nsec;
 
-	return odp_time_to_ns(time) / resolution;
+	return _odp_time_to_ns(time) / resolution;
 }
 
 odp_time_t odp_time_null(void)
 {
 	return (struct timespec) {0, 0};
 }
+
+int odp_time_local_init(void)
+{
+	int ret;
+	struct timespec time;
+
+	ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
+	start_local = ret ? odp_time_null() : time;
+
+	return ret;
+}
diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c
index 2e49f50..8737bc8 100644
--- a/test/performance/odp_pktio_perf.c
+++ b/test/performance/odp_pktio_perf.c
@@ -305,8 +305,8 @@  static void *run_thread_tx(void *arg)
 	int thr_id;
 	odp_queue_t outq;
 	pkt_tx_stats_t *stats;
-	odp_time_t start_time, cur_time, send_duration;
-	odp_time_t burst_start_time, burst_gap;
+	odp_time_t cur_time, send_time_end, send_duration;
+	odp_time_t burst_gap_end, burst_gap;
 	uint32_t batch_len;
 	int unsent_pkts = 0;
 	odp_event_t  tx_event[BATCH_LEN_MAX];
@@ -336,21 +336,19 @@  static void *run_thread_tx(void *arg)
 	odp_barrier_wait(&globals->tx_barrier);
 
 	cur_time     = odp_time_local();
-	start_time   = cur_time;
-	burst_start_time = odp_time_diff(cur_time, burst_gap);
-	while (odp_time_cmp(odp_time_diff(cur_time, start_time),
-			    send_duration) > 1) {
+	send_time_end = odp_time_sum(cur_time, send_duration);
+	burst_gap_end = cur_time;
+	while (odp_time_cmp(send_time_end, cur_time) > 0) {
 		unsigned alloc_cnt = 0, tx_cnt;
 
-		if (odp_time_cmp(odp_time_diff(cur_time, burst_start_time),
-				 burst_gap) > 1) {
+		if (odp_time_cmp(burst_gap_end, cur_time) > 0) {
 			cur_time = odp_time_local();
 			if (!odp_time_cmp(idle_start, ODP_TIME_NULL))
 				idle_start = cur_time;
 			continue;
 		}
 
-		if (odp_time_cmp(idle_start, ODP_TIME_NULL)) {
+		if (odp_time_cmp(idle_start, ODP_TIME_NULL) > 0) {
 			odp_time_t diff = odp_time_diff(cur_time, idle_start);
 
 			stats->s.idle_ticks =
@@ -359,7 +357,7 @@  static void *run_thread_tx(void *arg)
 			idle_start = ODP_TIME_NULL;
 		}
 
-		burst_start_time = odp_time_sum(burst_start_time, burst_gap);
+		burst_gap_end = odp_time_sum(burst_gap_end, burst_gap);
 
 		alloc_cnt = alloc_packets(tx_event, batch_len - unsent_pkts);
 		if (alloc_cnt != batch_len)
@@ -597,13 +595,13 @@  static int setup_txrx_masks(odp_cpumask_t *thd_mask_tx,
  */
 static void busy_loop_ns(uint64_t wait_ns)
 {
-	odp_time_t diff;
-	odp_time_t start_time = odp_time_local();
 	odp_time_t wait = odp_time_local_from_ns(wait_ns);
+	odp_time_t cur = odp_time_local();
+	odp_time_t end_time = odp_time_sum(cur, wait);
 
 	do {
-		diff = odp_time_diff(odp_time_local(), start_time);
-	} while (odp_time_cmp(wait, diff) > 0);
+		cur = odp_time_local();
+	} while (odp_time_cmp(end_time, cur) > 0);
 }
 
 /*
diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index 0a6592c..002c26b 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -336,18 +336,16 @@  static int destroy_inq(odp_pktio_t pktio)
 
 static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns)
 {
-	odp_time_t start, now, diff;
+	odp_time_t wait, end;
 	odp_event_t ev;
 
-	start = odp_time_local();
-
+	wait = odp_time_local_from_ns(ns);
+	end = odp_time_sum(odp_time_local(), wait);
 	do {
 		ev = odp_queue_deq(queue);
 		if (ev != ODP_EVENT_INVALID)
 			return ev;
-		now = odp_time_local();
-		diff = odp_time_diff(now, start);
-	} while (odp_time_to_ns(diff) < ns);
+	} while (odp_time_cmp(end, odp_time_local()) > 0);
 
 	return ODP_EVENT_INVALID;
 }
@@ -355,14 +353,14 @@  static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns)
 static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx,
 				    uint32_t seq, uint64_t ns)
 {
-	odp_time_t start, now, diff;
+	odp_time_t wait_time, end;
 	odp_event_t ev;
 	odp_packet_t pkt;
 	uint64_t wait;
 
-	start = odp_time_local();
 	wait = odp_schedule_wait_time(ns);
-
+	wait_time = odp_time_local_from_ns(ns);
+	end = odp_time_sum(odp_time_local(), wait_time);
 	do {
 		pkt = ODP_PACKET_INVALID;
 
@@ -388,10 +386,7 @@  static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx,
 
 			odp_packet_free(pkt);
 		}
-
-		now = odp_time_local();
-		diff = odp_time_diff(now, start);
-	} while (odp_time_to_ns(diff) < ns);
+	} while (odp_time_cmp(end, odp_time_local()) > 0);
 
 	CU_FAIL("failed to receive transmitted packet");