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 4C543A2F18 for ; Thu, 3 Oct 2019 14:28:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0DEBD1C038; Thu, 3 Oct 2019 14:28:09 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 7015C1C02F for ; Thu, 3 Oct 2019 14:28:07 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Oct 2019 05:28:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,252,1566889200"; d="scan'208";a="185898663" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by orsmga008.jf.intel.com with ESMTP; 03 Oct 2019 05:28:04 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.164]) by IRSMSX151.ger.corp.intel.com ([169.254.4.234]) with mapi id 14.03.0439.000; Thu, 3 Oct 2019 13:27:51 +0100 From: "Ananyev, Konstantin" To: "Ananyev, Konstantin" , Honnappa Nagarahalli , "olivier.matz@6wind.com" , "Wang, Yipeng1" , "Gobriel, Sameh" , "Richardson, Bruce" , "De Lara Guarch, Pablo" CC: "dev@dpdk.org" , Dharmik Thakkar , "Gavin Hu (Arm Technology China)" , "Ruifeng Wang (Arm Technology China)" , nd , nd , nd Thread-Topic: [dpdk-dev] [PATCH 2/5] lib/ring: add template to support different element sizes Thread-Index: AQHVXa90autmMQgO70SevYhypWJbKqdF3rfwgAEG+ICAAFeBkIABLVyAgACbGgCAAArJkA== Date: Thu, 3 Oct 2019 12:27:50 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580191970852@irsmsx105.ger.corp.intel.com> References: <20190828144614.25284-1-honnappa.nagarahalli@arm.com> <20190828144614.25284-3-honnappa.nagarahalli@arm.com> <2601191342CEEE43887BDE71AB977258019196E21E@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258019196FAC9@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580191970803@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772580191970803@irsmsx105.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYWQ2ZDZlMTItZTA5MS00YzgxLWIxNmUtNTk0NjkwYzRjZmIwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRE5kVTdHd3RkeEk2djFJOGFFSkhxXC9PM3R2Szk0WDJ2T2lYRkhMUmdueFdMcmFKTnp3M1JpWWlEQVN3SDBxVVwvIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/5] lib/ring: add template to support different element sizes 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" >=20 > > > > > > +++ b/lib/librte_ring/rte_ring_template.h > > > > > > @@ -0,0 +1,330 @@ > > > > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > > > > + * Copyright (c) 2019 Arm Limited */ > > > > > > + > > > > > > +#ifndef _RTE_RING_TEMPLATE_H_ > > > > > > +#define _RTE_RING_TEMPLATE_H_ > > > > > > + > > > > > > +#ifdef __cplusplus > > > > > > +extern "C" { > > > > > > +#endif > > > > > > + > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include #include > > > > > > +#include #include > > > > > > + > > > > > > +/* Ring API suffix name - used to append to API names */ #ifnd= ef > > > > > > +RTE_RING_TMPLT_API_SUFFIX #error RTE_RING_TMPLT_API_SUFFIX > > > not > > > > > > +defined #endif > > > > > > + > > > > > > +/* Ring's element size in bits, should be a power of 2 */ #ifn= def > > > > > > +RTE_RING_TMPLT_ELEM_SIZE #error RTE_RING_TMPLT_ELEM_SIZE > > > not > > > > > defined > > > > > > +#endif > > > > > > + > > > > > > +/* Type of ring elements */ > > > > > > +#ifndef RTE_RING_TMPLT_ELEM_TYPE > > > > > > +#error RTE_RING_TMPLT_ELEM_TYPE not defined #endif > > > > > > + > > > > > > +#define _rte_fuse(a, b) a##_##b > > > > > > +#define __rte_fuse(a, b) _rte_fuse(a, b) #define > > > > > > +__RTE_RING_CONCAT(a) __rte_fuse(a, RTE_RING_TMPLT_API_SUFFIX) > > > > > > + > > > > > > +/* Calculate the memory size needed for a ring */ > > > > > > +RTE_RING_TMPLT_EXPERIMENTAL ssize_t > > > > > > +__RTE_RING_CONCAT(rte_ring_get_memsize)(unsigned count); > > > > > > + > > > > > > +/* Create a new ring named *name* in memory. */ > > > > > > +RTE_RING_TMPLT_EXPERIMENTAL struct rte_ring * > > > > > > +__RTE_RING_CONCAT(rte_ring_create)(const char *name, unsigned > > > count, > > > > > > + int socket_id, unsigned flags); > > > > > > > > > > > > > > > Just an idea - probably same thing can be achieved in a different= way. > > > > > Instead of all these defines - replace ENQUEUE_PTRS/DEQUEUE_PTRS > > > > > macros with static inline functions and then make all internal fu= nctions, > > > i.e. > > > > > __rte_ring_do_dequeue() > > > > > to accept enqueue/dequeue function pointer as a parameter. > > > > > Then let say default rte_ring_mc_dequeue_bulk will do: > > > > > > > > > > rte_ring_mc_dequeue_bulk(struct rte_ring *r, void **obj_table, > > > > > unsigned int n, unsigned int *available) { > > > > > return __rte_ring_do_dequeue(r, obj_table, n, > > > RTE_RING_QUEUE_FIXED, > > > > > __IS_MC, available, dequeue_ptr_default);= } > > > > > > > > > > Then if someone will like to define ring functions forelt_size=3D= =3DX, > > > > > all he would need to do: > > > > > 1. define his own enqueue/dequeuer functions. > > > > > 2. do something like: > > > > > rte_ring_mc_dequeue_bulk(struct rte_ring *r, void **obj_table, > > > > > unsigned int n, unsigned int *available) { > > > > > return __rte_ring_do_dequeue(r, obj_table, n, > > > RTE_RING_QUEUE_FIXED, > > > > > __IS_MC, available, dequeue_X); } > > > > > > > > > > Konstantin > > > > Thanks for the feedback/idea. The goal of this patch was to make it > > > > simple enough to define APIs to store any element size without code > > > duplication. > > > > > > Well, then if we store elt_size inside the ring, it should be easy en= ough to add > > > to the API generic functions that would use memcpy(or rte_memcpy) for > > > enqueue/dequeue. > > > Yes, it might be slower than existing (8B per elem), but might be sti= ll > > > acceptable. > > The element size will be a constant in most use cases. If we keep the e= lement size as a parameter, it allows the compiler to do any loop > > unrolling and auto-vectorization optimizations on copying. > > Storing the element size will result in additional memory access. >=20 > I understand that, but for you case (rcu defer queue) you probably need h= ighest possible performance, right? Meant 'don't need' of course :) > I am sure there will be other cases where such slight perf degradation is= acceptatble. >=20 > > > > > > > > >With this patch, the user has to write ~4 lines of code to get APIs = for > > > >any element size. I would like to keep the goal still the same. > > > > > > > > If we have to avoid the macro-fest, the main problem that needs to = be > > > > addressed is - how to represent different sizes of element types in= a generic > > > way? IMO, we can do this by defining the element type to be a multipl= e of > > > uint32_t (I do not think we need to go to uint16_t). > > > > > > > > For ex: > > > > rte_ring_mp_enqueue_bulk_objs(struct rte_ring *r, > > > > uint32_t *obj_table, unsigned int num_objs, > > > > unsigned int n, > > > > enum rte_ring_queue_behavior behavior, unsigned int= is_sp, > > > > unsigned int *free_space) { } > > > > > > > > This approach would ensure that we have generic enough APIs and the= y > > > > can be used for elements of any size. But the element itself needs = to be a > > > multiple of 32b - I think this should not be a concern. > > > > > > > > The API suffix definitely needs to be better, any suggestions? > > >