| Message ID | 1418920838-17488-1-git-send-email-taras.kondratiuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 18, 2014 at 10:40 AM, Taras Kondratiuk < taras.kondratiuk@linaro.org> wrote: > > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > platform/linux-generic/include/odp_internal.h | 2 ++ > platform/linux-generic/odp_buffer_pool.c | 37 > +++++++++++++++++++++++++-- > platform/linux-generic/odp_init.c | 10 +++++++- > platform/linux-generic/odp_linux.c | 1 - > 4 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/include/odp_internal.h > b/platform/linux-generic/include/odp_internal.h > index 549d406..4fc8a5e 100644 > --- a/platform/linux-generic/include/odp_internal.h > +++ b/platform/linux-generic/include/odp_internal.h > @@ -29,6 +29,8 @@ int odp_shm_init_global(void); > int odp_shm_init_local(void); > > int odp_buffer_pool_init_global(void); > +int odp_buffer_pool_term_global(void); > +int odp_buffer_pool_term_local(void); > > int odp_pktio_init_global(void); > int odp_pktio_init_local(void); > diff --git a/platform/linux-generic/odp_buffer_pool.c > b/platform/linux-generic/odp_buffer_pool.c > index 48be24f..bb235ee 100644 > --- a/platform/linux-generic/odp_buffer_pool.c > +++ b/platform/linux-generic/odp_buffer_pool.c > @@ -55,6 +55,7 @@ typedef struct pool_table_t { > > /* The pool table */ > static pool_table_t *pool_tbl; > +static const char shm_name[] = "odp_buffer_pools"; > > /* Pool entry pointers (for inlining) */ > void *pool_entry_ptr[ODP_CONFIG_BUFFER_POOLS]; > @@ -67,7 +68,7 @@ int odp_buffer_pool_init_global(void) > uint32_t i; > odp_shm_t shm; > > - shm = odp_shm_reserve("odp_buffer_pools", > + shm = odp_shm_reserve(shm_name, > sizeof(pool_table_t), > sizeof(pool_entry_t), 0); > > @@ -95,10 +96,42 @@ int odp_buffer_pool_init_global(void) > return 0; > } > > +int odp_buffer_pool_term_global(void) > +{ > + odp_shm_t shm; > + int i; > + pool_entry_t *pool; > + int ret = 0; > + > + for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { > + pool = get_pool_entry(i); > + > + POOL_LOCK(&pool->s.lock); > + if (pool->s.pool_shm != ODP_SHM_INVALID) { > + ODP_ERR("Not destroyed pool: %s\n", pool->s.name); > + ret = -1; > + } > + POOL_UNLOCK(&pool->s.lock); > + } > + if (ret) > + return ret; > + > + shm = odp_shm_lookup(shm_name); > + if (shm == ODP_SHM_INVALID) > + return -1; > + ret = odp_shm_free(shm); > + > + return ret; > +} > + > +int odp_buffer_pool_term_local(void) > +{ > + _odp_flush_caches(); > + return 0; > +} > /** > * Buffer pool creation > */ > - > odp_buffer_pool_t odp_buffer_pool_create(const char *name, > odp_shm_t shm, > odp_buffer_pool_param_t *params) > diff --git a/platform/linux-generic/odp_init.c > b/platform/linux-generic/odp_init.c > index 7210430..80a3c19 100644 > --- a/platform/linux-generic/odp_init.c > +++ b/platform/linux-generic/odp_init.c > @@ -64,7 +64,10 @@ int odp_init_global(odp_init_t *params ODP_UNUSED, > > int odp_term_global(void) > { > - ODP_UNIMPLEMENTED(); > + if (odp_buffer_pool_term_global()) { > + ODP_ERR("ODP buffer pool term failed.\n"); > + return -1; > + } > return 0; > } > > @@ -90,5 +93,10 @@ int odp_init_local(void) > > int odp_term_local(void) > { > + if (odp_buffer_pool_term_local()) { > + ODP_ERR("ODP buffer pool local term failed.\n"); > + return -1; > + } > + > return (odp_thread_term_local() > 0) ? 1 : 0; > } > diff --git a/platform/linux-generic/odp_linux.c > b/platform/linux-generic/odp_linux.c > index 229d24e..92b8d53 100644 > --- a/platform/linux-generic/odp_linux.c > +++ b/platform/linux-generic/odp_linux.c > @@ -44,7 +44,6 @@ static void *odp_run_start_routine(void *arg) > } > > void *ret_ptr = start_args->start_routine(start_args->arg); > - _odp_flush_caches(); > int ret = odp_term_local(); > if (ret < 0) > ODP_ERR("Local term failed\n"); > -- > 1.9.1 > >
On 18 December 2014 at 17:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Thu, Dec 18, 2014 at 10:40 AM, Taras Kondratiuk < > taras.kondratiuk@linaro.org> wrote: >> >> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >> > > Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org> > This patch breaks the odp_init test, did you have a patch to repair the test ? Changes should never break regression without a fix being provided, but in this case as per our discussion on priority's earlier are hoping that someone else can take it from here ? > >> --- >> platform/linux-generic/include/odp_internal.h | 2 ++ >> platform/linux-generic/odp_buffer_pool.c | 37 >> +++++++++++++++++++++++++-- >> platform/linux-generic/odp_init.c | 10 +++++++- >> platform/linux-generic/odp_linux.c | 1 - >> 4 files changed, 46 insertions(+), 4 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp_internal.h >> b/platform/linux-generic/include/odp_internal.h >> index 549d406..4fc8a5e 100644 >> --- a/platform/linux-generic/include/odp_internal.h >> +++ b/platform/linux-generic/include/odp_internal.h >> @@ -29,6 +29,8 @@ int odp_shm_init_global(void); >> int odp_shm_init_local(void); >> >> int odp_buffer_pool_init_global(void); >> +int odp_buffer_pool_term_global(void); >> +int odp_buffer_pool_term_local(void); >> >> int odp_pktio_init_global(void); >> int odp_pktio_init_local(void); >> diff --git a/platform/linux-generic/odp_buffer_pool.c >> b/platform/linux-generic/odp_buffer_pool.c >> index 48be24f..bb235ee 100644 >> --- a/platform/linux-generic/odp_buffer_pool.c >> +++ b/platform/linux-generic/odp_buffer_pool.c >> @@ -55,6 +55,7 @@ typedef struct pool_table_t { >> >> /* The pool table */ >> static pool_table_t *pool_tbl; >> +static const char shm_name[] = "odp_buffer_pools"; >> >> /* Pool entry pointers (for inlining) */ >> void *pool_entry_ptr[ODP_CONFIG_BUFFER_POOLS]; >> @@ -67,7 +68,7 @@ int odp_buffer_pool_init_global(void) >> uint32_t i; >> odp_shm_t shm; >> >> - shm = odp_shm_reserve("odp_buffer_pools", >> + shm = odp_shm_reserve(shm_name, >> sizeof(pool_table_t), >> sizeof(pool_entry_t), 0); >> >> @@ -95,10 +96,42 @@ int odp_buffer_pool_init_global(void) >> return 0; >> } >> >> +int odp_buffer_pool_term_global(void) >> +{ >> + odp_shm_t shm; >> + int i; >> + pool_entry_t *pool; >> + int ret = 0; >> + >> + for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { >> + pool = get_pool_entry(i); >> + >> + POOL_LOCK(&pool->s.lock); >> + if (pool->s.pool_shm != ODP_SHM_INVALID) { >> + ODP_ERR("Not destroyed pool: %s\n", pool->s.name >> ); >> + ret = -1; >> + } >> + POOL_UNLOCK(&pool->s.lock); >> + } >> + if (ret) >> + return ret; >> + >> + shm = odp_shm_lookup(shm_name); >> + if (shm == ODP_SHM_INVALID) >> + return -1; >> + ret = odp_shm_free(shm); >> + >> + return ret; >> +} >> + >> +int odp_buffer_pool_term_local(void) >> +{ >> + _odp_flush_caches(); >> + return 0; >> +} >> /** >> * Buffer pool creation >> */ >> - >> odp_buffer_pool_t odp_buffer_pool_create(const char *name, >> odp_shm_t shm, >> odp_buffer_pool_param_t *params) >> diff --git a/platform/linux-generic/odp_init.c >> b/platform/linux-generic/odp_init.c >> index 7210430..80a3c19 100644 >> --- a/platform/linux-generic/odp_init.c >> +++ b/platform/linux-generic/odp_init.c >> @@ -64,7 +64,10 @@ int odp_init_global(odp_init_t *params ODP_UNUSED, >> >> int odp_term_global(void) >> { >> - ODP_UNIMPLEMENTED(); >> + if (odp_buffer_pool_term_global()) { >> + ODP_ERR("ODP buffer pool term failed.\n"); >> + return -1; >> + } >> return 0; >> } >> >> @@ -90,5 +93,10 @@ int odp_init_local(void) >> >> int odp_term_local(void) >> { >> + if (odp_buffer_pool_term_local()) { >> + ODP_ERR("ODP buffer pool local term failed.\n"); >> + return -1; >> + } >> + >> return (odp_thread_term_local() > 0) ? 1 : 0; >> } >> diff --git a/platform/linux-generic/odp_linux.c >> b/platform/linux-generic/odp_linux.c >> index 229d24e..92b8d53 100644 >> --- a/platform/linux-generic/odp_linux.c >> +++ b/platform/linux-generic/odp_linux.c >> @@ -44,7 +44,6 @@ static void *odp_run_start_routine(void *arg) >> } >> >> void *ret_ptr = start_args->start_routine(start_args->arg); >> - _odp_flush_caches(); >> int ret = odp_term_local(); >> if (ret < 0) >> ODP_ERR("Local term failed\n"); >> -- >> 1.9.1 >> >> > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
Good catch. The issue here is that odp_init_global() specifies a sequence of component initializers, and odp_term_global() must do the same in reverse but the others aren't ready yet. So odp_buffer_pool_term_global() must not be called prior to a bunch of other xxx_term_global() routines are called (which don't exist yet). Nothing wrong with odp_buffer_pool_term_global() as written, it's just that odp_term_global() can't call it until it's predecessor xxx_term_global() routines are written and tested/merged. The init sequence is: odp_shm_init_global() odp_thread_init_global() odp_buffer_pool_init_global() odp_queue_init_global() odp_schedule_init_global() odp_pktio_init_global() odp_crypto_init_global() odp_classification_init_global() which means that the sequence in odp_term_global() must be: odp_classification_term_global() odp_crypto_term_global() odp_pktio_term_global() odp_schedule_term_global() <-- This is the missing one the odp_init test trips over. odp_queue_term_global() odp_buffer_pool_term_global() odp_thread_term_global() odp_shm_term_global() On Thu, Dec 18, 2014 at 4:54 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > On 18 December 2014 at 17:27, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> >> >> On Thu, Dec 18, 2014 at 10:40 AM, Taras Kondratiuk < >> taras.kondratiuk@linaro.org> wrote: >>> >>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >>> >> >> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org> >> > > This patch breaks the odp_init test, did you have a patch to repair the > test ? > > Changes should never break regression without a fix being provided, but in > this case as per our discussion on priority's earlier are hoping that > someone else can take it from here ? > > >> >>> --- >>> platform/linux-generic/include/odp_internal.h | 2 ++ >>> platform/linux-generic/odp_buffer_pool.c | 37 >>> +++++++++++++++++++++++++-- >>> platform/linux-generic/odp_init.c | 10 +++++++- >>> platform/linux-generic/odp_linux.c | 1 - >>> 4 files changed, 46 insertions(+), 4 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/odp_internal.h >>> b/platform/linux-generic/include/odp_internal.h >>> index 549d406..4fc8a5e 100644 >>> --- a/platform/linux-generic/include/odp_internal.h >>> +++ b/platform/linux-generic/include/odp_internal.h >>> @@ -29,6 +29,8 @@ int odp_shm_init_global(void); >>> int odp_shm_init_local(void); >>> >>> int odp_buffer_pool_init_global(void); >>> +int odp_buffer_pool_term_global(void); >>> +int odp_buffer_pool_term_local(void); >>> >>> int odp_pktio_init_global(void); >>> int odp_pktio_init_local(void); >>> diff --git a/platform/linux-generic/odp_buffer_pool.c >>> b/platform/linux-generic/odp_buffer_pool.c >>> index 48be24f..bb235ee 100644 >>> --- a/platform/linux-generic/odp_buffer_pool.c >>> +++ b/platform/linux-generic/odp_buffer_pool.c >>> @@ -55,6 +55,7 @@ typedef struct pool_table_t { >>> >>> /* The pool table */ >>> static pool_table_t *pool_tbl; >>> +static const char shm_name[] = "odp_buffer_pools"; >>> >>> /* Pool entry pointers (for inlining) */ >>> void *pool_entry_ptr[ODP_CONFIG_BUFFER_POOLS]; >>> @@ -67,7 +68,7 @@ int odp_buffer_pool_init_global(void) >>> uint32_t i; >>> odp_shm_t shm; >>> >>> - shm = odp_shm_reserve("odp_buffer_pools", >>> + shm = odp_shm_reserve(shm_name, >>> sizeof(pool_table_t), >>> sizeof(pool_entry_t), 0); >>> >>> @@ -95,10 +96,42 @@ int odp_buffer_pool_init_global(void) >>> return 0; >>> } >>> >>> +int odp_buffer_pool_term_global(void) >>> +{ >>> + odp_shm_t shm; >>> + int i; >>> + pool_entry_t *pool; >>> + int ret = 0; >>> + >>> + for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { >>> + pool = get_pool_entry(i); >>> + >>> + POOL_LOCK(&pool->s.lock); >>> + if (pool->s.pool_shm != ODP_SHM_INVALID) { >>> + ODP_ERR("Not destroyed pool: %s\n", pool->s.name >>> ); >>> + ret = -1; >>> + } >>> + POOL_UNLOCK(&pool->s.lock); >>> + } >>> + if (ret) >>> + return ret; >>> + >>> + shm = odp_shm_lookup(shm_name); >>> + if (shm == ODP_SHM_INVALID) >>> + return -1; >>> + ret = odp_shm_free(shm); >>> + >>> + return ret; >>> +} >>> + >>> +int odp_buffer_pool_term_local(void) >>> +{ >>> + _odp_flush_caches(); >>> + return 0; >>> +} >>> /** >>> * Buffer pool creation >>> */ >>> - >>> odp_buffer_pool_t odp_buffer_pool_create(const char *name, >>> odp_shm_t shm, >>> odp_buffer_pool_param_t *params) >>> diff --git a/platform/linux-generic/odp_init.c >>> b/platform/linux-generic/odp_init.c >>> index 7210430..80a3c19 100644 >>> --- a/platform/linux-generic/odp_init.c >>> +++ b/platform/linux-generic/odp_init.c >>> @@ -64,7 +64,10 @@ int odp_init_global(odp_init_t *params ODP_UNUSED, >>> >>> int odp_term_global(void) >>> { >>> - ODP_UNIMPLEMENTED(); >>> + if (odp_buffer_pool_term_global()) { >>> + ODP_ERR("ODP buffer pool term failed.\n"); >>> + return -1; >>> + } >>> return 0; >>> } >>> >>> @@ -90,5 +93,10 @@ int odp_init_local(void) >>> >>> int odp_term_local(void) >>> { >>> + if (odp_buffer_pool_term_local()) { >>> + ODP_ERR("ODP buffer pool local term failed.\n"); >>> + return -1; >>> + } >>> + >>> return (odp_thread_term_local() > 0) ? 1 : 0; >>> } >>> diff --git a/platform/linux-generic/odp_linux.c >>> b/platform/linux-generic/odp_linux.c >>> index 229d24e..92b8d53 100644 >>> --- a/platform/linux-generic/odp_linux.c >>> +++ b/platform/linux-generic/odp_linux.c >>> @@ -44,7 +44,6 @@ static void *odp_run_start_routine(void *arg) >>> } >>> >>> void *ret_ptr = start_args->start_routine(start_args->arg); >>> - _odp_flush_caches(); >>> int ret = odp_term_local(); >>> if (ret < 0) >>> ODP_ERR("Local term failed\n"); >>> -- >>> 1.9.1 >>> >>> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP >
On 12/19/2014 01:20 AM, Bill Fischofer wrote: > Good catch. The issue here is that odp_init_global() specifies a > sequence of component initializers, and odp_term_global() must do the > same in reverse but the others aren't ready yet. So > odp_buffer_pool_term_global() must not be called prior to a bunch of > other xxx_term_global() routines are called (which don't exist yet). > Nothing wrong with odp_buffer_pool_term_global() as written, it's just > that odp_term_global() can't call it until it's predecessor > xxx_term_global() routines are written and tested/merged. > > The init sequence is: > odp_shm_init_global() > odp_thread_init_global() > odp_buffer_pool_init_global() > odp_queue_init_global() > odp_schedule_init_global() > odp_pktio_init_global() > odp_crypto_init_global() > odp_classification_init_global() > > which means that the sequence in odp_term_global() must be: > odp_classification_term_global() > odp_crypto_term_global() > odp_pktio_term_global() > odp_schedule_term_global() <-- This is the missing one the odp_init > test trips over. > odp_queue_term_global() > odp_buffer_pool_term_global() > odp_thread_term_global() > odp_shm_term_global() Right. Currently odp_buffer_pool_term_global() returns an error, because there are pending buffer pools at termination time. That is expected. But if we want to keep this test passing, then let's postpone this patch until other 'term' calls are introduced. Or we can keep ODP_ERR message, but remove error return from odp_buffer_pool_term_global().
On 19 December 2014 at 10:55, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 12/19/2014 01:20 AM, Bill Fischofer wrote: >> >> Good catch. The issue here is that odp_init_global() specifies a >> sequence of component initializers, and odp_term_global() must do the >> same in reverse but the others aren't ready yet. So >> odp_buffer_pool_term_global() must not be called prior to a bunch of >> other xxx_term_global() routines are called (which don't exist yet). >> Nothing wrong with odp_buffer_pool_term_global() as written, it's just >> that odp_term_global() can't call it until it's predecessor >> xxx_term_global() routines are written and tested/merged. >> >> The init sequence is: >> odp_shm_init_global() >> odp_thread_init_global() >> odp_buffer_pool_init_global() >> odp_queue_init_global() >> odp_schedule_init_global() >> odp_pktio_init_global() >> odp_crypto_init_global() >> odp_classification_init_global() >> >> which means that the sequence in odp_term_global() must be: >> odp_classification_term_global() >> odp_crypto_term_global() >> odp_pktio_term_global() >> odp_schedule_term_global() <-- This is the missing one the odp_init >> test trips over. >> odp_queue_term_global() >> odp_buffer_pool_term_global() >> odp_thread_term_global() >> odp_shm_term_global() > > > Right. Currently odp_buffer_pool_term_global() returns an error, > because there are pending buffer pools at termination time. That is > expected. But if we want to keep this test passing, then let's postpone > this patch until other 'term' calls are introduced. Or we can keep > ODP_ERR message, but remove error return from > odp_buffer_pool_term_global(). I think it is better to have the tests (for API't that we have specified) that fail rather than to remove such tests just to have only tests that pass. Failure is a fact of life, nothing wrong with that. Test logs with failed tests will constantly remind us of the missing or broken functionality. > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
I agree on the reminders, but would suggest that odp_term_global() be filled in with dummy calls in proper sequence so we can be reminded of all of the one's that still need to be filled in, as well as the sequence that they are needed in. On Fri, Dec 19, 2014 at 5:24 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > > On 19 December 2014 at 10:55, Taras Kondratiuk > <taras.kondratiuk@linaro.org> wrote: > > On 12/19/2014 01:20 AM, Bill Fischofer wrote: > >> > >> Good catch. The issue here is that odp_init_global() specifies a > >> sequence of component initializers, and odp_term_global() must do the > >> same in reverse but the others aren't ready yet. So > >> odp_buffer_pool_term_global() must not be called prior to a bunch of > >> other xxx_term_global() routines are called (which don't exist yet). > >> Nothing wrong with odp_buffer_pool_term_global() as written, it's just > >> that odp_term_global() can't call it until it's predecessor > >> xxx_term_global() routines are written and tested/merged. > >> > >> The init sequence is: > >> odp_shm_init_global() > >> odp_thread_init_global() > >> odp_buffer_pool_init_global() > >> odp_queue_init_global() > >> odp_schedule_init_global() > >> odp_pktio_init_global() > >> odp_crypto_init_global() > >> odp_classification_init_global() > >> > >> which means that the sequence in odp_term_global() must be: > >> odp_classification_term_global() > >> odp_crypto_term_global() > >> odp_pktio_term_global() > >> odp_schedule_term_global() <-- This is the missing one the odp_init > >> test trips over. > >> odp_queue_term_global() > >> odp_buffer_pool_term_global() > >> odp_thread_term_global() > >> odp_shm_term_global() > > > > > > Right. Currently odp_buffer_pool_term_global() returns an error, > > because there are pending buffer pools at termination time. That is > > expected. But if we want to keep this test passing, then let's postpone > > this patch until other 'term' calls are introduced. Or we can keep > > ODP_ERR message, but remove error return from > > odp_buffer_pool_term_global(). > I think it is better to have the tests (for API't that we have > specified) that fail rather than to remove such tests just to have > only tests that pass. > Failure is a fact of life, nothing wrong with that. Test logs with > failed tests will constantly remind us of the missing or broken > functionality. > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp >
We can XFAIL it in linux-generic temporarily if we take this patch in, please add that to the Makefile.am for linux-generic if we are headed that way. I think Yan is going to take a look at these termination functions. But this is a band aid until 1.0 only, nothing in linux-generic can be failing in 1.0 its self. On 19 December 2014 at 08:41, Bill Fischofer <bill.fischofer@linaro.org> wrote: > I agree on the reminders, but would suggest that odp_term_global() be > filled in with dummy calls in proper sequence so we can be reminded of all > of the one's that still need to be filled in, as well as the sequence that > they are needed in. > > On Fri, Dec 19, 2014 at 5:24 AM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: >> >> On 19 December 2014 at 10:55, Taras Kondratiuk >> <taras.kondratiuk@linaro.org> wrote: >> > On 12/19/2014 01:20 AM, Bill Fischofer wrote: >> >> >> >> Good catch. The issue here is that odp_init_global() specifies a >> >> sequence of component initializers, and odp_term_global() must do the >> >> same in reverse but the others aren't ready yet. So >> >> odp_buffer_pool_term_global() must not be called prior to a bunch of >> >> other xxx_term_global() routines are called (which don't exist yet). >> >> Nothing wrong with odp_buffer_pool_term_global() as written, it's just >> >> that odp_term_global() can't call it until it's predecessor >> >> xxx_term_global() routines are written and tested/merged. >> >> >> >> The init sequence is: >> >> odp_shm_init_global() >> >> odp_thread_init_global() >> >> odp_buffer_pool_init_global() >> >> odp_queue_init_global() >> >> odp_schedule_init_global() >> >> odp_pktio_init_global() >> >> odp_crypto_init_global() >> >> odp_classification_init_global() >> >> >> >> which means that the sequence in odp_term_global() must be: >> >> odp_classification_term_global() >> >> odp_crypto_term_global() >> >> odp_pktio_term_global() >> >> odp_schedule_term_global() <-- This is the missing one the odp_init >> >> test trips over. >> >> odp_queue_term_global() >> >> odp_buffer_pool_term_global() >> >> odp_thread_term_global() >> >> odp_shm_term_global() >> > >> > >> > Right. Currently odp_buffer_pool_term_global() returns an error, >> > because there are pending buffer pools at termination time. That is >> > expected. But if we want to keep this test passing, then let's postpone >> > this patch until other 'term' calls are introduced. Or we can keep >> > ODP_ERR message, but remove error return from >> > odp_buffer_pool_term_global(). >> I think it is better to have the tests (for API't that we have >> specified) that fail rather than to remove such tests just to have >> only tests that pass. >> Failure is a fact of life, nothing wrong with that. Test logs with >> failed tests will constantly remind us of the missing or broken >> functionality. >> >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >
diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h index 549d406..4fc8a5e 100644 --- a/platform/linux-generic/include/odp_internal.h +++ b/platform/linux-generic/include/odp_internal.h @@ -29,6 +29,8 @@ int odp_shm_init_global(void); int odp_shm_init_local(void); int odp_buffer_pool_init_global(void); +int odp_buffer_pool_term_global(void); +int odp_buffer_pool_term_local(void); int odp_pktio_init_global(void); int odp_pktio_init_local(void); diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c index 48be24f..bb235ee 100644 --- a/platform/linux-generic/odp_buffer_pool.c +++ b/platform/linux-generic/odp_buffer_pool.c @@ -55,6 +55,7 @@ typedef struct pool_table_t { /* The pool table */ static pool_table_t *pool_tbl; +static const char shm_name[] = "odp_buffer_pools"; /* Pool entry pointers (for inlining) */ void *pool_entry_ptr[ODP_CONFIG_BUFFER_POOLS]; @@ -67,7 +68,7 @@ int odp_buffer_pool_init_global(void) uint32_t i; odp_shm_t shm; - shm = odp_shm_reserve("odp_buffer_pools", + shm = odp_shm_reserve(shm_name, sizeof(pool_table_t), sizeof(pool_entry_t), 0); @@ -95,10 +96,42 @@ int odp_buffer_pool_init_global(void) return 0; } +int odp_buffer_pool_term_global(void) +{ + odp_shm_t shm; + int i; + pool_entry_t *pool; + int ret = 0; + + for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { + pool = get_pool_entry(i); + + POOL_LOCK(&pool->s.lock); + if (pool->s.pool_shm != ODP_SHM_INVALID) { + ODP_ERR("Not destroyed pool: %s\n", pool->s.name); + ret = -1; + } + POOL_UNLOCK(&pool->s.lock); + } + if (ret) + return ret; + + shm = odp_shm_lookup(shm_name); + if (shm == ODP_SHM_INVALID) + return -1; + ret = odp_shm_free(shm); + + return ret; +} + +int odp_buffer_pool_term_local(void) +{ + _odp_flush_caches(); + return 0; +} /** * Buffer pool creation */ - odp_buffer_pool_t odp_buffer_pool_create(const char *name, odp_shm_t shm, odp_buffer_pool_param_t *params) diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c index 7210430..80a3c19 100644 --- a/platform/linux-generic/odp_init.c +++ b/platform/linux-generic/odp_init.c @@ -64,7 +64,10 @@ int odp_init_global(odp_init_t *params ODP_UNUSED, int odp_term_global(void) { - ODP_UNIMPLEMENTED(); + if (odp_buffer_pool_term_global()) { + ODP_ERR("ODP buffer pool term failed.\n"); + return -1; + } return 0; } @@ -90,5 +93,10 @@ int odp_init_local(void) int odp_term_local(void) { + if (odp_buffer_pool_term_local()) { + ODP_ERR("ODP buffer pool local term failed.\n"); + return -1; + } + return (odp_thread_term_local() > 0) ? 1 : 0; } diff --git a/platform/linux-generic/odp_linux.c b/platform/linux-generic/odp_linux.c index 229d24e..92b8d53 100644 --- a/platform/linux-generic/odp_linux.c +++ b/platform/linux-generic/odp_linux.c @@ -44,7 +44,6 @@ static void *odp_run_start_routine(void *arg) } void *ret_ptr = start_args->start_routine(start_args->arg); - _odp_flush_caches(); int ret = odp_term_local(); if (ret < 0) ODP_ERR("Local term failed\n");
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> --- platform/linux-generic/include/odp_internal.h | 2 ++ platform/linux-generic/odp_buffer_pool.c | 37 +++++++++++++++++++++++++-- platform/linux-generic/odp_init.c | 10 +++++++- platform/linux-generic/odp_linux.c | 1 - 4 files changed, 46 insertions(+), 4 deletions(-)