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

Reply via email to