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 299E0A0C47; Tue, 15 Jun 2021 11:50:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E84A64067A; Tue, 15 Jun 2021 11:50:22 +0200 (CEST) Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) by mails.dpdk.org (Postfix) with ESMTP id 0CC5240140 for ; Tue, 15 Jun 2021 11:50:21 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id 8FCD358080F; Tue, 15 Jun 2021 05:50:20 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Tue, 15 Jun 2021 05:50:20 -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= vQ+XVjnO2GkNC2BvbPMwx/Oh3SzyxVaB163pOzfeXBI=; b=h31Oe+zXLJlEIrNk uTmQsWEjnm+OlV4mJOep04YoG5FfhD2k7/4Ye+dQUwD7QINnVTgybFvQjecfOUnA Zsa8vLB6Z4xxSJrPgSmYw+o2MnY5AljEUIMc5GSwOou2fvkyj3Jxk145rXYiSMoX JBAwLBeJwU270aN2E9a05NN0fi8u45pIvfGNFpF/Ckpkeph5nvcdynAB+0haes4K 8OY15TC2VDfovEbyVmDGxN1LvivuVJqwMh23m8XG/6SUhafTJtkz/m7wWCk/KCck 6m5BexxRd79eRT4x5nZO2AiJPt83/A22mI4R1mUVQhWsHWbKcwMci7qtrGBYkS7Y xBRzLg== 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=vQ+XVjnO2GkNC2BvbPMwx/Oh3SzyxVaB163pOzfeX BI=; b=Gs6YCybIRkNFq2YDppg1ywlVZO4kzwQI1BQ7swnv7OBm1nB3H5oRvuGOI XsYmcHiY/8dCVh7Lu3Ulli04m/ym1EOc54agebxgRR3C1bm0ZrUYm3qyJ4zn2e1Q vfhpQq4p5GNHvqjOGsdvA6V4G3pMBd+q4A/3MszU7KCBy0Qwd1rwKsvhoFPkfaZ/ S+n9ZiIE0H3TDJVBkb5PQJo4Eqg8NwK1TjmOcZv/QZxxQdL9heJ6y0N6Pc+Cdlex wTcmJ2Wlvs0QKVKq0CSC0DDlVKkf/ns4cJduKW2sVL7bh4lmwRQjHfi5BLvTbtOK zG1tKW/+idhXxYBj0y4btaRuqAiqw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrfedvjedgvddtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 15 Jun 2021 05:50:17 -0400 (EDT) From: Thomas Monjalon To: Jerin Jacob , "Ananyev, Konstantin" 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 11:50:16 +0200 Message-ID: <27997952.XWs9bGgn2z@thomas> In-Reply-To: References: <20210614105839.3379790-1-thomas@monjalon.net> <52380960.E65VIl4Blx@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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" 15/06/2021 11:33, Ananyev, Konstantin: > > 14/06/2021 17:48, Jerin Jacob: > > > On Mon, Jun 14, 2021 at 8:29 PM Ananyev, Konstantin > > > wrote: > > > > 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 and less error prone. > > > > At least for rte_ethdevs[] and friends. > > > > > > +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, > > 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-counter, rcu, etc.). No The old array is still there, so we don't need sync. > I.E. updater can free old array pointer *only* when it can guarantee that there are no > readers that still use it. No Reading an element is OK because the pointer to the element is not changed. Getting the pointer to an element from the index is the only thing which is blocking the freeing of an array, and I see no reason why dereferencing an index would be longer than 2 consecutive resizes of the array. > > but the old one is still usable until the next resize. > > Ok, but what is the guarantee that reader would *always* finish till next resize? > As an example of such race condition: > > /* global one */ > struct rte_parray pa; > > /* thread #1, tries to read elem from the array */ > .... > int **x = pa->array; We should not save the array pointer. Each index must be dereferenced with the macro getting the current array pointer. So the interrupt is during dereference of a single index. > /* 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 */ > rte_parray_alloc(&pa, ...); > ..... > /* causes second resize(), x now points to freed memory */ > rte_parray_alloc(&pa, ...); > ... 2 resizes is a very long time, it is at minimum 33 allocations! > /* at that point thread #1 resumes: */ > > /* contents of x[0] are undefined, 'p' could point anywhere, > might cause segfault or silent memory corruption */ > int *p = x[0]; > > > Yes probability of such situation is quite small. > But it is still possible. In device probing, I don't see how it is realistically possible: 33 device allocations during 1 device index being dereferenced. I agree it is tricky, but that's the whole point of finding tricks to keep fast code. > > I think we don't need more.