The term routines cover normal termination. That means the application is responsible for all "bookend" calls. If the application called odp_pool_create() then it is the application's responsibility to call odp_pool_destroy() as part of its normal termination path. As a result odp_pool_term_global() simply verifies that this has been done--it does not do this for the application.
This is consistent with all other term routines. On Wed, Feb 25, 2015 at 6:54 AM, Maxim Uvarov <[email protected]> wrote: > On 02/25/2015 02:03 PM, Bill Fischofer wrote: > >> odp_pool_create() issues an odp_shm_reserve() call. odp_pool_destroy() >> issues a corresponding odp_shm_free() call. What else is needed? Presumably >> any needed map/unmap calls are internal to the odp_shm_xxx( routines, no? >> > yes, odp_shm_free() frees shm, which is the same as > pool->s.pool_base_addr. But I still think that pool termination has to be > simple: > for (i = 0; i < ODP_CONFIG_POOLS; i++) { > pool = get_pool_entry(i); > ret += odp_pool_destroy(pool_index_to_handle(pool)); > } > return ret; > > The reasons are: > 1. That flag needed to be accounted: > if (!pool->s.flags.user_supplied_shm) > odp_shm_free(pool->s.pool_shm); > You can not free pool which was not automatically allocated. > > 2. Set to invalid: > pool->s.pool_shm = ODP_SHM_INVALID > > 3. No idea what should we do if there are still some packets. Probably > also fail to terminate pool. > /* Call fails if pool has allocated buffers */ > if (odp_atomic_load_u32(&pool->s.bufcount) < pool->s.buf_num) { > POOL_UNLOCK(&pool->s.lock); > return -1; > } > > Thanks, > Maxim. > > >> On Wed, Feb 25, 2015 at 3:27 AM, Maxim Uvarov <[email protected] >> <mailto:[email protected]>> wrote: >> >> On 02/25/2015 01:21 AM, Bill Fischofer wrote: >> >> Maxim, there's no need to do all that since odp_pool_create() >> handles a full re-init of the pool structures as part of >> (re)allocating it for another create operation. >> >> Actually, having vestigial info about past usage can be >> valuable for debugging core dumps. A pool is unallocated if >> pool->s.pool_shm == ODP_SHM_INVALID and that's the key >> variable that determines whether the rest of the items in the >> struct are active. >> >> Bill >> >> But at least we need unmap(pool->s.pool_base_addr), to prevent >> memory leaks right? >> >> Maxim. >> >> On Tue, Feb 24, 2015 at 7:19 AM, Maxim Uvarov >> <[email protected] <mailto:[email protected]> >> <mailto:[email protected] >> <mailto:[email protected]>>> wrote: >> >> On 02/23/2015 11:36 PM, Robbie King wrote: >> >> +int odp_pool_term_global(void) >> +{ >> + int i; >> + pool_entry_t *pool; >> + int ret = 0; >> + int rc = 0; >> + >> + for (i = 0; i < ODP_CONFIG_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 <http://s.name> <http://s.name>); >> + rc = -1; >> + } >> + POOL_UNLOCK(&pool->s.lock); >> + } >> >> what this for does? >> you should do: >> pool->s.name <http://s.name> <http://s.name>[0] = 0; >> pool->s.flags.has_name = 1; >> unmap(pool->s.pool_base_addr); >> >> Also I would like to zero everything that odp_pool_create() >> created. Including atomic counters. >> >> Thanks, >> Maxim. >> >> + >> + ret = >> odp_shm_free(odp_shm_lookup(SHM_DEFAULT_NAME)); >> + if (ret < 0) { >> + ODP_ERR("shm free failed for %s", >> SHM_DEFAULT_NAME); >> + rc = -1; >> + } >> + >> + return rc; >> +} >> + >> >> >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected]> >> <mailto:[email protected] >> <mailto:[email protected]>> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
