Skip to content

Commit 4cb4f97

Browse files
dingtianhongdavem330
authored andcommitted
bonding: rebuild the lock use for bond_mii_monitor()
The bond_mii_monitor() still use bond lock to protect bond slave list, it is no effect, I have 2 way to fix the problem, move the RTNL to the top of the function, or add RCU to protect the bond slave list, according to the Jay Vosburgh's opinion, 10 times one second is a truely big performance loss if use RTNL to protect the whole monitor, so I would take the advice and use RCU to protect the bond slave list. The bond_has_slave() will not protect by anything, there will no things happen if the slave list is be changed, unless the bond was free, but it will not happened before the monitor, the bond will closed before be freed. The peers notify for the bond will calling curr_active_slave, so derefence the slave to make sure we will accessing the same slave if the curr_active_slave changed, as the rcu dereference need in read-side critical sector and bond_change_active_slave() will call it with no RCU hold, so add peer notify in rcu_read_lock which will be nested in monitor. Suggested-by: Jay Vosburgh <[email protected]> Suggested-by: Veaceslav Falico <[email protected]> Signed-off-by: Ding Tianhong <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent b2e7ace commit 4cb4f97

File tree

1 file changed

+11
-13
lines changed

1 file changed

+11
-13
lines changed

drivers/net/bonding/bond_main.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,11 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
815815

816816
static bool bond_should_notify_peers(struct bonding *bond)
817817
{
818-
struct slave *slave = bond->curr_active_slave;
818+
struct slave *slave;
819+
820+
rcu_read_lock();
821+
slave = rcu_dereference(bond->curr_active_slave);
822+
rcu_read_unlock();
819823

820824
pr_debug("bond_should_notify_peers: bond %s slave %s\n",
821825
bond->dev->name, slave ? slave->dev->name : "NULL");
@@ -1919,7 +1923,7 @@ static int bond_miimon_inspect(struct bonding *bond)
19191923

19201924
ignore_updelay = !bond->curr_active_slave ? true : false;
19211925

1922-
bond_for_each_slave(bond, slave, iter) {
1926+
bond_for_each_slave_rcu(bond, slave, iter) {
19231927
slave->new_link = BOND_LINK_NOCHANGE;
19241928

19251929
link_state = bond_check_dev_link(bond, slave->dev, 0);
@@ -2117,41 +2121,35 @@ void bond_mii_monitor(struct work_struct *work)
21172121
bool should_notify_peers = false;
21182122
unsigned long delay;
21192123

2120-
read_lock(&bond->lock);
2121-
21222124
delay = msecs_to_jiffies(bond->params.miimon);
21232125

21242126
if (!bond_has_slaves(bond))
21252127
goto re_arm;
21262128

2129+
rcu_read_lock();
2130+
21272131
should_notify_peers = bond_should_notify_peers(bond);
21282132

21292133
if (bond_miimon_inspect(bond)) {
2130-
read_unlock(&bond->lock);
2134+
rcu_read_unlock();
21312135

21322136
/* Race avoidance with bond_close cancel of workqueue */
21332137
if (!rtnl_trylock()) {
2134-
read_lock(&bond->lock);
21352138
delay = 1;
21362139
should_notify_peers = false;
21372140
goto re_arm;
21382141
}
21392142

2140-
read_lock(&bond->lock);
2141-
21422143
bond_miimon_commit(bond);
21432144

2144-
read_unlock(&bond->lock);
21452145
rtnl_unlock(); /* might sleep, hold no other locks */
2146-
read_lock(&bond->lock);
2147-
}
2146+
} else
2147+
rcu_read_unlock();
21482148

21492149
re_arm:
21502150
if (bond->params.miimon)
21512151
queue_delayed_work(bond->wq, &bond->mii_work, delay);
21522152

2153-
read_unlock(&bond->lock);
2154-
21552153
if (should_notify_peers) {
21562154
if (!rtnl_trylock())
21572155
return;

0 commit comments

Comments
 (0)