IB multicast cleanup patches V2

PATCH Applied Christoph Lameter 4 participants 7 messages Dec 21, 2015 – Dec 23, 2015 4 messages not in the archive InfiniBand

lore

Share & tools

lore

Christoph Lameter

author

2015-12-21 14:42 UTC #0 lore raw

cover IB multicast cleanup patches V2

V1->V2
- Add Reviewed by's for first patch from Ira Weiny
- Change name of ipoib_check_mcast_sendonly() to ipoib_check_and_add_mcast_sendonly() as requested by Ira

This patchset cleans up the code a bit after the last round of multicast patches related to the sendonly join logic. Some of the bits of code landed in ipoib_main.c instead of ipoib_multicast.c.

- Move the multicastbits into that file so that everything is neatly together
- Reduce the number of functions exported from ipoib_multicast.c

This patchset can be retrieved from a git repo on kernel.org via

git pull git://git.kernel.org/pub/scm/linux/kernel/git/christoph/rdma.git cleanup

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Christoph Lameter

author

2015-12-21 14:42 UTC #1 lore raw

PATCH 1/2 Isolate common list remove code

Code cleanup to remove multicast specific code from ipoib_main.c

The removal of a list of multicast groups occurs in three places. Create a new function ipoib_mcast_remove_list(). Use this new function in ipoib_main.c too.
That in turn allows the dropping of two functions that were exported from ipoib_multicast.c for expiration of mc groups.

Reviewed-by Iraq Weiny <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/ulp/ipoib/ipoib.h           |  3 +--
 drivers/infiniband/ulp/ipoib/ipoib_main.c      |  7 ++-----
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 24 ++++++++++++++----------
 3 files changed, 17 insertions(+), 17 deletions(-)

drivers/infiniband/ulp/ipoib/ipoib.h +1 -2

index 3ede103..989c409 100644
@@ -495,7 +495,6 @@ void ipoib_dev_cleanup(struct net_device *dev);
 void ipoib_mcast_join_task(struct work_struct *work);
 void ipoib_mcast_carrier_on_task(struct work_struct *work);
 void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb);
-void ipoib_mcast_free(struct ipoib_mcast *mc);
 
 void ipoib_mcast_restart_task(struct work_struct *work);
 int ipoib_mcast_start_thread(struct net_device *dev);

@@ -549,7 +548,7 @@ void ipoib_path_iter_read(struct ipoib_path_iter *iter,
 
 int ipoib_mcast_attach(struct net_device *dev, u16 mlid,
 		       union ib_gid *mgid, int set_qkey);
-int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast);
+void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list);
 struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid);
 
 int ipoib_init_qp(struct net_device *dev);

drivers/infiniband/ulp/ipoib/ipoib_main.c +2 -5

index 7d32818..483ff20 100644
@@ -1150,7 +1150,7 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv *priv)
 	unsigned long flags;
 	int i;
 	LIST_HEAD(remove_list);
-	struct ipoib_mcast *mcast, *tmcast;
+	struct ipoib_mcast *mcast;
 	struct net_device *dev = priv->dev;
 
 	if (test_bit(IPOIB_STOP_NEIGH_GC, &priv->flags))

@@ -1207,10 +1207,7 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv *priv)
 
 out_unlock:
 	spin_unlock_irqrestore(&priv->lock, flags);
-	list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {
-		ipoib_mcast_leave(dev, mcast);
-		ipoib_mcast_free(mcast);
-	}
+	ipoib_mcast_remove_list(dev, &remove_list);
 }
 
 static void ipoib_reap_neigh(struct work_struct *work)

drivers/infiniband/ulp/ipoib/ipoib_multicast.c +14 ~~-10~~

index f357ca6..8acb420a 100644
@@ -106,7 +106,7 @@ static void __ipoib_mcast_schedule_join_thread(struct ipoib_dev_priv *priv,
 		queue_delayed_work(priv->wq, &priv->mcast_task, 0);
 }
 
-void ipoib_mcast_free(struct ipoib_mcast *mcast)
+static void ipoib_mcast_free(struct ipoib_mcast *mcast)
 {
 	struct net_device *dev = mcast->dev;
 	int tx_dropped = 0;

@@ -677,7 +677,7 @@ int ipoib_mcast_stop_thread(struct net_device *dev)
 	return 0;
 }
 
-int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast)
+static int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	int ret = 0;

@@ -704,6 +704,16 @@ int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast)
 	return 0;
 }
 
+void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list)
+{
+	struct ipoib_mcast *mcast, *tmcast;
+
+	list_for_each_entry_safe(mcast, tmcast, remove_list, list) {
+		ipoib_mcast_leave(dev, mcast);
+		ipoib_mcast_free(mcast);
+	}
+}
+
 void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);

@@ -810,10 +820,7 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
 		if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
 			wait_for_completion(&mcast->done);
 
-	list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {
-		ipoib_mcast_leave(dev, mcast);
-		ipoib_mcast_free(mcast);
-	}
+	ipoib_mcast_remove_list(dev, &remove_list);
 }
 
 static int ipoib_mcast_addr_is_valid(const u8 *addr, const u8 *broadcast)

@@ -939,10 +946,7 @@ void ipoib_mcast_restart_task(struct work_struct *work)
 		if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
 			wait_for_completion(&mcast->done);
 
-	list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {
-		ipoib_mcast_leave(mcast->dev, mcast);
-		ipoib_mcast_free(mcast);
-	}
+	ipoib_mcast_remove_list(mcast->dev, &remove_list);
 
 	/*
 	 * Double check that we are still up

Signature · 7 lines

2.5.0


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christoph Lameter

author

2015-12-21 14:42 UTC #6 lore raw

PATCH 2/2 Move multicast specific code out of ipoib_main.c

In reply to Christoph Lameter

V1->V2:
- Rename function as requested by Ira

Code cleanup to move multicast specific code that checks for a sendonly join to ipoib_multicast.c. This allows the removal of the export of __ipoib_mcast_find().

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/ulp/ipoib/ipoib.h           |  3 ++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 13 +------------
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 21 ++++++++++++++++++++-
 3 files changed, 23 insertions(+), 14 deletions(-)

drivers/infiniband/ulp/ipoib/ipoib.h +2 -1

index 989c409..a924933 100644
@@ -549,7 +549,8 @@ void ipoib_path_iter_read(struct ipoib_path_iter *iter,
 int ipoib_mcast_attach(struct net_device *dev, u16 mlid,
 		       union ib_gid *mgid, int set_qkey);
 void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list);
-struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid);
+void ipoib_check_and_add_mcast_sendonly(struct ipoib_dev_priv *priv, u8 *mgid,
+				struct list_head *remove_list);
 
 int ipoib_init_qp(struct net_device *dev);
 int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca);

drivers/infiniband/ulp/ipoib/ipoib_main.c +1 ~~-12~~

index 483ff20..620d9ca 100644
@@ -1150,7 +1150,6 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv *priv)
 	unsigned long flags;
 	int i;
 	LIST_HEAD(remove_list);
-	struct ipoib_mcast *mcast;
 	struct net_device *dev = priv->dev;
 
 	if (test_bit(IPOIB_STOP_NEIGH_GC, &priv->flags))

@@ -1179,18 +1178,8 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv *priv)
 							  lockdep_is_held(&priv->lock))) != NULL) {
 			/* was the neigh idle for two GC periods */
 			if (time_after(neigh_obsolete, neigh->alive)) {
-				u8 *mgid = neigh->daddr + 4;
 
-				/* Is this multicast ? */
-				if (*mgid == 0xff) {
-					mcast = __ipoib_mcast_find(dev, mgid);
-
-					if (mcast && test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) {
-						list_del(&mcast->list);
-						rb_erase(&mcast->rb_node, &priv->multicast_tree);
-						list_add_tail(&mcast->list, &remove_list);
-					}
-				}
+				ipoib_check_and_add_mcast_sendonly(priv, neigh->daddr + 4, &remove_list);
 
 				rcu_assign_pointer(*np,
 						   rcu_dereference_protected(neigh->hnext,

drivers/infiniband/ulp/ipoib/ipoib_multicast.c +20 -1

index 8acb420a..ab79b87 100644
@@ -153,7 +153,7 @@ static struct ipoib_mcast *ipoib_mcast_alloc(struct net_device *dev,
 	return mcast;
 }
 
-struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid)
+static struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct rb_node *n = priv->multicast_tree.rb_node;

@@ -704,6 +704,25 @@ static int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast)
 	return 0;
 }
 
+/*
+ * Check if the multicast group is sendonly. If so remove it from the maps
+ * and add to the remove list
+ */
+void ipoib_check_and_add_mcast_sendonly(struct ipoib_dev_priv *priv, u8 *mgid,
+				struct list_head *remove_list)
+{
+	/* Is this multicast ? */
+	if (*mgid == 0xff) {
+		struct ipoib_mcast *mcast = __ipoib_mcast_find(priv->dev, mgid);
+
+		if (mcast && test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) {
+			list_del(&mcast->list);
+			rb_erase(&mcast->rb_node, &priv->multicast_tree);
+			list_add_tail(&mcast->list, remove_list);
+		}
+	}
+}
+
 void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list)
 {
 	struct ipoib_mcast *mcast, *tmcast;

Signature · 7 lines

2.5.0


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Leon Romanovsky

2015-12-21 14:57 UTC #3 lore raw

Re: [PATCH 1/2] Isolate common list remove code

In reply to a message not in the archive

Quoted 9 lines · Christoph Lameter

Code cleanup to remove multicast specific code from ipoib_main.c
The removal of a list of multicast groups occurs in three places. Create a new function ipoib_mcast_remove_list(). Use this new function in ipoib_main.c too.
That in turn allows the dropping of two functions that were exported from ipoib_multicast.c for expiration of mc groups.
Reviewed-by Iraq Weiny <[email protected]>

Iraq Weiny --> Ira Weiny

....

Quoted 1 line

+void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list)

Will it be beneficial to inline this function?

drivers/infiniband/ulp/ipoib/ipoib_multicast.c int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast) · line 716

Quoted 9 lines

drivers/infiniband/ulp/ipoib/ipoib_multicast.c +9 -0

+{
+	struct ipoib_mcast *mcast, *tmcast;
+
+	list_for_each_entry_safe(mcast, tmcast, remove_list, list) {
+		ipoib_mcast_leave(dev, mcast);
+		ipoib_mcast_free(mcast);
+	}
+}
+

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Christoph Lameter

author

2015-12-21 15:08 UTC #5 lore raw

Re: [PATCH 1/2] Isolate common list remove code

In reply to a message not in the archive

Quoted 11 lines · Leon Romanovsky

Quoted 9 lines · Christoph Lameter
Code cleanup to remove multicast specific code from ipoib_main.c
The removal of a list of multicast groups occurs in three places. Create a new function ipoib_mcast_remove_list(). Use this new function in ipoib_main.c too.
That in turn allows the dropping of two functions that were exported from ipoib_multicast.c for expiration of mc groups.
Reviewed-by Iraq Weiny <[email protected]>
Iraq Weiny --> Ira Weiny

Ohh.. Bad typo.

Quoted 2 lines

Quoted 1 line
+void ipoib_mcast_remove_list(struct net_device *dev, struct list_head *remove_list)
Will it be beneficial to inline this function?

As far as I know it is not run in a latency critical context and the code is too heavy for that. In particular we are calling other functions that are not inlined.

Quoted 10 lines

drivers/infiniband/ulp/ipoib/ipoib_multicast.c int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast) · line 716
Quoted 9 lines
drivers/infiniband/ulp/ipoib/ipoib_multicast.c +9 -0
+{
+	struct ipoib_mcast *mcast, *tmcast;
+
+	list_for_each_entry_safe(mcast, tmcast, remove_list, list) {
+		ipoib_mcast_leave(dev, mcast);
+		ipoib_mcast_free(mcast);
+	}
+}
+

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Doug Ledford

Applied

2015-12-23 18:11 UTC #8 lore raw

In reply to a message not in the archive

Quoted 9 lines · Christoph Lameter

V1->V2
- Add Reviewed by's for first patch from Ira Weiny
- Change name of ipoib_check_mcast_sendonly() to ipoib_check_and_add_mcast_sendonly() as requested by Ira
This patchset cleans up the code a bit after the last round of multicast patches related to the sendonly join logic. Some of the bits of code landed in ipoib_main.c instead of ipoib_multicast.c.

Quoted 7 lines

- Move the multicastbits into that file so that everything is neatly together
- Reduce the number of functions exported from ipoib_multicast.c
This patchset can be retrieved from a git repo on kernel.org via
git pull git://git.kernel.org/pub/scm/linux/kernel/git/christoph/rdma.git cleanup

Thanks, applied (with Ira's name fixed).

Signature · 2 lines

Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD

ira.weiny

2015-12-23 20:02 UTC #10 lore raw

In reply to a message not in the archive

Quoted 13 lines · Doug Ledford

Quoted 12 lines · Christoph Lameter
V1->V2
- Add Reviewed by's for first patch from Ira Weiny
- Change name of ipoib_check_mcast_sendonly() to ipoib_check_and_add_mcast_sendonly() as requested by Ira
This patchset cleans up the code a bit after the last round of multicast patches related to the sendonly join logic. Some of the bits of code landed in ipoib_main.c instead of ipoib_multicast.c.
- Move the multicastbits into that file so that everything is neatly together
- Reduce the number of functions exported from ipoib_multicast.c

Quoted 6 lines

Quoted 4 lines
This patchset can be retrieved from a git repo on kernel.org via
git pull git://git.kernel.org/pub/scm/linux/kernel/git/christoph/rdma.git cleanup
Thanks, applied (with Ira's name fixed).

Thanks! :-D

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html