On Thu, Jan 16, 2020 at 10:56 AM Honnappa Nagarahalli
<honnappa.nagaraha...@arm.com> wrote:
>
> Use custom element size ring APIs to replace event ring
> implementation. This avoids code duplication.
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> Reviewed-by: Gavin Hu <gavin...@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljed...@arm.com>
Please change the subject to eventdev: xxxxxx.
With the above change, LGTM
Reviewed-by: Jerin Jacob <jer...@marvell.com>
> ---
> lib/librte_eventdev/rte_event_ring.c | 147 ++-------------------------
> lib/librte_eventdev/rte_event_ring.h | 45 ++++----
> 2 files changed, 24 insertions(+), 168 deletions(-)
>
> diff --git a/lib/librte_eventdev/rte_event_ring.c
> b/lib/librte_eventdev/rte_event_ring.c
> index 50190de01..d27e23901 100644
> --- a/lib/librte_eventdev/rte_event_ring.c
> +++ b/lib/librte_eventdev/rte_event_ring.c
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: BSD-3-Clause
> * Copyright(c) 2017 Intel Corporation
> + * Copyright(c) 2019 Arm Limited
> */
>
> #include <sys/queue.h>
> @@ -11,13 +12,6 @@
> #include <rte_eal_memconfig.h>
> #include "rte_event_ring.h"
>
> -TAILQ_HEAD(rte_event_ring_list, rte_tailq_entry);
> -
> -static struct rte_tailq_elem rte_event_ring_tailq = {
> - .name = RTE_TAILQ_EVENT_RING_NAME,
> -};
> -EAL_REGISTER_TAILQ(rte_event_ring_tailq)
> -
> int
> rte_event_ring_init(struct rte_event_ring *r, const char *name,
> unsigned int count, unsigned int flags)
> @@ -35,150 +29,21 @@ struct rte_event_ring *
> rte_event_ring_create(const char *name, unsigned int count, int socket_id,
> unsigned int flags)
> {
> - char mz_name[RTE_MEMZONE_NAMESIZE];
> - struct rte_event_ring *r;
> - struct rte_tailq_entry *te;
> - const struct rte_memzone *mz;
> - ssize_t ring_size;
> - int mz_flags = 0;
> - struct rte_event_ring_list *ring_list = NULL;
> - const unsigned int requested_count = count;
> - int ret;
> -
> - ring_list = RTE_TAILQ_CAST(rte_event_ring_tailq.head,
> - rte_event_ring_list);
> -
> - /* for an exact size ring, round up from count to a power of two */
> - if (flags & RING_F_EXACT_SZ)
> - count = rte_align32pow2(count + 1);
> - else if (!rte_is_power_of_2(count)) {
> - rte_errno = EINVAL;
> - return NULL;
> - }
> -
> - ring_size = sizeof(*r) + (count * sizeof(struct rte_event));
> -
> - ret = snprintf(mz_name, sizeof(mz_name), "%s%s",
> - RTE_RING_MZ_PREFIX, name);
> - if (ret < 0 || ret >= (int)sizeof(mz_name)) {
> - rte_errno = ENAMETOOLONG;
> - return NULL;
> - }
> -
> - te = rte_zmalloc("RING_TAILQ_ENTRY", sizeof(*te), 0);
> - if (te == NULL) {
> - RTE_LOG(ERR, RING, "Cannot reserve memory for tailq\n");
> - rte_errno = ENOMEM;
> - return NULL;
> - }
> -
> - rte_mcfg_tailq_write_lock();
> -
> - /*
> - * reserve a memory zone for this ring. If we can't get rte_config or
> - * we are secondary process, the memzone_reserve function will set
> - * rte_errno for us appropriately - hence no check in this this
> function
> - */
> - mz = rte_memzone_reserve(mz_name, ring_size, socket_id, mz_flags);
> - if (mz != NULL) {
> - r = mz->addr;
> - /* Check return value in case rte_ring_init() fails on size */
> - int err = rte_event_ring_init(r, name, requested_count,
> flags);
> - if (err) {
> - RTE_LOG(ERR, RING, "Ring init failed\n");
> - if (rte_memzone_free(mz) != 0)
> - RTE_LOG(ERR, RING, "Cannot free memzone\n");
> - rte_free(te);
> - rte_mcfg_tailq_write_unlock();
> - return NULL;
> - }
> -
> - te->data = (void *) r;
> - r->r.memzone = mz;
> -
> - TAILQ_INSERT_TAIL(ring_list, te, next);
> - } else {
> - r = NULL;
> - RTE_LOG(ERR, RING, "Cannot reserve memory\n");
> - rte_free(te);
> - }
> - rte_mcfg_tailq_write_unlock();
> -
> - return r;
> + return (struct rte_event_ring *)rte_ring_create_elem(name,
> + sizeof(struct rte_event),
> + count, socket_id, flags);
> }
>
>
> struct rte_event_ring *
> rte_event_ring_lookup(const char *name)
> {
> - struct rte_tailq_entry *te;
> - struct rte_event_ring *r = NULL;
> - struct rte_event_ring_list *ring_list;
> -
> - ring_list = RTE_TAILQ_CAST(rte_event_ring_tailq.head,
> - rte_event_ring_list);
> -
> - rte_mcfg_tailq_read_lock();
> -
> - TAILQ_FOREACH(te, ring_list, next) {
> - r = (struct rte_event_ring *) te->data;
> - if (strncmp(name, r->r.name, RTE_RING_NAMESIZE) == 0)
> - break;
> - }
> -
> - rte_mcfg_tailq_read_unlock();
> -
> - if (te == NULL) {
> - rte_errno = ENOENT;
> - return NULL;
> - }
> -
> - return r;
> + return (struct rte_event_ring *)rte_ring_lookup(name);
> }
>
> /* free the ring */
> void
> rte_event_ring_free(struct rte_event_ring *r)
> {
> - struct rte_event_ring_list *ring_list = NULL;
> - struct rte_tailq_entry *te;
> -
> - if (r == NULL)
> - return;
> -
> - /*
> - * Ring was not created with rte_event_ring_create,
> - * therefore, there is no memzone to free.
> - */
> - if (r->r.memzone == NULL) {
> - RTE_LOG(ERR, RING,
> - "Cannot free ring (not created with
> rte_event_ring_create()");
> - return;
> - }
> -
> - if (rte_memzone_free(r->r.memzone) != 0) {
> - RTE_LOG(ERR, RING, "Cannot free memory\n");
> - return;
> - }
> -
> - ring_list = RTE_TAILQ_CAST(rte_event_ring_tailq.head,
> - rte_event_ring_list);
> - rte_mcfg_tailq_write_lock();
> -
> - /* find out tailq entry */
> - TAILQ_FOREACH(te, ring_list, next) {
> - if (te->data == (void *) r)
> - break;
> - }
> -
> - if (te == NULL) {
> - rte_mcfg_tailq_write_unlock();
> - return;
> - }
> -
> - TAILQ_REMOVE(ring_list, te, next);
> -
> - rte_mcfg_tailq_write_unlock();
> -
> - rte_free(te);
> + rte_ring_free((struct rte_ring *)r);
> }
> diff --git a/lib/librte_eventdev/rte_event_ring.h
> b/lib/librte_eventdev/rte_event_ring.h
> index 827a3209e..c0861b0ec 100644
> --- a/lib/librte_eventdev/rte_event_ring.h
> +++ b/lib/librte_eventdev/rte_event_ring.h
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: BSD-3-Clause
> * Copyright(c) 2016-2017 Intel Corporation
> + * Copyright(c) 2019 Arm Limited
> */
>
> /**
> @@ -19,6 +20,7 @@
> #include <rte_memory.h>
> #include <rte_malloc.h>
> #include <rte_ring.h>
> +#include <rte_ring_elem.h>
> #include "rte_eventdev.h"
>
> #define RTE_TAILQ_EVENT_RING_NAME "RTE_EVENT_RING"
> @@ -88,22 +90,17 @@ rte_event_ring_enqueue_burst(struct rte_event_ring *r,
> const struct rte_event *events,
> unsigned int n, uint16_t *free_space)
> {
> - uint32_t prod_head, prod_next;
> - uint32_t free_entries;
> + unsigned int num;
> + uint32_t space;
>
> - n = __rte_ring_move_prod_head(&r->r, r->r.prod.single, n,
> - RTE_RING_QUEUE_VARIABLE,
> - &prod_head, &prod_next, &free_entries);
> - if (n == 0)
> - goto end;
> + num = rte_ring_enqueue_burst_elem(&r->r, events,
> + sizeof(struct rte_event), n,
> + &space);
>
> - ENQUEUE_PTRS(&r->r, &r[1], prod_head, events, n, struct rte_event);
> -
> - update_tail(&r->r.prod, prod_head, prod_next, r->r.prod.single, 1);
> -end:
> if (free_space != NULL)
> - *free_space = free_entries - n;
> - return n;
> + *free_space = space;
> +
> + return num;
> }
>
> /**
> @@ -129,23 +126,17 @@ rte_event_ring_dequeue_burst(struct rte_event_ring *r,
> struct rte_event *events,
> unsigned int n, uint16_t *available)
> {
> - uint32_t cons_head, cons_next;
> - uint32_t entries;
> -
> - n = __rte_ring_move_cons_head(&r->r, r->r.cons.single, n,
> - RTE_RING_QUEUE_VARIABLE,
> - &cons_head, &cons_next, &entries);
> - if (n == 0)
> - goto end;
> + unsigned int num;
> + uint32_t remaining;
>
> - DEQUEUE_PTRS(&r->r, &r[1], cons_head, events, n, struct rte_event);
> + num = rte_ring_dequeue_burst_elem(&r->r, events,
> + sizeof(struct rte_event), n,
> + &remaining);
>
> - update_tail(&r->r.cons, cons_head, cons_next, r->r.cons.single, 0);
> -
> -end:
> if (available != NULL)
> - *available = entries - n;
> - return n;
> + *available = remaining;
> +
> + return num;
> }
>
> /*
> --
> 2.17.1
>
