| Message ID | 1408607536-28411-1-git-send-email-petri.savolainen@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 <petri.savolainen@linaro.org> > --- > 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 <anders.roxell@linaro.org> 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 > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
> -----Original Message----- > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > Sent: Thursday, August 21, 2014 11:44 AM > To: Petri Savolainen > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle to > zero > > 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 <petri.savolainen@linaro.org> > > --- > > 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; Anders, I think you meant this one. It's renamed already in this patch. > > 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. Done already. See above. > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > > And Maxim can cleanup the commit message before he push this, so you > don't have to resend the patch. OK. -Petri > 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 > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > -- > Anders Roxell > anders.roxell@linaro.org > M: +46 709 71 42 85 | IRC: roxell
On 2014-08-21 09:39, Savolainen, Petri (NSN - FI/Espoo) wrote: > > > > -----Original Message----- > > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > > Sent: Thursday, August 21, 2014 11:44 AM > > To: Petri Savolainen > > Cc: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle to > > zero > > > > 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 <petri.savolainen@linaro.org> > > > --- > > > 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; > > > Anders, I think you meant this one. It's renamed already in this patch. Yes you are correct! Sorry that I missed that! =/ > > > > > 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. > > Done already. See above. Great! =) Cheers, Anders > > > > > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > > > > And Maxim can cleanup the commit message before he push this, so you > > don't have to resend the patch. > > OK. > > -Petri > > > > > 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 > > > lng-odp@lists.linaro.org > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > -- > > Anders Roxell > > anders.roxell@linaro.org > > M: +46 709 71 42 85 | IRC: roxell
This patch is ready for merge. Waiting for review tags... -Petri > -----Original Message----- > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > Sent: Thursday, August 21, 2014 12:47 PM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: Petri Savolainen; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle to > zero > > On 2014-08-21 09:39, Savolainen, Petri (NSN - FI/Espoo) wrote: > > > > > > > -----Original Message----- > > > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > > > Sent: Thursday, August 21, 2014 11:44 AM > > > To: Petri Savolainen > > > Cc: lng-odp@lists.linaro.org > > > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle > to > > > zero > > > > > > 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 <petri.savolainen@linaro.org> > > > > --- > > > > 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; > > > > > > Anders, I think you meant this one. It's renamed already in this > patch. > > Yes you are correct! > Sorry that I missed that! =/ > > > > > > > > > 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. > > > > Done already. See above. > > Great! =) > > Cheers, > Anders > > > > > > > > > > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > > > > > > And Maxim can cleanup the commit message before he push this, so > you > > > don't have to resend the patch. > > > > OK. > > > > -Petri > > > > > > > > > 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 > > > > lng-odp@lists.linaro.org > > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > > Anders Roxell > > > anders.roxell@linaro.org > > > M: +46 709 71 42 85 | IRC: roxell
On 26 August 2014 13:10, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > This patch is ready for merge. Waiting for review tags... > Please rebase this on origin/master and send it again... because it doesn't apply now... =( I agree it was ready for merge... Cheers, Anders > > -Petri > > > -----Original Message----- > > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > > Sent: Thursday, August 21, 2014 12:47 PM > > To: Savolainen, Petri (NSN - FI/Espoo) > > Cc: Petri Savolainen; lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle to > > zero > > > > On 2014-08-21 09:39, Savolainen, Petri (NSN - FI/Espoo) wrote: > > > > > > > > > > -----Original Message----- > > > > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > > > > Sent: Thursday, August 21, 2014 11:44 AM > > > > To: Petri Savolainen > > > > Cc: lng-odp@lists.linaro.org > > > > Subject: Re: [lng-odp] [PATCH v3] Change invalid buffer pool handle > > to > > > > zero > > > > > > > > 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 <petri.savolainen@linaro.org> > > > > > --- > > > > > 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; > > > > > > > > > Anders, I think you meant this one. It's renamed already in this > > patch. > > > > Yes you are correct! > > Sorry that I missed that! =/ > > > > > > > > > > > > > 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. > > > > > > Done already. See above. > > > > Great! =) > > > > Cheers, > > Anders > > > > > > > > > > > > > > > > Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > > > > > > > > And Maxim can cleanup the commit message before he push this, so > > you > > > > don't have to resend the patch. > > > > > > OK. > > > > > > -Petri > > > > > > > > > > > > > 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 > > > > > lng-odp@lists.linaro.org > > > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > > > -- > > > > Anders Roxell > > > > anders.roxell@linaro.org > > > > M: +46 709 71 42 85 | IRC: roxell > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
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; 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);
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 Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- 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(-)