diff options
author | David S. Miller <davem@davemloft.net> | 2014-01-13 22:22:35 -0800 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-01-13 22:22:35 -0800 |
commit | 2afe02eed1ac565e5ba90254706e67f670ae41a2 (patch) | |
tree | 8cf62ce9ae669d5587bbc7b9dc8ee504eecaed56 | |
parent | 853dc21bfe15f545347b5c82aad02735d552110d (diff) | |
parent | 49b7624eda6867d2803bcc5bbf6f25936184304a (diff) |
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: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/net/bonding/bond_3ad.c | 37 |
1 files changed, 19 insertions, 18 deletions
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 29db1caee764..cce1f1bf90b4 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -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 * found. + * The caller must hold RCU or RTNL lock. */ static inline struct aggregator *__get_first_agg(struct port *port) { struct bonding *bond = __get_bond_by_port(port); struct slave *first_slave; + struct aggregator *agg; /* If there's no bond for this port, or bond has no slaves */ if (bond == NULL) @@ -155,9 +157,10 @@ static inline struct aggregator *__get_first_agg(struct port *port) rcu_read_lock(); first_slave = bond_first_slave_rcu(bond); + agg = first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL; 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) /** * __get_active_agg - get the current active aggregator * @aggregator: the aggregator we're looking at + * + * Caller must hold RCU lock. */ 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 slave *slave; - rcu_read_lock(); bond_for_each_slave_rcu(bond, slave, iter) - if (SLAVE_AD_INFO(slave).aggregator.is_active) { - rcu_read_unlock(); + if (SLAVE_AD_INFO(slave).aggregator.is_active) return &(SLAVE_AD_INFO(slave).aggregator); - } - rcu_read_unlock(); return NULL; } @@ -1496,11 +1497,11 @@ static void ad_agg_selection_logic(struct aggregator *agg) struct slave *slave; struct port *port; + rcu_read_lock(); origin = agg; active = __get_active_agg(agg); best = (active && agg_device_up(active)) ? active : NULL; - rcu_read_lock(); bond_for_each_slave_rcu(bond, slave, iter) { agg = &(SLAVE_AD_INFO(slave).aggregator); @@ -2327,32 +2328,32 @@ int bond_3ad_set_carrier(struct bonding *bond) { struct aggregator *active; struct slave *first_slave; + int ret = 1; rcu_read_lock(); first_slave = bond_first_slave_rcu(bond); - rcu_read_unlock(); - if (!first_slave) - return 0; + if (!first_slave) { + ret = 0; + goto out; + } active = __get_active_agg(&(SLAVE_AD_INFO(first_slave).aggregator)); if (active) { /* are enough slaves available to consider link up? */ if (active->num_of_ports < bond->params.min_links) { if (netif_carrier_ok(bond->dev)) { netif_carrier_off(bond->dev); - return 1; + goto out; } } else if (!netif_carrier_ok(bond->dev)) { netif_carrier_on(bond->dev); - return 1; + goto out; } - return 0; - } - - if (netif_carrier_ok(bond->dev)) { + } else if (netif_carrier_ok(bond->dev)) { netif_carrier_off(bond->dev); - return 1; } - return 0; +out: + rcu_read_unlock(); + return ret; } /** |