aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/net/bonding/bond_alb.c
diff options
context:
space:
mode:
authordingtianhong <dingtianhong@huawei.com>2013-12-12 21:19:32 -0500
committerDavid S. Miller <davem@davemloft.net>2013-12-14 01:58:01 -0500
commitb2e7aceb00b2621431e220748f7158b131b89e7b (patch)
tree2f2a187827d9203946d12877971479bac4c8dff0 /drivers/net/bonding/bond_alb.c
parente57a784d8cae429f5b697fe55abf420181d9ff09 (diff)
bonding: remove the no effect lock for bond_select_active_slave()
The bond slave list was no longer protected by bond lock and only protected by RTNL or RCU, so anywhere that use bond lock to protect slave list is meaningless. remove the release and acquire bond lock for bond_select_active_slave(). The curr_active_slave could only be changed in 3 place: 1. enslave slave. 2. release slave. 3. change_active_slave. all above place were holding bond lock, RTNL and curr_slave_lock together, it is tedious and meaningless, obviously bond lock is no need here, but RTNL or curr_slave_lock is needed, so if you want to access active slave, you have to choose one lock, RTNL or curr_slave_lock, if RTNL is exist, no need to add curr_slave_lock, otherwise curr_slave_lock is better, because of the performance. there are several place calling bond_select_active_slave() and bond_change_active_slave(), the next step I will clean these place and remove the no effect lock. there are some document changed together when update the function. Suggested-by: Jay Vosburgh <fubar@us.ibm.com> Suggested-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/bonding/bond_alb.c')
-rw-r--r--drivers/net/bonding/bond_alb.c12
1 files changed, 3 insertions, 9 deletions
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 2250b063ab89..7b806e330364 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -469,7 +469,7 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])
469 469
470/* slave being removed should not be active at this point 470/* slave being removed should not be active at this point
471 * 471 *
472 * Caller must hold bond lock for read 472 * Caller must hold rtnl.
473 */ 473 */
474static void rlb_clear_slave(struct bonding *bond, struct slave *slave) 474static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
475{ 475{
@@ -1679,14 +1679,11 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
1679 * If new_slave is NULL, caller must hold curr_slave_lock or 1679 * If new_slave is NULL, caller must hold curr_slave_lock or
1680 * bond->lock for write. 1680 * bond->lock for write.
1681 * 1681 *
1682 * If new_slave is not NULL, caller must hold RTNL, bond->lock for 1682 * If new_slave is not NULL, caller must hold RTNL, curr_slave_lock
1683 * read and curr_slave_lock for write. Processing here may sleep, so 1683 * for write. Processing here may sleep, so no other locks may be held.
1684 * no other locks may be held.
1685 */ 1684 */
1686void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave) 1685void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave)
1687 __releases(&bond->curr_slave_lock) 1686 __releases(&bond->curr_slave_lock)
1688 __releases(&bond->lock)
1689 __acquires(&bond->lock)
1690 __acquires(&bond->curr_slave_lock) 1687 __acquires(&bond->curr_slave_lock)
1691{ 1688{
1692 struct slave *swap_slave; 1689 struct slave *swap_slave;
@@ -1722,7 +1719,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
1722 tlb_clear_slave(bond, new_slave, 1); 1719 tlb_clear_slave(bond, new_slave, 1);
1723 1720
1724 write_unlock_bh(&bond->curr_slave_lock); 1721 write_unlock_bh(&bond->curr_slave_lock);
1725 read_unlock(&bond->lock);
1726 1722
1727 ASSERT_RTNL(); 1723 ASSERT_RTNL();
1728 1724
@@ -1748,11 +1744,9 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
1748 /* swap mac address */ 1744 /* swap mac address */
1749 alb_swap_mac_addr(swap_slave, new_slave); 1745 alb_swap_mac_addr(swap_slave, new_slave);
1750 alb_fasten_mac_swap(bond, swap_slave, new_slave); 1746 alb_fasten_mac_swap(bond, swap_slave, new_slave);
1751 read_lock(&bond->lock);
1752 } else { 1747 } else {
1753 /* set the new_slave to the bond mac address */ 1748 /* set the new_slave to the bond mac address */
1754 alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr); 1749 alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr);
1755 read_lock(&bond->lock);
1756 alb_send_learning_packets(new_slave, bond->dev->dev_addr); 1750 alb_send_learning_packets(new_slave, bond->dev->dev_addr);
1757 } 1751 }
1758 1752