From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 047E9F72 for ; Fri, 18 Jan 2019 13:14:06 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jan 2019 04:14:06 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,491,1539673200"; d="scan'208";a="115672383" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by fmsmga007.fm.intel.com with ESMTP; 18 Jan 2019 04:14:04 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.116]) by IRSMSX107.ger.corp.intel.com ([169.254.10.127]) with mapi id 14.03.0415.000; Fri, 18 Jan 2019 12:14:03 +0000 From: "Ananyev, Konstantin" To: Honnappa Nagarahalli , "dev@dpdk.org" , "stephen@networkplumber.org" , "paulmck@linux.ibm.com" CC: "Gavin Hu (Arm Technology China)" , Dharmik Thakkar , nd , nd Thread-Topic: [RFC v2 1/2] rcu: add RCU library supporting QSBR mechanism Thread-Index: AQHUmZwkA3lN9JDuXEiclvOGJBGOtKWwUixggACXpdCAAUkpAIACgqOwgABi02A= Date: Fri, 18 Jan 2019 12:14:03 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258010D9058F6@irsmsx105.ger.corp.intel.com> References: <20181122033055.3431-1-honnappa.nagarahalli@arm.com> <20181222021420.5114-1-honnappa.nagarahalli@arm.com> <20181222021420.5114-2-honnappa.nagarahalli@arm.com> <2601191342CEEE43887BDE71AB977258010D904212@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258010D904AC7@irsmsx105.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTg1ZmNkMjgtZjFkOC00ZmJhLWJiZTctZTM4OTI1YjdiZjgzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicXZTOEJ1M3VBampaa1FwOHNtSDZWb0srdk9WR21lMDRnc01DMmFXVFFXZWphTE9sSFZVeWs1d2txRVwvNVBpUVMifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC v2 1/2] rcu: add RCU library supporting QSBR mechanism 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: , X-List-Received-Date: Fri, 18 Jan 2019 12:14:07 -0000 > > > > > diff --git a/lib/librte_rcu/rte_rcu_qsbr.h > > > > > b/lib/librte_rcu/rte_rcu_qsbr.h new file mode 100644 index > > > > > 000000000..c818e77fd > > > > > --- /dev/null > > > > > +++ b/lib/librte_rcu/rte_rcu_qsbr.h > > > > > @@ -0,0 +1,321 @@ > > > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > > > + * Copyright (c) 2018 Arm Limited */ > > > > > + > > > > > +#ifndef _RTE_RCU_QSBR_H_ > > > > > +#define _RTE_RCU_QSBR_H_ > > > > > + > > > > > +/** > > > > > + * @file > > > > > + * RTE Quiescent State Based Reclamation (QSBR) > > > > > + * > > > > > + * Quiescent State (QS) is any point in the thread execution > > > > > + * where the thread does not hold a reference to shared memory. > > > > > + * A critical section for a data structure can be a quiescent > > > > > +state for > > > > > + * another data structure. > > > > > + * > > > > > + * This library provides the ability to identify quiescent state= . > > > > > + */ > > > > > + > > > > > +#ifdef __cplusplus > > > > > +extern "C" { > > > > > +#endif > > > > > + > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > +#include > > > > > + > > > > > +/**< Maximum number of reader threads supported. */ #define > > > > > +RTE_RCU_MAX_THREADS 128 > > > > > + > > > > > +#if !RTE_IS_POWER_OF_2(RTE_RCU_MAX_THREADS) > > > > > +#error RTE_RCU_MAX_THREADS must be a power of 2 #endif > > > > > + > > > > > +/**< Number of array elements required for the bit-map */ #defin= e > > > > > +RTE_QSBR_BIT_MAP_ELEMS > > (RTE_RCU_MAX_THREADS/(sizeof(uint64_t) > > > > * 8)) > > > > > + > > > > > +/* Thread IDs are stored as a bitmap of 64b element array. Given > > > > > +thread id > > > > > + * needs to be converted to index into the array and the id > > > > > +within > > > > > + * the array element. > > > > > + */ > > > > > +#define RTE_QSBR_THR_INDEX_SHIFT 6 #define > > RTE_QSBR_THR_ID_MASK > > > > > +0x3f > > > > > + > > > > > +/* Worker thread counter */ > > > > > +struct rte_rcu_qsbr_cnt { > > > > > + uint64_t cnt; /**< Quiescent state counter. */ } > > > > > +__rte_cache_aligned; > > > > > + > > > > > +/** > > > > > + * RTE thread Quiescent State structure. > > > > > + */ > > > > > +struct rte_rcu_qsbr { > > > > > + uint64_t reg_thread_id[RTE_QSBR_BIT_MAP_ELEMS] > > > > __rte_cache_aligned; > > > > > + /**< Registered reader thread IDs - reader threads reporting > > > > > + * on this QS variable represented in a bit map. > > > > > + */ > > > > > + > > > > > + uint64_t token __rte_cache_aligned; > > > > > + /**< Counter to allow for multiple simultaneous QS queries */ > > > > > + > > > > > + struct rte_rcu_qsbr_cnt w[RTE_RCU_MAX_THREADS] > > > > __rte_cache_aligned; > > > > > + /**< QS counter for each reader thread, counts upto > > > > > + * current value of token. > > > > > > > > As I understand you decided to stick with neutral thread_id and let > > > > user define what exactly thread_id is (lcore, syste, thread id, som= ething > > else)? > > > Yes, that is correct. I will reply to the other thread to continue th= e discussion. > > > > > > > If so, can you probably get rid of RTE_RCU_MAX_THREADS limitation? > > > I am not seeing this as a limitation. The user can change this if req= uired. May > > be I should change it as follows: > > > #ifndef RTE_RCU_MAX_THREADS > > > #define RTE_RCU_MAX_THREADS 128 > > > #endif > > > > Yep, that's better, though it would still require user to rebuild the c= ode if he > > would like to increase total number of threads supported. > Agree >=20 > > Though it seems relatively simply to extend current code to support dyn= amic > > max thread num here (2 variable arrays plus shift value plus mask). > Agree, supporting dynamic 'max thread num' is simple. But this means memo= ry needs to be allocated to the arrays. The API > 'rte_rcu_qsbr_init' has to take max thread num as the parameter. We also = have to introduce another API to free this memory. This will > become very similar to alloc/free APIs I had in the v1. > I hope I am following you well, please correct me if not. I think we can still leave alloc/free tasks to the user. We probabply just need extra function rte_rcu_qsbr_size(uint32_ max_threads= ) to help user calculate required size. rte_rcu_qsbr_init() might take as an additional parameter 'size' to make ch= ecks. Thought about something like that: size_t sz =3D rte_rcu_qsbr_size(max_threads); struct rte_rcu_qsbr *qsbr =3D alloc_aligned(CACHE_LINE, sz); rte_rcu_qsbr_init(qsbr, max_threads, sz); ... Konstantin >=20 > > > > > > > > > I.E. struct rte_rcu_qsbr_cnt w[] and allow user at init time to > > > > define max number of threads allowed. > > > > Or something like: > > > > #define RTE_RCU_QSBR_DEF(name, max_thread) struct name { \ > > > > uint64_t reg_thread_id[ALIGN_CEIL(max_thread, 64) >> 6]; \ > > > > ... > > > > struct rte_rcu_qsbr_cnt w[max_thread]; \ } > > > I am trying to understand this. I am not following why 'name' is > > > required? Would the user call 'RTE_RCU_QSBR_DEF' in the application > > header file? > > > > My thought here was to allow user to define his own structures, dependi= ng on > > the number of max threads he needs/wants: > > RTE_RCU_QSBR_DEF(rte_rcu_qsbr_128, 128); > > RTE_RCU_QSBR_DEF(rte_rcu_qsbr_64, 64); ... > Thank you for the clarification, I follow you now. However, it will not s= olve the problem of dynamic max thread num. Changes to the max > number of threads will require recompilation. >=20 > > Konstantin