Commit 2afe02ee authored by David S. Miller's avatar David S. Miller

Merge branch 'bonding_rcu'

Veaceslav Falico says:

====================
bonding: fix bond_3ad RCU usage

While digging through bond_3ad.c I've found that the RCU usage there is
just wrong - it's used as a kind of mutex/spinlock instead of RCU.

v3->v4: remove useless goto and wrap __get_first_agg() in proper RCU.

v2->v3: make bond_3ad_set_carrier() use RCU read lock for the whole
function, so that all other functions will be protected by RCU as well.
This way we can use _rcu variants everywhere.

v1->v2: use generic primitives instead of _rcu ones cause we can hold RTNL
lock without RCU one, which is still safe.

This patchset is on top of bond_3ad.c cleanup:
http://www.spinics.net/lists/netdev/msg265447.html
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 853dc21b 49b7624e
...@@ -143,11 +143,13 @@ static inline struct bonding *__get_bond_by_port(struct port *port) ...@@ -143,11 +143,13 @@ static inline struct bonding *__get_bond_by_port(struct port *port)
* *
* Return the aggregator of the first slave in @bond, or %NULL if it can't be * Return the aggregator of the first slave in @bond, or %NULL if it can't be
* found. * found.
* The caller must hold RCU or RTNL lock.
*/ */
static inline struct aggregator *__get_first_agg(struct port *port) static inline struct aggregator *__get_first_agg(struct port *port)
{ {
struct bonding *bond = __get_bond_by_port(port); struct bonding *bond = __get_bond_by_port(port);
struct slave *first_slave; struct slave *first_slave;
struct aggregator *agg;
/* If there's no bond for this port, or bond has no slaves */ /* If there's no bond for this port, or bond has no slaves */
if (bond == NULL) if (bond == NULL)
...@@ -155,9 +157,10 @@ static inline struct aggregator *__get_first_agg(struct port *port) ...@@ -155,9 +157,10 @@ static inline struct aggregator *__get_first_agg(struct port *port)
rcu_read_lock(); rcu_read_lock();
first_slave = bond_first_slave_rcu(bond); first_slave = bond_first_slave_rcu(bond);
agg = first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL;
rcu_read_unlock(); rcu_read_unlock();
return first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL; return agg;
} }
/** /**
...@@ -675,6 +678,8 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator) ...@@ -675,6 +678,8 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
/** /**
* __get_active_agg - get the current active aggregator * __get_active_agg - get the current active aggregator
* @aggregator: the aggregator we're looking at * @aggregator: the aggregator we're looking at
*
* Caller must hold RCU lock.
*/ */
static struct aggregator *__get_active_agg(struct aggregator *aggregator) static struct aggregator *__get_active_agg(struct aggregator *aggregator)
{ {
...@@ -682,13 +687,9 @@ static struct aggregator *__get_active_agg(struct aggregator *aggregator) ...@@ -682,13 +687,9 @@ static struct aggregator *__get_active_agg(struct aggregator *aggregator)
struct list_head *iter; struct list_head *iter;
struct slave *slave; struct slave *slave;
rcu_read_lock();
bond_for_each_slave_rcu(bond, slave, iter) bond_for_each_slave_rcu(bond, slave, iter)
if (SLAVE_AD_INFO(slave).aggregator.is_active) { if (SLAVE_AD_INFO(slave).aggregator.is_active)
rcu_read_unlock();
return &(SLAVE_AD_INFO(slave).aggregator); return &(SLAVE_AD_INFO(slave).aggregator);
}
rcu_read_unlock();
return NULL; return NULL;
} }
...@@ -1496,11 +1497,11 @@ static void ad_agg_selection_logic(struct aggregator *agg) ...@@ -1496,11 +1497,11 @@ static void ad_agg_selection_logic(struct aggregator *agg)
struct slave *slave; struct slave *slave;
struct port *port; struct port *port;
rcu_read_lock();
origin = agg; origin = agg;
active = __get_active_agg(agg); active = __get_active_agg(agg);
best = (active && agg_device_up(active)) ? active : NULL; best = (active && agg_device_up(active)) ? active : NULL;
rcu_read_lock();
bond_for_each_slave_rcu(bond, slave, iter) { bond_for_each_slave_rcu(bond, slave, iter) {
agg = &(SLAVE_AD_INFO(slave).aggregator); agg = &(SLAVE_AD_INFO(slave).aggregator);
...@@ -2327,32 +2328,32 @@ int bond_3ad_set_carrier(struct bonding *bond) ...@@ -2327,32 +2328,32 @@ int bond_3ad_set_carrier(struct bonding *bond)
{ {
struct aggregator *active; struct aggregator *active;
struct slave *first_slave; struct slave *first_slave;
int ret = 1;
rcu_read_lock(); rcu_read_lock();
first_slave = bond_first_slave_rcu(bond); first_slave = bond_first_slave_rcu(bond);
rcu_read_unlock(); if (!first_slave) {
if (!first_slave) ret = 0;
return 0; goto out;
}
active = __get_active_agg(&(SLAVE_AD_INFO(first_slave).aggregator)); active = __get_active_agg(&(SLAVE_AD_INFO(first_slave).aggregator));
if (active) { if (active) {
/* are enough slaves available to consider link up? */ /* are enough slaves available to consider link up? */
if (active->num_of_ports < bond->params.min_links) { if (active->num_of_ports < bond->params.min_links) {
if (netif_carrier_ok(bond->dev)) { if (netif_carrier_ok(bond->dev)) {
netif_carrier_off(bond->dev); netif_carrier_off(bond->dev);
return 1; goto out;
} }
} else if (!netif_carrier_ok(bond->dev)) { } else if (!netif_carrier_ok(bond->dev)) {
netif_carrier_on(bond->dev); netif_carrier_on(bond->dev);
return 1; goto out;
} }
return 0; } else if (netif_carrier_ok(bond->dev)) {
}
if (netif_carrier_ok(bond->dev)) {
netif_carrier_off(bond->dev); netif_carrier_off(bond->dev);
return 1;
} }
return 0; out:
rcu_read_unlock();
return ret;
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment