bonding: restructure and simplify bond_for_each_slave_next()
Subsystems (3)
PATCH 4/5 bonding: restructure and simplify bond_for_each_slave_next()
remove the wordy int and add bond_for_each_slave_next_rcu().
Signed-off-by Ding Tianhong <[email protected]>
--- drivers/net/bonding/bond_alb.c | 3 +-- drivers/net/bonding/bond_main.c | 6 ++---- drivers/net/bonding/bonding.h | 19 +++++++++++++++---- 3 files changed, 18 insertions(+), 10 deletions(-)
drivers/net/bonding/bond_alb.c +1 -2
@@ -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
@@ -904,7 +904,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;
@@ -923,7 +922,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 &&
@@ -2891,7 +2890,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);
@@ -2923,7 +2921,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 +15 -4
@@ -119,14 +119,25 @@* bond_for_each_slave_from - iterate the slaves list from a starting point * @bond: the bond holding this list. * @pos: current slave. -* @cnt: counter for max number of moves * @start: starting point. * * 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 = bond_next_slave(bond, start); pos && pos != start; \ + pos = bond_next_slave(bond, pos)) + +/** +* bond_for_each_slave_from_rcu - iterate the slaves list from a starting point +* @bond: the bond holding this list. +* @pos: current slave. +* @start: starting point. +* +* Caller must hold rcu_read_lock +*/ +#define bond_for_each_slave_from_rcu (bond, pos, start) \ + for (pos = bond_next_slave_rcu(bond, start); pos && pos != start; \ + pos = bond_next_slave_rcu(bond, pos))/** * bond_for_each_slave - iterate over all slaves
Signature · 1 line
1.8.2.1
Quoted 90 lines · Ding Tianhong
remove the wordy int and add
bond_for_each_slave_next_rcu().Signed-off-by Ding Tianhong <[email protected]>--- drivers/net/bonding/bond_alb.c | 3 +-- drivers/net/bonding/bond_main.c | 6 ++---- drivers/net/bonding/bonding.h | 19 +++++++++++++++---- 3 files changed, 18 insertions(+), 10 deletions(-)drivers/net/bonding/bond_alb.c +1
-2@@ -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@@ -904,7 +904,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;@@ -923,7 +922,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 &&@@ -2891,7 +2890,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);@@ -2923,7 +2921,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 +6
-4@@ -119,14 +119,25 @@* bond_for_each_slave_from - iterate the slaves list from a starting point * @bond: the bond holding this list. * @pos: current slave. -* @cnt: counter for max number of moves * @start: starting point. * * 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 = bond_next_slave(bond, start); pos && pos != start; \ + pos = bond_next_slave(bond, pos)) + +/** +* bond_for_each_slave_from_rcu - iterate the slaves list from a starting point
drivers/net/bonding/bonding.h line 141
Quoted 10 lines
drivers/net/bonding/bonding.h +9
-0+* @bond: the bond holding this list. +* @pos: current slave. +* @start: starting point. +* +* Caller must hold rcu_read_lock +*/ +#define bond_for_each_slave_from_rcu (bond, pos, start) \ + for (pos = bond_next_slave_rcu(bond, start); pos && pos != start; \ + pos = bond_next_slave_rcu(bond, pos))
One question here: what if "start" gets deleted while we're traversing the list, could this lead to an infinite loop ?
Quoted 3 lines
drivers/net/bonding/bonding.h +0
-0/** * bond_for_each_slave - iterate over all slaves
Quoted 29 lines
Quoted 29 lines
drivers/net/bonding/bond_main.c +0
-0slave->link =BOND_LINK_BACK ;bond_set_slave_active_flags (slave);drivers/net/bonding/bonding.h +10
-4@@ -119,14 +119,25 @@* bond_for_each_slave_from - iterate the slaves list from a starting point * @bond: the bond holding this list. * @pos: current slave. -* @cnt: counter for max number of moves * @start: starting point. * * 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 = bond_next_slave(bond, start); pos && pos != start; \ + pos = bond_next_slave(bond, pos)) + +/** +* bond_for_each_slave_from_rcu - iterate the slaves list from a starting point +* @bond: the bond holding this list. +* @pos: current slave. +* @start: starting point. +*
Quoted 9 lines
drivers/net/bonding/bonding.h line 141
Quoted 6 lines
drivers/net/bonding/bonding.h +5
-0+* Caller must hold rcu_read_lock +*/ +#define bond_for_each_slave_from_rcu (bond, pos, start) \ + for (pos = bond_next_slave_rcu(bond, start); pos && pos != start; \ + pos = bond_next_slave_rcu(bond, pos))One question here: what if "start" gets deleted while we're traversing the list, could this lead to an infinite loop ?
agree, so the start should not be delete while traversing the list, the func should not run without protection.
Quoted 7 lines
drivers/net/bonding/bonding.h line 143
Quoted 3 lines
drivers/net/bonding/bonding.h +0
-0/** * bond_for_each_slave - iterate over all slaves .