[openib-general] ipoib_multicast_leak.patch

[Roland Dreier]

 > ipoib_multicast_leak.patch

It seems things can be simplified still further.  I didn't see a
reason to have both ipoib_mcast_dev_flush() and ipoib_mcast_dev_down(),
so I rolled everything up into ipoib_mcast_dev_flush().  This passes
light testing here (and I checked that mcast allocs are balanced by
mcast frees).  How does this look to you?

 - R.

--- infiniband/ulp/ipoib/ipoib_multicast.c	(revision 4973)
+++ infiniband/ulp/ipoib/ipoib_multicast.c	(working copy)
@@ -742,50 +742,23 @@ void ipoib_mcast_dev_flush(struct net_de
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	LIST_HEAD(remove_list);
-	struct ipoib_mcast *mcast, *tmcast, *nmcast;
+	struct ipoib_mcast *mcast, *tmcast;
 	unsigned long flags;
 
 	ipoib_dbg_mcast(priv, "flushing multicast list\n");
 
 	spin_lock_irqsave(&priv->lock, flags);
-	list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) {
-		nmcast = ipoib_mcast_alloc(dev, 0);
-		if (nmcast) {
-			nmcast->flags =
-				mcast->flags & (1 << IPOIB_MCAST_FLAG_SENDONLY);
-
-			nmcast->mcmember.mgid = mcast->mcmember.mgid;
-
-			/* Add the new group in before the to-be-destroyed group */
-			list_add_tail(&nmcast->list, &mcast->list);
-			list_del_init(&mcast->list);
-
-			rb_replace_node(&mcast->rb_node, &nmcast->rb_node,
-					&priv->multicast_tree);
 
-			list_add_tail(&mcast->list, &remove_list);
-		} else {
-			ipoib_warn(priv, "could not reallocate multicast group "
-				   IPOIB_GID_FMT "\n",
-				   IPOIB_GID_ARG(mcast->mcmember.mgid));
-		}
+	list_for_each_entry_safe(mcast, tmcast, &priv->multicast_list, list) {
+		list_del(&mcast->list);
+		rb_erase(&mcast->rb_node, &priv->multicast_tree);
+		list_add_tail(&mcast->list, &remove_list);
 	}
 
 	if (priv->broadcast) {
-		nmcast = ipoib_mcast_alloc(dev, 0);
-		if (nmcast) {
-			nmcast->mcmember.mgid = priv->broadcast->mcmember.mgid;
-
-			rb_replace_node(&priv->broadcast->rb_node,
-					&nmcast->rb_node,
-					&priv->multicast_tree);
-
-			list_add_tail(&priv->broadcast->list, &remove_list);
-			priv->broadcast = nmcast;
-		} else
-			ipoib_warn(priv, "could not reallocate broadcast group "
-                        	          IPOIB_GID_FMT "\n",
-                                	  IPOIB_GID_ARG(priv->broadcast->mcmember.mgid));
+ 		rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree);
+		list_add_tail(&priv->broadcast->list, &remove_list);
+		priv->broadcast = NULL;
 	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
@@ -796,24 +769,6 @@ void ipoib_mcast_dev_flush(struct net_de
 	}
 }
 
-void ipoib_mcast_dev_down(struct net_device *dev)
-{
-	struct ipoib_dev_priv *priv = netdev_priv(dev);
-	unsigned long flags;
-
-	/* Delete broadcast since it will be recreated */
-	if (priv->broadcast) {
-		ipoib_dbg_mcast(priv, "deleting broadcast group\n");
-
-		spin_lock_irqsave(&priv->lock, flags);
-		rb_erase(&priv->broadcast->rb_node, &priv->multicast_tree);
-		spin_unlock_irqrestore(&priv->lock, flags);
-		ipoib_mcast_leave(dev, priv->broadcast);
-		ipoib_mcast_free(priv->broadcast);
-		priv->broadcast = NULL;
-	}
-}
-
 void ipoib_mcast_restart_task(void *dev_ptr)
 {
 	struct net_device *dev = dev_ptr;
--- infiniband/ulp/ipoib/ipoib_ib.c	(revision 4973)
+++ infiniband/ulp/ipoib/ipoib_ib.c	(working copy)
@@ -453,17 +453,8 @@ int ipoib_ib_dev_down(struct net_device 
 	}
 
 	ipoib_mcast_stop_thread(dev, 1);
-
-	/*
-	 * Flush the multicast groups first so we stop any multicast joins. The
-	 * completion thread may have already died and we may deadlock waiting
-	 * for the completion thread to finish some multicast joins.
-	 */
 	ipoib_mcast_dev_flush(dev);
 
-	/* Delete broadcast and local addresses since they will be recreated */
-	ipoib_mcast_dev_down(dev);
-
 	ipoib_flush_paths(dev);
 
 	return 0;
@@ -624,9 +615,7 @@ void ipoib_ib_dev_cleanup(struct net_dev
 	ipoib_dbg(priv, "cleaning up ib_dev\n");
 
 	ipoib_mcast_stop_thread(dev, 1);
-
-	/* Delete the broadcast address and the local address */
-	ipoib_mcast_dev_down(dev);
+	ipoib_mcast_dev_flush(dev);
 
 	ipoib_transport_dev_cleanup(dev);
 }