From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 71F85F94 for ; Wed, 31 Aug 2016 11:59:35 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP; 31 Aug 2016 02:59:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,261,1470726000"; d="scan'208";a="755375388" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by FMSMGA003.fm.intel.com with ESMTP; 31 Aug 2016 02:59:33 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 31 Aug 2016 10:59:25 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.102]) by irsmsx111.ger.corp.intel.com ([169.254.2.19]) with mapi id 14.03.0248.002; Wed, 31 Aug 2016 10:59:26 +0100 From: "Ananyev, Konstantin" To: Vladyslav Buslov CC: "dev@dpdk.org" Thread-Topic: [PATCH] acl: use rte_calloc for temporary memory allocation Thread-Index: AQHR98bMloFFvwZovkmV/gPzRtdsvaBiVocggAB/W0CAABJCgA== Date: Wed, 31 Aug 2016 09:59:25 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B94FEA@irsmsx105.ger.corp.intel.com> References: <20160816140128.10149-1-vladyslav.buslov@harmonicinc.com> <20160816140128.10149-2-vladyslav.buslov@harmonicinc.com> <2601191342CEEE43887BDE71AB97725836B94E0C@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-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] acl: use rte_calloc for temporary memory allocation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Aug 2016 09:59:36 -0000 Hi Vlad,=20 >=20 > Hi Konstantin, >=20 > Thanks for your feedback. >=20 > Would you accept this change as config file compile-time parameter with l= ibc calloc as default? > It is one line change only so it is easy to ifdef. It is an option, but the main requirements from the community is to minimize number of build time config options, and instead provide ability to the user to configure things at runtime. That's why I thought about something like: + /* use EAL hugepages for temporary memory allocations, + * might improve build time, build increases hugepages + * demand significantly. + */ #define RTE_ACL_CFG_FLAG_HMEM 1 struct rte_acl_config { uint32_t num_categories; /**< Number of categories to build with. */ uint32_t num_fields; /**< Number of field definitions. */ struct rte_acl_field_def defs[RTE_ACL_MAX_FIELDS]; /**< array of field definitions. */ size_t max_size; /**< max memory limit for internal run-time structures. */ + uint64_t flags; };=20 And then change tb_pool() to use either calloc() or rte_alloc() based on th= e flags value. Another, probably even better and more flexible way is to allow user to spe= cify his own alloc routine: struct rte_acl_config { uint32_t num_categories; /**< Number of categories to build with. */ uint32_t num_fields; /**< Number of field definitions. */ struct rte_acl_field_def defs[RTE_ACL_MAX_FIELDS]; /**< array of field definitions. */ size_t max_size; /**< max memory limit for internal run-time structures. */ + void *(tballoc)( (size_t, size_t); + void (*tbfree)(void *); }; In that case user can provide his own alloc/free based on rte_calloc/rte_fr= ee or even on something else. The only drawback I see with that approaches, that in that case, you'll nee= d to follow ABI compliance rules, which probably means that your change might not make = 16.11. Konstantin=20 >=20 > Regards, > Vlad >=20 > -----Original Message----- > From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com] > Sent: Wednesday, August 31, 2016 4:28 AM > To: Vladyslav Buslov > Cc: dev@dpdk.org > Subject: RE: [PATCH] acl: use rte_calloc for temporary memory allocation >=20 > Hi Vladyslav, >=20 > > -----Original Message----- > > From: Vladyslav Buslov [mailto:vladyslav.buslov@harmonicinc.com] > > Sent: Tuesday, August 16, 2016 3:01 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: [PATCH] acl: use rte_calloc for temporary memory allocation > > > > Acl build process uses significant amount of memory which degrades > > performance by causing page walks when memory is allocated on regular h= eap using libc calloc. > > > > This commit changes tb_mem to allocate temporary memory on huge pages w= ith rte_calloc. >=20 > We deliberately used standard system memory allocation routines (calloc/f= ree) here. > With current design build phase was never considered to be an 'RT' phase = operation. > It is pretty cpu and memory expensive. > So if we'll use RTE memory for build phase it could easily consume all (o= r most) of it, and might cause DPDK process failure or degradation. > If you really feel that you (and other users) would benefit from using rt= e_calloc here (I personally still in doubt), then at least it should be a > new field inside rte_acl_config, that would allow user to control that be= havior. > Though, as I said above, librte_acl was never designed to ' to allocate t= ens of thousands of ACLs at runtime'. > To add ability to add/delete rules at runtime while keeping lookup time r= easonably low some new approach need to be introduced. > Konstantin >=20 > > > > Signed-off-by: Vladyslav Buslov > > --- > > lib/librte_acl/tb_mem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_acl/tb_mem.c b/lib/librte_acl/tb_mem.c index > > 157e608..c373673 100644 > > --- a/lib/librte_acl/tb_mem.c > > +++ b/lib/librte_acl/tb_mem.c > > @@ -52,7 +52,7 @@ tb_pool(struct tb_mem_pool *pool, size_t sz) > > size_t size; > > > > size =3D sz + pool->alignment - 1; > > - block =3D calloc(1, size + sizeof(*pool->block)); > > + block =3D rte_calloc("ACL_TBMEM_BLOCK", 1, size + > > +sizeof(*pool->block), 0); > > if (block =3D=3D NULL) { > > RTE_LOG(ERR, MALLOC, "%s(%zu)\n failed, currently allocated " > > "by pool: %zu bytes\n", __func__, sz, pool->alloc); > > -- > > 2.8.3