On 25/04/2017, 14:32, "Savolainen, Petri (Nokia - FI/Espoo)" <petri.savolai...@nokia-bell-labs.com> wrote:
> > >> -----Original Message----- >> From: Ola Liljedahl [mailto:ola.liljed...@arm.com] >> Sent: Tuesday, April 25, 2017 1:56 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- >> labs.com>; Brian Brooks <brian.bro...@arm.com>; lng-odp@lists.linaro.org >> Cc: nd <n...@arm.com> >> Subject: Re: [lng-odp] [PATCH] test: odp_sched_latency: robust draining >>of >> queues >> >> Another thing. >> >> >> On 25/04/2017, 12:26, "Savolainen, Petri (Nokia - FI/Espoo)" >> <petri.savolai...@nokia-bell-labs.com> wrote: >> >> > >> > >> >> -----Original Message----- >> >> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of >> >>Brian >> >> Brooks >> >> Sent: Monday, April 24, 2017 11:59 PM >> >> To: lng-odp@lists.linaro.org >> >> Cc: Ola Liljedahl <ola.liljed...@arm.com> >> >> Subject: [lng-odp] [PATCH] test: odp_sched_latency: robust draining >>of >> >> queues >> >> >> >> 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. >> >> >> > >> > odp_schedule_pause(); >> > >> > while (1) { >> > ev = odp_schedule(&src_queue, ODP_SCHED_NO_WAIT); >> > >> > if (ev == ODP_EVENT_INVALID) >> > break; >> > >> > if (odp_queue_enq(src_queue, ev)) { >> > LOG_ERR("[%i] Queue enqueue failed.\n", >> thr); >> > odp_event_free(ev); >> > return -1; >> > } >> > } >> > >> > odp_schedule_resume(); >> Is it good to call odp_schedule_resume() here? Isn¹t it legal and >>possible >> that the scheduler does or requests some speculative prescheduling in >>the >> resume call? Thus defying the schedule_pause and draining of >>prescheduled >> (stashed) events happening just before. > > >The loop above ensures that other threads proceed while this thread waits. > >Resume should not reserve a schedule context (do pre-scheduling), only >schedule() does reserve and free a context. The spec does not say this. /** * Resume scheduling * * Resume global scheduling for this thread. After this call, all schedule calls * will schedule normally (perform global scheduling). */ void odp_schedule_resume(void); “Resume scheduling” could easily be interpreted as allowing pre-scheduling or enabling some “global scheduler” to schedule events for this thread. I can easily imagine that when using a HW scheduler with some scheduling latency, one would always send a scheduling request in advance in order to hide the latency. The description for odp_schedule() is written as if pre-scheduling or stashing cannot occur. > The clear_sched_queues() loops as long as there are events and stops >when sees an EVENT_INVALID == no context. Yes I have added a loop calling odp_schedule() until it returns EVENT_INVALID. > >-Petri > > >> >> >> > >> > odp_barrier_wait(&globals->barrier); >> > >> > clear_sched_queues(); >> > >> > >> >What is the issue that this patch fixes? This sequence should be quite >> >robust already since no new enqueues happen after the barrier. In a >> >simple test code like this, the latency from last enq() (through the >> >barrier) to schedule loop (in clear_sched_queues()) could be overcome >> >just by not exiting after the first EVENT_INVALID from scheduler, but >> >after N EVENT_INVALIDs in a row. >> > >> >Also in your patch, thread should exit only after scheduler returns >> >EVENT_INVALID. >> > >> > >> >-Petri >> > >
