1.Release context is just to be added for scalable scheduler in scheduler_test_groups(). I think it does no harms to other scheduler here. 2.This code is to be removed for the scalable scheduler, We use ring buffer to implement the queue. So it is possible the enqueue operation failed if the ring buffer is full.
Kevin 2017-04-07 16:22 GMT+08:00 Savolainen, Petri (Nokia - FI/Espoo) < [email protected]>: > > > > -----Original Message----- > > From: lng-odp [mailto:[email protected]] On Behalf Of > Kevin > > Wang > > Sent: Friday, April 07, 2017 11:07 AM > > To: [email protected] > > Cc: Kevin Wang <[email protected]> > > Subject: [lng-odp] [PATCH] validation: scheduler: Release context before > > the end of the scheduler test > > > > If the scheduler sync type is atomic or ordered, > > need to release the context. > > Release context is actually a hint. It does not guarantee that context is > released. Application needs to call schedule() and receive _EVENT_INVALID > to be sure that it does not hold a context anymore. > > > > > > Signed-off-by: Kevin Wang <[email protected]> > > Reviewed-by: Ola Liljedahl <[email protected]> > > --- > > .../common_plat/validation/api/scheduler/scheduler.c | 20 > +++++++++++---- > > ----- > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/test/common_plat/validation/api/scheduler/scheduler.c > > b/test/common_plat/validation/api/scheduler/scheduler.c > > index 952561c..2631001 100644 > > --- a/test/common_plat/validation/api/scheduler/scheduler.c > > +++ b/test/common_plat/validation/api/scheduler/scheduler.c > > @@ -129,6 +129,14 @@ static int exit_schedule_loop(void) > > return ret; > > } > > > > +static void release_context(odp_schedule_sync_t sync) > > +{ > > + if (sync == ODP_SCHED_SYNC_ATOMIC) > > + odp_schedule_release_atomic(); > > + else if (sync == ODP_SCHED_SYNC_ORDERED) > > + odp_schedule_release_ordered(); > > +} > > + > > void scheduler_test_wait_time(void) > > { > > int i; > > @@ -251,8 +259,7 @@ void scheduler_test_queue_destroy(void) > > CU_ASSERT_FATAL(u32[0] == MAGIC); > > > > odp_buffer_free(buf); > > - odp_schedule_release_ordered(); > > - > > + release_context(qp.sched.sync); > > CU_ASSERT_FATAL(odp_queue_destroy(queue) == 0); > > } > > > > @@ -478,6 +485,7 @@ void scheduler_test_groups(void) > > odp_schedule_group_leave(mygrp1, &mymask); > > odp_schedule_group_leave(mygrp2, &mymask); > > > > + release_context(qp.sched.sync); > > > Why this hint was added? Maybe a proper pause()/schedule() sequence is > actually missing here ? > > > > > /* Done with queues for this round */ > > CU_ASSERT_FATAL(odp_queue_destroy(queue_grp1) == 0); > > CU_ASSERT_FATAL(odp_queue_destroy(queue_grp2) == 0); > > @@ -820,12 +828,7 @@ static int schedule_common_(void *arg) > > } > > } > > > > - if (sync == ODP_SCHED_SYNC_ATOMIC) > > - odp_schedule_release_atomic(); > > - > > - if (sync == ODP_SCHED_SYNC_ORDERED) > > - odp_schedule_release_ordered(); > > - > > + release_context(sync); > > odp_ticketlock_lock(&globals->lock); > > > > globals->buf_count -= num; > > @@ -959,7 +962,6 @@ static void fill_queues(thread_args_t *args) > > } > > > > ret = odp_queue_enq(queue, ev); > > - CU_ASSERT_FATAL(ret == 0); > > > Why this line was removed? Is it bug in the test? Did you really meant to > remove the line? It does not belong under the subject line of this patch. > > -Petri > > > > > > if (ret) > > > > odp_buffer_free(buf); > > -- > > 2.7.4 > > -- Thanks, Kevin
