Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page: platform/linux-generic/include/odp_buffer_internal.h line 17 @@ -41,11 +41,19 @@ typedef struct seg_entry_t { uint32_t len; } seg_entry_t; +typedef union buffer_index_t { + uint32_t u32; + + struct { + uint32_t pool :8; + uint32_t buffer :24;
Comment: We originally had this index partitioning based on `ODP_CONFIG_POOLS`. Do we want to return to that here? If not, we at least need an `ODP_STATIC_ASSERT()` to ensure that `ODP_CONFIG_POOLS < 256` or else bad things will happen here. > Bill Fischofer(Bill-Fischofer-Linaro) wrote: > This routine can be optimized to: > ``` > return ring->head == ring->tail; > ``` >> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >> Your invariant is the queue is empty when `head == tail` therefore the queue >> is full when `abs(tail - head) == mask`, so the correct calculation here is: >> >> `num = mask - abs(tail - head);` >> >> The effect is that a queue can only hold `size - 1` elements, otherwise you >> cannot distinguish between a full and an empty queue without another bit of >> metadata, which is a cost you're trying to avoid. >> >> This is somewhat problematic if the caller is trying to be "optimal" by >> specifying a power of two in the `size` parameter of the `odp_queue_param_t` >> passed to `odp_queue_create()`. For this reason we may wish to return a >> `max_size` of a power of 2 - 1 in `odp_queue_capability()` as part of this >> patch series. >>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>> This only works if `size` is a power of 2. Should be documented as such, >>> since this is an internal routine. In this case an `ODP_ASSERT(size == >>> ROUNDUP_POWER2_U32(size))` for this requirement would be a useful debugging >>> aid. >>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>> should be `num = abs(tail - head);` to deal with wrap arounds, otherwise >>>> may be misinterpreted as overly large since it's `uint32_t`. Note that GCC >>>> and clang recognize `abs()` and treat it as a builtin, so there's no >>>> actual `stdlib.h` call here. >>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>> Extra blank line should be removed (nit). https://github.com/Linaro/odp/pull/492#discussion_r169819392 updated_at 2018-02-22 09:32:40