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 <edumazet@google.com>
Acked-by: Veaceslav Falico <vfalico@gmail.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 46dcb7b..27ce838 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -498,11 +498,11 @@
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 @@
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 @@
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 @@
*/
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 @@
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 @@
* 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 @@
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 @@
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 @@
/*-------------------------------- Monitoring -------------------------------*/
-
+/* called with rcu_read_lock() */
static int bond_miimon_inspect(struct bonding *bond)
{
int link_state, commit = 0;
@@ -1886,7 +1891,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 {