From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 727BEC48BE5 for ; Tue, 15 Jun 2021 06:52:53 +0000 (UTC) Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by mail.kernel.org (Postfix) with ESMTP id A3070613B6 for ; Tue, 15 Jun 2021 06:52:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A3070613B6 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=monjalon.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AEAF94067A; Tue, 15 Jun 2021 08:52:51 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by mails.dpdk.org (Postfix) with ESMTP id 304DF40140 for ; Tue, 15 Jun 2021 08:52:50 +0200 (CEST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailnew.nyi.internal (Postfix) with ESMTP id 85C065806C3; Tue, 15 Jun 2021 02:52:48 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Tue, 15 Jun 2021 02:52:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= Bh5xq+Kl0cCiHzoWgHCqS3quMk8+EHHqwdS/oQpJx8E=; b=aUwQiz6ndEyrjCl0 Y03bPTRg0x6eKCYbRqA/p+jcr49gXNXP7zRPh0x4MTWA+XoS40kCFfKH9xDPCZXI rUO+qM+TNJO8mhRU0ijyeBZtJvYo1am+gmSUwv6ptW+a0WeTEOiX2F6t9qdDsIsN Fu4s+wjPBE4V1nV4ckSxqiqiSWn7sucmBQf633BJBqVQxkM+7QiHfd2APTc58ZzH n1D4tEM4QvQSptoRHYjDIrO0o1ehhyopnrxjU2e4goyjn/ThjqyylDGxV/ZOXSTd xLZwNNUchPXcqPEeDO7l/XgJudi8YRRfUPZ/gijFeuGIY+G4eL+P3Iox5OjhMokN OmMD0Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=Bh5xq+Kl0cCiHzoWgHCqS3quMk8+EHHqwdS/oQpJx 8E=; b=hvk6UkebWY4eNFVnRLuUcmreG7lGXDoOsfGcyBTHtXnaZz5EVUscEh5NK Vy+pYAxsfZAYqfthPpGrqXwOcOV9LQTq6+o0f0vfwuhueYq2wc7rfqqPFmSGhkmU 6jhhquHf4pTF9q4F7ceh50K8CgIm/GOr6/z/7jYHjo3uMB5m+KZ994IhgaBXeP4g AAGMVIC+b5PzxwYTUverfXGi6YsO95iigQsJ1MhENOpZivNKXs/uhSYFzS56SKy/ 5QdbEdh/bf2FG46sBab/A3XtojKgJ7c9T9Z8OaxtSxuUOvsfCfPRKaG0o5AJkK11 SJ8Un1SOWrhFyDDsPa9WxoDsaQmQQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrfedviedgudduhecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthhqredttddtudenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpeefgeffiefhfeettdfhvdfgteekffffudekvedtvedtvdfgveeu udevgedvgeegtdenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 15 Jun 2021 02:52:45 -0400 (EDT) From: Thomas Monjalon To: "Ananyev, Konstantin" , Jerin Jacob Cc: "Richardson, Bruce" , Morten =?ISO-8859-1?Q?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" Date: Tue, 15 Jun 2021 08:52:44 +0200 Message-ID: <52380960.E65VIl4Blx@thomas> In-Reply-To: References: <20210614105839.3379790-1-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" 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 Mon= jalon > > > > > > 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 runtime, > > > > > > being as efficient as static arrays, but still limited to a max= imum. > > > > > > > > > > > > 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 performa= nce. > > > > > > > > > > > > After resize, the previous array is kept until the next resize > > > > > > to avoid crashs during a read without any lock. > > > > > > > > > > > > Each element is a pointer to a memory chunk dynamically allocat= ed. > > > > > > This is not good for cache locality but it allows to keep the s= ame > > > > > > 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-time > > > > > > maximums. > > > > > > > > > > I get the purpose and overall intention of this library. > > > > > > > > > > I probably already mentioned that I prefer "embedded style progra= mming" with fixed size arrays, rather than runtime configurability. It's > > > my personal opinion, and the DPDK Tech Board clearly prefers reducing= 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 libra= ry. :-) > > > > > > > > > > This library is likely to become a core library of DPDK, so I thi= nk 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 sh= ould 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 inte= nded 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 app= lications might also need dynamically sized arrays for their > > > application specific per-port runtime data, and this library could se= rve 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 the= 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 convinc= ed that > > > > we should switch away from the flat arrays or that we need fully dy= namic > > > > arrays that grow/shrink at runtime for ethdevs. I would suggest a h= alf-way > > > > house here, where we keep the ethdevs as an array, but one allocate= d/sized > > > > at runtime rather than statically. This would allow us to have a > > > > compile-time default value, but, for use cases that need it, allow = use of a > > > > flag e.g. "max-ethdevs" to change the size of the parameter given = to the > > > > malloc call for the array. This max limit could then be provided t= o apps > > > > too if they want to match any array sizes. [Alternatively those app= s could > > > > check the provided size and error out if the size has been increase= d beyond > > > > what the app is designed to use?]. There would be no extra derefere= nces per > > > > rx/tx burst call in this scenario so performance should be the same= as > > > > before (potentially better if array is in hugepage memory, I suppos= e). > > > > > > 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 for = 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 require > > extra synchronization for each read-write access to such parray element= (lock, rcu, ...). > > I think what Bruce suggests will be much ligther, easier to implement a= nd less error prone. > > At least for rte_ethdevs[] and friends. >=20 > +1 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, but the old one is still usable until the next resize. I think we don't need more.