Re: [dpdk-dev] [PATCH v2] librte_ether: release memory in uninit function.

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2015/07/08 18:59, Thomas Monjalon wrote:
> 2015-07-08 09:49, Iremonger, Bernard:
>> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
>>> On 2015/07/07 19:53, Iremonger, Bernard wrote:
>>>> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
>>>>> 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.
> It appears that the API is not defined.
> None of these assumptions is written in rte_ethdev.h.
> Please continue the discussion by submitting a patch fixing the doxygen
> comments of these functions.
> How an application developper is supposed to use these functions without
> having a clear explanation of their roles in doxygen?

Yes, this assumption is only written in DPDK user's guide.
So we should add description to doxygen. I will take care of it.

Anyway, I will add description to doxygen like user's guide.
 (A) stop() and close() must be called before detach().
 (B) close() releases all most all resources.
Probably Bernard and Michael patches are fit for above restrictions.

It may be good to add below in DPDK-2.2, though supporting below feature
will be more user friendly.
 (C) Even if stop() and close() are not called, detach() will take care
of it.

Probably, it's not so much time left before releasing DPDK-2.1.
So how about keeping current restriction written in user's guide, then
add a new feature in DPDK-2.2.

Regards,
Tetsuya