This can go into master/api-next as an independent patch. Agree?
On 4 April 2017 at 21:22, Brian Brooks <[email protected]> wrote: > On 04/04 17:26:12, Bill Fischofer wrote: >> On Tue, Apr 4, 2017 at 3:37 PM, Brian Brooks <[email protected]> wrote: >> > On 04/04 21:59:15, Maxim Uvarov wrote: >> >> On 04/04/17 21:47, Brian Brooks wrote: >> >> > Signed-off-by: Ola Liljedahl <[email protected]> >> >> > Reviewed-by: Brian Brooks <[email protected]> >> >> > Reviewed-by: Honnappa Nagarahalli <[email protected]> >> >> > Reviewed-by: Kevin Wang <[email protected]> >> >> > --- >> >> > test/common_plat/performance/odp_scheduling.c | 12 ++++++++++-- >> >> > 1 file changed, 10 insertions(+), 2 deletions(-) >> >> > >> >> > diff --git a/test/common_plat/performance/odp_scheduling.c >> >> > b/test/common_plat/performance/odp_scheduling.c >> >> > index c74a0713..38e76257 100644 >> >> > --- a/test/common_plat/performance/odp_scheduling.c >> >> > +++ b/test/common_plat/performance/odp_scheduling.c >> >> > @@ -273,7 +273,7 @@ static int test_plain_queue(int thr, test_globals_t >> >> > *globals) >> >> > test_message_t *t_msg; >> >> > odp_queue_t queue; >> >> > uint64_t c1, c2, cycles; >> >> > - int i; >> >> > + int i, j; >> >> > >> >> > /* Alloc test message */ >> >> > buf = odp_buffer_alloc(globals->pool); >> >> > @@ -307,7 +307,15 @@ static int test_plain_queue(int thr, >> >> > test_globals_t *globals) >> >> > return -1; >> >> > } >> >> > >> >> > - ev = odp_queue_deq(queue); >> >> > + /* When enqueue and dequeue are decoupled (e.g. not using a >> >> > + * common lock), an enqueued event may not be immediately >> >> > + * visible to dequeue. So we just try again for a while. */ >> >> > + for (j = 0; j < 100; j++) { >> >> >> >> where 100 number comes from? >> > >> > It is the retry count. Perhaps it could be a bit lower, or a bit higher, >> > but >> > it works well. >> >> Actually, it's incorrect. What happens if all 100 retries fail? You'll >> call odp_buffer_from_event() for ODP_EVENT_INVALID, which is >> undefined. > > Incorrect? :) The point is that an event may not be immediately available > to dequeue after it has been enqueued. This is due to the way that a > concurrent > ring buffer behaves in a multi-threaded environment. The approach here is > just to retry the dequeue a couple times (100 times actually) before moving > on to the rest of code. Perhaps 100 times is too many times, but some amount > of retry is needed. > > If this is not desirable, then I think it would be more accurate to consider > odp_queue_enq() / odp_queue_deq() as async APIs -or- MT-unsafe (must be called > from one thread at a time in order to ensure the behavior that an event is > immediately available for dequeue once it has been enqueued). > >> > >> >> Maxim. >> >> >> >> > + ev = odp_queue_deq(queue); >> >> > + if (ev != ODP_EVENT_INVALID) >> >> > + break; >> >> > + odp_cpu_pause(); >> >> > + } >> >> > >> >> > buf = odp_buffer_from_event(ev); >> >> > >> >> > >> >>
