Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page: platform/linux-generic/odp_queue.c line 92 @@ -143,8 +150,10 @@ static int queue_capability(odp_queue_capability_t *capa) capa->max_sched_groups = sched_fn->num_grps(); capa->sched_prios = odp_schedule_num_prio(); capa->plain.max_num = capa->max_queues; + capa->plain.max_size = CONFIG_QUEUE_SIZE;
Comment: As noted earlier, due to "losing" one entry to distinguish queue empty/full, this should be returned as `CONFIG_QUEUE_SIZE - 1`, and we also need to ensure that `CONFIG_QUEUE_SIZE` is itself a power of 2. > Bill Fischofer(Bill-Fischofer-Linaro) wrote: > Since you're initializing `index.pool` and `index.buffer` there's no need to > set `index.u32` here. >> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >> 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_r169821543 updated_at 2018-02-22 09:32:40