[PATCH v2 0/2] xen-netback: fix memory leaks on XenBus disconnect

Just split the initial patch in two as proposed by Wei.

Since the approach for locking netdev statistics is inconsistent (tends not
to have any locking at all) accross the kernel we'd better to rely on our
internal lock for this purpose.

Igor Druzhinin (2):
  xen-netback: fix memory leaks on XenBus disconnect
  xen-netback: protect resource cleaning on XenBus disconnect

 drivers/net/xen-netback/interface.c |  6 ++++--
 drivers/net/xen-netback/xenbus.c    | 13 +++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

-- 
1.8.3.1

vif->lock is used to protect statistics gathering agents from using the
queue structure during cleaning.

Signed-off-by: Igor Druzhinin <redacted>
---
 drivers/net/xen-netback/interface.c | 6 ++++--
 drivers/net/xen-netback/xenbus.c    | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 41c69b3..c48252a 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c

@@ -230,18 +230,18 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev)
 {
 	struct xenvif *vif = netdev_priv(dev);
 	struct xenvif_queue *queue = NULL;
-	unsigned int num_queues = vif->num_queues;
 	unsigned long rx_bytes = 0;
 	unsigned long rx_packets = 0;
 	unsigned long tx_bytes = 0;
 	unsigned long tx_packets = 0;
 	unsigned int index;
 
+	spin_lock(&vif->lock);
 	if (vif->queues == NULL)
 		goto out;
 
 	/* Aggregate tx and rx stats from each queue */
-	for (index = 0; index < num_queues; ++index) {
+	for (index = 0; index < vif->num_queues; ++index) {
 		queue = &vif->queues[index];
 		rx_bytes += queue->stats.rx_bytes;
 		rx_packets += queue->stats.rx_packets;

@@ -250,6 +250,8 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev)
 	}
 
 out:
+	spin_unlock(&vif->lock);
+
 	vif->dev->stats.rx_bytes = rx_bytes;
 	vif->dev->stats.rx_packets = rx_packets;
 	vif->dev->stats.tx_bytes = tx_bytes;

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 3e99071..d82cd71 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c

@@ -503,9 +503,11 @@ static void backend_disconnect(struct backend_info *be)
 		for (queue_index = 0; queue_index < be->vif->num_queues; ++queue_index)
 			xenvif_deinit_queue(&be->vif->queues[queue_index]);
 
+		spin_lock(&be->vif->lock);
 		vfree(be->vif->queues);
 		be->vif->num_queues = 0;
 		be->vif->queues = NULL;
+		spin_unlock(&be->vif->lock);
 
 		xenvif_disconnect_ctrl(be->vif);
 	}

-- 
1.8.3.1

Eliminate memory leaks introduced several years ago by cleaning the
queue resources which are allocated on XenBus connection event. Namely, queue
structure array and pages used for IO rings.

Signed-off-by: Igor Druzhinin <redacted>
---
 drivers/net/xen-netback/xenbus.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 6c57b02..3e99071 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c

@@ -493,11 +493,20 @@ static int backend_create_xenvif(struct backend_info *be)
 static void backend_disconnect(struct backend_info *be)
 {
 	if (be->vif) {
+		unsigned int queue_index;
+
 		xen_unregister_watchers(be->vif);
 #ifdef CONFIG_DEBUG_FS
 		xenvif_debugfs_delif(be->vif);
 #endif /* CONFIG_DEBUG_FS */
 		xenvif_disconnect_data(be->vif);
+		for (queue_index = 0; queue_index < be->vif->num_queues; ++queue_index)
+			xenvif_deinit_queue(&be->vif->queues[queue_index]);
+
+		vfree(be->vif->queues);
+		be->vif->num_queues = 0;
+		be->vif->queues = NULL;
+
 		xenvif_disconnect_ctrl(be->vif);
 	}
 }

@@ -1026,6 +1035,8 @@ static void connect(struct backend_info *be)
 err:
 	if (be->vif->num_queues > 0)
 		xenvif_disconnect_data(be->vif); /* Clean up existing queues */
+	for (queue_index = 0; queue_index < be->vif->num_queues; ++queue_index)
+		xenvif_deinit_queue(&be->vif->queues[queue_index]);
 	vfree(be->vif->queues);
 	be->vif->queues = NULL;
 	be->vif->num_queues = 0;

-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

-----Original Message-----
From: Igor Druzhinin [mailto:igor.druzhinin@citrix.com]
Sent: 17 January 2017 20:50
To: Wei Liu <redacted>
Cc: Paul Durrant <redacted>; xen-devel@lists.xenproject.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Igor Druzhinin
[off-list ref]
Subject: [PATCH v2 1/2] xen-netback: fix memory leaks on XenBus
disconnect

Eliminate memory leaks introduced several years ago by cleaning the
queue resources which are allocated on XenBus connection event. Namely,
queue
structure array and pages used for IO rings.

Signed-off-by: Igor Druzhinin <redacted>

Reviewed-by: Paul Durrant <redacted>

-----Original Message-----
From: Igor Druzhinin [mailto:igor.druzhinin@citrix.com]
Sent: 17 January 2017 20:50
To: Wei Liu <redacted>
Cc: Paul Durrant <redacted>; xen-devel@lists.xenproject.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Igor Druzhinin
[off-list ref]
Subject: [PATCH v2 2/2] xen-netback: protect resource cleaning on XenBus
disconnect

vif->lock is used to protect statistics gathering agents from using the
queue structure during cleaning.

Signed-off-by: Igor Druzhinin <redacted>

Reviewed-by: Paul Durrant <redacted>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

On Tue, Jan 17, 2017 at 08:49:38PM +0000, Igor Druzhinin wrote:

vif->lock is used to protect statistics gathering agents from using the
queue structure during cleaning.

Signed-off-by: Igor Druzhinin <redacted>

Acked-by: Wei Liu <redacted>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

On Tue, Jan 17, 2017 at 08:49:37PM +0000, Igor Druzhinin wrote:

Eliminate memory leaks introduced several years ago by cleaning the
queue resources which are allocated on XenBus connection event. Namely, queue
structure array and pages used for IO rings.

Signed-off-by: Igor Druzhinin <redacted>

Acked-by: Wei Liu <redacted>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

From: Igor Druzhinin <redacted>
Date: Tue, 17 Jan 2017 20:49:36 +0000

Just split the initial patch in two as proposed by Wei.

Since the approach for locking netdev statistics is inconsistent (tends not
to have any locking at all) accross the kernel we'd better to rely on our
internal lock for this purpose.

Series applied, thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel