From 4740d6382790077f22c606d03804f5d9f15b90d7 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Tue, 15 Jul 2014 06:56:55 -0700 Subject: bonding: add proper __rcu annotation for curr_active_slave RCU was added to bonding in linux-3.12 but lacked proper sparse annotations. Using __rcu annotation actually helps to spot all accesses to bond->curr_active_slave are correctly protected, with LOCKDEP support. Signed-off-by: Eric Dumazet Acked-by: Veaceslav Falico Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 53 ++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 24 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 46dcb7b6216f..27ce838d45d6 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -498,11 +498,11 @@ static int bond_set_promiscuity(struct bonding *bond, int inc) int err = 0; if (bond_uses_primary(bond)) { + struct slave *curr_active = bond_deref_active_protected(bond); + /* write lock already acquired */ - if (bond->curr_active_slave) { - err = dev_set_promiscuity(bond->curr_active_slave->dev, - inc); - } + if (curr_active) + err = dev_set_promiscuity(curr_active->dev, inc); } else { struct slave *slave; @@ -524,11 +524,11 @@ static int bond_set_allmulti(struct bonding *bond, int inc) int err = 0; if (bond_uses_primary(bond)) { + struct slave *curr_active = bond_deref_active_protected(bond); + /* write lock already acquired */ - if (bond->curr_active_slave) { - err = dev_set_allmulti(bond->curr_active_slave->dev, - inc); - } + if (curr_active) + err = dev_set_allmulti(curr_active->dev, inc); } else { struct slave *slave; @@ -713,7 +713,7 @@ out: static bool bond_should_change_active(struct bonding *bond) { struct slave *prim = bond->primary_slave; - struct slave *curr = bond->curr_active_slave; + struct slave *curr = bond_deref_active_protected(bond); if (!prim || !curr || curr->link != BOND_LINK_UP) return true; @@ -792,7 +792,11 @@ static bool bond_should_notify_peers(struct bonding *bond) */ void bond_change_active_slave(struct bonding *bond, struct slave *new_active) { - struct slave *old_active = bond->curr_active_slave; + struct slave *old_active; + + old_active = rcu_dereference_protected(bond->curr_active_slave, + !new_active || + lockdep_is_held(&bond->curr_slave_lock)); if (old_active == new_active) return; @@ -900,7 +904,7 @@ void bond_select_active_slave(struct bonding *bond) int rv; best_slave = bond_find_best_slave(bond); - if (best_slave != bond->curr_active_slave) { + if (best_slave != bond_deref_active_protected(bond)) { bond_change_active_slave(bond, best_slave); rv = bond_set_carrier(bond); if (!rv) @@ -1531,7 +1535,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) * anyway (it holds no special properties of the bond device), * so we can change it without calling change_active_interface() */ - if (!bond->curr_active_slave && new_slave->link == BOND_LINK_UP) + if (!rcu_access_pointer(bond->curr_active_slave) && + new_slave->link == BOND_LINK_UP) rcu_assign_pointer(bond->curr_active_slave, new_slave); break; @@ -1602,7 +1607,7 @@ err_detach: vlan_vids_del_by_dev(slave_dev, bond_dev); if (bond->primary_slave == new_slave) bond->primary_slave = NULL; - if (bond->curr_active_slave == new_slave) { + if (rcu_access_pointer(bond->curr_active_slave) == new_slave) { block_netpoll_tx(); write_lock_bh(&bond->curr_slave_lock); bond_change_active_slave(bond, NULL); @@ -1704,7 +1709,7 @@ static int __bond_release_one(struct net_device *bond_dev, bond_is_active_slave(slave) ? "active" : "backup", slave_dev->name); - oldcurrent = bond->curr_active_slave; + oldcurrent = rcu_access_pointer(bond->curr_active_slave); bond->current_arp_slave = NULL; @@ -1878,7 +1883,7 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in /*-------------------------------- Monitoring -------------------------------*/ - +/* called with rcu_read_lock() */ static int bond_miimon_inspect(struct bonding *bond) { int link_state, commit = 0; @@ -1886,7 +1891,7 @@ static int bond_miimon_inspect(struct bonding *bond) struct slave *slave; bool ignore_updelay; - ignore_updelay = !bond->curr_active_slave ? true : false; + ignore_updelay = !rcu_dereference(bond->curr_active_slave); bond_for_each_slave_rcu(bond, slave, iter) { slave->new_link = BOND_LINK_NOCHANGE; @@ -2046,7 +2051,7 @@ static void bond_miimon_commit(struct bonding *bond) bond_alb_handle_link_change(bond, slave, BOND_LINK_DOWN); - if (slave == bond->curr_active_slave) + if (slave == rcu_access_pointer(bond->curr_active_slave)) goto do_failover; continue; @@ -2416,7 +2421,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work) rcu_read_lock(); - oldcurrent = ACCESS_ONCE(bond->curr_active_slave); + oldcurrent = rcu_dereference(bond->curr_active_slave); /* see if any of the previous devices are up now (i.e. they have * xmt and rcv traffic). the curr_active_slave does not come into * the picture unless it is null. also, slave->last_link_up is not @@ -2607,8 +2612,8 @@ static void bond_ab_arp_commit(struct bonding *bond) case BOND_LINK_UP: trans_start = dev_trans_start(slave->dev); - if (bond->curr_active_slave != slave || - (!bond->curr_active_slave && + if (rtnl_dereference(bond->curr_active_slave) != slave || + (!rtnl_dereference(bond->curr_active_slave) && bond_time_in_interval(bond, trans_start, 1))) { slave->link = BOND_LINK_UP; if (bond->current_arp_slave) { @@ -2621,7 +2626,7 @@ static void bond_ab_arp_commit(struct bonding *bond) pr_info("%s: link status definitely up for interface %s\n", bond->dev->name, slave->dev->name); - if (!bond->curr_active_slave || + if (!rtnl_dereference(bond->curr_active_slave) || (slave == bond->primary_slave)) goto do_failover; @@ -2640,7 +2645,7 @@ static void bond_ab_arp_commit(struct bonding *bond) pr_info("%s: link status definitely down for interface %s, disabling it\n", bond->dev->name, slave->dev->name); - if (slave == bond->curr_active_slave) { + if (slave == rtnl_dereference(bond->curr_active_slave)) { bond->current_arp_slave = NULL; goto do_failover; } @@ -3097,8 +3102,8 @@ static int bond_open(struct net_device *bond_dev) if (bond_has_slaves(bond)) { read_lock(&bond->curr_slave_lock); bond_for_each_slave(bond, slave, iter) { - if (bond_uses_primary(bond) - && (slave != bond->curr_active_slave)) { + if (bond_uses_primary(bond) && + slave != rcu_access_pointer(bond->curr_active_slave)) { bond_set_slave_inactive_flags(slave, BOND_SLAVE_NOTIFY_NOW); } else { -- cgit v1.2.3-55-g7522