diff options
author | Nikolay Aleksandrov <nikolay@redhat.com> | 2014-09-11 16:49:23 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-09-13 16:29:06 -0400 |
commit | 62c5f5185397f4bd8e5defe6fcb86420deeb2b38 (patch) | |
tree | 890f1c02606fbda231565b7f95a10b05cbd47629 /drivers/net/bonding | |
parent | 86e749866d7c6b0ee1f9377cf7142f2690596a05 (diff) |
bonding: alb: remove curr_slave_lock
First in rlb_teach_disabled_mac_on_primary() it's okay to remove
curr_slave_lock as all callers except bond_alb_monitor() already hold
RTNL, and in case bond_alb_monitor() is executing we can at most have a
period with bad throughput (very unlikely though).
In bond_alb_monitor() it's okay to remove the read_lock as the slave
list is walked with RCU and the worst that could happen is another
transmitter at the same time and thus for a period which currently is 10
seconds (bond_alb.h: BOND_ALB_LP_TICKS).
And bond_alb_handle_active_change() is okay because it's always called
with RTNL. Removed the ASSERT_RTNL() because it'll be inserted in the
parent function in a following patch.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/bonding')
-rw-r--r-- | drivers/net/bonding/bond_alb.c | 39 |
1 files changed, 3 insertions, 36 deletions
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 028496205f39..cf4ede8594ff 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c | |||
@@ -447,7 +447,7 @@ static struct slave *__rlb_next_rx_slave(struct bonding *bond) | |||
447 | /* teach the switch the mac of a disabled slave | 447 | /* teach the switch the mac of a disabled slave |
448 | * on the primary for fault tolerance | 448 | * on the primary for fault tolerance |
449 | * | 449 | * |
450 | * Caller must hold bond->curr_slave_lock for write or bond lock for write | 450 | * Caller must hold RTNL |
451 | */ | 451 | */ |
452 | static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[]) | 452 | static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[]) |
453 | { | 453 | { |
@@ -512,12 +512,8 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave) | |||
512 | 512 | ||
513 | _unlock_rx_hashtbl_bh(bond); | 513 | _unlock_rx_hashtbl_bh(bond); |
514 | 514 | ||
515 | write_lock_bh(&bond->curr_slave_lock); | ||
516 | |||
517 | if (slave != bond_deref_active_protected(bond)) | 515 | if (slave != bond_deref_active_protected(bond)) |
518 | rlb_teach_disabled_mac_on_primary(bond, slave->dev->dev_addr); | 516 | rlb_teach_disabled_mac_on_primary(bond, slave->dev->dev_addr); |
519 | |||
520 | write_unlock_bh(&bond->curr_slave_lock); | ||
521 | } | 517 | } |
522 | 518 | ||
523 | static void rlb_update_client(struct rlb_client_info *client_info) | 519 | static void rlb_update_client(struct rlb_client_info *client_info) |
@@ -1595,13 +1591,6 @@ void bond_alb_monitor(struct work_struct *work) | |||
1595 | if (bond_info->lp_counter >= BOND_ALB_LP_TICKS(bond)) { | 1591 | if (bond_info->lp_counter >= BOND_ALB_LP_TICKS(bond)) { |
1596 | bool strict_match; | 1592 | bool strict_match; |
1597 | 1593 | ||
1598 | /* change of curr_active_slave involves swapping of mac addresses. | ||
1599 | * in order to avoid this swapping from happening while | ||
1600 | * sending the learning packets, the curr_slave_lock must be held for | ||
1601 | * read. | ||
1602 | */ | ||
1603 | read_lock(&bond->curr_slave_lock); | ||
1604 | |||
1605 | bond_for_each_slave_rcu(bond, slave, iter) { | 1594 | bond_for_each_slave_rcu(bond, slave, iter) { |
1606 | /* If updating current_active, use all currently | 1595 | /* If updating current_active, use all currently |
1607 | * user mac addreses (!strict_match). Otherwise, only | 1596 | * user mac addreses (!strict_match). Otherwise, only |
@@ -1613,17 +1602,11 @@ void bond_alb_monitor(struct work_struct *work) | |||
1613 | alb_send_learning_packets(slave, slave->dev->dev_addr, | 1602 | alb_send_learning_packets(slave, slave->dev->dev_addr, |
1614 | strict_match); | 1603 | strict_match); |
1615 | } | 1604 | } |
1616 | |||
1617 | read_unlock(&bond->curr_slave_lock); | ||
1618 | |||
1619 | bond_info->lp_counter = 0; | 1605 | bond_info->lp_counter = 0; |
1620 | } | 1606 | } |
1621 | 1607 | ||
1622 | /* rebalance tx traffic */ | 1608 | /* rebalance tx traffic */ |
1623 | if (bond_info->tx_rebalance_counter >= BOND_TLB_REBALANCE_TICKS) { | 1609 | if (bond_info->tx_rebalance_counter >= BOND_TLB_REBALANCE_TICKS) { |
1624 | |||
1625 | read_lock(&bond->curr_slave_lock); | ||
1626 | |||
1627 | bond_for_each_slave_rcu(bond, slave, iter) { | 1610 | bond_for_each_slave_rcu(bond, slave, iter) { |
1628 | tlb_clear_slave(bond, slave, 1); | 1611 | tlb_clear_slave(bond, slave, 1); |
1629 | if (slave == rcu_access_pointer(bond->curr_active_slave)) { | 1612 | if (slave == rcu_access_pointer(bond->curr_active_slave)) { |
@@ -1633,9 +1616,6 @@ void bond_alb_monitor(struct work_struct *work) | |||
1633 | bond_info->unbalanced_load = 0; | 1616 | bond_info->unbalanced_load = 0; |
1634 | } | 1617 | } |
1635 | } | 1618 | } |
1636 | |||
1637 | read_unlock(&bond->curr_slave_lock); | ||
1638 | |||
1639 | bond_info->tx_rebalance_counter = 0; | 1619 | bond_info->tx_rebalance_counter = 0; |
1640 | } | 1620 | } |
1641 | 1621 | ||
@@ -1775,21 +1755,14 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char | |||
1775 | * Set the bond->curr_active_slave to @new_slave and handle | 1755 | * Set the bond->curr_active_slave to @new_slave and handle |
1776 | * mac address swapping and promiscuity changes as needed. | 1756 | * mac address swapping and promiscuity changes as needed. |
1777 | * | 1757 | * |
1778 | * If new_slave is NULL, caller must hold curr_slave_lock for write | 1758 | * Caller must hold RTNL |
1779 | * | ||
1780 | * If new_slave is not NULL, caller must hold RTNL, curr_slave_lock | ||
1781 | * for write. Processing here may sleep, so no other locks may be held. | ||
1782 | */ | 1759 | */ |
1783 | void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave) | 1760 | void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave) |
1784 | __releases(&bond->curr_slave_lock) | ||
1785 | __acquires(&bond->curr_slave_lock) | ||
1786 | { | 1761 | { |
1787 | struct slave *swap_slave; | 1762 | struct slave *swap_slave; |
1788 | struct slave *curr_active; | 1763 | struct slave *curr_active; |
1789 | 1764 | ||
1790 | curr_active = rcu_dereference_protected(bond->curr_active_slave, | 1765 | curr_active = rtnl_dereference(bond->curr_active_slave); |
1791 | !new_slave || | ||
1792 | lockdep_is_held(&bond->curr_slave_lock)); | ||
1793 | if (curr_active == new_slave) | 1766 | if (curr_active == new_slave) |
1794 | return; | 1767 | return; |
1795 | 1768 | ||
@@ -1820,10 +1793,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave | |||
1820 | tlb_clear_slave(bond, swap_slave, 1); | 1793 | tlb_clear_slave(bond, swap_slave, 1); |
1821 | tlb_clear_slave(bond, new_slave, 1); | 1794 | tlb_clear_slave(bond, new_slave, 1); |
1822 | 1795 | ||
1823 | write_unlock_bh(&bond->curr_slave_lock); | ||
1824 | |||
1825 | ASSERT_RTNL(); | ||
1826 | |||
1827 | /* in TLB mode, the slave might flip down/up with the old dev_addr, | 1796 | /* in TLB mode, the slave might flip down/up with the old dev_addr, |
1828 | * and thus filter bond->dev_addr's packets, so force bond's mac | 1797 | * and thus filter bond->dev_addr's packets, so force bond's mac |
1829 | */ | 1798 | */ |
@@ -1852,8 +1821,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave | |||
1852 | alb_send_learning_packets(new_slave, bond->dev->dev_addr, | 1821 | alb_send_learning_packets(new_slave, bond->dev->dev_addr, |
1853 | false); | 1822 | false); |
1854 | } | 1823 | } |
1855 | |||
1856 | write_lock_bh(&bond->curr_slave_lock); | ||
1857 | } | 1824 | } |
1858 | 1825 | ||
1859 | /* Called with RTNL */ | 1826 | /* Called with RTNL */ |