On 15.09.15 14:42, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
EXT Ivan Khoronzhuk
Sent: Friday, September 11, 2015 1:05 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] [PATCH v2 5/5] performance: odp_pktio_perf: fix
potential overflow for burst_gap
The direct comparing of "cur_cycles" and "next_tx_cycles" is not
valid, as "next_tx_cycles" can be overflowed and comparison will
give wrong result. So use odp_time_diff_cycles() for that, as it
takes in account ticks overflow.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
test/performance/odp_pktio_perf.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/test/performance/odp_pktio_perf.c
b/test/performance/odp_pktio_perf.c
index ac32b15..85ef2bc 100644
--- a/test/performance/odp_pktio_perf.c
+++ b/test/performance/odp_pktio_perf.c
@@ -303,7 +303,7 @@ static void *run_thread_tx(void *arg)
int thr_id;
odp_queue_t outq;
pkt_tx_stats_t *stats;
- uint64_t next_tx_cycles, start_cycles, cur_cycles, send_duration;
+ uint64_t burst_start_cycles, start_cycles, cur_cycles,
send_duration;
uint64_t burst_gap_cycles;
uint32_t batch_len;
int unsent_pkts = 0;
@@ -334,11 +334,12 @@ static void *run_thread_tx(void *arg)
cur_cycles = odp_time_cycles();
start_cycles = cur_cycles;
- next_tx_cycles = cur_cycles;
+ burst_start_cycles = odp_time_diff_cycles(cur_cycles,
burst_gap_cycles);
Shouldn't this be:
burst_start_cycles = cur_cycles + burst_gap_cycles;
,which works as long as cycle count wraps at UINT64_MAX. Maybe we need a cpu.h
API for summing two values with correct wrapping.
It's initialization for burst gap, which is changing while send_duration.
Current algorithm don't wait "burst gap" at first iteration, my intention to
not change it.
So I've used diff, in another case it waits one init gap.
In case of cur_cycles + burst_gap_cycles it waits 2 x burst_gap. So it's not
correct.
I suppose here shouldn't be functional changes.
The cpu API doesn't have sum function and is not for this case, we need time
here, that is
Time API is indented for. The new Time API is going to be added after this
series and will
contain sum function which will replace "+" on odp_time_sum(). Current API supposes that
"+"
correctly handles UINT64_MAX wrap and doesn't contain sum function.
while (odp_time_diff_cycles(start_cycles, cur_cycles) <
send_duration) {
unsigned alloc_cnt = 0, tx_cnt;
- if (cur_cycles < next_tx_cycles) {
+ if (odp_time_diff_cycles(burst_start_cycles, cur_cycles)
+ < burst_gap_cycles) {
cur_cycles = odp_time_cycles();
if (idle_start == 0)
idle_start = cur_cycles;
@@ -351,7 +352,7 @@ static void *run_thread_tx(void *arg)
idle_start = 0;
}
- next_tx_cycles += burst_gap_cycles;
+ burst_start_cycles += burst_gap_cycles;
alloc_cnt = alloc_packets(tx_event, batch_len -
unsent_pkts);
if (alloc_cnt != batch_len)
--
1.9.1
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
