| Message ID | 1446675194-30401-1-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 4 November 2015 at 17:13, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > It's more convenient to pass parameters in order, like t2 - t1, > when t2 is supposed to be more. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > > The same should be done for CPU cycle API > > example/generator/odp_generator.c | 2 +- > include/odp/api/time.h | 4 ++-- > platform/linux-generic/odp_schedule.c | 2 +- > platform/linux-generic/odp_time.c | 2 +- > test/performance/odp_pktio_perf.c | 8 ++++---- > test/validation/pktio/pktio.c | 4 ++-- > test/validation/time/time.c | 4 ++-- > 7 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/example/generator/odp_generator.c > b/example/generator/odp_generator.c > index 60e015b..ae5d9dc 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -604,7 +604,7 @@ static void print_global_stats(int num_workers) > } > > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > if (odp_time_cycles_to_ns(diff) < > verbose_interval * ODP_TIME_SEC) { > continue; > diff --git a/include/odp/api/time.h b/include/odp/api/time.h > index b0072fc..4c7f5c2 100644 > --- a/include/odp/api/time.h > +++ b/include/odp/api/time.h > @@ -40,12 +40,12 @@ uint64_t odp_time_cycles(void); > /** > * Time difference > * > - * @param t1 First time stamp > * @param t2 Second time stamp > + * @param t1 First time stamp > * > * @return Difference of time stamps in CPU cycles > */ > -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); > +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1); > This could easily break apps by silently reversing the meaning, at a minimum the release notes really need to draw attention to this. > > > /** > diff --git a/platform/linux-generic/odp_schedule.c > b/platform/linux-generic/odp_schedule.c > index 6df8073..418b497 100644 > --- a/platform/linux-generic/odp_schedule.c > +++ b/platform/linux-generic/odp_schedule.c > @@ -608,7 +608,7 @@ static int schedule_loop(odp_queue_t *out_queue, > uint64_t wait, > } > > cycle = odp_time_cycles(); > - diff = odp_time_diff_cycles(start_cycle, cycle); > + diff = odp_time_diff_cycles(cycle, start_cycle); > > if (wait < diff) > break; > diff --git a/platform/linux-generic/odp_time.c > b/platform/linux-generic/odp_time.c > index abafd12..6b1aca5 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -19,7 +19,7 @@ uint64_t odp_time_cycles(void) > return odp_cpu_cycles(); > } > > -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2) > +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1) > { > return _odp_cpu_cycles_diff(t1, t2); > } > diff --git a/test/performance/odp_pktio_perf.c > b/test/performance/odp_pktio_perf.c > index efd26dc..16614bb 100644 > --- a/test/performance/odp_pktio_perf.c > +++ b/test/performance/odp_pktio_perf.c > @@ -336,11 +336,11 @@ static void *run_thread_tx(void *arg) > > cur_cycles = odp_time_cycles(); > start_cycles = cur_cycles; > - burst_start_cycles = odp_time_diff_cycles(burst_gap_cycles, > cur_cycles); > - while (odp_time_diff_cycles(start_cycles, cur_cycles) < > send_duration) { > + burst_start_cycles = odp_time_diff_cycles(cur_cycles, > burst_gap_cycles); > + while (odp_time_diff_cycles(cur_cycles, start_cycles) < > send_duration) { > unsigned alloc_cnt = 0, tx_cnt; > > - if (odp_time_diff_cycles(burst_start_cycles, cur_cycles) > + if (odp_time_diff_cycles(cur_cycles, burst_start_cycles) > < > burst_gap_cycles) { > cur_cycles = odp_time_cycles(); > if (idle_start == 0) > @@ -350,7 +350,7 @@ static void *run_thread_tx(void *arg) > > if (idle_start) { > stats->s.idle_cycles += odp_time_diff_cycles( > - idle_start, > cur_cycles); > + cur_cycles, > idle_start); > idle_start = 0; > } > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index ff62b3c..4a798e3 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -345,7 +345,7 @@ static odp_event_t queue_deq_wait_time(odp_queue_t > queue, uint64_t ns) > if (ev != ODP_EVENT_INVALID) > return ev; > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > } while (odp_time_cycles_to_ns(diff) < ns); > > return ODP_EVENT_INVALID; > @@ -389,7 +389,7 @@ static odp_packet_t wait_for_packet(pktio_info_t > *pktio_rx, > } > > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > } while (odp_time_cycles_to_ns(diff) < ns); > > CU_FAIL("failed to receive transmitted packet"); > diff --git a/test/validation/time/time.c b/test/validation/time/time.c > index 41db0e9..b3741ab 100644 > --- a/test/validation/time/time.c > +++ b/test/validation/time/time.c > @@ -27,7 +27,7 @@ void time_test_odp_cycles_diff(void) > cycles2 = odp_time_cycles(); > CU_ASSERT(cycles2 > cycles1); > > - diff = odp_time_diff_cycles(cycles1, cycles2); > + diff = odp_time_diff_cycles(cycles2, cycles1); > CU_ASSERT(diff > 0); > } > > @@ -38,7 +38,7 @@ void time_test_odp_cycles_negative_diff(void) > > cycles1 = 10; > cycles2 = 5; > - diff = odp_time_diff_cycles(cycles1, cycles2); > + diff = odp_time_diff_cycles(cycles2, cycles1); > CU_ASSERT(diff > 0); > } > > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
On 05.11.15 00:18, Mike Holmes wrote: > > > On 4 November 2015 at 17:13, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org <mailto:ivan.khoronzhuk@linaro.org>> wrote: > > It's more convenient to pass parameters in order, like t2 - t1, > when t2 is supposed to be more. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org <mailto:ivan.khoronzhuk@linaro.org>> > --- > > The same should be done for CPU cycle API > > example/generator/odp_generator.c | 2 +- > include/odp/api/time.h | 4 ++-- > platform/linux-generic/odp_schedule.c | 2 +- > platform/linux-generic/odp_time.c | 2 +- > test/performance/odp_pktio_perf.c | 8 ++++---- > test/validation/pktio/pktio.c | 4 ++-- > test/validation/time/time.c | 4 ++-- > 7 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c > index 60e015b..ae5d9dc 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -604,7 +604,7 @@ static void print_global_stats(int num_workers) > } > > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > if (odp_time_cycles_to_ns(diff) < > verbose_interval * ODP_TIME_SEC) { > continue; > diff --git a/include/odp/api/time.h b/include/odp/api/time.h > index b0072fc..4c7f5c2 100644 > --- a/include/odp/api/time.h > +++ b/include/odp/api/time.h > @@ -40,12 +40,12 @@ uint64_t odp_time_cycles(void); > /** > * Time difference > * > - * @param t1 First time stamp > * @param t2 Second time stamp > + * @param t1 First time stamp > * > * @return Difference of time stamps in CPU cycles > */ > -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); > +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1); > > This could easily break apps by silently reversing the meaning, at a minimum the release notes really need to draw attention to this. If it's OK to change, It can be changed in the same patch that replaces odp_time_diff_cycles() on odp_time_diff() in "avoid cycles" series. At least it can draw attention a little more. Just wanted to underline it in separate patch here. The same can be done for CPU cycle API. > > > > /** > diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c > index 6df8073..418b497 100644 > --- a/platform/linux-generic/odp_schedule.c > +++ b/platform/linux-generic/odp_schedule.c > @@ -608,7 +608,7 @@ static int schedule_loop(odp_queue_t *out_queue, uint64_t wait, > } > > cycle = odp_time_cycles(); > - diff = odp_time_diff_cycles(start_cycle, cycle); > + diff = odp_time_diff_cycles(cycle, start_cycle); > > if (wait < diff) > break; > diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c > index abafd12..6b1aca5 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -19,7 +19,7 @@ uint64_t odp_time_cycles(void) > return odp_cpu_cycles(); > } > > -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2) > +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1) > { > return _odp_cpu_cycles_diff(t1, t2); > } > diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c > index efd26dc..16614bb 100644 > --- a/test/performance/odp_pktio_perf.c > +++ b/test/performance/odp_pktio_perf.c > @@ -336,11 +336,11 @@ static void *run_thread_tx(void *arg) > > cur_cycles = odp_time_cycles(); > start_cycles = cur_cycles; > - burst_start_cycles = odp_time_diff_cycles(burst_gap_cycles, cur_cycles); > - while (odp_time_diff_cycles(start_cycles, cur_cycles) < send_duration) { > + burst_start_cycles = odp_time_diff_cycles(cur_cycles, burst_gap_cycles); > + while (odp_time_diff_cycles(cur_cycles, start_cycles) < send_duration) { > unsigned alloc_cnt = 0, tx_cnt; > > - if (odp_time_diff_cycles(burst_start_cycles, cur_cycles) > + if (odp_time_diff_cycles(cur_cycles, burst_start_cycles) > < burst_gap_cycles) { > cur_cycles = odp_time_cycles(); > if (idle_start == 0) > @@ -350,7 +350,7 @@ static void *run_thread_tx(void *arg) > > if (idle_start) { > stats->s.idle_cycles += odp_time_diff_cycles( > - idle_start, cur_cycles); > + cur_cycles, idle_start); > idle_start = 0; > } > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index ff62b3c..4a798e3 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -345,7 +345,7 @@ static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns) > if (ev != ODP_EVENT_INVALID) > return ev; > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > } while (odp_time_cycles_to_ns(diff) < ns); > > return ODP_EVENT_INVALID; > @@ -389,7 +389,7 @@ static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx, > } > > now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > + diff = odp_time_diff_cycles(now, start); > } while (odp_time_cycles_to_ns(diff) < ns); > > CU_FAIL("failed to receive transmitted packet"); > diff --git a/test/validation/time/time.c b/test/validation/time/time.c > index 41db0e9..b3741ab 100644 > --- a/test/validation/time/time.c > +++ b/test/validation/time/time.c > @@ -27,7 +27,7 @@ void time_test_odp_cycles_diff(void) > cycles2 = odp_time_cycles(); > CU_ASSERT(cycles2 > cycles1); > > - diff = odp_time_diff_cycles(cycles1, cycles2); > + diff = odp_time_diff_cycles(cycles2, cycles1); > CU_ASSERT(diff > 0); > } > > @@ -38,7 +38,7 @@ void time_test_odp_cycles_negative_diff(void) > > cycles1 = 10; > cycles2 = 5; > - diff = odp_time_diff_cycles(cycles1, cycles2); > + diff = odp_time_diff_cycles(cycles2, cycles1); > CU_ASSERT(diff > 0); > } > > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs > > __ > >
diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c index 60e015b..ae5d9dc 100644 --- a/example/generator/odp_generator.c +++ b/example/generator/odp_generator.c @@ -604,7 +604,7 @@ static void print_global_stats(int num_workers) } now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); + diff = odp_time_diff_cycles(now, start); if (odp_time_cycles_to_ns(diff) < verbose_interval * ODP_TIME_SEC) { continue; diff --git a/include/odp/api/time.h b/include/odp/api/time.h index b0072fc..4c7f5c2 100644 --- a/include/odp/api/time.h +++ b/include/odp/api/time.h @@ -40,12 +40,12 @@ uint64_t odp_time_cycles(void); /** * Time difference * - * @param t1 First time stamp * @param t2 Second time stamp + * @param t1 First time stamp * * @return Difference of time stamps in CPU cycles */ -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1); /** diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c index 6df8073..418b497 100644 --- a/platform/linux-generic/odp_schedule.c +++ b/platform/linux-generic/odp_schedule.c @@ -608,7 +608,7 @@ static int schedule_loop(odp_queue_t *out_queue, uint64_t wait, } cycle = odp_time_cycles(); - diff = odp_time_diff_cycles(start_cycle, cycle); + diff = odp_time_diff_cycles(cycle, start_cycle); if (wait < diff) break; diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c index abafd12..6b1aca5 100644 --- a/platform/linux-generic/odp_time.c +++ b/platform/linux-generic/odp_time.c @@ -19,7 +19,7 @@ uint64_t odp_time_cycles(void) return odp_cpu_cycles(); } -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2) +uint64_t odp_time_diff_cycles(uint64_t t2, uint64_t t1) { return _odp_cpu_cycles_diff(t1, t2); } diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c index efd26dc..16614bb 100644 --- a/test/performance/odp_pktio_perf.c +++ b/test/performance/odp_pktio_perf.c @@ -336,11 +336,11 @@ static void *run_thread_tx(void *arg) cur_cycles = odp_time_cycles(); start_cycles = cur_cycles; - burst_start_cycles = odp_time_diff_cycles(burst_gap_cycles, cur_cycles); - while (odp_time_diff_cycles(start_cycles, cur_cycles) < send_duration) { + burst_start_cycles = odp_time_diff_cycles(cur_cycles, burst_gap_cycles); + while (odp_time_diff_cycles(cur_cycles, start_cycles) < send_duration) { unsigned alloc_cnt = 0, tx_cnt; - if (odp_time_diff_cycles(burst_start_cycles, cur_cycles) + if (odp_time_diff_cycles(cur_cycles, burst_start_cycles) < burst_gap_cycles) { cur_cycles = odp_time_cycles(); if (idle_start == 0) @@ -350,7 +350,7 @@ static void *run_thread_tx(void *arg) if (idle_start) { stats->s.idle_cycles += odp_time_diff_cycles( - idle_start, cur_cycles); + cur_cycles, idle_start); idle_start = 0; } diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index ff62b3c..4a798e3 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -345,7 +345,7 @@ static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns) if (ev != ODP_EVENT_INVALID) return ev; now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); + diff = odp_time_diff_cycles(now, start); } while (odp_time_cycles_to_ns(diff) < ns); return ODP_EVENT_INVALID; @@ -389,7 +389,7 @@ static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx, } now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); + diff = odp_time_diff_cycles(now, start); } while (odp_time_cycles_to_ns(diff) < ns); CU_FAIL("failed to receive transmitted packet"); diff --git a/test/validation/time/time.c b/test/validation/time/time.c index 41db0e9..b3741ab 100644 --- a/test/validation/time/time.c +++ b/test/validation/time/time.c @@ -27,7 +27,7 @@ void time_test_odp_cycles_diff(void) cycles2 = odp_time_cycles(); CU_ASSERT(cycles2 > cycles1); - diff = odp_time_diff_cycles(cycles1, cycles2); + diff = odp_time_diff_cycles(cycles2, cycles1); CU_ASSERT(diff > 0); } @@ -38,7 +38,7 @@ void time_test_odp_cycles_negative_diff(void) cycles1 = 10; cycles2 = 5; - diff = odp_time_diff_cycles(cycles1, cycles2); + diff = odp_time_diff_cycles(cycles2, cycles1); CU_ASSERT(diff > 0); }
It's more convenient to pass parameters in order, like t2 - t1, when t2 is supposed to be more. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- The same should be done for CPU cycle API example/generator/odp_generator.c | 2 +- include/odp/api/time.h | 4 ++-- platform/linux-generic/odp_schedule.c | 2 +- platform/linux-generic/odp_time.c | 2 +- test/performance/odp_pktio_perf.c | 8 ++++---- test/validation/pktio/pktio.c | 4 ++-- test/validation/time/time.c | 4 ++-- 7 files changed, 13 insertions(+), 13 deletions(-)