diff options
author | dingtianhong <dingtianhong@huawei.com> | 2013-12-12 21:19:32 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-12-14 01:58:01 -0500 |
commit | b2e7aceb00b2621431e220748f7158b131b89e7b (patch) | |
tree | 2f2a187827d9203946d12877971479bac4c8dff0 /drivers/net/bonding/bond_alb.c | |
parent | e57a784d8cae429f5b697fe55abf420181d9ff09 (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.c | 12 |
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 | */ |
474 | static void rlb_clear_slave(struct bonding *bond, struct slave *slave) | 474 | static 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 | */ |
1686 | void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave) | 1685 | void 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 | ||