bonding: remove the counter and simplify bond_for_each_slave_from()

PATCH In review Ding Tianhong 2 participants 2 messages Sep 25, 2013

Subsystems (3)

Networking Network drivers Bonding driver

lore

Share & tools

lore

Ding Tianhong

author

2013-09-25 09:54 UTC #0 lore raw

PATCH v5 5/6 bonding: remove the counter and simplify bond_for_each_slave_from()

Restructure the bond_for_each_slave_from(), remove the checking for bond->slave_cnt,
make the new loop be more simple and racy.

Signed-off-by Ding Tianhong <[email protected]>

Cc Nikolay Aleksandrov <[email protected]>

Cc Veaceslav Falico <[email protected]>

---
 drivers/net/bonding/bond_alb.c  | 3 +--
 drivers/net/bonding/bond_main.c | 6 ++----
 drivers/net/bonding/bonding.h   | 6 +++---
 3 files changed, 6 insertions(+), 9 deletions(-)

drivers/net/bonding/bond_alb.c +1 -2

index f428ef57..4813dc6 100644
@@ -383,7 +383,6 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
 {
 	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
 	struct slave *rx_slave, *slave, *start_at;
-	int i = 0;
 
 	if (bond_info->next_rx_slave)
 		start_at = bond_info->next_rx_slave;

@@ -392,7 +391,7 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
 
 	rx_slave = NULL;
 
-	bond_for_each_slave_from(bond, slave, i, start_at) {
+	bond_for_each_slave_from(bond, slave, start_at) {
 		if (SLAVE_IS_OK(slave)) {
 			if (!rx_slave) {
 				rx_slave = slave;

drivers/net/bonding/bond_main.c +2 -4

index 55bbb8b..fbe25bc 100644
@@ -782,7 +782,6 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
 	struct slave *new_active, *old_active;
 	struct slave *bestslave = NULL;
 	int mintime = bond->params.updelay;
-	int i;
 
 	new_active = bond->curr_active_slave;

@@ -801,7 +800,7 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
 	/* remember where to stop iterating over the slaves */
 	old_active = new_active;
 
-	bond_for_each_slave_from(bond, new_active, i, old_active) {
+	bond_for_each_slave_from(bond, new_active, old_active) {
 		if (new_active->link == BOND_LINK_UP) {
 			return new_active;
 		} else if (new_active->link == BOND_LINK_BACK &&

@@ -2756,7 +2755,6 @@ do_failover:
 static void bond_ab_arp_probe(struct bonding *bond)
 {
 	struct slave *slave, *next_slave;
-	int i;
 
 	read_lock(&bond->curr_slave_lock);

@@ -2788,7 +2786,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
 
 	/* search for next candidate */
 	next_slave = bond_next_slave(bond, bond->current_arp_slave);
-	bond_for_each_slave_from(bond, slave, i, next_slave) {
+	bond_for_each_slave_from(bond, slave, next_slave) {
 		if (IS_UP(slave->dev)) {
 			slave->link = BOND_LINK_BACK;
 			bond_set_slave_active_flags(slave);

drivers/net/bonding/bonding.h +3 -3

index eb36f57..e5734ad 100644
@@ -130,9 +130,9 @@
  *
  * Caller must hold bond->lock
  */
-#define bond_for_each_slave_from(bond, pos, cnt, start) \
-	for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
-	     cnt++, pos = bond_next_slave(bond, pos))
+#define bond_for_each_slave_from(bond, pos, start) \
+	for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
+	    (pos) : (pos = NULL))
 
 /**
  * bond_for_each_slave - iterate over all slaves

Signature · 1 line

1.8.0

Veaceslav Falico

2013-09-25 10:12 UTC #1 lore raw

Quoted 3 lines · Ding Tianhong

Restructure the bond_for_each_slave_from(),
remove the checking for bond->slave_cnt,
make the new loop be more simple and racy.

The whole bond_for_each_slave_from() should be removed and replaced with a standard bond_for_each_slave() - which is already in the process:

http://patchwork.ozlabs.org/patch/277701/
http://patchwork.ozlabs.org/patch/277717/
http://patchwork.ozlabs.org/patch/277716/
http://patchwork.ozlabs.org/patch/277702/
http://patchwork.ozlabs.org/patch/277703/

After that we're ready to use the standard and less painful list_for_each_entry() analogue - bond_for_each_slave() - which can afterwards be easily RCUified by just replacing bond_for_each_slave() with bond_for_each_slave_rcu().

Quoted 98 lines