> -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa at igel.co.jp] > Sent: Wednesday, July 8, 2015 4:48 AM > To: Iremonger, Bernard; dev at dpdk.org > Cc: Qiu, Michael; thomas.monjalon at 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. > > On 2015/07/07 19:53, Iremonger, Bernard wrote: > >> -----Original Message----- > >> From: Tetsuya Mukawa [mailto:mukawa at igel.co.jp] > >> Sent: Tuesday, July 7, 2015 4:38 AM > >> To: dev at dpdk.org > >> Cc: Qiu, Michael; Iremonger, Bernard; thomas.monjalon at 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. > >> > >> 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 > > call > rte_eth_dev_close() or rte_eth_dev_detach() which calls > rte_eth_dev_uninit() 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(). > > Hi Bernard, > > All DPDK applications that uses hotpluggingmust call rte_eth_dev_stop() and > rte_eth_dev_close()APIs before detaching ports. > (This is described in DPDK documentation) Could we ignore applications that > only calls rte_eth_dev_detach()? > > > This patch is intended to address the rte_eth_dev_detach() case only > (hotplug case). > > Perhaps there should be a separate patch to address the > rte_eth_dev_close() case. > > We all needs to have consensus about how to implement finalization code in > close() and uninit(). > Probably one of options will be implementing finalization code in > close() as much as possible. > > Tetsuya,
Hi Tetsuya, Testpmd enforces the dev_stop(), dev_close() and dev_detach() sequence. There is no reason why an application cannot call dev_detach() directly. During internal review of PMD dev_uninit() functions it was decided to ensure that this sequence was adhered to by calling dev_close() which calls dev_stop() from the dev_uninit() function. In the PMD hotplug patches the following calling sequence is implemented: dev_uninit() calls dev_close() calls dev_stop(). At present most of the finalization code is implemented in dev_close() and dev_stop(). The dev_uninit() functions implement what is left of the finalization code. Regards, Bernard. > > > 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. > >> +1 > >> > >> I assume that once close() is called, we cannot start the port again > >> without hotplugging. > >> This is my premise. > >> > >> 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(). > >> > >> 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. > >> > >>> 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. > >> 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. > >> > >> Regards, > >> Tetsuya