| Message ID | 1419021288-29268-3-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
Typo in the title (odp_schdule_one). Presumably Maxim can fix during merge? On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > example/timer/odp_timer_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/timer/odp_timer_test.c > b/example/timer/odp_timer_test.c > index 0d6e31a..6d2609a 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args) > while (1) { > odp_timeout_t tmo; > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > tmo = odp_timeout_from_buffer(buf); > tick = odp_timeout_tick(tmo); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 12/19/2014 11:37 PM, Bill Fischofer wrote: > Typo in the title (odp_schdule_one). Presumably Maxim can fix during > merge? > Yes, that is not the problem. Maxim. > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > --- > example/timer/odp_timer_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/timer/odp_timer_test.c > b/example/timer/odp_timer_test.c > index 0d6e31a..6d2609a 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, > test_args_t *args) > while (1) { > odp_timeout_t tmo; > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > tmo = odp_timeout_from_buffer(buf); > tick = odp_timeout_tick(tmo); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
I have found a problem with this patch (sorry Mike). The odp_timer_test example needs some more changes for the removal of odp_schedule_one() not to potentially hang the program. I have outlined this in a mail to Mike, hopefully he can provide an updated patch. On 19 December 2014 at 21:34, Mike Holmes <mike.holmes@linaro.org> wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > example/timer/odp_timer_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c > index 0d6e31a..6d2609a 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args) > while (1) { > odp_timeout_t tmo; > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > tmo = odp_timeout_from_buffer(buf); > tick = odp_timeout_tick(tmo); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
This is what you need to do in order to replace the per-thread
"remain" counter with a shared counter.
File scope:
+/** @private Number of timeouts to receive */
+static odp_atomic_u32_t remain;
In test_abs_timeouts():
- int remain = args->tmo_count;
- while (remain != 0) {
+ while ((int)odp_atomic_load_u32(&remain) > 0) {
- remain--;
+ odp_atomic_dec_u32(&remain);
In main() before creating the worker threads:
+ /* Initialize number of timeouts to receive */
+ odp_atomic_init_u32(&remain, args.tmo_count * num_workers);
Seems to work fine with odp_schedule() in my code (which uses the new
timer API, that's why I can't give you a proper patch). You can see
that timeout buffers are very unevenly scheduled to the different
threads.
On 19 December 2014 at 22:16, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
> I have found a problem with this patch (sorry Mike). The
> odp_timer_test example needs some more changes for the removal of
> odp_schedule_one() not to potentially hang the program. I have
> outlined this in a mail to Mike, hopefully he can provide an updated
> patch.
>
> On 19 December 2014 at 21:34, Mike Holmes <mike.holmes@linaro.org> wrote:
>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>> example/timer/odp_timer_test.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c
>> index 0d6e31a..6d2609a 100644
>> --- a/example/timer/odp_timer_test.c
>> +++ b/example/timer/odp_timer_test.c
>> @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args)
>> while (1) {
>> odp_timeout_t tmo;
>>
>> - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT);
>> + buf = odp_schedule(&queue, ODP_SCHED_WAIT);
>>
>> tmo = odp_timeout_from_buffer(buf);
>> tick = odp_timeout_tick(tmo);
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
These tests are admittedly artificial, but do we really want to encourage this style of programming? Threads are cheap so the notion that a thread would be in a scheduler loop and then decide to stop doing that and go off and do something else seems strange. Effectively isn't this trying to mix control and data plane sort of logic in the same thread? Worker threads should simply do work and not worry about doing anything else. In that case the question of whether the implementation is or is not using some sort of local scheduler cache is transparent. It's the same as with buffer pools. The fact that a local buffer cache may or may not be used by the implementation doesn't "bleed through" the ODP APIs. I'd think the ODP scheduling APIs would be cleaner and more portable if the same applied there. Threads/processes can terminate for any number of reasons, and a robust system needs to take that into account. In the case of buffer caches, for example, if a thread terminates then any locally cached buffers need to be flushed back to the pool from which they came. That's why there are hooks in odp_term_local() to do that. The scheduler should be no different. If a thread calls odp_term_local() then any "cached" scheduler items should similarly be rescheduled elsewhere without the application needing to be explicitly aware that this happened. On Mon, Dec 22, 2014 at 7:10 AM, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > > > Because the thread will exit the schedule loop, it has to pause first and > then run sched loop until the potential per thread local scheduler cache is > empty (see under). > > > > -Petri > > > > int done = 0; > > > > while (1) { > > odp_timeout_t tmo; > > > > if (done) > > buf = odp_schedule(&queue, ODP_SCHED_NO_WAIT); > > else > > buf = odp_schedule(&queue, > ODP_SCHED_WAIT); > > > > if (buf == ODP_BUFFER_INVALID) > > break; > > > > tmo = odp_timeout_from_buffer(buf); > > tick = odp_timeout_tick(tmo); > > > > EXAMPLE_DBG(" [%i] timeout, tick > %"PRIu64"\n", thr, tick); > > > > odp_buffer_free(buf); > > > > num--; > > > > if (num == 0) { > > odp_schedule_pause(); > > done = 1; > > continue; > > } > > > > tick += period; > > > > odp_timer_absolute_tmo(test_timer, tick, > > queue, > ODP_BUFFER_INVALID); > > } > > > > > > *From:* lng-odp-bounces@lists.linaro.org [mailto: > lng-odp-bounces@lists.linaro.org] *On Behalf Of *ext Bill Fischofer > *Sent:* Friday, December 19, 2014 10:38 PM > *To:* Mike Holmes > *Cc:* lng-odp-forward > *Subject:* Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove > use of odp_schdule_one > > > > Typo in the title (odp_schdule_one). Presumably Maxim can fix during > merge? > > > > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > example/timer/odp_timer_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/timer/odp_timer_test.c > b/example/timer/odp_timer_test.c > index 0d6e31a..6d2609a 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args) > while (1) { > odp_timeout_t tmo; > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > tmo = odp_timeout_from_buffer(buf); > tick = odp_timeout_tick(tmo); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 22 December 2014 at 14:10, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: > > > Because the thread will exit the schedule loop, it has to pause first and > then run sched loop until the potential per thread local scheduler cache is > empty (see under). In this specific case, the example terminates when the specified number of timeouts have been received and processed. There is no need to pause the scheduler and drain any prescheduled buffers because when the remain counter reaches zero, all threads are done and should terminate. This is a timer example, not a scheduler example. For a scheduler example, we should demonstrate the proper usage of odp_scheduler_pause() and resume. > > > > -Petri > > > > int done = 0; > > > > while (1) { > > odp_timeout_t tmo; > > > > if (done) > > buf = odp_schedule(&queue, ODP_SCHED_NO_WAIT); > > else > > buf = odp_schedule(&queue, > ODP_SCHED_WAIT); > > > > if (buf == ODP_BUFFER_INVALID) > > break; > > > > tmo = odp_timeout_from_buffer(buf); > > tick = odp_timeout_tick(tmo); > > > > EXAMPLE_DBG(" [%i] timeout, tick > %"PRIu64"\n", thr, tick); > > > > odp_buffer_free(buf); > > > > num--; > > > > if (num == 0) { > > odp_schedule_pause(); > > done = 1; > > continue; > > } > > > > tick += period; > > > > odp_timer_absolute_tmo(test_timer, tick, > > queue, > ODP_BUFFER_INVALID); > > } > > > > > > From: lng-odp-bounces@lists.linaro.org > [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext Bill Fischofer > Sent: Friday, December 19, 2014 10:38 PM > To: Mike Holmes > Cc: lng-odp-forward > Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove use of > odp_schdule_one > > > > Typo in the title (odp_schdule_one). Presumably Maxim can fix during merge? > > > > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > example/timer/odp_timer_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c > index 0d6e31a..6d2609a 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args) > while (1) { > odp_timeout_t tmo; > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > tmo = odp_timeout_from_buffer(buf); > tick = odp_timeout_tick(tmo); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
If the scheduler expects threads to process buffers indefinitely then perhaps that's what threads should do. ODP is constructing both a set of APIs as well as recommendations for how applications should be structured for optimal scalability and portability. Applications that choose not to follow these recommendations may incur additional overhead, but that's their choice. The notion that worker threads need to keep changing their mind about what their task is seems very strange. Beyond that, it's the responsibility of the ODP implementation to manage orderly thread termination in a graceful manner. If a thread calls odp_term_local() then any "prestaged" events need to be rescheduled to other threads by the implementation. It's unacceptable to deadlock in such situations as a result of resource leaks like this. On Tue, Dec 23, 2014 at 3:51 AM, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > > > > -----Original Message----- > > From: ext Ola Liljedahl [mailto:ola.liljedahl@linaro.org] > > Sent: Monday, December 22, 2014 4:23 PM > > To: Savolainen, Petri (NSN - FI/Espoo) > > Cc: ext Bill Fischofer; Mike Holmes; lng-odp-forward > > Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove use > > of odp_schdule_one > > > > On 22 December 2014 at 14:10, Savolainen, Petri (NSN - FI/Espoo) > > <petri.savolainen@nsn.com> wrote: > > > > > > > > > Because the thread will exit the schedule loop, it has to pause first > > and > > > then run sched loop until the potential per thread local scheduler > cache > > is > > > empty (see under). > > In this specific case, the example terminates when the specified > > number of timeouts have been received and processed. There is no need > > to pause the scheduler and drain any prescheduled buffers because when > > the remain counter reaches zero, all threads are done and should > > terminate. > > > > This is a timer example, not a scheduler example. For a scheduler > > example, we should demonstrate the proper usage of > > odp_scheduler_pause() and resume. > > > A throughput optimized scheduler may have pre-scheduled multiple buffers > (incl tmo notifications) to a thread local cache. Scheduler expects the > thread to continue process new buffers infinitely. Only way for application > to tell it's going to take a pause on processing those is > odp_schedule_pause() call. If application would not do that (and drain any > remaining buffers) those queues pre-scheduled to the thread would suffer a > long delay (or even deadlock if the thread exits). > > So it's an issue any thread need to handle before exiting the schedule > loop. Also in the test would hang if one thread exits and fails to process > all pre-scheduled timeouts. Other threads would wait infinitely those > missing tmos. > > > -Petri > > > > > > > > > > > > > > > > > > -Petri > > > > > > > > > > > > int done = 0; > > > > > > > > > > > > while (1) { > > > > > > odp_timeout_t tmo; > > > > > > > > > > > > if (done) > > > > > > buf = odp_schedule(&queue, ODP_SCHED_NO_WAIT); > > > > > > else > > > > > > buf = odp_schedule(&queue, > > > ODP_SCHED_WAIT); > > > > > > > > > > > > if (buf == ODP_BUFFER_INVALID) > > > > > > break; > > > > > > > > > > > > tmo = odp_timeout_from_buffer(buf); > > > > > > tick = odp_timeout_tick(tmo); > > > > > > > > > > > > EXAMPLE_DBG(" [%i] timeout, tick > > > %"PRIu64"\n", thr, tick); > > > > > > > > > > > > odp_buffer_free(buf); > > > > > > > > > > > > num--; > > > > > > > > > > > > if (num == 0) { > > > > > > odp_schedule_pause(); > > > > > > done = 1; > > > > > > continue; > > > > > > } > > > > > > > > > > > > tick += period; > > > > > > > > > > > > odp_timer_absolute_tmo(test_timer, tick, > > > > > > > > queue, > > > ODP_BUFFER_INVALID); > > > > > > } > > > > > > > > > > > > > > > > > > From: lng-odp-bounces@lists.linaro.org > > > [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext Bill > > Fischofer > > > Sent: Friday, December 19, 2014 10:38 PM > > > To: Mike Holmes > > > Cc: lng-odp-forward > > > Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove > > use of > > > odp_schdule_one > > > > > > > > > > > > Typo in the title (odp_schdule_one). Presumably Maxim can fix during > > merge? > > > > > > > > > > > > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> > > wrote: > > > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > > --- > > > example/timer/odp_timer_test.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/example/timer/odp_timer_test.c > > b/example/timer/odp_timer_test.c > > > index 0d6e31a..6d2609a 100644 > > > --- a/example/timer/odp_timer_test.c > > > +++ b/example/timer/odp_timer_test.c > > > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t > > *args) > > > while (1) { > > > odp_timeout_t tmo; > > > > > > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > > > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > > > > > > tmo = odp_timeout_from_buffer(buf); > > > tick = odp_timeout_tick(tmo); > > > -- > > > 2.1.0 > > > > > > > > > _______________________________________________ > > > lng-odp mailing list > > > lng-odp@lists.linaro.org > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > > > > _______________________________________________ > > > lng-odp mailing list > > > lng-odp@lists.linaro.org > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > >
On 23 December 2014 at 10:51, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: > > >> -----Original Message----- >> From: ext Ola Liljedahl [mailto:ola.liljedahl@linaro.org] >> Sent: Monday, December 22, 2014 4:23 PM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: ext Bill Fischofer; Mike Holmes; lng-odp-forward >> Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove use >> of odp_schdule_one >> >> On 22 December 2014 at 14:10, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolainen@nsn.com> wrote: >> > >> > >> > Because the thread will exit the schedule loop, it has to pause first >> and >> > then run sched loop until the potential per thread local scheduler cache >> is >> > empty (see under). >> In this specific case, the example terminates when the specified >> number of timeouts have been received and processed. There is no need >> to pause the scheduler and drain any prescheduled buffers because when >> the remain counter reaches zero, all threads are done and should >> terminate. >> >> This is a timer example, not a scheduler example. For a scheduler >> example, we should demonstrate the proper usage of >> odp_scheduler_pause() and resume. > > > A throughput optimized scheduler may have pre-scheduled multiple buffers (incl tmo notifications) to a thread local cache. Scheduler expects the thread to continue process new buffers infinitely. Only way for application to tell it's going to take a pause on processing those is odp_schedule_pause() call. If application would not do that (and drain any remaining buffers) those queues pre-scheduled to the thread would suffer a long delay (or even deadlock if the thread exits). > > So it's an issue any thread need to handle before exiting the schedule loop. Also in the test would hang if one thread exits and fails to process all pre-scheduled timeouts. Other threads would wait infinitely those missing tmos. I understand all of this and this is how a proper scheduler example should work. But this example attempts to be a simplified timer example. When the specified number of timeouts have been received, the worker threads and the whole program terminates (and this is definitively completely artificial from a networking perspective). Yes we don't don't do this in a way that's nice to the scheduler but this isn't really the scope of this example IMO. If the timer example can't just be a simplified timer example perhaps we should put a bullet in it? > > > -Petri > > >> >> >> > >> > >> > >> > -Petri >> > >> > >> > >> > int done = 0; >> > >> > >> > >> > while (1) { >> > >> > odp_timeout_t tmo; >> > >> > >> > >> > if (done) >> > >> > buf = odp_schedule(&queue, ODP_SCHED_NO_WAIT); >> > >> > else >> > >> > buf = odp_schedule(&queue, >> > ODP_SCHED_WAIT); >> > >> > >> > >> > if (buf == ODP_BUFFER_INVALID) >> > >> > break; >> > >> > >> > >> > tmo = odp_timeout_from_buffer(buf); >> > >> > tick = odp_timeout_tick(tmo); >> > >> > >> > >> > EXAMPLE_DBG(" [%i] timeout, tick >> > %"PRIu64"\n", thr, tick); >> > >> > >> > >> > odp_buffer_free(buf); >> > >> > >> > >> > num--; >> > >> > >> > >> > if (num == 0) { >> > >> > odp_schedule_pause(); >> > >> > done = 1; >> > >> > continue; >> > >> > } >> > >> > >> > >> > tick += period; >> > >> > >> > >> > odp_timer_absolute_tmo(test_timer, tick, >> > >> > >> queue, >> > ODP_BUFFER_INVALID); >> > >> > } >> > >> > >> > >> > >> > >> > From: lng-odp-bounces@lists.linaro.org >> > [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext Bill >> Fischofer >> > Sent: Friday, December 19, 2014 10:38 PM >> > To: Mike Holmes >> > Cc: lng-odp-forward >> > Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove >> use of >> > odp_schdule_one >> > >> > >> > >> > Typo in the title (odp_schdule_one). Presumably Maxim can fix during >> merge? >> > >> > >> > >> > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> > >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > --- >> > example/timer/odp_timer_test.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/example/timer/odp_timer_test.c >> b/example/timer/odp_timer_test.c >> > index 0d6e31a..6d2609a 100644 >> > --- a/example/timer/odp_timer_test.c >> > +++ b/example/timer/odp_timer_test.c >> > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t >> *args) >> > while (1) { >> > odp_timeout_t tmo; >> > >> > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); >> > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); >> > >> > tmo = odp_timeout_from_buffer(buf); >> > tick = odp_timeout_tick(tmo); >> > -- >> > 2.1.0 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >
Why can't this example be exactly how it's currently written provided that any needed scheduler cache cleanup is done as part of thread termination, as it should be. On Tue, Dec 23, 2014 at 7:17 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > > On 23 December 2014 at 10:51, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com> wrote: > > > > > >> -----Original Message----- > >> From: ext Ola Liljedahl [mailto:ola.liljedahl@linaro.org] > >> Sent: Monday, December 22, 2014 4:23 PM > >> To: Savolainen, Petri (NSN - FI/Espoo) > >> Cc: ext Bill Fischofer; Mike Holmes; lng-odp-forward > >> Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove > use > >> of odp_schdule_one > >> > >> On 22 December 2014 at 14:10, Savolainen, Petri (NSN - FI/Espoo) > >> <petri.savolainen@nsn.com> wrote: > >> > > >> > > >> > Because the thread will exit the schedule loop, it has to pause first > >> and > >> > then run sched loop until the potential per thread local scheduler > cache > >> is > >> > empty (see under). > >> In this specific case, the example terminates when the specified > >> number of timeouts have been received and processed. There is no need > >> to pause the scheduler and drain any prescheduled buffers because when > >> the remain counter reaches zero, all threads are done and should > >> terminate. > >> > >> This is a timer example, not a scheduler example. For a scheduler > >> example, we should demonstrate the proper usage of > >> odp_scheduler_pause() and resume. > > > > > > A throughput optimized scheduler may have pre-scheduled multiple buffers > (incl tmo notifications) to a thread local cache. Scheduler expects the > thread to continue process new buffers infinitely. Only way for application > to tell it's going to take a pause on processing those is > odp_schedule_pause() call. If application would not do that (and drain any > remaining buffers) those queues pre-scheduled to the thread would suffer a > long delay (or even deadlock if the thread exits). > > > > So it's an issue any thread need to handle before exiting the schedule > loop. Also in the test would hang if one thread exits and fails to process > all pre-scheduled timeouts. Other threads would wait infinitely those > missing tmos. > I understand all of this and this is how a proper scheduler example > should work. But this example attempts to be a simplified timer > example. When the specified number of timeouts have been received, the > worker threads and the whole program terminates (and this is > definitively completely artificial from a networking perspective). Yes > we don't don't do this in a way that's nice to the scheduler but this > isn't really the scope of this example IMO. > > If the timer example can't just be a simplified timer example perhaps > we should put a bullet in it? > > > > > > > -Petri > > > > > >> > >> > >> > > >> > > >> > > >> > -Petri > >> > > >> > > >> > > >> > int done = 0; > >> > > >> > > >> > > >> > while (1) { > >> > > >> > odp_timeout_t tmo; > >> > > >> > > >> > > >> > if (done) > >> > > >> > buf = odp_schedule(&queue, ODP_SCHED_NO_WAIT); > >> > > >> > else > >> > > >> > buf = > odp_schedule(&queue, > >> > ODP_SCHED_WAIT); > >> > > >> > > >> > > >> > if (buf == ODP_BUFFER_INVALID) > >> > > >> > break; > >> > > >> > > >> > > >> > tmo = odp_timeout_from_buffer(buf); > >> > > >> > tick = odp_timeout_tick(tmo); > >> > > >> > > >> > > >> > EXAMPLE_DBG(" [%i] timeout, tick > >> > %"PRIu64"\n", thr, tick); > >> > > >> > > >> > > >> > odp_buffer_free(buf); > >> > > >> > > >> > > >> > num--; > >> > > >> > > >> > > >> > if (num == 0) { > >> > > >> > odp_schedule_pause(); > >> > > >> > done = 1; > >> > > >> > continue; > >> > > >> > } > >> > > >> > > >> > > >> > tick += period; > >> > > >> > > >> > > >> > odp_timer_absolute_tmo(test_timer, tick, > >> > > >> > > >> queue, > >> > ODP_BUFFER_INVALID); > >> > > >> > } > >> > > >> > > >> > > >> > > >> > > >> > From: lng-odp-bounces@lists.linaro.org > >> > [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext Bill > >> Fischofer > >> > Sent: Friday, December 19, 2014 10:38 PM > >> > To: Mike Holmes > >> > Cc: lng-odp-forward > >> > Subject: Re: [lng-odp] [PATCH v2 2/3] example: odp_timer_test: remove > >> use of > >> > odp_schdule_one > >> > > >> > > >> > > >> > Typo in the title (odp_schdule_one). Presumably Maxim can fix during > >> merge? > >> > > >> > > >> > > >> > On Fri, Dec 19, 2014 at 2:34 PM, Mike Holmes <mike.holmes@linaro.org> > >> wrote: > >> > > >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > >> > --- > >> > example/timer/odp_timer_test.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/example/timer/odp_timer_test.c > >> b/example/timer/odp_timer_test.c > >> > index 0d6e31a..6d2609a 100644 > >> > --- a/example/timer/odp_timer_test.c > >> > +++ b/example/timer/odp_timer_test.c > >> > @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t > >> *args) > >> > while (1) { > >> > odp_timeout_t tmo; > >> > > >> > - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); > >> > + buf = odp_schedule(&queue, ODP_SCHED_WAIT); > >> > > >> > tmo = odp_timeout_from_buffer(buf); > >> > tick = odp_timeout_tick(tmo); > >> > -- > >> > 2.1.0 > >> > > >> > > >> > _______________________________________________ > >> > lng-odp mailing list > >> > lng-odp@lists.linaro.org > >> > http://lists.linaro.org/mailman/listinfo/lng-odp > >> > > >> > > >> > _______________________________________________ > >> > lng-odp mailing list > >> > lng-odp@lists.linaro.org > >> > http://lists.linaro.org/mailman/listinfo/lng-odp > >> > >
diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c index 0d6e31a..6d2609a 100644 --- a/example/timer/odp_timer_test.c +++ b/example/timer/odp_timer_test.c @@ -84,7 +84,7 @@ static void test_abs_timeouts(int thr, test_args_t *args) while (1) { odp_timeout_t tmo; - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT); + buf = odp_schedule(&queue, ODP_SCHED_WAIT); tmo = odp_timeout_from_buffer(buf); tick = odp_timeout_tick(tmo);
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- example/timer/odp_timer_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)