On 2014-08-21 10:52, Petri Savolainen wrote: > For consistency and easier debugging, use zero as the value of > an invalid pool handle (in linux-generic). > > v2: rename pool id > v3: rename pool handles
v2 and v3 should be added below "---", because that should not be in the commit message. > > Signed-off-by: Petri Savolainen <[email protected]> > --- > include/odp_buffer_pool.h | 2 +- > .../linux-generic/include/odp_buffer_internal.h | 4 +- > .../include/odp_buffer_pool_internal.h | 6 +- > platform/linux-generic/odp_buffer.c | 2 +- > platform/linux-generic/odp_buffer_pool.c | 68 > ++++++++++++++-------- > 5 files changed, 50 insertions(+), 32 deletions(-) > > diff --git a/include/odp_buffer_pool.h b/include/odp_buffer_pool.h > index 26d9f14..fe88898 100644 > --- a/include/odp_buffer_pool.h > +++ b/include/odp_buffer_pool.h > @@ -27,7 +27,7 @@ extern "C" { > #define ODP_BUFFER_POOL_NAME_LEN 32 > > /** Invalid buffer pool */ > -#define ODP_BUFFER_POOL_INVALID (0xffffffff) > +#define ODP_BUFFER_POOL_INVALID 0 > > /** ODP buffer pool */ > typedef uint32_t odp_buffer_pool_t; > diff --git a/platform/linux-generic/include/odp_buffer_internal.h > b/platform/linux-generic/include/odp_buffer_internal.h > index bffd0dd..2002b51 100644 > --- a/platform/linux-generic/include/odp_buffer_internal.h > +++ b/platform/linux-generic/include/odp_buffer_internal.h > @@ -48,7 +48,7 @@ typedef union odp_buffer_bits_t { > odp_buffer_t handle; > > struct { > - uint32_t pool:ODP_BUFFER_POOL_BITS; > + uint32_t pool_id:ODP_BUFFER_POOL_BITS; > uint32_t index:ODP_BUFFER_INDEX_BITS; > }; > } odp_buffer_bits_t; > @@ -91,7 +91,7 @@ typedef struct odp_buffer_hdr_t { > odp_atomic_int_t ref_count; /* reference count */ > odp_buffer_scatter_t scatter; /* Scatter/gather list */ > int type; /* type of next header */ > - odp_buffer_pool_t pool; /* buffer pool */ > + odp_buffer_pool_t pool_hdl; /* buffer pool handle */ > > } odp_buffer_hdr_t; > > diff --git a/platform/linux-generic/include/odp_buffer_pool_internal.h > b/platform/linux-generic/include/odp_buffer_pool_internal.h > index 1c0a9fc..e0210bd 100644 > --- a/platform/linux-generic/include/odp_buffer_pool_internal.h > +++ b/platform/linux-generic/include/odp_buffer_pool_internal.h > @@ -51,7 +51,7 @@ struct pool_entry_s { > uint64_t free_bufs; > char name[ODP_BUFFER_POOL_NAME_LEN]; > > - odp_buffer_pool_t pool ODP_ALIGNED_CACHE; > + odp_buffer_pool_t pool_hdl ODP_ALIGNED_CACHE; > uintptr_t buf_base; > size_t buf_size; > size_t buf_offset; > @@ -68,7 +68,7 @@ struct pool_entry_s { > extern void *pool_entry_ptr[]; > > > -static inline void *get_pool_entry(odp_buffer_pool_t pool_id) > +static inline void *get_pool_entry(uint32_t pool_id) > { > return pool_entry_ptr[pool_id]; > } > @@ -83,7 +83,7 @@ static inline odp_buffer_hdr_t *odp_buf_to_hdr(odp_buffer_t > buf) > odp_buffer_hdr_t *hdr; > > handle.u32 = buf; > - pool_id = handle.pool; > + pool_id = handle.pool_id; > index = handle.index; > > #ifdef POOL_ERROR_CHECK > diff --git a/platform/linux-generic/odp_buffer.c > b/platform/linux-generic/odp_buffer.c > index 0169eec..e54e0e7 100644 > --- a/platform/linux-generic/odp_buffer.c > +++ b/platform/linux-generic/odp_buffer.c > @@ -61,7 +61,7 @@ int odp_buffer_snprint(char *str, size_t n, odp_buffer_t > buf) > len += snprintf(&str[len], n-len, > "Buffer\n"); > len += snprintf(&str[len], n-len, > - " pool %i\n", hdr->pool); > + " pool %i\n", hdr->pool_hdl); > len += snprintf(&str[len], n-len, > " index %"PRIu32"\n", hdr->index); > len += snprintf(&str[len], n-len, > diff --git a/platform/linux-generic/odp_buffer_pool.c > b/platform/linux-generic/odp_buffer_pool.c > index a48781a..e538f04 100644 > --- a/platform/linux-generic/odp_buffer_pool.c > +++ b/platform/linux-generic/odp_buffer_pool.c > @@ -80,25 +80,40 @@ void *pool_entry_ptr[ODP_CONFIG_BUFFER_POOLS]; > static __thread odp_buffer_chunk_hdr_t *local_chunk[ODP_CONFIG_BUFFER_POOLS]; > > > +static inline odp_buffer_pool_t pool_index_to_handle(uint32_t pool_id) > +{ > + return pool_id + 1; > +} > + > + > +static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl) > +{ > + return pool_hdl -1; > +} > + > + > static inline void set_handle(odp_buffer_hdr_t *hdr, > pool_entry_t *pool, uint32_t index) > { > - uint32_t pool_id = (uint32_t) pool->s.pool; > + odp_buffer_pool_t pool_hdl = pool->s.pool_hdl; > + uint32_t pool_id = pool_handle_to_index(pool_hdl); > > - if (pool_id > ODP_CONFIG_BUFFER_POOLS) > - ODP_ERR("set_handle: Bad pool id\n"); > + if (pool_id >= ODP_CONFIG_BUFFER_POOLS) { > + ODP_ERR("set_handle: Bad pool handle %u\n", pool_hdl); > + exit(0); > + } > > if (index > ODP_BUFFER_MAX_INDEX) > ODP_ERR("set_handle: Bad buffer index\n"); > > - hdr->handle.pool = pool_id; > - hdr->handle.index = index; > + hdr->handle.pool_id = pool_id; > + hdr->handle.index = index; > } > > > int odp_buffer_pool_init_global(void) > { > - odp_buffer_pool_t i; > + uint32_t i; > > pool_tbl = odp_shm_reserve("odp_buffer_pools", > sizeof(pool_table_t), > @@ -113,7 +128,7 @@ int odp_buffer_pool_init_global(void) > /* init locks */ > pool_entry_t *pool = &pool_tbl->pool[i]; > LOCK_INIT(&pool->s.lock); > - pool->s.pool = i; > + pool->s.pool_hdl = pool_index_to_handle(i); > > pool_entry_ptr[i] = pool; > } > @@ -257,11 +272,11 @@ static void fill_hdr(void *ptr, pool_entry_t *pool, > uint32_t index, > > set_handle(hdr, pool, index); > > - hdr->addr = &buf_data[pool->s.buf_offset - pool->s.hdr_size]; > - hdr->index = index; > - hdr->size = pool->s.user_size; > - hdr->pool = pool->s.pool; > - hdr->type = buf_type; > + hdr->addr = &buf_data[pool->s.buf_offset - pool->s.hdr_size]; > + hdr->index = index; > + hdr->size = pool->s.user_size; > + hdr->pool_hdl = pool->s.pool_hdl; > + hdr->type = buf_type; urgh... I'm not sure show we should do this, and its not in the scope of this patch. However, I will raise the question here anyway. In the struct pool_entry_s we have (file platform/linux-generic/include/odp_buffer_pool_internal.h): odp_buffer_pool_t pool ODP_ALIGNED_CACHE; Shouldn't that be renamed to pool_hdl? I think this needs to be renamed as well, do we need to have some sort of naming convention for this? If we agree that this shouldn't be changed in this patch. Reviewed-by: Anders Roxell <[email protected]> And Maxim can cleanup the commit message before he push this, so you don't have to resend the patch. Cheers, Anders > > check_align(pool, hdr); > } > @@ -363,9 +378,9 @@ odp_buffer_pool_t odp_buffer_pool_create(const char *name, > size_t buf_size, size_t buf_align, > int buf_type) > { > - odp_buffer_pool_t i; > + odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID; > pool_entry_t *pool; > - odp_buffer_pool_t pool_id = ODP_BUFFER_POOL_INVALID; > + uint32_t i; > > for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { > pool = get_pool_entry(i); > @@ -388,20 +403,20 @@ odp_buffer_pool_t odp_buffer_pool_create(const char > *name, > > UNLOCK(&pool->s.lock); > > - pool_id = i; > + pool_hdl = pool->s.pool_hdl; > break; > } > > UNLOCK(&pool->s.lock); > } > > - return pool_id; > + return pool_hdl; > } > > > odp_buffer_pool_t odp_buffer_pool_lookup(const char *name) > { > - odp_buffer_pool_t i; > + uint32_t i; > pool_entry_t *pool; > > for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) { > @@ -411,7 +426,7 @@ odp_buffer_pool_t odp_buffer_pool_lookup(const char *name) > if (strcmp(name, pool->s.name) == 0) { > /* found it */ > UNLOCK(&pool->s.lock); > - return i; > + return pool->s.pool_hdl; > } > UNLOCK(&pool->s.lock); > } > @@ -420,11 +435,12 @@ odp_buffer_pool_t odp_buffer_pool_lookup(const char > *name) > } > > > -odp_buffer_t odp_buffer_alloc(odp_buffer_pool_t pool_id) > +odp_buffer_t odp_buffer_alloc(odp_buffer_pool_t pool_hdl) > { > pool_entry_t *pool; > odp_buffer_chunk_hdr_t *chunk; > odp_buffer_bits_t handle; > + uint32_t pool_id = pool_handle_to_index(pool_hdl); > > pool = get_pool_entry(pool_id); > chunk = local_chunk[pool_id]; > @@ -462,12 +478,12 @@ odp_buffer_t odp_buffer_alloc(odp_buffer_pool_t pool_id) > void odp_buffer_free(odp_buffer_t buf) > { > odp_buffer_hdr_t *hdr; > - odp_buffer_pool_t pool_id; > + uint32_t pool_id; > pool_entry_t *pool; > odp_buffer_chunk_hdr_t *chunk_hdr; > > hdr = odp_buf_to_hdr(buf); > - pool_id = hdr->pool; > + pool_id = pool_handle_to_index(hdr->pool_hdl); > pool = get_pool_entry(pool_id); > chunk_hdr = local_chunk[pool_id]; > > @@ -496,21 +512,23 @@ odp_buffer_pool_t odp_buffer_pool(odp_buffer_t buf) > odp_buffer_hdr_t *hdr; > > hdr = odp_buf_to_hdr(buf); > - return hdr->pool; > + return hdr->pool_hdl; > } > > > -void odp_buffer_pool_print(odp_buffer_pool_t pool_id) > +void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl) > { > pool_entry_t *pool; > odp_buffer_chunk_hdr_t *chunk_hdr; > uint32_t i; > + uint32_t pool_id; > > - pool = get_pool_entry(pool_id); > + pool_id = pool_handle_to_index(pool_hdl); > + pool = get_pool_entry(pool_id); > > printf("Pool info\n"); > printf("---------\n"); > - printf(" pool %i\n", pool->s.pool); > + printf(" pool %i\n", pool->s.pool_hdl); > printf(" name %s\n", pool->s.name); > printf(" pool base %p\n", pool->s.pool_base_addr); > printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base); > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp -- Anders Roxell [email protected] M: +46 709 71 42 85 | IRC: roxell _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
