That is the plan. Working through latest issues now. Bill
On Thu, Nov 13, 2014 at 5:54 AM, Mike Holmes <[email protected]> wrote: > > > 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
