Good catch. Yes, the inner routines are missing an &. Will send out v2 to correct that shortly.
Bill On Tue, Nov 11, 2014 at 2:26 AM, Taras Kondratiuk < [email protected]> wrote: > On 11/10/2014 08:07 PM, Bill Fischofer wrote: > > Thanks. See inline comments: > > > > On Mon, Nov 10, 2014 at 11:37 AM, Taras Kondratiuk > > <[email protected] <mailto:[email protected]>> > wrote: > > > > On 11/10/2014 07:10 PM, Bill Fischofer wrote: > > > > Signed-off-by: Bill Fischofer <[email protected] > > <mailto:[email protected]>> > > --- > > Versions of this patch > > == > > v1 - Original (missing files) > > v2 - Added missing odp_buffer_inlines.h > > v3 - Added missing odph_tcp.h > > v4 - (mispublished as v3): Allow NULL arguments to > > odp_buffer_pool_create() > > v5 - Correctly handle timer allocs (no blocks), add support for > > buf_init calls > > on allocation. > > v6 - Add remaining segment APIs + unmaps + additional (void *) > > casts needed > > for some C compiler environments, as noted by Taras > > > > > > > > + > > + pool->s.bufcount = 0; > > + pool->s.buf_freelist = NULL; > > + pool->s.blk_freelist = NULL; > > > > > > Looks like pool->s.blk_freelist and pool->s.buf_freelist are not > > updated anywhere. And then dereferenced in ret_buf(). > > > > > > They are initialized in the lines just above your comment. Not sure > > what problem you see here. The ret_buf()/ret_blk() routines push > > buffers/blocks onto their respective freelists, so the contents of these > > pointers become the next pointer for the freed elements. Result is that > > the NULL propagates to the last element of the list, and is restored to > > the list anchor when the last element on that list is popped off of it. > > These are push/pop stack structures. > > That's is not how it works right now. Current code does: > 1. pool->s.blk_freelist = NULL; > 2. later odp_cs(pool->blk_freelist, oldhead, newhead) -> > __sync_bool_compare_and_swap(pool->blk_freelist, oldhead, newhead) > 3. __sync_bool_compare_and_swap() dereferences blk_freelist and get > segfault, because it is NULL. > > To get behavior that you have described a *pointer* to blk_freelist > have to passed in odp_cs instead of blk_freelist itself. > > -odp_cs(pool->blk_freelist, oldhead, newhead) > +odp_cs(&pool->blk_freelist, oldhead, newhead) > > > > > > > + > > + uint8_t *buf = pool->s.udata_base_addr - > buf_stride; > > > > > > pool->s.udata_base_addr is not initialized here. > > Should it be just udata_base_addr? > > > > > > No, it is initialized a few lines above in the statement: > > uint8_t *udata_base_addr = pool->s.pool_base_addr + mdata_size; > > uint8_t *block_base_addr = udata_base_addr + udata_size; > > That's the point. Local udata_base_addr is initialized, but then > uninitialized pool->s.udata_base_addr is used, which leads to segfault. > > Maybe you have sent not the latest patch? >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
