From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B11ACA0C49; Tue, 15 Jun 2021 11:34:06 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 24A8E4067A; Tue, 15 Jun 2021 11:34:06 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id B066640140 for ; Tue, 15 Jun 2021 11:34:04 +0200 (CEST) IronPort-SDR: wsaucSAV2VedZZQLs2ErOlvKa/iqYKxDjMWwWGKfjQ3AqVksLc/Hm/dQQSENf87zW6xCavox7Q Ghqcx4prndZA== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="202931362" X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="202931362" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 02:34:03 -0700 IronPort-SDR: r2W0/aky9WYvLFtcJn0W8WkHgnpbT6/8wC99fSvfOxTIeBWv2xaOplVswzDkZlelo/FFQrAm9E rRpAjFYgpHcQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="421066958" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by orsmga002.jf.intel.com with ESMTP; 15 Jun 2021 02:34:03 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4; Tue, 15 Jun 2021 02:34:02 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4 via Frontend Transport; Tue, 15 Jun 2021 02:34:02 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.175) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.4; Tue, 15 Jun 2021 02:34:02 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BM69kvGAZAqCVY65dALc8mFSOS0Nw5kQBkDW/hImlEEsHXpa29XRv43PtWTUxg5gZIr0pBK2e5dxbCGZbCBWnj9tOQktj3zpHBHAznPOeUKjTN+LrnhtZacTb3NLVW6MCz/NmpskTskRXY1dZoS2j+xGWiMullhOLGk1RQxTzxIJMiPCVzBMEw+f6g93mLmWd7G0elfg9r2OPcnA5U/d12AKYAHG+G/Nh5JjBHQ63qb3T5vBm6GFWLUgoEP3OP3wUIJtAiW5xHQ21d0yX1/3GGeKA5/Y0sFLyu5S38eTDSdsGwYwzo758EB2p9FncoYBQ2ZhlBn6ZTL4WDRl2MZtFg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4sVEHtNrnPCpdYrE97rrecYlVFXaZDDR9zrkROcgb4s=; b=XM8ccQ4eXBolnyEl/3XD9HTNXmlgkOAYVBrno9z3NQhtHNXGEDlLBCyuNfeFEylOiPUXsWJGIIPQfX5bgvpi7qtAQWmE5WjfBGRrgoLA7VDvpuN3KXOlYT5O3boyVl5Fq0i/4yo3XnCA9Gi2qeMsV24Iwffqm6EhFvyolDMuyO5ND/oG/bvl+dXnyyH1ClwsRHAv2FoAzFQLQvvVmkUIfnOfU8+WPTozLltQdyGQuzdHVy4/5FPNke3RgxtRbFs21NuCXCbPFzYOV7C9V9DNbYu42gPBaT1LjWaHLz4FzQq5XAGSRKdHGowPtmi2I8brkb7K70trII8Dkf1LLI3NUA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4sVEHtNrnPCpdYrE97rrecYlVFXaZDDR9zrkROcgb4s=; b=kwTsqxPIrpT37IuOMEqFhZgc+Mjwm0FGKYUz29qEjxOC9woPdTTcDYgP2F8kCZRICJEHey3jLaJYN9jsEhME4p9q6BmAhkCM+ZQ9/V9gnoFLHVQVb9glJntqVXqsjExbQPWsgKbbPcXvCRnjjxRiHCOz60k/Z6H4As+p7ffWhRI= Received: from DM6PR11MB4491.namprd11.prod.outlook.com (2603:10b6:5:204::19) by DM6PR11MB3227.namprd11.prod.outlook.com (2603:10b6:5:5d::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4219.21; Tue, 15 Jun 2021 09:33:58 +0000 Received: from DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::7dc4:66b0:f76b:6d48]) by DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::7dc4:66b0:f76b:6d48%7]) with mapi id 15.20.4219.025; Tue, 15 Jun 2021 09:33:58 +0000 From: "Ananyev, Konstantin" To: Thomas Monjalon , Jerin Jacob CC: "Richardson, Bruce" , =?iso-8859-1?Q?Morten_Br=F8rup?= , "dev@dpdk.org" , "olivier.matz@6wind.com" , "andrew.rybchenko@oktetlabs.ru" , "honnappa.nagarahalli@arm.com" , "Yigit, Ferruh" , "jerinj@marvell.com" , "gakhil@marvell.com" Thread-Topic: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays Thread-Index: AQHXYQxT3nKj3OToOE+g04oypCLg2asTbdkAgAAOoICAAATCgIAAFeZggAAQUgCAAPyKAIAAIe3Q Date: Tue, 15 Jun 2021 09:33:58 +0000 Message-ID: References: <20210614105839.3379790-1-thomas@monjalon.net> <52380960.E65VIl4Blx@thomas> In-Reply-To: <52380960.E65VIl4Blx@thomas> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=intel.com; x-originating-ip: [109.255.184.192] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: db46f3b1-21cb-4e4a-2be0-08d92fe0b3a0 x-ms-traffictypediagnostic: DM6PR11MB3227: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: es+ls43sedv8IrHQm5BohItfwwjbzNEwAGth3fbebQBVaYaD+6i9v4ZR0+3E5S8r+dXVcfEV90lvV9YkBxTX20YWREqFtAm1KWAT9FeUfSLNPrB5P26nYBQDJOfCbpN22Dep3j8qg926iFteGx5pOKpYgcsEXN+J3IubtHXYJ6iDD7hymWz6gKZ/cyh2K9UqtNOoPo8Nd+qUK+x0fCt3Z9ZBYwHiLC2iWTlrmt9h7J5t2bRlcIs/amoagqJh7sb/80xdt2Q0RZR7S1TBi00/blIJHoTub1z0R5Si5q9PEs+4CAP/ZyTfLh/8z7VKd/lBFlgJ4qaYXO2pZxNfUXCZY4nXCvhYEdthlT746lJAipTyYbDqSIpO7TSpCNme7RoHgupIVM/1cclqbdDmxzurju3e6vvzeKCv0LaKb6U6WCr4qSo8tdu/Qjbz3Vfe3OzFeu5yMpJftuZN3k2sql7UjsIcn4UwEmqophVNGTPF5T+epCworF4lBWc3IT/Bh1VYC6WCZ1Fh6OUTPFzefWHk+y407kPpMrWFmgAGcpXNhcRXxyTA3uIAH7lzHZmIY76pvBkNGYtdVDtC7mFQ/JQvbT5aGmBgJGE5dORT6Qka8pM= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB4491.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(136003)(39860400002)(396003)(346002)(366004)(376002)(8936002)(26005)(6506007)(53546011)(52536014)(186003)(8676002)(55236004)(86362001)(55016002)(7696005)(4326008)(9686003)(38100700002)(316002)(71200400001)(76116006)(122000001)(110136005)(5660300002)(2906002)(66446008)(83380400001)(66556008)(66574015)(64756008)(54906003)(66476007)(478600001)(66946007)(33656002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?ONNQIoUx4y1AvTV7anYkk6cbCcgdkII+UOxgwIKPJa5dt3RPIwD59mOwdf?= =?iso-8859-1?Q?qo/73nox8gqNRQsMGUZgsBsybxOVx29jl9yVgnovXtHGfCvD+QTkWuE/Np?= =?iso-8859-1?Q?H79x4y9UeKTbyLdsSirYPjV1pDVSLR3c/YFItGtudCe9/p9WaKQztjY3k3?= =?iso-8859-1?Q?2YVGukyt4i+2xu3QCIn3fhlubjozsRNx6d8i9K0DI/cdnejx3SuhgzjR/6?= =?iso-8859-1?Q?3l9l7W6cYPi238h9z+WryVcRI9NoXaCzgATddeaqnwEUxvLBCfr94UqsEv?= =?iso-8859-1?Q?9GICumMPsdeWUbXdvc/W++UBolEi/Z4lxbqTPI61fKljzS7D5WNrvozvaY?= =?iso-8859-1?Q?4qalYQAmq2RDn3e61e6w63InozPMum77mGGPcU8zkiVq35S5Y9O0hfnEBv?= =?iso-8859-1?Q?hWmtTiUy5VAvXxKBbhUCRldNf6o1j+l0ct+0C3IeXMK6HGw3+XvrJZQfup?= =?iso-8859-1?Q?lkS6C+5Mj8/jWAlw69mJJrbP1cX2vtpmuxRwRxwT2DbZqx5XNtBdKPOau8?= =?iso-8859-1?Q?zgxjDAua/OqMkPNtNzfkejM5ZV880I6LGhIP106dRMng4LOebWhhOTVEkg?= =?iso-8859-1?Q?4Yy8SQj29rUv4poQB7ipOBK57dxwszhiAOB8I87WbbJZRyzmN83q7D8M6l?= =?iso-8859-1?Q?PjLWhR1nRgKnL5dHnA2sR53VB7TYmxSqsJee8EaAVEUyRAu+wULf4mERad?= =?iso-8859-1?Q?izfE60f+n0NgYXtwrw3AdBg3LzsrwR8cQaL5KsmU0/i46s8IcAFl8gz3RO?= =?iso-8859-1?Q?O4rP7JNdCsSd6onxuGMJ+JAH70OeGgL/+QL9to25C8//khraqGlumiOsF0?= =?iso-8859-1?Q?a3BbJdETWKPhpeYp4F/Qrz/MrPjaMdCJSpQGzGq0JjFuht2z8Ww5pWu+os?= =?iso-8859-1?Q?HakOOp3PRGkRIXywMrDy9Nnsl9K4S/dYe4G97xFgojuLGOe/sG7v/EcIn6?= =?iso-8859-1?Q?7kDSqM0kqRUKJqbQsoKsDt1Z2vWYFBY3LNgyGR2jEZIbxPthUquxxlWJtt?= =?iso-8859-1?Q?GDIGSDTXd07QjGiI17Lnxr0uZZUC6ymsfgDCJ8lISH2UAULA2dGKQ+l5XQ?= =?iso-8859-1?Q?sNZVY/tdwUptev9/aUCn/HCfTz9g196qkG2sWoTXHaEtHyxz82YYwwHgk7?= =?iso-8859-1?Q?vMci35ljlnb59bAQjK8H3sDh5It+F7Ae8FTeu9f1XWJHAzB3uDO+ESBZcM?= =?iso-8859-1?Q?ftkhZd9+CHGA9njRETX9wmozNsAuuRE+z8CCeHQAnsWloFzZxxUMPdtMdM?= =?iso-8859-1?Q?STo6FVOPcm3BOeNfMn19RQqt9Q5B2PRWrUCoNsupZrtGoLrMcGHOYFtuJt?= =?iso-8859-1?Q?VrbdAkeQCLJJgmy4ZbP4tfiTElcpGzLnJ6eB0yZmKn/8mge3RvuYA813iW?= =?iso-8859-1?Q?iiyiFKknzI?= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4491.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: db46f3b1-21cb-4e4a-2be0-08d92fe0b3a0 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Jun 2021 09:33:58.1794 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Nbb4Xz87xRR979MoNG/Fg+50rGdiadNdLYsckuq4UxdVUKyKgQjZaJzKER/mVD212G8uRrfhcKzRblD0/zU3AQ3MwFTseIH96OR2DTzIq28= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3227 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" > 14/06/2021 17:48, Jerin Jacob: > > On Mon, Jun 14, 2021 at 8:29 PM Ananyev, Konstantin > > wrote: > > > > 14/06/2021 15:15, Bruce Richardson: > > > > > On Mon, Jun 14, 2021 at 02:22:42PM +0200, Morten Br=F8rup wrote: > > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas M= onjalon > > > > > > > Sent: Monday, 14 June 2021 12.59 > > > > > > > > > > > > > > Performance of access in a fixed-size array is very good > > > > > > > because of cache locality > > > > > > > and because there is a single pointer to dereference. > > > > > > > The only drawback is the lack of flexibility: > > > > > > > the size of such an array cannot be increase at runtime. > > > > > > > > > > > > > > An approach to this problem is to allocate the array at runti= me, > > > > > > > being as efficient as static arrays, but still limited to a m= aximum. > > > > > > > > > > > > > > That's why the API rte_parray is introduced, > > > > > > > allowing to declare an array of pointer which can be resized > > > > > > > dynamically > > > > > > > and automatically at runtime while keeping a good read perfor= mance. > > > > > > > > > > > > > > After resize, the previous array is kept until the next resiz= e > > > > > > > to avoid crashs during a read without any lock. > > > > > > > > > > > > > > Each element is a pointer to a memory chunk dynamically alloc= ated. > > > > > > > This is not good for cache locality but it allows to keep the= same > > > > > > > memory per element, no matter how the array is resized. > > > > > > > Cache locality could be improved with mempools. > > > > > > > The other drawback is having to dereference one more pointer > > > > > > > to read an element. > > > > > > > > > > > > > > There is not much locks, so the API is for internal use only. > > > > > > > This API may be used to completely remove some compilation-ti= me > > > > > > > maximums. > > > > > > > > > > > > I get the purpose and overall intention of this library. > > > > > > > > > > > > I probably already mentioned that I prefer "embedded style prog= ramming" with fixed size arrays, rather than runtime > configurability. It's > > > > my personal opinion, and the DPDK Tech Board clearly prefers reduci= ng the amount of compile time configurability, so there is no way > for > > > > me to stop this progress, and I do not intend to oppose to this lib= rary. :-) > > > > > > > > > > > > This library is likely to become a core library of DPDK, so I t= hink it is important getting it right. Could you please mention a few > examples > > > > where you think this internal library should be used, and where it = should not be used. Then it is easier to discuss if the border line > between > > > > control path and data plane is correct. E.g. this library is not in= tended to be used for dynamically sized packet queues that grow and > shrink in > > > > the fast path. > > > > > > > > > > > > If the library becomes a core DPDK library, it should probably = be public instead of internal. E.g. if the library is used to make > > > > RTE_MAX_ETHPORTS dynamic instead of compile time fixed, then some a= pplications might also need dynamically sized arrays for their > > > > application specific per-port runtime data, and this library could = serve that purpose too. > > > > > > > > > > > > > > > > Thanks Thomas for starting this discussion and Morten for follow-= up. > > > > > > > > > > My thinking is as follows, and I'm particularly keeping in mind t= he cases > > > > > of e.g. RTE_MAX_ETHPORTS, as a leading candidate here. > > > > > > > > > > While I dislike the hard-coded limits in DPDK, I'm also not convi= nced that > > > > > we should switch away from the flat arrays or that we need fully = dynamic > > > > > arrays that grow/shrink at runtime for ethdevs. I would suggest a= half-way > > > > > house here, where we keep the ethdevs as an array, but one alloca= ted/sized > > > > > at runtime rather than statically. This would allow us to have a > > > > > compile-time default value, but, for use cases that need it, allo= w use of a > > > > > flag e.g. "max-ethdevs" to change the size of the parameter give= n to the > > > > > malloc call for the array. This max limit could then be provided= to apps > > > > > too if they want to match any array sizes. [Alternatively those a= pps could > > > > > check the provided size and error out if the size has been increa= sed beyond > > > > > what the app is designed to use?]. There would be no extra derefe= rences per > > > > > rx/tx burst call in this scenario so performance should be the sa= me as > > > > > before (potentially better if array is in hugepage memory, I supp= ose). > > > > > > > > I think we need some benchmarks to decide what is the best tradeoff= . > > > > I spent time on this implementation, but sorry I won't have time fo= r benchmarks. > > > > Volunteers? > > > > > > I had only a quick look at your approach so far. > > > But from what I can read, in MT environment your suggestion will requ= ire > > > extra synchronization for each read-write access to such parray eleme= nt (lock, rcu, ...). > > > I think what Bruce suggests will be much ligther, easier to implement= and less error prone. > > > At least for rte_ethdevs[] and friends. > > > > +1 >=20 > Please could you have a deeper look and tell me why we need more locks? > The element pointers doesn't change. > Only the array pointer change at resize, Yes, array pointer changes at resize, and reader has to read that value to access elements in the parray. Which means that we need some sync between readers and updaters to avoid reader using stale pointer (ref-count= er, rcu, etc.). I.E. updater can free old array pointer *only* when it can guarantee that t= here are no readers that still use it. =20 > but the old one is still usable until the next resize. Ok, but what is the guarantee that reader would *always* finish till next r= esize? As an example of such race condition: /* global one */ struct rte_parray pa; /* thread #1, tries to read elem from the array */=20 .... int **x =3D pa->array;=20 /* thread # 1 get suspended for a while at that point */ /* meanwhile thread #2 does: */ .... /* causes first resize(), x still valid, points to pa->old_array */=20 rte_parray_alloc(&pa, ...);=20 ..... /* causes second resize(), x now points to freed memory */ rte_parray_alloc(&pa, ...); ... /* at that point thread #1 resumes: */ /* contents of x[0] are undefined, 'p' could point anywhere, might cause segfault or silent memory corruption */ =20 int *p =3D x[0]; Yes probability of such situation is quite small. But it is still possible. > I think we don't need more.