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 E18EAA0C4E; Fri, 15 Oct 2021 15:46:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C9A2B411CB; Fri, 15 Oct 2021 15:46:46 +0200 (CEST) Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) by mails.dpdk.org (Postfix) with ESMTP id AFDB7410F1 for ; Fri, 15 Oct 2021 15:46:45 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id 4876C5800BC; Fri, 15 Oct 2021 09:46:44 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Fri, 15 Oct 2021 09:46:44 -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= cJy4sr6iDp88QP5JwxKz61Z33ahdxS6GobVQIJLRASY=; b=j/umbitF2HJYTN1c YIov5SfRPy+rMl4PdNeh7LkpNdAu/0hFy3zSFzzV+gmvfPImOVJE9zMSxMxOhfWo f8ronYucyiYLN8q8fdVStGn7cw1Q3WkUkwoNswyKxmhLG2FeXuM2A5tdYzMfvYqa UzHsv6gb2yZRDHWjZZevW+xCeW84X/st7q1KBa57iUQGNGD6lw8JywsqrMRZC0wP cH/D7ocIOP3x1nkI5T+A+w2OdVDYaQbDk1H+4zgECR+WId9wAUTZK9kSX2B8mtU6 i8lq0tkMSPW1JtbU0wR1jbdn4D+Zkx5debo2Cm4Dx7AXuqVzEuHAE1HnmZDCSDfh EvFQrg== 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=cJy4sr6iDp88QP5JwxKz61Z33ahdxS6GobVQIJLRA SY=; b=l8CzXvKiA/eeaihClvgqoVyCVZwJDKh3XAnyt7C/PiAPZs85ZfhRbObKj YbjMNajWE6hoc5Ev77JYDvkJ8MjtAnEH5cbG5gMZu97Oo7GpKIqTDem6P3D7ei0C GB3xIWhI1SZ/TfRR5il+6V1OmQ19zRSZMXvrfjKGzS+vKTnSwcd3CxqJPWKqIJ/H 4EOQfLjf3J2n1GkSe03Wh9lGnZmWTVYPlrH0nKOfX7ToCDZ1fSGdEZUAvTAGoRNe MxZ/ZfYYg1vDoGX4sH/c+6UloYdWkjx8ev9aW4iofC1e0xI+RnEO0dARi10aMsm4 5sJHsGOqzKjXqgZyBYePHePw2fhrg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvddugedgieejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefhvffufffkjghfggfgtgesthfure dttddtvdenucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshes mhhonhhjrghlohhnrdhnvghtqeenucggtffrrghtthgvrhhnpedugefgvdefudfftdefge elgffhueekgfffhfeujedtteeutdejueeiiedvffegheenucevlhhushhtvghrufhiiigv pedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrd hnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 15 Oct 2021 09:46:39 -0400 (EDT) From: Thomas Monjalon To: fengchengwen Cc: dev@dpdk.org, ferruh.yigit@intel.com, bruce.richardson@intel.com, jerinj@marvell.com, jerinjacobk@gmail.com, andrew.rybchenko@oktetlabs.ru, mb@smartsharesystems.com, nipun.gupta@nxp.com, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com, honnappa.nagarahalli@arm.com, david.marchand@redhat.com, sburla@marvell.com, pkapoor@marvell.com, konstantin.ananyev@intel.com, conor.walsh@intel.com, kevin.laatz@intel.com Date: Fri, 15 Oct 2021 15:46:36 +0200 Message-ID: <3921430.YgJSOKJIcn@thomas> In-Reply-To: <73748677-58a7-34f6-56f5-0e2b090f669a@huawei.com> References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <2810284.T8RqjUy1RU@thomas> <73748677-58a7-34f6-56f5-0e2b090f669a@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v25 1/6] dmadev: introduce DMA device library 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/10/2021 11:59, fengchengwen: > On 2021/10/15 16:29, Thomas Monjalon wrote: > > 13/10/2021 09:41, Thomas Monjalon: > >> 13/10/2021 02:21, fengchengwen: > >>> On 2021/10/13 3:09, Thomas Monjalon wrote: > >>>> 11/10/2021 09:33, Chengwen Feng: > >>>>> +static void > >>>>> +dma_release(struct rte_dma_dev *dev) > >>>>> +{ > >>>>> + rte_free(dev->dev_private); > >>>>> + memset(dev, 0, sizeof(struct rte_dma_dev)); > >>>>> +} > >> [...] > >>>>> +int > >>>>> +rte_dma_pmd_release(const char *name) > >>>>> +{ > >>>>> + struct rte_dma_dev *dev; > >>>>> + > >>>>> + if (dma_check_name(name) != 0) > >>>>> + return -EINVAL; > >>>>> + > >>>>> + dev = dma_find_by_name(name); > >>>>> + if (dev == NULL) > >>>>> + return -EINVAL; > >>>>> + > >>>>> + dma_release(dev); > >>>>> + return 0; > >>>>> +} > >>>> > >>>> Trying to understand the logic of creation/destroy. > >>>> skeldma_probe > >>>> \-> skeldma_create > >>>> \-> rte_dma_pmd_allocate > >>>> \-> dma_allocate > >>>> \-> dma_data_prepare > >>>> \-> dma_dev_data_prepare > >>>> skeldma_remove > >>>> \-> skeldma_destroy > >>>> \-> rte_dma_pmd_release > >>>> \-> dma_release > >>> > >>> This patch only provide device allocate function, the 2st patch provide extra logic: > >>> > >>> diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c > >>> index 42a4693bd9..a6a5680d2b 100644 > >>> --- a/lib/dmadev/rte_dmadev.c > >>> +++ b/lib/dmadev/rte_dmadev.c > >>> @@ -201,6 +201,9 @@ rte_dma_pmd_release(const char *name) > >>> if (dev == NULL) > >>> return -EINVAL; > >>> > >>> + if (dev->state == RTE_DMA_DEV_READY) > >>> + return rte_dma_close(dev->dev_id); > >>> + > >>> dma_release(dev); > >>> return 0; > >>> } > >>> > >>> So the skeldma remove will be: > >>> > >>> skeldma_remove > >>> \-> skeldma_destroy > >>> \-> rte_dma_pmd_release > >>> \-> rte_dma_close > >>> \-> dma_release > >> > >> OK, in this case, no need to dma_release from rte_dma_pmd_release, > >> because it is already called from rte_dma_close. > > > > Ping for reply please. > > Sorry, I think the previous reply was enough, Let me explain: No, if previous answer was enough, I would not add a new comment. Please read again: " no need to dma_release from rte_dma_pmd_release, because it is already called from rte_dma_close " > The PMD use following logic create dmadev: > skeldma_probe > \-> skeldma_create > \-> rte_dma_pmd_allocate > \-> dma_allocate > \-> mark dmadev state to READY. > > The PMD remove will be: > skeldma_remove > \-> skeldma_destroy > \-> rte_dma_pmd_release > \-> rte_dma_close > \-> dma_release > > The application close dmadev: > rte_dma_close > \-> dma_release > > in the above case, the PMD remove and application close both call rte_dma_close, > I think that's what you expect. > > > skeldma is simple, please let me give you a complicated example: > hisi_dma_probe > \-> hisi_dma_create > \-> rte_dma_pmd_allocate > \-> dma_allocate > \-> hisi_hw_init > \-> if init fail, call rte_dma_pmd_release. > \-> dma_release > \-> if init OK, mark dmadev state to READY. > > as you can see, if hisi_hw_init fail, it call rte_dma_pmd_release to release dmadev, > it will direct call dma_release. > if hisi_hw_init success, it mean the hardware also OK, then mark dmadev state to > READY. if PMD remove the dmadev it will call rte_dma_close because the dmadev's state > is READY, and the application could also call rte_dma_close to destroy dmadev. > > > The rte_dma_pmd_release take two function: > 1. if the dmadev's hardware part init fail, the PMD could use this function release the > dmadev. > 2. if the dmadev's hardware part init success, the PMD could use this function destroy > the dmadev. > > > If we don't have the rte_dma_pmd_release function, we should export dma_release function > which invoked when the hardware init fail. > > And if we keep rte_dma_pmd_release, it correspond the rte_dma_pmd_allocate, the PMD just > invoke rte_dma_pmd_release to handle above both cases (hardware part init fail when probe > and remove phase). You are justifying the existence of the functions, OK, but I am just discussing one call of the function which is useless. Anyway, now I am in the process of merging v26, so I will send a fix.