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, > 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
