[openib-general] [PATCH 1 of 2] ipoib: pass all of multicast.c through ipoib_workqueue

Michael S. Tsirkin mst at mellanox.co.il
Mon Nov 21 08:34:58 PST 2005


There appear to be several races in IPoIB multicast code:
for example when a MAD event may start the multicast thread, while ipoib_stop
tries to stop it, leaving a thread running after the device is removed.
This patch forces all external callers of multicast.c into ipoib_workqueue,
which avoids the need for explicit synchronisation.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_ib.c
===================================================================
--- linux-2.6.14-dbg.orig/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2005-11-20 11:21:39.000000000 +0200
+++ linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2005-11-20 12:04:32.000000000 +0200
@@ -431,7 +431,8 @@ int ipoib_ib_dev_up(struct net_device *d
 
 	set_bit(IPOIB_FLAG_OPER_UP, &priv->flags);
 
-	return ipoib_mcast_start_thread(dev);
+	queue_work(ipoib_workqueue, &priv->start_task);
+	return 0;
 }
 
 int ipoib_ib_dev_down(struct net_device *dev)
@@ -452,19 +453,8 @@ int ipoib_ib_dev_down(struct net_device 
 		flush_workqueue(ipoib_workqueue);
 	}
 
-	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);
+	queue_work(ipoib_workqueue, &priv->down_task);
+	flush_workqueue(ipoib_workqueue);
 
 	return 0;
 }
@@ -619,11 +609,8 @@ 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);
-
+	queue_work(ipoib_workqueue, &priv->cleanup_task);
+	flush_workqueue(ipoib_workqueue);
 	ipoib_transport_dev_cleanup(dev);
 }
 
Index: linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
===================================================================
--- linux-2.6.14-dbg.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2005-11-20 11:21:39.000000000 +0200
+++ linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_multicast.c	2005-11-20 12:19:03.000000000 +0200
@@ -573,7 +573,7 @@ void ipoib_mcast_join_task(void *dev_ptr
 	netif_carrier_on(dev);
 }
 
-int ipoib_mcast_start_thread(struct net_device *dev)
+static void ipoib_mcast_start_thread(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 
@@ -583,11 +583,9 @@ int ipoib_mcast_start_thread(struct net_
 	if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags))
 		queue_work(ipoib_workqueue, &priv->mcast_task);
 	up(&mcast_mutex);
-
-	return 0;
 }
 
-int ipoib_mcast_stop_thread(struct net_device *dev, int flush)
+static void ipoib_mcast_stop_thread(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ipoib_mcast *mcast;
@@ -599,9 +597,6 @@ int ipoib_mcast_stop_thread(struct net_d
 	cancel_delayed_work(&priv->mcast_task);
 	up(&mcast_mutex);
 
-	if (flush)
-		flush_workqueue(ipoib_workqueue);
-
 	if (priv->broadcast && priv->broadcast->query) {
 		ib_sa_cancel_query(priv->broadcast->query_id, priv->broadcast->query);
 		priv->broadcast->query = NULL;
@@ -618,8 +613,6 @@ int ipoib_mcast_stop_thread(struct net_d
 			wait_for_completion(&mcast->done);
 		}
 	}
-
-	return 0;
 }
 
 static int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast)
@@ -737,7 +730,7 @@ out:
 	spin_unlock(&priv->lock);
 }
 
-void ipoib_mcast_dev_flush(struct net_device *dev)
+static void ipoib_mcast_dev_flush(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	LIST_HEAD(remove_list);
@@ -793,7 +786,7 @@ void ipoib_mcast_dev_flush(struct net_de
 	}
 }
 
-void ipoib_mcast_dev_down(struct net_device *dev)
+static void ipoib_mcast_dev_down(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	unsigned long flags;
@@ -822,7 +815,7 @@ void ipoib_mcast_restart_task(void *dev_
 
 	ipoib_dbg_mcast(priv, "restarting multicast task\n");
 
-	ipoib_mcast_stop_thread(dev, 0);
+	ipoib_mcast_stop_thread(dev);
 
 	spin_lock_irqsave(&priv->lock, flags);
 
@@ -908,6 +901,41 @@ void ipoib_mcast_restart_task(void *dev_
 		ipoib_mcast_start_thread(dev);
 }
 
+void ipoib_mcast_down_task(void *dev_ptr)
+{
+	struct net_device *dev = dev_ptr;
+
+	ipoib_mcast_stop_thread(dev);
+
+	/*
+	 * 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);
+}
+
+void ipoib_mcast_cleanup_task(void *dev_ptr)
+{
+	struct net_device *dev = dev_ptr;
+	ipoib_mcast_stop_thread(dev);
+
+	/* Delete the broadcast address and the local address */
+	ipoib_mcast_dev_down(dev);
+}
+
+void ipoib_mcast_start_task(void *dev_ptr)
+{
+	struct net_device *dev = dev_ptr;
+	ipoib_mcast_start_thread(dev);
+}
+
+
 #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG
 
 struct ipoib_mcast_iter *ipoib_mcast_iter_init(struct net_device *dev)
Index: linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- linux-2.6.14-dbg.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c	2005-11-20 11:21:39.000000000 +0200
+++ linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib_main.c	2005-11-20 11:57:00.000000000 +0200
@@ -899,6 +899,9 @@ static void ipoib_setup(struct net_devic
 	INIT_WORK(&priv->mcast_task,   ipoib_mcast_join_task,    priv->dev);
 	INIT_WORK(&priv->flush_task,   ipoib_ib_dev_flush,       priv->dev);
 	INIT_WORK(&priv->restart_task, ipoib_mcast_restart_task, priv->dev);
+	INIT_WORK(&priv->down_task,    ipoib_mcast_down_task,    priv->dev);
+	INIT_WORK(&priv->cleanup_task, ipoib_mcast_cleanup_task, priv->dev);
+	INIT_WORK(&priv->start_task,   ipoib_mcast_start_task,   priv->dev);
 	INIT_WORK(&priv->ah_reap_task, ipoib_reap_ah,            priv->dev);
 }
 
Index: linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- linux-2.6.14-dbg.orig/drivers/infiniband/ulp/ipoib/ipoib.h	2005-11-20 11:21:39.000000000 +0200
+++ linux-2.6.14-dbg/drivers/infiniband/ulp/ipoib/ipoib.h	2005-11-20 12:18:43.000000000 +0200
@@ -137,6 +137,9 @@ struct ipoib_dev_priv {
 	struct work_struct mcast_task;
 	struct work_struct flush_task;
 	struct work_struct restart_task;
+	struct work_struct down_task;
+	struct work_struct cleanup_task;
+	struct work_struct start_task;
 	struct work_struct ah_reap_task;
 
 	struct ib_device *ca;
@@ -263,11 +266,9 @@ void ipoib_mcast_send(struct net_device 
 		      struct sk_buff *skb);
 
 void ipoib_mcast_restart_task(void *dev_ptr);
-int ipoib_mcast_start_thread(struct net_device *dev);
-int ipoib_mcast_stop_thread(struct net_device *dev, int flush);
-
-void ipoib_mcast_dev_down(struct net_device *dev);
-void ipoib_mcast_dev_flush(struct net_device *dev);
+void ipoib_mcast_start_task(void *dev_ptr);
+void ipoib_mcast_down_task(void *dev_ptr);
+void ipoib_mcast_cleanup_task(void *dev_ptr);
 
 #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG
 struct ipoib_mcast_iter *ipoib_mcast_iter_init(struct net_device *dev);

-- 
MST



More information about the general mailing list