From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
Subject: Re: [PATCH v2] librte_ether: release memory in uninit
 function.
Date: Tue, 7 Jul 2015 10:53:05 +0000
Message-ID: <8CEF83825BEC744B83065625E567D7C204A467FF@IRSMSX108.ger.corp.intel.com>
References: <PATCH>
 <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>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
To: Tetsuya Mukawa <mukawa@igel.co.jp>, "dev@dpdk.org" <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 1BD6AA10
 for <dev@dpdk.org>; Tue,  7 Jul 2015 12:53:08 +0200 (CEST)
In-Reply-To: <559B49A0.1060904@igel.co.jp>
Content-Language: en-US
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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