On 02/25/2015 04:02 PM, Bill Fischofer wrote:
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.
From that point of view patches are ok.
Maxim.
On Wed, Feb 25, 2015 at 6:54 AM, Maxim Uvarov <[email protected]
<mailto:[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]>
<mailto:[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]>>
<mailto:[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>
<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>
<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]>>
<mailto:[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
