On Mon, Apr 24, 2017 at 3:58 PM, Brian Brooks <brian.bro...@arm.com> wrote:
> From: Ola Liljedahl <ola.liljed...@arm.com>
>
> In order to robustly drain all queues when the benchmark has
> ended, we enqueue a special event on every queue and invoke
> the scheduler until all such events have been received.
>
> Signed-off-by: Ola Liljedahl <ola.liljed...@arm.com>
> Reviewed-by: Brian Brooks <brian.bro...@arm.com>
> ---
> test/common_plat/performance/odp_sched_latency.c | 51
> ++++++++++++++++++------
> 1 file changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/test/common_plat/performance/odp_sched_latency.c
> b/test/common_plat/performance/odp_sched_latency.c
> index 2b28cd7b..7ba99fc6 100644
> --- a/test/common_plat/performance/odp_sched_latency.c
> +++ b/test/common_plat/performance/odp_sched_latency.c
> @@ -57,9 +57,10 @@ ODP_STATIC_ASSERT(LO_PRIO_QUEUES <= MAX_QUEUES, "Too
> many LO priority queues");
>
> /** Test event types */
> typedef enum {
> - WARM_UP, /**< Warm up event */
> - TRAFFIC, /**< Event used only as traffic load */
> - SAMPLE /**< Event used to measure latency */
> + WARM_UP, /**< Warm up event */
> + COOL_DOWN,/**< Last event on queue */
> + TRAFFIC, /**< Event used only as traffic load */
> + SAMPLE /**< Event used to measure latency */
> } event_type_t;
>
> /** Test event */
> @@ -114,16 +115,40 @@ typedef struct {
> *
> * Retry to be sure that all buffers have been scheduled.
> */
> -static void clear_sched_queues(void)
> +static void clear_sched_queues(test_globals_t *globals)
> {
> odp_event_t ev;
> + odp_buffer_t buf;
> + test_event_t *event;
> + int i, j;
> + uint32_t numtogo = 0;
>
int numtogo would be more standard here, and consistent with i, j
immediately above.
>
> - while (1) {
> - ev = odp_schedule(NULL, ODP_SCHED_NO_WAIT);
> -
> - if (ev == ODP_EVENT_INVALID)
> - break;
> -
> + /* Enqueue special cool_down events on all queues (one per queue)
> */
> + for (i = 0; i < NUM_PRIOS; i++) {
> + for (j = 0; j < globals->args.prio[i].queues; j++) {
> + buf = odp_buffer_alloc(globals->pool);
> + if (buf == ODP_BUFFER_INVALID) {
> + LOG_ERR("Buffer alloc failed.\n");
> + return;
> + }
>
This isn't terribly robust. In the unlikely event that this call fails
you're leaving a bunch of events (including the previously successfully
allocated COOL_DOWN markers) on the queues. At minimum you should just
break out of this "marking" phase and proceed to drain the remaining queues
for the numgoto markers already on them. More robust would be to
preallocate all the markers needed up front or else do each queue
individually to avoid having to buffer the markers.
> + event = odp_buffer_addr(buf);
> + event->type = COOL_DOWN;
> + ev = odp_buffer_to_event(buf);
> + if (odp_queue_enq(globals->queue[i][j], ev)) {
> + LOG_ERR("Queue enqueue failed.\n");
> + odp_event_free(ev);
> + return;
> + }
> + numtogo++;
> + }
> + }
> + /* Invoke scheduler until all cool_down events have been received
> */
> + while (numtogo != 0) {
>
while (numtogo > 0) would be more standard here.
> + ev = odp_schedule(NULL, ODP_SCHED_WAIT);
> + buf = odp_buffer_from_event(ev);
> + event = odp_buffer_addr(buf);
> + if (event->type == COOL_DOWN)
> + numtogo--;
> odp_event_free(ev);
> }
> }
> @@ -394,10 +419,10 @@ static int test_schedule(int thr, test_globals_t
> *globals)
>
> odp_barrier_wait(&globals->barrier);
>
> - clear_sched_queues();
> -
> - if (thr == MAIN_THREAD)
> + if (thr == MAIN_THREAD) {
> + clear_sched_queues(globals);
> print_results(globals);
> + }
>
> return 0;
> }
> --
> 2.12.2
>
>
