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().
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
