Re: [Xen-devel] [PATCH] xen: netback: remove redundant xenvif_put

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2013-02-19 at 08:03 +0000, Jan Beulich wrote:
> The core of
> it is that the put here parallels the one in netbk_tx_err(), and
> the one in xenvif_carrier_off() matches the get from
> xenvif_connect() (which normally would be done on the path
> coming through xenvif_disconnect()). 

A few people have made this mistake already, perhaps this helps make it
more obvious what is going on...

8<--------------------------------

>From 7b93077e2cda5881b594d9c7e733e617df87d7c0 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 19 Feb 2013 10:46:12 +0000
Subject: [PATCH] xen/netback: refactor xenvif_carrier_on from xenvif_connect

Several people have been confused by the vif reference count taken by
xenvif_connect() being dropped in xenvif_carrier_off(). Factor out bringing
the carrier up (and the associated reference grab) to try and make the
relationship more obvious.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/net/xen-netback/interface.c |   49 +++++++++++++++++++---------------
 1 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index d984141..059d726 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -307,6 +307,32 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	return vif;
 }
 
+static void xenvif_carrier_on(struct xenvif *vif)
+{
+	xenvif_get(vif);
+
+	rtnl_lock();
+	if (!vif->can_sg && vif->dev->mtu > ETH_DATA_LEN)
+		dev_set_mtu(vif->dev, ETH_DATA_LEN);
+	netdev_update_features(vif->dev);
+	netif_carrier_on(vif->dev);
+	if (netif_running(vif->dev))
+		xenvif_up(vif);
+	rtnl_unlock();
+}
+
+void xenvif_carrier_off(struct xenvif *vif)
+{
+	struct net_device *dev = vif->dev;
+
+	rtnl_lock();
+	netif_carrier_off(dev); /* discard queued packets */
+	if (netif_running(dev))
+		xenvif_down(vif);
+	rtnl_unlock();
+	xenvif_put(vif);
+}
+
 int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 		   unsigned long rx_ring_ref, unsigned int evtchn)
 {
@@ -328,16 +354,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	vif->irq = err;
 	disable_irq(vif->irq);
 
-	xenvif_get(vif);
-
-	rtnl_lock();
-	if (!vif->can_sg && vif->dev->mtu > ETH_DATA_LEN)
-		dev_set_mtu(vif->dev, ETH_DATA_LEN);
-	netdev_update_features(vif->dev);
-	netif_carrier_on(vif->dev);
-	if (netif_running(vif->dev))
-		xenvif_up(vif);
-	rtnl_unlock();
+	xenvif_carrier_on(vif);
 
 	return 0;
 err_unmap:
@@ -346,18 +363,6 @@ err:
 	return err;
 }
 
-void xenvif_carrier_off(struct xenvif *vif)
-{
-	struct net_device *dev = vif->dev;
-
-	rtnl_lock();
-	netif_carrier_off(dev); /* discard queued packets */
-	if (netif_running(dev))
-		xenvif_down(vif);
-	rtnl_unlock();
-	xenvif_put(vif);
-}
-
 void xenvif_disconnect(struct xenvif *vif)
 {
 	if (netif_carrier_ok(vif->dev))
-- 
1.7.2.5