| Message ID | 1422978253-7721-7-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 2015-02-03 16:44, Ola Liljedahl wrote: > Ensure we run at least one worker thread. > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > --- > (This document/code contribution attached is provided under the terms of > agreement LES-LTM-21309) > > test/validation/odp_timer.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c > index 5218406..1f673c0 100644 > --- a/test/validation/odp_timer.c > +++ b/test/validation/odp_timer.c > @@ -270,9 +270,13 @@ static void test_odp_timer_all(void) > { > odp_pool_param_t params; > odp_timer_pool_param_t tparam; > - /* This is a stressfull test - need to reserve some cpu cycles > - * @TODO move to test/performance */ > - int num_workers = min(odp_sys_cpu_count()-1, MAX_WORKERS); > + /* Reserve at least one core for running other processes so the timer > + * test hopefully can run undisturbed and thus get better timing > + * results. */ > + int num_workers = min(odp_sys_cpu_count() - 1, MAX_WORKERS); > + /* On a single-CPU machine run at least one thread */ > + if (num_workers < 1) > + num_workers = 1; > > /* Create timeout pools */ > params.tmo.num = (NTIMERS + 1) * num_workers; > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 3 February 2015 at 22:42, Anders Roxell <anders.roxell@linaro.org> wrote: > On 2015-02-03 16:44, Ola Liljedahl wrote: >> Ensure we run at least one worker thread. >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> Fabulous! Have you reviewed the whole patch set now (this was the last one)? If I had received all the review feedback from the start, I could have done one update and made the split into six separate patches then. That would have been more efficient use of *my* time (don't know about others). > >> --- >> (This document/code contribution attached is provided under the terms of >> agreement LES-LTM-21309) >> >> test/validation/odp_timer.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c >> index 5218406..1f673c0 100644 >> --- a/test/validation/odp_timer.c >> +++ b/test/validation/odp_timer.c >> @@ -270,9 +270,13 @@ static void test_odp_timer_all(void) >> { >> odp_pool_param_t params; >> odp_timer_pool_param_t tparam; >> - /* This is a stressfull test - need to reserve some cpu cycles >> - * @TODO move to test/performance */ >> - int num_workers = min(odp_sys_cpu_count()-1, MAX_WORKERS); >> + /* Reserve at least one core for running other processes so the timer >> + * test hopefully can run undisturbed and thus get better timing >> + * results. */ >> + int num_workers = min(odp_sys_cpu_count() - 1, MAX_WORKERS); >> + /* On a single-CPU machine run at least one thread */ >> + if (num_workers < 1) >> + num_workers = 1; >> >> /* Create timeout pools */ >> params.tmo.num = (NTIMERS + 1) * num_workers; >> -- >> 1.9.1 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
On 2015-02-03 23:09, Ola Liljedahl wrote: > On 3 February 2015 at 22:42, Anders Roxell <anders.roxell@linaro.org> wrote: > > On 2015-02-03 16:44, Ola Liljedahl wrote: > >> Ensure we run at least one worker thread. > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > Fabulous! Have you reviewed the whole patch set now (this was the last one)? > > If I had received all the review feedback from the start, I could have > done one update and made the split into six separate patches then. 1. If everything is in one patch its hard to see everything and do a through review. 2. For this patch set I did send additional review comments on some patches a couple of versions ago and they didn't get addressed. Cheers, Anders
On 4 February 2015 at 00:06, Anders Roxell <anders.roxell@linaro.org> wrote: > On 2015-02-03 23:09, Ola Liljedahl wrote: >> On 3 February 2015 at 22:42, Anders Roxell <anders.roxell@linaro.org> wrote: >> > On 2015-02-03 16:44, Ola Liljedahl wrote: >> >> Ensure we run at least one worker thread. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> > >> > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> >> Fabulous! Have you reviewed the whole patch set now (this was the last one)? >> >> If I had received all the review feedback from the start, I could have >> done one update and made the split into six separate patches then. > > 1. If everything is in one patch its hard to see everything and do a > through review. > 2. For this patch set I did send additional review comments on some patches a > couple of versions ago and they didn't get addressed. Link? > > Cheers, > Anders
This comment I believe: This change LOG_DBG belongs in this patch: "[PATCHv4 4/5] validation: odp_timer.c: cunit cleanup" I have taken care of it now. On 4 February 2015 at 10:50, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 4 February 2015 at 00:06, Anders Roxell <anders.roxell@linaro.org> wrote: >> On 2015-02-03 23:09, Ola Liljedahl wrote: >>> On 3 February 2015 at 22:42, Anders Roxell <anders.roxell@linaro.org> wrote: >>> > On 2015-02-03 16:44, Ola Liljedahl wrote: >>> >> Ensure we run at least one worker thread. >>> >> >>> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >>> > >>> > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> >>> Fabulous! Have you reviewed the whole patch set now (this was the last one)? >>> >>> If I had received all the review feedback from the start, I could have >>> done one update and made the split into six separate patches then. >> >> 1. If everything is in one patch its hard to see everything and do a >> through review. >> 2. For this patch set I did send additional review comments on some patches a >> couple of versions ago and they didn't get addressed. > Link? > >> >> Cheers, >> Anders
diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c index 5218406..1f673c0 100644 --- a/test/validation/odp_timer.c +++ b/test/validation/odp_timer.c @@ -270,9 +270,13 @@ static void test_odp_timer_all(void) { odp_pool_param_t params; odp_timer_pool_param_t tparam; - /* This is a stressfull test - need to reserve some cpu cycles - * @TODO move to test/performance */ - int num_workers = min(odp_sys_cpu_count()-1, MAX_WORKERS); + /* Reserve at least one core for running other processes so the timer + * test hopefully can run undisturbed and thus get better timing + * results. */ + int num_workers = min(odp_sys_cpu_count() - 1, MAX_WORKERS); + /* On a single-CPU machine run at least one thread */ + if (num_workers < 1) + num_workers = 1; /* Create timeout pools */ params.tmo.num = (NTIMERS + 1) * num_workers;
Ensure we run at least one worker thread. Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (This document/code contribution attached is provided under the terms of agreement LES-LTM-21309) test/validation/odp_timer.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)