From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A6230A3295 for ; Wed, 23 Oct 2019 11:59:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7A6421BFF2; Wed, 23 Oct 2019 11:59:53 +0200 (CEST) Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id D97381BFEF for ; Wed, 23 Oct 2019 11:59:51 +0200 (CEST) Received: from glumotte.dev.6wind.com (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id A33CE3356A7; Wed, 23 Oct 2019 11:59:51 +0200 (CEST) Date: Wed, 23 Oct 2019 11:59:51 +0200 From: Olivier Matz To: Honnappa Nagarahalli Cc: sthemmin@microsoft.com, jerinj@marvell.com, bruce.richardson@intel.com, david.marchand@redhat.com, pbhagavatula@marvell.com, konstantin.ananyev@intel.com, drc@linux.vnet.ibm.com, hemant.agrawal@nxp.com, dev@dpdk.org, dharmik.thakkar@arm.com, ruifeng.wang@arm.com, gavin.hu@arm.com Message-ID: <20191023095951.GD25286@glumotte.dev.6wind.com> References: <20190906190510.11146-1-honnappa.nagarahalli@arm.com> <20191021002300.26497-1-honnappa.nagarahalli@arm.com> <20191021002300.26497-3-honnappa.nagarahalli@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191021002300.26497-3-honnappa.nagarahalli@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [RFC v6 2/6] lib/ring: apis to support configurable element size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > Reviewed-by: Dharmik Thakkar > Reviewed-by: Gavin Hu > Reviewed-by: Ruifeng Wang > --- > 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.