diff options
author | dingtianhong <dingtianhong@huawei.com> | 2013-12-12 21:20:02 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-12-14 01:58:02 -0500 |
commit | eb9fa4b0199f62df3d174d32b4bd534df8ba4533 (patch) | |
tree | d5ce36e8379818ff29a0d53b1c13e81a14efc183 /drivers/net/bonding | |
parent | e001bfad913bf119fb67c1e8dd2d4ec1f5d392fa (diff) |
bonding: rebuild the lock use for bond_activebackup_arp_mon()
The bond_activebackup_arp_mon() use the bond lock for read to
protect the slave list, it is no effect, and the RTNL is only
called for bond_ab_arp_commit() and peer notify, for the performance
better, use RCU to replace with the bond lock, to the bond slave
list need to called in RCU, add a new bond_first_slave_rcu()
to get the first slave in RCU protection.
In bond_ab_arp_probe(), the bond->current_arp_slave may changd
if bond release slave, just like:
bond_ab_arp_probe() bond_release()
cpu 0 cpu 1
...
if (bond->current_arp_slave...) ...
... bond->current_arp_slave = NULl
bond->current_arp_slave->dev->name ...
So the current_arp_slave need to dereference in the section.
Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
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')
-rw-r--r-- | drivers/net/bonding/bond_main.c | 45 |
1 files changed, 20 insertions, 25 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 1781ea620d76..bad1bf949546 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c | |||
@@ -2517,7 +2517,7 @@ re_arm: | |||
2517 | * place for the slave. Returns 0 if no changes are found, >0 if changes | 2517 | * place for the slave. Returns 0 if no changes are found, >0 if changes |
2518 | * to link states must be committed. | 2518 | * to link states must be committed. |
2519 | * | 2519 | * |
2520 | * Called with bond->lock held for read. | 2520 | * Called with rcu_read_lock hold. |
2521 | */ | 2521 | */ |
2522 | static int bond_ab_arp_inspect(struct bonding *bond) | 2522 | static int bond_ab_arp_inspect(struct bonding *bond) |
2523 | { | 2523 | { |
@@ -2526,7 +2526,7 @@ static int bond_ab_arp_inspect(struct bonding *bond) | |||
2526 | struct slave *slave; | 2526 | struct slave *slave; |
2527 | int commit = 0; | 2527 | int commit = 0; |
2528 | 2528 | ||
2529 | bond_for_each_slave(bond, slave, iter) { | 2529 | bond_for_each_slave_rcu(bond, slave, iter) { |
2530 | slave->new_link = BOND_LINK_NOCHANGE; | 2530 | slave->new_link = BOND_LINK_NOCHANGE; |
2531 | last_rx = slave_last_rx(bond, slave); | 2531 | last_rx = slave_last_rx(bond, slave); |
2532 | 2532 | ||
@@ -2588,7 +2588,7 @@ static int bond_ab_arp_inspect(struct bonding *bond) | |||
2588 | * Called to commit link state changes noted by inspection step of | 2588 | * Called to commit link state changes noted by inspection step of |
2589 | * active-backup mode ARP monitor. | 2589 | * active-backup mode ARP monitor. |
2590 | * | 2590 | * |
2591 | * Called with RTNL and bond->lock for read. | 2591 | * Called with RTNL hold. |
2592 | */ | 2592 | */ |
2593 | static void bond_ab_arp_commit(struct bonding *bond) | 2593 | static void bond_ab_arp_commit(struct bonding *bond) |
2594 | { | 2594 | { |
@@ -2663,19 +2663,20 @@ do_failover: | |||
2663 | /* | 2663 | /* |
2664 | * Send ARP probes for active-backup mode ARP monitor. | 2664 | * Send ARP probes for active-backup mode ARP monitor. |
2665 | * | 2665 | * |
2666 | * Called with bond->lock held for read. | 2666 | * Called with rcu_read_lock hold. |
2667 | */ | 2667 | */ |
2668 | static void bond_ab_arp_probe(struct bonding *bond) | 2668 | static void bond_ab_arp_probe(struct bonding *bond) |
2669 | { | 2669 | { |
2670 | struct slave *slave, *before = NULL, *new_slave = NULL; | 2670 | struct slave *slave, *before = NULL, *new_slave = NULL, |
2671 | *curr_arp_slave = rcu_dereference(bond->current_arp_slave); | ||
2671 | struct list_head *iter; | 2672 | struct list_head *iter; |
2672 | bool found = false; | 2673 | bool found = false; |
2673 | 2674 | ||
2674 | read_lock(&bond->curr_slave_lock); | 2675 | read_lock(&bond->curr_slave_lock); |
2675 | 2676 | ||
2676 | if (bond->current_arp_slave && bond->curr_active_slave) | 2677 | if (curr_arp_slave && bond->curr_active_slave) |
2677 | pr_info("PROBE: c_arp %s && cas %s BAD\n", | 2678 | pr_info("PROBE: c_arp %s && cas %s BAD\n", |
2678 | bond->current_arp_slave->dev->name, | 2679 | curr_arp_slave->dev->name, |
2679 | bond->curr_active_slave->dev->name); | 2680 | bond->curr_active_slave->dev->name); |
2680 | 2681 | ||
2681 | if (bond->curr_active_slave) { | 2682 | if (bond->curr_active_slave) { |
@@ -2691,15 +2692,15 @@ static void bond_ab_arp_probe(struct bonding *bond) | |||
2691 | * for becoming the curr_active_slave | 2692 | * for becoming the curr_active_slave |
2692 | */ | 2693 | */ |
2693 | 2694 | ||
2694 | if (!bond->current_arp_slave) { | 2695 | if (!curr_arp_slave) { |
2695 | bond->current_arp_slave = bond_first_slave(bond); | 2696 | curr_arp_slave = bond_first_slave_rcu(bond); |
2696 | if (!bond->current_arp_slave) | 2697 | if (!curr_arp_slave) |
2697 | return; | 2698 | return; |
2698 | } | 2699 | } |
2699 | 2700 | ||
2700 | bond_set_slave_inactive_flags(bond->current_arp_slave); | 2701 | bond_set_slave_inactive_flags(curr_arp_slave); |
2701 | 2702 | ||
2702 | bond_for_each_slave(bond, slave, iter) { | 2703 | bond_for_each_slave_rcu(bond, slave, iter) { |
2703 | if (!found && !before && IS_UP(slave->dev)) | 2704 | if (!found && !before && IS_UP(slave->dev)) |
2704 | before = slave; | 2705 | before = slave; |
2705 | 2706 | ||
@@ -2722,7 +2723,7 @@ static void bond_ab_arp_probe(struct bonding *bond) | |||
2722 | pr_info("%s: backup interface %s is now down.\n", | 2723 | pr_info("%s: backup interface %s is now down.\n", |
2723 | bond->dev->name, slave->dev->name); | 2724 | bond->dev->name, slave->dev->name); |
2724 | } | 2725 | } |
2725 | if (slave == bond->current_arp_slave) | 2726 | if (slave == curr_arp_slave) |
2726 | found = true; | 2727 | found = true; |
2727 | } | 2728 | } |
2728 | 2729 | ||
@@ -2736,8 +2737,7 @@ static void bond_ab_arp_probe(struct bonding *bond) | |||
2736 | bond_set_slave_active_flags(new_slave); | 2737 | bond_set_slave_active_flags(new_slave); |
2737 | bond_arp_send_all(bond, new_slave); | 2738 | bond_arp_send_all(bond, new_slave); |
2738 | new_slave->jiffies = jiffies; | 2739 | new_slave->jiffies = jiffies; |
2739 | bond->current_arp_slave = new_slave; | 2740 | rcu_assign_pointer(bond->current_arp_slave, new_slave); |
2740 | |||
2741 | } | 2741 | } |
2742 | 2742 | ||
2743 | void bond_activebackup_arp_mon(struct work_struct *work) | 2743 | void bond_activebackup_arp_mon(struct work_struct *work) |
@@ -2747,43 +2747,38 @@ void bond_activebackup_arp_mon(struct work_struct *work) | |||
2747 | bool should_notify_peers = false; | 2747 | bool should_notify_peers = false; |
2748 | int delta_in_ticks; | 2748 | int delta_in_ticks; |
2749 | 2749 | ||
2750 | read_lock(&bond->lock); | ||
2751 | |||
2752 | delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval); | 2750 | delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval); |
2753 | 2751 | ||
2754 | if (!bond_has_slaves(bond)) | 2752 | if (!bond_has_slaves(bond)) |
2755 | goto re_arm; | 2753 | goto re_arm; |
2756 | 2754 | ||
2755 | rcu_read_lock(); | ||
2756 | |||
2757 | should_notify_peers = bond_should_notify_peers(bond); | 2757 | should_notify_peers = bond_should_notify_peers(bond); |
2758 | 2758 | ||
2759 | if (bond_ab_arp_inspect(bond)) { | 2759 | if (bond_ab_arp_inspect(bond)) { |
2760 | read_unlock(&bond->lock); | 2760 | rcu_read_unlock(); |
2761 | 2761 | ||
2762 | /* Race avoidance with bond_close flush of workqueue */ | 2762 | /* Race avoidance with bond_close flush of workqueue */ |
2763 | if (!rtnl_trylock()) { | 2763 | if (!rtnl_trylock()) { |
2764 | read_lock(&bond->lock); | ||
2765 | delta_in_ticks = 1; | 2764 | delta_in_ticks = 1; |
2766 | should_notify_peers = false; | 2765 | should_notify_peers = false; |
2767 | goto re_arm; | 2766 | goto re_arm; |
2768 | } | 2767 | } |
2769 | 2768 | ||
2770 | read_lock(&bond->lock); | ||
2771 | |||
2772 | bond_ab_arp_commit(bond); | 2769 | bond_ab_arp_commit(bond); |
2773 | 2770 | ||
2774 | read_unlock(&bond->lock); | ||
2775 | rtnl_unlock(); | 2771 | rtnl_unlock(); |
2776 | read_lock(&bond->lock); | 2772 | rcu_read_lock(); |
2777 | } | 2773 | } |
2778 | 2774 | ||
2779 | bond_ab_arp_probe(bond); | 2775 | bond_ab_arp_probe(bond); |
2776 | rcu_read_unlock(); | ||
2780 | 2777 | ||
2781 | re_arm: | 2778 | re_arm: |
2782 | if (bond->params.arp_interval) | 2779 | if (bond->params.arp_interval) |
2783 | queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks); | 2780 | queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks); |
2784 | 2781 | ||
2785 | read_unlock(&bond->lock); | ||
2786 | |||
2787 | if (should_notify_peers) { | 2782 | if (should_notify_peers) { |
2788 | if (!rtnl_trylock()) | 2783 | if (!rtnl_trylock()) |
2789 | return; | 2784 | return; |