[dpdk-dev,v2] net/vhost: stop dev in close and address mem leak

Message ID 1491524797-27299-1-git-send-email-sabhang@brocade.com (mailing list archive)
State Accepted, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Sagar Abhang April 7, 2017, 12:26 a.m. UTC
  Move the call to stop the device inside the close routine because close
needs to stop the device if it isn't stopped.

Free the allocated queue buffers in close instead of doing so in remove.
Original code had these clean ups in remove which was causing memory
leak.

Signed-off-by: Sagar Abhang <sabhang@brocade.com>
---

 v2:
 - Modified the log message as requested.

 drivers/net/vhost/rte_eth_vhost.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
  

Comments

Yuanhan Liu April 13, 2017, 1:45 a.m. UTC | #1
On Thu, Apr 06, 2017 at 05:26:37PM -0700, Sagar Abhang wrote:
> Move the call to stop the device inside the close routine because close
> needs to stop the device if it isn't stopped.
> 
> Free the allocated queue buffers in close instead of doing so in remove.
> Original code had these clean ups in remove which was causing memory
> leak.
> 
> Signed-off-by: Sagar Abhang <sabhang@brocade.com>

Applied to dpdk-next-virtio.

	--yliu
  

Patch

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 41ce5fc..b6fc94a 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -791,11 +791,14 @@  eth_dev_close(struct rte_eth_dev *dev)
 {
 	struct pmd_internal *internal;
 	struct internal_list *list;
+	unsigned int i;
 
 	internal = dev->data->dev_private;
 	if (!internal)
 		return;
 
+	eth_dev_stop(dev);
+
 	rte_vhost_driver_unregister(internal->iface_name);
 
 	list = find_internal_resource(internal->iface_name);
@@ -807,9 +810,17 @@  eth_dev_close(struct rte_eth_dev *dev)
 	pthread_mutex_unlock(&internal_list_lock);
 	rte_free(list);
 
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
+		rte_free(dev->data->rx_queues[i]);
+	for (i = 0; i < dev->data->nb_tx_queues; i++)
+		rte_free(dev->data->tx_queues[i]);
+
+	rte_free(dev->data->mac_addrs);
 	free(internal->dev_name);
 	free(internal->iface_name);
 	rte_free(internal);
+
+	dev->data->dev_private = NULL;
 }
 
 static int
@@ -1198,7 +1209,6 @@  static int
 rte_pmd_vhost_remove(const char *name)
 {
 	struct rte_eth_dev *eth_dev = NULL;
-	unsigned int i;
 
 	RTE_LOG(INFO, PMD, "Un-Initializing pmd_vhost for %s\n", name);
 
@@ -1207,19 +1217,11 @@  rte_pmd_vhost_remove(const char *name)
 	if (eth_dev == NULL)
 		return -ENODEV;
 
-	eth_dev_stop(eth_dev);
-
 	eth_dev_close(eth_dev);
 
 	rte_free(vring_states[eth_dev->data->port_id]);
 	vring_states[eth_dev->data->port_id] = NULL;
 
-	for (i = 0; i < eth_dev->data->nb_rx_queues; i++)
-		rte_free(eth_dev->data->rx_queues[i]);
-	for (i = 0; i < eth_dev->data->nb_tx_queues; i++)
-		rte_free(eth_dev->data->tx_queues[i]);
-
-	rte_free(eth_dev->data->mac_addrs);
 	rte_free(eth_dev->data);
 
 	rte_eth_dev_release_port(eth_dev);