From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Rybchenko Subject: Re: [RFC] ethdev: complete closing to free all resources Date: Wed, 12 Sep 2018 18:44:17 +0300 Message-ID: <69a4709e-c588-69a9-c916-15094eed5a6e@solarflare.com> References: <20180907233929.21950-1-thomas@monjalon.net> <2007089.03mgKk1tIn@xps> <402147ea-d88a-27a0-25a9-905c79fb5b89@solarflare.com> <3176257.grtejztXq6@xps> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: , To: Thomas Monjalon Return-path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id C0FEA34F3 for ; Wed, 12 Sep 2018 17:44:54 +0200 (CEST) In-Reply-To: <3176257.grtejztXq6@xps> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 09/12/2018 05:57 PM, Thomas Monjalon wrote: > 10/09/2018 10:54, Andrew Rybchenko: >> On 09/10/2018 11:42 AM, Thomas Monjalon wrote: >>> 10/09/2018 10:03, Andrew Rybchenko: >>>> On 09/08/2018 02:39 AM, Thomas Monjalon wrote: >>>>> After closing a port, it cannot be restarted. >>>>> So there is no reason to not free all associated resources. >>>>> >>>>> The last step was done with rte_eth_dev_detach() which is deprecated. >>>>> Instead of removing the associated rte_device, the driver should check >>>>> if no more port (ethdev, cryptodev, etc) is still open for the device. >>>>> Then the device resources can be freed by the driver inside the >>>>> dev_close() driver callback operation. >>>>> >>>>> The last ethdev freeing (dev_private and final release), which were done >>>>> by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close(). >>>> For me, it sounds more logical to kill dev_close and keep detach. >>>> IMHO, dev_close is artificial and hardly useful. detach is a local pair >>>> to attach. >>> I don't get your point. >>> >>> In order to free a port, we need close + detach. >>> We can keep only one. >>> I choose close because: >>> 1) attach/detach are deprecated >>> 2) probe/close is a more obvious pair >>> 3) we need the driver to free the lower level resources >> Yes, I'm sorry I used bad terminology. >> We have probe/remove pair for both PCI and vdev drivers and I mean >> that remove is a better candidate to be kept (as a pair for probe which >> allocates all resources). > OK, yes probe/remove is the pair at EAL level. > But if we want to request removal at ethdev level, rte_eth_dev_close > is the function. > > Note that there is no function to request creation of an ethdev port. > Adding a new port is done only by the PMD during probing (rte_bus level). The overall picture is too vague. May be it is simply incomplete looking at the patch only. I understand that restructuring is required looking at rte_eth_dev_destroy() (which is used by i40e and ixgbe drivers only) and rte_eth_dev_pci_release() used by other PCI drivers. Right now it looks symmetric at least on drivers level which provides init callback to probe to do driver-specific job and uninit callback to remove to free driver-specific resources. I can't say if the patch is right or wrong direction since final design is unclear.