On 13 November 2014 04:59, Taras Kondratiuk <[email protected]>
wrote:
> On 11/13/2014 01:22 AM, Bill Fischofer wrote:
>
> [snip]
>
> -odp_buffer_pool_t odp_buffer_pool_create(const char *name,
>> - void *base_addr, uint64_t size,
>> - size_t buf_size, size_t
>> buf_align,
>> - int buf_type)
>> -{
>> - odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID;
>> - pool_entry_t *pool;
>> - uint32_t i;
>> + /* found free pool */
>> + size_t block_size, mdata_size, udata_size;
>>
>> - for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) {
>> - pool = get_pool_entry(i);
>> + strncpy(pool->s.name, name,
>> + ODP_BUFFER_POOL_NAME_LEN - 1);
>> + pool->s.name[ODP_BUFFER_POOL_NAME_LEN - 1] = 0;
>>
>> - LOCK(&pool->s.lock);
>> + pool->s.params = *params;
>> + pool->s.init_params = *init_params;
>>
>> - if (pool->s.buf_base == 0) {
>> - /* found free pool */
>> + mdata_size = params->buf_num * buf_stride;
>> + udata_size = params->buf_num * udata_stride;
>> + block_size = params->buf_num * blk_size;
>>
>> - strncpy(pool->s.name, name,
>> - ODP_BUFFER_POOL_NAME_LEN - 1);
>> - pool->s.name[ODP_BUFFER_POOL_NAME_LEN - 1] = 0;
>> - pool->s.pool_base_addr = base_addr;
>> - pool->s.pool_size = size;
>> - pool->s.user_size = buf_size;
>> - pool->s.user_align = buf_align;
>> - pool->s.buf_type = buf_type;
>> + pool->s.pool_size = ODP_PAGE_SIZE_ROUNDUP(mdata_size +
>> + udata_size +
>> + block_size);
>>
>> - link_bufs(pool);
>> + pool->s.shm = odp_shm_reserve(pool->s.name,
>> pool->s.pool_size,
>> + ODP_PAGE_SIZE, 0);
>>
>> + if (pool->s.shm == ODP_SHM_INVALID) {
>> UNLOCK(&pool->s.lock);
>> -
>> - pool_hdl = pool->s.pool_hdl;
>> - break;
>> + return ODP_BUFFER_POOL_INVALID;
>> }
>>
>> + /* Now safe to unlock since pool entry has been allocated
>> */
>> UNLOCK(&pool->s.lock);
>> +
>> + pool->s.pool_base_addr = (uint8_t
>> *)odp_shm_addr(pool->s.shm);
>> + pool->s.flags.unsegmented = unsegmented;
>> + pool->s.seg_size = unsegmented ?
>> + blk_size : ODP_CONFIG_BUF_SEG_SIZE;
>> + uint8_t *udata_base_addr = pool->s.pool_base_addr +
>> mdata_size;
>> + uint8_t *block_base_addr = udata_base_addr + udata_size;
>> +
>> + /* bufcount will decrement down to 0 as we populate
>> freelist */
>> + pool->s.bufcount = params->buf_num;
>> + pool->s.buf_stride = buf_stride;
>> + pool->s.udata_stride = udata_stride;
>> + pool->s.high_wm = 0;
>> + pool->s.low_wm = 0;
>> + pool->s.headroom = 0;
>> + pool->s.tailroom = 0;
>> + pool->s.buf_freelist = NULL;
>> + pool->s.blk_freelist = NULL;
>> +
>> + uint8_t *buf = pool->s.udata_base_addr - buf_stride;
>> + uint8_t *udat = (udata_stride == 0) ? NULL :
>> + block_base_addr - udata_stride;
>>
>
> Maybe my previous 3 comments about this issue were not clear.
> What happens here:
> - pool->s.udata_base_addr is not initialized. Let's say it is 0.
> - buf = pool->s.udata_base_addr - buf_stride => buf = -buf_stride
> - Then buf is dereferenced => segfault.
>
> +
>> + /* Init buffer common header and add to pool buffer
>> freelist */
>> + do {
>> + odp_buffer_hdr_t *tmp =
>> + (odp_buffer_hdr_t *)(void *)buf;
>> +
>> + /* Initialize buffer metadata */
>> + tmp->buf_hdl.handle =
>> odp_buffer_encode_handle(tmp);
>>
>
> In this version you have reordered tmp fields initialization which caused
> one more issue.
>
> odp_buffer_encode_handle() uses tmp->pool_hdl inside which is
> initialized four lines below => segfault.
>
> + tmp->size = 0;
>> + tmp->ref_count = 0;
>> + tmp->type = params->buf_type;
>> + tmp->pool_hdl = pool->s.pool_hdl;
>> + tmp->udata_addr = (void *)udat;
>> + tmp->udata_size = init_params->udata_size;
>> + tmp->segcount = 0;
>> + tmp->segsize = pool->s.seg_size;
>> +
>>
>
> This is not RFC series anymore, so I would expect that at least basic
> testing was done with available tests like odp_example or at least
> odp_init. On my setup all series including this one fail on
> odp_init_global().
>
>
>
Bill I think you are setting up a git repo that hosts your code already and
I think that is the answer
It will be better to take this off the list until that repo passes all the
unit test Taras is developing with you.
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
--
*Mike Holmes*
Linaro Sr Technical Manager
LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
