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 079CBA0C47; Thu, 14 Oct 2021 12:35:41 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BC75F4112E; Thu, 14 Oct 2021 12:35:40 +0200 (CEST) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mails.dpdk.org (Postfix) with ESMTP id BDACA40041 for ; Thu, 14 Oct 2021 12:35:39 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 4AF9D5C0186; Thu, 14 Oct 2021 06:35:39 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 14 Oct 2021 06:35:39 -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= kXxPPHM2QlVeNaaV2q6LsgnYwpHI1R+xtVO5utRKS2Y=; b=gT0C3X98pGqVfAu4 lKYHdD+oyN69PGKW57LDcWT0dJUOphGbn3kSxpiA4CfJyib5K/cR5RywBkTVa4sE EOYcrLi43TVCONXpZqAKzkDq/LkBJq8w+wWpbD2SJ+ylaFT7R+eTH2/6Oo7J9J1L FZXCBR1sr96C/ywM7EEh7zlTxV5HQY8Uyv7bgSZ5c6NAN7KgPDpqXvRmHA3/rf5u T7ufIisAe8xJ/PVweiqpUVaIkVKbt3AwjImrTe5WF8LsNoMAApUtQ5SSnLGruJ2J VmyH0pQj+4iriptd99lXvLyj6aotkcIoeK6yX4qxLG+JwozdX6cWMFGTTnOxL7EP yxi6iQ== 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=kXxPPHM2QlVeNaaV2q6LsgnYwpHI1R+xtVO5utRKS 2Y=; b=OWdcWzrJ46j0HfEG/yhTOhaX0dDPbYqu2h6LNjIvf/znEpmIbH6au5BQF aBDD+cDKaZNu/Atoi5TWnBEZ+JEpPdECdsBEmnuRksRT1fcAiL9GsXetlILaZNnV Y+GPtgTXwy/KmLtcFmtOBsRGJ9d1QEPsMTv8YUiJwYMEl3u+i3uTlb1x2IEZZkps PrXa6j09Z5PjtwYi9u0AKg6SHrIhzGpv8hjJCY5JcagKr4tFiA/lpkZB5HBWZ8XC P3ugqGeMs7sl+8xzRZi+UOmuHeK+avk4K7wZCryKAWBxSsMJOFsGavqfvueeOX+z EFZqF+d7clvJTPZXERoQEKiSkdKug== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvdduvddgvdelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 14 Oct 2021 06:35:37 -0400 (EDT) From: Thomas Monjalon To: Harman Kalra Cc: David Marchand , "dev@dpdk.org" , Raslan Darawsheh , Ray Kinsella , Dmitry Kozlyuk , "viacheslavo@nvidia.com" , "matan@nvidia.com" Date: Thu, 14 Oct 2021 12:35:34 +0200 Message-ID: <2130106.uiXJXLAz8d@thomas> In-Reply-To: References: <20210826145726.102081-1-hkalra@marvell.com> <4395254.EVvzvEdfqG@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v1 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" 14/10/2021 12:31, Harman Kalra: > From: Thomas Monjalon > > 14/10/2021 11:31, Harman Kalra: > > > From: Thomas Monjalon > > > > 13/10/2021 20:52, Thomas Monjalon: > > > > > 13/10/2021 19:57, Harman Kalra: > > > > > > From: dev On Behalf Of Harman Kalra > > > > > > > From: Thomas Monjalon > > > > > > > > 04/10/2021 11:57, David Marchand: > > > > > > > > > On Mon, Oct 4, 2021 at 10:51 AM Harman Kalra > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > +struct rte_intr_handle > > > > > > > > > > > > +*rte_intr_handle_instance_alloc(int > > > > size, > > > > > > > > > > > > + > > > > > > > > > > > > +bool > > > > > > > > > > > > +from_hugepage) { > > > > > > > > > > > > + struct rte_intr_handle *intr_handle; > > > > > > > > > > > > + int i; > > > > > > > > > > > > + > > > > > > > > > > > > + if (from_hugepage) > > > > > > > > > > > > + intr_handle = rte_zmalloc(NULL, > > > > > > > > > > > > + size * sizeof(struct rte_intr_handle), > > > > > > > > > > > > + 0); > > > > > > > > > > > > + else > > > > > > > > > > > > + intr_handle = calloc(1, size * > > > > > > > > > > > > + sizeof(struct rte_intr_handle)); > > > > > > > > > > > > > > > > > > > > > > We can call DPDK allocator in all cases. > > > > > > > > > > > That would avoid headaches on why multiprocess does > > > > > > > > > > > not work in some rarely tested cases. > > > > [...] > > > > > > > > I agree with David. > > > > > > > > I prefer a simpler API which always use rte_malloc, and make > > > > > > > > sure interrupts are always handled between rte_eal_init and > > > > rte_eal_cleanup. > > > > [...] > > > > > > > There are couple of more dependencies on glibc heap APIs: > > > > > > > 1. "rte_eal_alarm_init()" allocates an interrupt instance > > > > > > > which is used for timerfd, is called before > > > > > > > "rte_eal_memory_init()" which does the memseg init. > > > > > > > Not sure what all challenges we may face in moving alarm_init > > > > > > > after memory_init as it might break some subsystem inits. > > > > > > > Other option could be to allocate interrupt instance for > > > > > > > timerfd on first alarm_setup call. > > > > > > > > > > Indeed it is an issue. > > > > > > > > > > [...] > > > > > > > > > > > > There are many other drivers which statically declares the > > > > > > > interrupt handles inside their respective private structures > > > > > > > and memory for those structure was allocated from heap. For > > > > > > > such drivers I allocated interrupt instances also using glibc heap > > APIs. > > > > > > > > > > Could you use rte_malloc in these drivers? > > > > > > > > If we take the direction of 2 different allocations mode for the > > > > interrupts, I suggest we make it automatic without any API parameter. > > > > We don't have any function to check rte_malloc readiness I think. > > > > But we can detect whether shared memory is ready with this check: > > > > rte_eal_get_configuration()->mem_config->magic == RTE_MAGIC This > > > > check is true at the end of rte_eal_init, so it is false during probing. > > > > Would it be enough? Or should we implement rte_malloc_is_ready()? > > > > > > Hi Thomas, > > > > > > It's a very good suggestion. Let's implement "rte_malloc_is_ready()" > > > which could be as simple as " rte_eal_get_configuration()->mem_config- > > >magic == RTE_MAGIC" check. > > > There may be more consumers for this API in future. > > > > You cannot rely on the magic because it is set only after probing. > > For such API you need to have another internal flag to check that malloc is > > setup. > > Yeah, got that. You mean in case of bus probing although rte_malloc is setup > but eal_mcfg_complete() is calledt done yet. So we should set another malloc > specific flag at the end of rte_eal_memory_init(). Correct? I think the new internal flag should be at the end of rte_eal_malloc_heap_init(). Then a rte_internal function rte_malloc_is_ready() should check this flag. > But just for understanding, as David suggested that we preserve keep this flag > then why not use it, have rte_malloc and malloc bits and make a decision. > Let driver has the flexibility to choose. Do you see any harm in this? Which flag?