On 15.09.15 15:45, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Tuesday, September 15, 2015 3:08 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH v2 5/5] performance: odp_pktio_perf: fix
potential overflow for burst_gap
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.
For example:
burst_gap_cycles = 1k; // e.g. 1msec
cur_cycles = 1M; // wraps at 10M
burst_start_cycles = odp_time_diff_cycles(cur_cycles, burst_gap_cycles);
burst_start_cycles = 10M - 1M + 1k = 9 001 000
I've checked it some time ago and it was working, I remember I corrected this,
strange.
Seems I forgot it to swap.
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) {
This spins back to the top of the while loop until 'cur_cycles' has passed
'burst_start_cycles' by ' burst_gap_cycles'. So, in the example the first
packet is sent at cur_cycles == 9 002 000 cycles.
-Petri
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
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
