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 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 ; 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: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvddvvddgledtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 19 Oct 2021 11:58:40 -0400 (EDT) From: Thomas Monjalon To: Stephen Hemminger , Harman Kalra Cc: "david.marchand@redhat.com" , "dmitry.kozliuk@gmail.com" , dev@dpdk.org, Ray Kinsella Date: Tue, 19 Oct 2021 17:58:38 +0200 Message-ID: <11513915.eqH0zLBXoz@thomas> In-Reply-To: References: <20210826145726.102081-1-hkalra@marvell.com> <20211018155654.0d3ffbed@hermes.local> 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 19/10/2021 10:32, Harman Kalra: > From: Stephen Hemminger > > On Tue, 19 Oct 2021 01:07:02 +0530 > > Harman Kalra 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.