| Message ID | 1435242628-8619-1-git-send-email-bernard.iremonger@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 248DFC66A; Thu, 25 Jun 2015 16:30:36 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 13F5BC604 for <dev@dpdk.org>; Thu, 25 Jun 2015 16:30:34 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 25 Jun 2015 07:30:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,677,1427785200"; d="scan'208";a="514134216" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by FMSMGA003.fm.intel.com with ESMTP; 25 Jun 2015 07:30:34 -0700 Received: from sivswdev01.ir.intel.com (sivswdev01.ir.intel.com [10.237.217.45]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id t5PEUVFR021527; Thu, 25 Jun 2015 15:30:31 +0100 Received: from sivswdev01.ir.intel.com (localhost [127.0.0.1]) by sivswdev01.ir.intel.com with ESMTP id t5PEUV8P008661; Thu, 25 Jun 2015 15:30:31 +0100 Received: (from bairemon@localhost) by sivswdev01.ir.intel.com with id t5PEUVLC008657; Thu, 25 Jun 2015 15:30:31 +0100 From: Bernard Iremonger <bernard.iremonger@intel.com> To: dev@dpdk.org Date: Thu, 25 Jun 2015 15:30:28 +0100 Message-Id: <1435242628-8619-1-git-send-email-bernard.iremonger@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <PATCH> References: <PATCH> Subject: [dpdk-dev] [PATCH] 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 <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> |
Commit Message
Iremonger, Bernard
June 25, 2015, 2:30 p.m. UTC
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
lib/librte_ether/rte_ethdev.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
Comments
On Thu, 25 Jun 2015 15:30:28 +0100 Bernard Iremonger <bernard.iremonger@intel.com> wrote: > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > --- > lib/librte_ether/rte_ethdev.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index e13fde5..2404556 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -369,8 +369,14 @@ rte_eth_dev_uninit(struct rte_pci_device *pci_dev) > /* free ether device */ > rte_eth_dev_release_port(eth_dev); > > - if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + rte_free(eth_dev->data->rx_queues); > + rte_free(eth_dev->data->tx_queues); > rte_free(eth_dev->data->dev_private); > + rte_free(eth_dev->data->mac_addrs); > + rte_free(eth_dev->data->hash_mac_addrs); > + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); Glad to see this problem addressed. I would prefer that the component that created the object be responsible for doing its own cleanup.
Hi Bernard, > -----Original Message----- > From: Iremonger, Bernard > Sent: Thursday, June 25, 2015 3:30 PM > To: dev@dpdk.org > Cc: Zhang, Helin; Ananyev, Konstantin; Qiu, Michael; mukawa@igel.co.jp; Iremonger, Bernard > Subject: [PATCH] librte_ether: release memory in uninit function. > > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > --- > lib/librte_ether/rte_ethdev.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index e13fde5..2404556 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -369,8 +369,14 @@ rte_eth_dev_uninit(struct rte_pci_device *pci_dev) > /* free ether device */ > rte_eth_dev_release_port(eth_dev); > > - if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + rte_free(eth_dev->data->rx_queues); > + rte_free(eth_dev->data->tx_queues); > rte_free(eth_dev->data->dev_private); > + rte_free(eth_dev->data->mac_addrs); > + rte_free(eth_dev->data->hash_mac_addrs); Sorry, but I don't understand why you put last 2 rte_free()s here. You already do relese mac_addrs and hash_mac_addrs memory at each PMD _uninit routine. Plus, as Stephen said - it would be better if same component (PMD in that case) would do both alloc and free. Apart from that, patch looks good to me. Konstantin > + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); > + } > > eth_dev->pci_dev = NULL; > eth_dev->driver = NULL; > -- > 1.7.4.1
> -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, June 25, 2015 7:33 PM > To: Iremonger, Bernard; dev@dpdk.org > Cc: Zhang, Helin; Qiu, Michael; mukawa@igel.co.jp > Subject: RE: [PATCH] librte_ether: release memory in uninit function. > > Hi Bernard, > > > -----Original Message----- > > From: Iremonger, Bernard > > Sent: Thursday, June 25, 2015 3:30 PM > > To: dev@dpdk.org > > Cc: Zhang, Helin; Ananyev, Konstantin; Qiu, Michael; > > mukawa@igel.co.jp; Iremonger, Bernard > > Subject: [PATCH] librte_ether: release memory in uninit function. > > > > > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > > --- > > lib/librte_ether/rte_ethdev.c | 8 +++++++- > > 1 files changed, 7 insertions(+), 1 deletions(-) > > > > diff --git a/lib/librte_ether/rte_ethdev.c > > b/lib/librte_ether/rte_ethdev.c index e13fde5..2404556 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -369,8 +369,14 @@ rte_eth_dev_uninit(struct rte_pci_device > *pci_dev) > > /* free ether device */ > > rte_eth_dev_release_port(eth_dev); > > > > - if (rte_eal_process_type() == RTE_PROC_PRIMARY) > > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > > + rte_free(eth_dev->data->rx_queues); > > + rte_free(eth_dev->data->tx_queues); > > rte_free(eth_dev->data->dev_private); > > + rte_free(eth_dev->data->mac_addrs); > > + rte_free(eth_dev->data->hash_mac_addrs); > > Sorry, but I don't understand why you put last 2 rte_free()s here. > You already do relese mac_addrs and hash_mac_addrs memory at each PMD > _uninit routine. > Plus, as Stephen said - it would be better if same component (PMD in that > case) would do both alloc and free. > Apart from that, patch looks good to me. > > Konstantin Hi Konstantin, I thought it might be safer to free the memory here rather than relying on the PMD to do it. It is probably better if the PMD who does the alloc also does the free. I will send a v2 of the patch. Regards, Bernard. > > > > + memset(eth_dev->data, 0, sizeof(struct > rte_eth_dev_data)); > > + } > > > > eth_dev->pci_dev = NULL; > > eth_dev->driver = NULL; > > -- > > 1.7.4.1
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index e13fde5..2404556 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -369,8 +369,14 @@ rte_eth_dev_uninit(struct rte_pci_device *pci_dev) /* free ether device */ rte_eth_dev_release_port(eth_dev); - if (rte_eal_process_type() == RTE_PROC_PRIMARY) + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + rte_free(eth_dev->data->rx_queues); + rte_free(eth_dev->data->tx_queues); rte_free(eth_dev->data->dev_private); + rte_free(eth_dev->data->mac_addrs); + rte_free(eth_dev->data->hash_mac_addrs); + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); + } eth_dev->pci_dev = NULL; eth_dev->driver = NULL;