From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C39C5A0C41; Tue, 19 Oct 2021 17:58:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B3893411E0; Tue, 19 Oct 2021 17:58:45 +0200 (CEST) Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by mails.dpdk.org (Postfix) with ESMTP id 25CAA411C1 for <dev@dpdk.org>; Tue, 19 Oct 2021 17:58:44 +0200 (CEST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 07F403201CAD; Tue, 19 Oct 2021 11:58:41 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Tue, 19 Oct 2021 11:58:42 -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=fm2; bh= BnMORYZJUhHx4yTfc8RL1jEjOML1Rs/NNR4w7XO9HK0=; b=ic8llGo4Sde41sZJ KBEXF0zHtjuf/gbfWvJAECtB0ePMkN2ZF61wK+CYcJN8hBCz2+JiY9GG6sGTZ9wR CDUzCybRu5HDHCEWv+EVHdsNth/M4nDrUCd/xhjc3QoaYnVW3/c5KlBCFV4AiwqJ xHk8/ZD/OFY+yHFSnyc9KDaQPAutJ5s3ohOYVy1Id1c72+fHUzlOEQpow4Yx8Rv4 hllNknAradlyDrRRwvpf8m3DatFCHSg3bwpVJ6Xaa3S3Q4gvov/SBg2FY/l51qD5 yIKwZnqqK8rYZBObzxM8iqyFX/TooINwcU4ZkPsAgTQDO5N0skmwBCOTtVMM2wnt gxSrWg== 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=fm1; bh=BnMORYZJUhHx4yTfc8RL1jEjOML1Rs/NNR4w7XO9H K0=; b=bdLpuN8LslMDZAlXLXE+63uufWybZpwkADzVcA9xbkNv3FyJnE1Nl/G71 Bng05lsh1NdEYiHaA7S+uL8aa90geqUSbMnAby4y9hT0kzIwUKIr9LQ9CCQH5uOE jsEUE1efgG8KeH5R8aD5itH3lbC1jarb/tFDnAFeE7XWxlyP9O6Fzc/2CTZ6ZG2F haao7NgnOtwD8MNu1v9VtGhBMhHJr6XM9oO7VKC28gVj7kRNP3O6aP6pJKu2h/hK PWPQPYLPtqL18dROARSX7HWip4MmcUnNFb3PTFBWqhQHck/ByW1AY7ouMYvFNF3h Qgr2LLUCyp81LNWnqs5fXi1ogYNVg== X-ME-Sender: <xms:MetuYXu9uy8ddl5Bsk3ireHH5ynVu-K5I5Ye1WGwXl2j_XFTgcbH2g> <xme:MetuYYdLhp6pLy9Jgo-x8ARYtiTVlT40X2_OE4bAmeMJAvXzd943DFRaW84DKvBbt -0j78siu_vFwhevIg> X-ME-Received: <xmr:MetuYazK3dDJBFQWzNUW-HkNMJ1exFmVyxy3DOUpAv1gH9z_G8ByBjzr39QLLrZ2hgXx7YJjyCFjYh8i77QIunSb0w> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvddvvddgledtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: <xmx:MetuYWM_-XXtUcE5OMsK0PKOlqAocq_chwJlXEdFz7v_UDWRYjETNg> <xmx:MetuYX-zmq823lxH7J4664KANXaPn8t_irOhFhgCExAozHZmhBeNTA> <xmx:MetuYWUjHd7gsnkGDyFoFpqVlyZjvti-FEgLkSwvJcxMIbZGKynlyA> <xmx:MetuYTb6wfxb3FdGGU0KtKhEl2Hq8Ml06d8DiMnrp8EW-PiC93j4tg> Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 19 Oct 2021 11:58:40 -0400 (EDT) From: Thomas Monjalon <thomas@monjalon.net> To: Stephen Hemminger <stephen@networkplumber.org>, Harman Kalra <hkalra@marvell.com> Cc: "david.marchand@redhat.com" <david.marchand@redhat.com>, "dmitry.kozliuk@gmail.com" <dmitry.kozliuk@gmail.com>, dev@dpdk.org, Ray Kinsella <mdr@ashroe.eu> Date: Tue, 19 Oct 2021 17:58:38 +0200 Message-ID: <11513915.eqH0zLBXoz@thomas> In-Reply-To: <BL1PR18MB41970024F2181D6A1ADB7E46C5BD9@BL1PR18MB4197.namprd18.prod.outlook.com> References: <20210826145726.102081-1-hkalra@marvell.com> <20211018155654.0d3ffbed@hermes.local> <BL1PR18MB41970024F2181D6A1ADB7E46C5BD9@BL1PR18MB4197.namprd18.prod.outlook.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v3 2/7] eal/interrupts: implement get set APIs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> 19/10/2021 10:32, Harman Kalra: > From: Stephen Hemminger <stephen@networkplumber.org> > > On Tue, 19 Oct 2021 01:07:02 +0530 > > Harman Kalra <hkalra@marvell.com> wrote: > > > + /* Detect if DPDK malloc APIs are ready to be used. */ > > > + mem_allocator = rte_malloc_is_ready(); > > > + if (mem_allocator) > > > + intr_handle = rte_zmalloc(NULL, sizeof(struct > > rte_intr_handle), > > > + 0); > > > + else > > > + intr_handle = calloc(1, sizeof(struct rte_intr_handle)); > > > > This is problematic way to do this. > > The reason to use rte_malloc vs malloc should be determined by usage. > > > > If the pointer will be shared between primary/secondary process then it has > > to be in hugepages (ie rte_malloc). If it is not shared then then use regular > > malloc. > > > > But what you have done is created a method which will be a latent bug for > > anyone using primary/secondary process. > > > > Either: > > intr_handle is not allowed to be used in secondary. > > Then always use malloc(). > > Or. > > intr_handle can be used by both primary and secondary. > > Then always use rte_malloc(). > > Any code path that allocates intr_handle before pool is > > ready is broken. > > Hi Stephan, > > Till V2, I implemented this API in a way where user of the API can choose > If he wants intr handle to be allocated using malloc or rte_malloc by passing > a flag arg to the rte_intr_instanc_alloc API. User of the API will best know if > the intr handle is to be shared with secondary or not. Yes the caller should know, but it makes usage more difficult. Using rte_malloc always is simpler. > But after some discussions and suggestions from the community we decided > to drop that flag argument and auto detect on whether rte_malloc APIs are > ready to be used and thereafter make all further allocations via rte_malloc. > Currently alarm subsystem (or any driver doing allocation in constructor) gets > interrupt instance allocated using glibc malloc that too because rte_malloc* > is not ready by rte_eal_alarm_init(), while all further consumers gets instance > allocated via rte_malloc. Yes the general case is to allocate after rte_malloc is ready. Anyway a constructor should not allocate complicate things. > I think this should not cause any issue in primary/secondary model as all interrupt > instance pointer will be shared. Infact to avoid any surprises of primary/secondary > not working we thought of making all allocations via rte_malloc. Yes > David, Thomas, Dmitry, please add if I missed anything. I understand Stephen's concern but I think this choice is a good compromise. Ideally we should avoid doing real stuff in constructors. > Can we please conclude on this series APIs as API freeze deadline (rc1) is very near. I vote for keeping this design.