From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 16BACA051A;
	Fri, 17 Jan 2020 15:42:18 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 0280F1AFF;
	Fri, 17 Jan 2020 15:42:16 +0100 (CET)
Received: from mail-io1-f68.google.com (mail-io1-f68.google.com
 [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id D8485235
 for <dev@dpdk.org>; Fri, 17 Jan 2020 15:42:14 +0100 (CET)
Received: by mail-io1-f68.google.com with SMTP id n21so26212305ioo.10
 for <dev@dpdk.org>; Fri, 17 Jan 2020 06:42:14 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc; bh=A193Qklf2LxCeta2fw0Vf1XP5ozjLMCydZxCAZalSDs=;
 b=Ls2HeYYfX1Lu8S6Z9bch2JZ/6kwyyOMrrG77y9C8dBa5+jCmGLM+hJ1YZRrLU7Jhvc
 jhb9SA01bo/G8WQRnmDGbKJEwF6GbXt68I5hIIQz1H4/ywDD/AK3YzzG+dLXb4NtxwVv
 Bkg9hbqcpNH/pJcZ8DK8cZEa4XEHzeler32JGNnG9aTarPB2NlDWGh8mrW3kqeXF7964
 M3eeBIQtiMQLzxNbnRggDw+uEZhL7Dh7wIno5iHtmDXQvvO6Ar0b08jomjrk8if9mZ+w
 bv28JOet+tGjAGmFkQ6lL/v6zwyl9O97vybsnmp49ZJSUmnfRPDct8SoTSDVvbKzf3vn
 THCA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=A193Qklf2LxCeta2fw0Vf1XP5ozjLMCydZxCAZalSDs=;
 b=J+nxgwLlTsifB6YuQsktlsGO1hc0x1tF0P0lw9mmCUBB9yfS/WdxsbohPYml/Pqvv1
 f5Tey0mB5DHMMdpbJB+HyqGaI3FJRwbBYD+uYmOy4fFi8+lQc8XOkydI+6y+yvr8F1JI
 ZCTdiXCfP6RJf2oL60IbBpI/HZl2XvsKqplTLLxqLU0dUl3U87/GV5BZS4dVdE4uZxMC
 qcyIdkgPo9WYtZZ3iOKOkMx49+KpwMFFsdbRYVsDFt8vjEWjAaZyf3bNUSM8FhnS40lZ
 goTUzih1KkztfnRijEcJjPo8M14fp1zCh6EyI/p0lKkWQ+PM2eTP9zvMnSyfO6Y5PVKi
 UGGQ==
X-Gm-Message-State: APjAAAXPDikl7y9NmuYpBlgcn1cibcS+vYwS3+SJNqYmA1iaLN+GJmDD
 0KNWK7aki3rMQYyyoUKQAqcwJnl0FPSWHzPXktQ=
X-Google-Smtp-Source: APXvYqwsZEHMfmtkA+j4Wyg4UXaRBNqMZWv8dFBJTjpX5a9VxNx3Y4lddgPu6BoKqfae5hrfu5TNB8Nf5kZOv0ROsJI=
X-Received: by 2002:a5d:8cce:: with SMTP id k14mr32633172iot.294.1579272134047; 
 Fri, 17 Jan 2020 06:42:14 -0800 (PST)
MIME-Version: 1.0
References: <20190906190510.11146-1-honnappa.nagarahalli@arm.com>
 <20200116052511.8557-1-honnappa.nagarahalli@arm.com>
 <20200116052511.8557-7-honnappa.nagarahalli@arm.com>
In-Reply-To: <20200116052511.8557-7-honnappa.nagarahalli@arm.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Fri, 17 Jan 2020 20:11:57 +0530
Message-ID: <CALBAE1NitLbH4A6yctPdb0ZWmWndaMck7tS56ArMVvKM6=cOyw@mail.gmail.com>
To: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Cc: Olivier Matz <olivier.matz@6wind.com>,
 Stephen Hemminger <sthemmin@microsoft.com>, 
 Jerin Jacob <jerinj@marvell.com>, "Richardson,
 Bruce" <bruce.richardson@intel.com>, 
 David Marchand <david.marchand@redhat.com>,
 Pavan Nikhilesh <pbhagavatula@marvell.com>, 
 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
 Yipeng Wang <yipeng1.wang@intel.com>, 
 dpdk-dev <dev@dpdk.org>, Dharmik Thakkar <dharmik.thakkar@arm.com>, 
 "Ruifeng Wang (Arm Technology China)" <ruifeng.wang@arm.com>,
 Gavin Hu <gavin.hu@arm.com>, nd <nd@arm.com>, 
 "Van Haaren, Harry" <harry.van.haaren@intel.com>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v9 6/6] lib/eventdev: use custom element size
 ring for event rings
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Jan 16, 2020 at 10:56 AM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>
> Use custom element size ring APIs to replace event ring
> implementation. This avoids code duplication.
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

Please change the subject to eventdev: xxxxxx.

With the above change, LGTM

Reviewed-by: Jerin Jacob <jerinj@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
>