On Sun, Oct 20, 2019 at 07:22:56PM -0500, Honnappa Nagarahalli wrote:
> Current APIs assume ring elements to be pointers. However, in many
> use cases, the size can be different. Add new APIs to support
> configurable ring element sizes.
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> Reviewed-by: Dharmik Thakkar <dharmik.thak...@arm.com>
> Reviewed-by: Gavin Hu <gavin...@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
> ---
> lib/librte_ring/Makefile | 3 +-
> lib/librte_ring/meson.build | 4 +
> lib/librte_ring/rte_ring.c | 44 +-
> lib/librte_ring/rte_ring.h | 1 +
> lib/librte_ring/rte_ring_elem.h | 946 +++++++++++++++++++++++++++
> lib/librte_ring/rte_ring_version.map | 2 +
> 6 files changed, 991 insertions(+), 9 deletions(-)
> create mode 100644 lib/librte_ring/rte_ring_elem.h
(...)
> +/* the actual enqueue of pointers on the ring.
> + * Placed here since identical code needed in both
> + * single and multi producer enqueue functions.
> + */
> +#define ENQUEUE_PTRS_ELEM(r, ring_start, prod_head, obj_table, esize, n) do
> { \
> + if (esize == 4) \
> + ENQUEUE_PTRS_32(r, ring_start, prod_head, obj_table, n); \
> + else if (esize == 8) \
> + ENQUEUE_PTRS_64(r, ring_start, prod_head, obj_table, n); \
> + else if (esize == 16) \
> + ENQUEUE_PTRS_128(r, ring_start, prod_head, obj_table, n); \
> +} while (0)
My initial thinking was that it could be a static inline functions
instead of macros. I see that patches 5 and 6 are changing it. I wonder
however if patches 5 and 6 shouldn't be merged and moved before this
one: it would avoid to introduce new macros that will be removed after.
(...)
> +/**
> + * @internal Enqueue several objects on the ring
> + *
> + * @param r
> + * A pointer to the ring structure.
> + * @param obj_table
> + * A pointer to a table of void * pointers (objects).
> + * @param esize
> + * The size of ring element, in bytes. It must be a multiple of 4.
> + * Currently, sizes 4, 8 and 16 are supported. This should be the same
> + * as passed while creating the ring, otherwise the results are undefined.
The comment "It must be a multiple of 4" and "Currently, sizes 4, 8 and 16 are
supported" are redundant (it appears several times in the file). The second one
should be removed by patch 5 (I think it is missing?).
But if patch 5 and 6 are moved before this one, only "It must be a multiple of
4" would be needed I think, and there would be no transition with only 3
supported sizes.
