> -----Original Message-----
> From: EXT Ivan Khoronzhuk [mailto:[email protected]]
> Sent: Tuesday, September 15, 2015 3:08 PM
> To: Savolainen, Petri (Nokia - FI/Espoo); [email protected]
> 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:[email protected]] On Behalf Of
> >> EXT Ivan Khoronzhuk
> >> Sent: Friday, September 11, 2015 1:05 PM
> >> To: [email protected]
> >> 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 <[email protected]>
> >> ---
> >> 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
>
> >
> >
> >
> >
> >
> >> 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
> >> [email protected]
> >> https://lists.linaro.org/mailman/listinfo/lng-odp
>
> --
> Regards,
> Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
