From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 4F3A1C37E for ; Mon, 29 Jun 2015 18:42:38 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 29 Jun 2015 09:42:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,699,1432623600"; d="scan'208";a="752618719" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga002.fm.intel.com with ESMTP; 29 Jun 2015 09:42:26 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.201]) by IRSMSX104.ger.corp.intel.com ([169.254.5.171]) with mapi id 14.03.0224.002; Mon, 29 Jun 2015 17:42:24 +0100 From: "Iremonger, Bernard" To: "Qiu, Michael" , "dev@dpdk.org" Thread-Topic: [PATCH v2] librte_ether: release memory in uninit function. Thread-Index: AQHQr/MWrMnfGSFKL02MpXHseHw6cp3DptjA Date: Mon, 29 Jun 2015 16:42:23 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C204A42F5F@IRSMSX108.ger.corp.intel.com> References: <1435311160-8679-1-git-send-email-bernard.iremonger@intel.com> <533710CFB86FA344BFBF2D6802E60286046A625A@SHSMSX101.ccr.corp.intel.com> <8CEF83825BEC744B83065625E567D7C204A42C88@IRSMSX108.ger.corp.intel.com> <533710CFB86FA344BFBF2D6802E60286046A6ED8@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286046A6ED8@SHSMSX101.ccr.corp.intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] librte_ether: release memory in uninit function. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Jun 2015 16:42:39 -0000 > -----Original Message----- > From: Qiu, Michael > Sent: Monday, June 29, 2015 4:22 PM > To: Iremonger, Bernard; dev@dpdk.org > Cc: Zhang, Helin; Ananyev, Konstantin; mukawa@igel.co.jp; Stephen > Hemminger > Subject: Re: [PATCH v2] librte_ether: release memory in uninit function. >=20 > On 2015/6/29 18:20, Iremonger, Bernard wrote: > > > >> -----Original Message----- > >> From: Qiu, Michael > >> Sent: Monday, June 29, 2015 9:55 AM > >> To: Iremonger, Bernard; dev@dpdk.org > >> Cc: Zhang, Helin; Ananyev, Konstantin; mukawa@igel.co.jp; Stephen > >> Hemminger > >> Subject: Re: [PATCH v2] librte_ether: release memory in uninit functio= n. > >> > >> On 6/26/2015 5:32 PM, Iremonger, Bernard wrote: > >>> Changes in v2: > >>> do not free mac_addrs and hash_mac_addrs here. > >>> > >>> Signed-off-by: Bernard Iremonger > >>> --- > >>> lib/librte_ether/rte_ethdev.c | 6 +++++- > >>> 1 files changed, 5 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/lib/librte_ether/rte_ethdev.c > >>> b/lib/librte_ether/rte_ethdev.c index e13fde5..7ae101a 100644 > >>> --- a/lib/librte_ether/rte_ethdev.c > >>> +++ b/lib/librte_ether/rte_ethdev.c > >>> @@ -369,8 +369,12 @@ rte_eth_dev_uninit(struct rte_pci_device > >> *pci_dev) > >>> /* free ether device */ > >>> rte_eth_dev_release_port(eth_dev); > >>> > >>> - if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) > >>> + if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) { > >>> + rte_free(eth_dev->data->rx_queues); > >>> + rte_free(eth_dev->data->tx_queues); > >>> rte_free(eth_dev->data->dev_private); > >>> + memset(eth_dev->data, 0, sizeof(struct > >> rte_eth_dev_data)); > >>> + } > >>> > >>> eth_dev->pci_dev =3D NULL; > >>> eth_dev->driver =3D NULL; > >> > >> Actually, This could be put in rte_eth_dev_close() becasue queues > >> should be released when closed. > >> > >> Also before free dev->data->rx_queues you should make sure > >> dev->data->rx_queues[i] has been freed in PMD close() function, So > >> dev->data->this > >> two should be better done at the same time, ether in > >> rte_eth_dev_close() or in PMD close() function. For hotplug in fm10k, > >> I put it in PMD close() function. > >> > >> Thanks, > >> Michael > > Hi Michael, > > > > The consensus is that the rx_queue and tx_queue memory should not be > released in the PMD as it is not allocated by the PMD. The memory is > allocated in rte_eth_dev_rx_queue_config() and > rte_eth_dev_tx_queue_config(), which are both called from > rte_eth_dev_configure() which is called by the application (for example > test_pmd). So it seems to make sense to free this memory in > rte_eth_dev_uninit(). >=20 > It really make sense to free memory in rte_ether level, but when close a = port > with out detached? just as stop --> close() --> quit(), the memory will n= ot be > released :) >=20 In the above scenario lots of memory will not be released. This is why the detach() and the underlying dev_uninit() functions were int= roduced. The dev_uninit() functions currently call dev_close() which in turn calls = dev_stop() which calls dev_clear_queues().=20 The dev_clear_queues() function does not release the queue_memory or the q= ueue array memory. The queue memory is now released in the dev_uninit() and= the queue array memory is released in the rte_eth_dev_uninit() function. If the queue array memory is released in rte_eth_dev_close() then the relea= se of the queue_memory will have to be moved to the dev_close() functions f= rom the dev_uninit() functions. This will impact all the existing PMD hotp= lug patches. It will also change the existing dev_close() functionality. My preference is to leave the existing dev_close() functions unchanged as f= ar as possible and to do what needs to be done in the dev_uninit() function= s. We probably need the view of the maintainers as to whether this should be d= one in the close() or uninit() functions. =20 Regards, Bernard. =20