From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 1BD6AA10 for ; Tue, 7 Jul 2015 12:53:08 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 07 Jul 2015 03:53:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,422,1432623600"; d="scan'208";a="742095066" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga001.fm.intel.com with ESMTP; 07 Jul 2015 03:53:07 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.201]) by IRSMSX151.ger.corp.intel.com ([169.254.4.108]) with mapi id 14.03.0224.002; Tue, 7 Jul 2015 11:53:06 +0100 From: "Iremonger, Bernard" To: Tetsuya Mukawa , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] librte_ether: release memory in uninit function. Thread-Index: AQHQr/MWrMnfGSFKL02MpXHseHw6cp3DptjAgAvF98qAAHP60A== Date: Tue, 7 Jul 2015 10:53:05 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C204A467FF@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> <8CEF83825BEC744B83065625E567D7C204A42F5F@IRSMSX108.ger.corp.intel.com> <533710CFB86FA344BFBF2D6802E60286046A75A4@SHSMSX101.ccr.corp.intel.com> <533710CFB86FA344BFBF2D6802E60286046AE60D@SHSMSX101.ccr.corp.intel.com> <559B49A0.1060904@igel.co.jp> In-Reply-To: <559B49A0.1060904@igel.co.jp> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] 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: Tue, 07 Jul 2015 10:53:10 -0000 > -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > Sent: Tuesday, July 7, 2015 4:38 AM > To: dev@dpdk.org > Cc: Qiu, Michael; Iremonger, Bernard; thomas.monjalon@6wind.com; > Ananyev, Konstantin; Stephen Hemminger; Zhang, Helin; Chen, Jing D; Neil > Horman > Subject: Re: [dpdk-dev] [PATCH v2] librte_ether: release memory in uninit > function. >=20 > On 2015/07/06 20:35, Qiu, Michael wrote: > > Hi, all > > > > As we has gap on the memory release action to be done in which step, I > > appreciate all your comments on this patch. > > > > Currently, the correct quit sequence for testpmd is stop() ---> > > port_stop() --> port_close() --> quit(). This will lead lots of memory > > not released by default, like queues. There is also the scenario (outside of testpmd) where an application can ca= ll rte_eth_dev_close() or rte_eth_dev_detach() which calls rte_eth_dev_unin= it() directly from an application. In these cases the memory allocated in the EAL layer should be released in = both rte_eth_dev_close() and rte_eth_dev_detach(). This patch is intended to address the rte_eth_dev_detach() case only (hotpl= ug case). Perhaps there should be a separate patch to address the rte_eth_dev_close()= case. Regards, Bernard. > > > > We have 3 potential proposals(normal case means without hotplug): > > > > 1. Those memory release in close() other left in uninit() > > This will work fine for both hotplug case and normal case. >=20 > +1 >=20 > I assume that once close() is called, we cannot start the port again with= out > hotplugging. > This is my premise. >=20 > It might be good that we move finalization code to close() as much as > possible, because of followings. > 1. Anyway, once close() is called, we cannot start the port again. So it = should > be ok to free resources in close(). > 2. Non-hotplugging applications can release resources if we implement > finalization code to close(). >=20 > Also I guess Stephen's suggestion is important to keep code clean. > It may be good that 'dev->data->rx/tx_queues[queue_id]' are freed in > rx/tx_queue_release() of each PMD, and rx/tx_queue_release() will be > called by rte_eth_dev_close(). Also 'dev->data->rx/tx_queues[]' should be > freed in ethdev.c. >=20 > > 3. Combine close() and uninit(), only one will be left. > > This will work fine for both hotplug case and normal case. But it > > may change a lot, such as logic. >=20 > I guess this will be second option. But I agree we need to change a lot.= Also > after enabling hotplug by default, when someone only wants to close the > port, it will be impossible. >=20 > Regards, > Tetsuya