On 5 April 2017 at 00:26, Bill Fischofer <[email protected]> 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.
That's what the code did before. And it is followed by code which
assumes ODP_EVENT_INVALID can be returned.
buf = odp_buffer_from_event(ev);
if (!odp_buffer_is_valid(buf)) {
LOG_ERR(" [%i] Queue empty.\n", thr);
return -1;
}
Some similar calls, e.g. odp_packet_from_event(), explicitly checks
for ODP_EVENT_INVALID and returns ODP_PACKET_INVALID. As
odp_buffer_from_event() is just a static cast, an invalid input just
generates some invalid output (it might still be a undefined
operation).
Possibly we need to fix both the ODP implementation (no check for
invalid event handles because operations on those are anyway undefined
and invalid handles should not be treated as valid inputs) and test
programs which need to check for invalid event handles before passing
handles to other ODP functions.
>
>>
>>> Maxim.
>>>
>>> > + ev = odp_queue_deq(queue);
>>> > + if (ev != ODP_EVENT_INVALID)
>>> > + break;
>>> > + odp_cpu_pause();
>>> > + }
>>> >
>>> > buf = odp_buffer_from_event(ev);
>>> >
>>> >
>>>
