bonding: Convert miimon to new locking

Convert mii (link state) monitor to acquire correct locks for failover events. In particular, failovers generally require RTNL at a low level (when manipulating device MAC addresses, for example) and no other locks. The high level monitor is responsible for acquiring a known set of locks, RTNL, the bond->lock for read and the slave_lock for write, and the low level failover processing can then release appropriate locks as needed. This patch provides the high level portion. As it is undesirable to acquire RTNL for every monitor pass (which may occur as often as every 10 ms), the miimon has been converted to do conditional locking. A first pass inspects all slaves to determine if any action is required, and if so, a second pass (after acquring RTNL) is done to perform any actions (doing a complete rescan, as the situation may have changed when all locks were released). Signed-off-by: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com> Signed-off-by: Jeff Garzik <jeff@garzik.org>

bonding: Convert miimon to new locking
Convert mii (link state) monitor to acquire correct locks for failover events. In particular, failovers generally require RTNL at a low level (when manipulating device MAC addresses, for example) and no other locks. The high level monitor is responsible for acquiring a known set of locks, RTNL, the bond->lock for read and the slave_lock for write, and the low level failover processing can then release appropriate locks as needed. This patch provides the high level portion. As it is undesirable to acquire RTNL for every monitor pass (which may occur as often as every 10 ms), the miimon has been converted to do conditional locking. A first pass inspects all slaves to determine if any action is required, and if so, a second pass (after acquring RTNL) is done to perform any actions (doing a complete rescan, as the situation may have changed when all locks were released). Signed-off-by: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com> Signed-off-by: Jeff Garzik <jeff@garzik.org>
Jay Vosburgh · Jeff Garzik
1 parent cf5f904493
Showing 1 changed file with 51 additions and 19 deletions Side-by-side Diff
drivers/net/bonding/bond_main.c
@@ -2088,28 +2088,26 @@
  
 /*-------------------------------- Monitoring -------------------------------*/
  
-/* this function is called regularly to monitor each slave's link. */
-void bond_mii_monitor(struct work_struct *work)
+/*
+ * if !have_locks, return nonzero if a failover is necessary.  if
+ * have_locks, do whatever failover activities are needed.
+ *
+ * This is to separate the inspection and failover steps for locking
+ * purposes; failover requires rtnl, but acquiring it for every
+ * inspection is undesirable, so a wrapper first does inspection, and
+ * the acquires the necessary locks and calls again to perform
+ * failover if needed.  Since all locks are dropped, a complete
+ * restart is needed between calls.
+ */
+static int __bond_mii_monitor(struct bonding *bond, int have_locks)
 {
-	struct bonding *bond = container_of(work, struct bonding,
-					    mii_work.work);
 	struct slave *slave, *oldcurrent;
 	int do_failover = 0;
-	int delta_in_ticks;
 	int i;
  
-	read_lock(&bond->lock);
-
-	delta_in_ticks = (bond->params.miimon * HZ) / 1000;
-
-	if (bond->kill_timers) {
+	if (bond->slave_cnt == 0)
 		goto out;
-	}
  
-	if (bond->slave_cnt == 0) {
-		goto re_arm;
-	}
-
 	/* we will try to read the link status of each of our slaves, and
 	 * set their IFF_RUNNING flag appropriately. For each slave not
 	 * supporting MII status, we won't do anything so that a user-space
@@ -2175,6 +2173,9 @@
 			if (link_state != BMSR_LSTATUS) {
 				/* link stays down */
 				if (slave->delay <= 0) {
+					if (!have_locks)
+						return 1;
+
 					/* link down for too long time */
 					slave->link = BOND_LINK_DOWN;
  
@@ -2258,6 +2259,9 @@
 			} else {
 				/* link stays up */
 				if (slave->delay == 0) {
+					if (!have_locks)
+						return 1;
+
 					/* now the link has been up for long time enough */
 					slave->link = BOND_LINK_UP;
 					slave->jiffies = jiffies;
  
  
@@ -2331,13 +2335,41 @@
 	} else
 		bond_set_carrier(bond);
  
-re_arm:
-	if (bond->params.miimon)
-		queue_delayed_work(bond->wq, &bond->mii_work, delta_in_ticks);
 out:
-	read_unlock(&bond->lock);
+	return 0;
 }
  
+/*
+ * bond_mii_monitor
+ *
+ * Really a wrapper that splits the mii monitor into two phases: an
+ * inspection, then (if inspection indicates something needs to be
+ * done) an acquisition of appropriate locks followed by another pass
+ * to implement whatever link state changes are indicated.
+ */
+void bond_mii_monitor(struct work_struct *work)
+{
+	struct bonding *bond = container_of(work, struct bonding,
+					    mii_work.work);
+	unsigned long delay;
+
+	read_lock(&bond->lock);
+	if (bond->kill_timers) {
+		read_unlock(&bond->lock);
+		return;
+	}
+	if (__bond_mii_monitor(bond, 0)) {
+		read_unlock(&bond->lock);
+		rtnl_lock();
+		read_lock(&bond->lock);
+		__bond_mii_monitor(bond, 1);
+		rtnl_unlock();
+	}
+
+	delay = ((bond->params.miimon * HZ) / 1000) ? : 1;
+	read_unlock(&bond->lock);
+	queue_delayed_work(bond->wq, &bond->mii_work, delay);
+}
  
 static __be32 bond_glean_dev_ip(struct net_device *dev)
 {
...	...	@@ -2088,28 +2088,26 @@
2088	2088
2089	2089	/-------------------------------- Monitoring -------------------------------/
2090	2090
2091		-/* this function is called regularly to monitor each slave's link. */
2092		-void bond_mii_monitor(struct work_struct *work)
	2091	+/*
	2092	+ * if !have_locks, return nonzero if a failover is necessary. if
	2093	+ * have_locks, do whatever failover activities are needed.
	2094	+ *
	2095	+ * This is to separate the inspection and failover steps for locking
	2096	+ * purposes; failover requires rtnl, but acquiring it for every
	2097	+ * inspection is undesirable, so a wrapper first does inspection, and
	2098	+ * the acquires the necessary locks and calls again to perform
	2099	+ * failover if needed. Since all locks are dropped, a complete
	2100	+ * restart is needed between calls.
	2101	+ */
	2102	+static int __bond_mii_monitor(struct bonding *bond, int have_locks)
2093	2103	{
2094		- struct bonding *bond = container_of(work, struct bonding,
2095		- mii_work.work);
2096	2104	struct slave slave, oldcurrent;
2097	2105	int do_failover = 0;
2098		- int delta_in_ticks;
2099	2106	int i;
2100	2107
2101		- read_lock(&bond->lock);
2102		-
2103		- delta_in_ticks = (bond->params.miimon * HZ) / 1000;
2104		-
2105		- if (bond->kill_timers) {
	2108	+ if (bond->slave_cnt == 0)
2106	2109	goto out;
2107		- }
2108	2110
2109		- if (bond->slave_cnt == 0) {
2110		- goto re_arm;
2111		- }
2112		-
2113	2111	/* we will try to read the link status of each of our slaves, and
2114	2112	* set their IFF_RUNNING flag appropriately. For each slave not
2115	2113	* supporting MII status, we won't do anything so that a user-space
...	...	@@ -2175,6 +2173,9 @@
2175	2173	if (link_state != BMSR_LSTATUS) {
2176	2174	/* link stays down */
2177	2175	if (slave->delay <= 0) {
	2176	+ if (!have_locks)
	2177	+ return 1;
	2178	+
2178	2179	/* link down for too long time */
2179	2180	slave->link = BOND_LINK_DOWN;
2180	2181
...	...	@@ -2258,6 +2259,9 @@
2258	2259	} else {
2259	2260	/* link stays up */
2260	2261	if (slave->delay == 0) {
	2262	+ if (!have_locks)
	2263	+ return 1;
	2264	+
2261	2265	/* now the link has been up for long time enough */
2262	2266	slave->link = BOND_LINK_UP;
2263	2267	slave->jiffies = jiffies;
2264	2268
2265	2269
...	...	@@ -2331,13 +2335,41 @@
2331	2335	} else
2332	2336	bond_set_carrier(bond);
2333	2337
2334		-re_arm:
2335		- if (bond->params.miimon)
2336		- queue_delayed_work(bond->wq, &bond->mii_work, delta_in_ticks);
2337	2338	out:
2338		- read_unlock(&bond->lock);
	2339	+ return 0;
2339	2340	}
2340	2341
	2342	+/*
	2343	+ * bond_mii_monitor
	2344	+ *
	2345	+ * Really a wrapper that splits the mii monitor into two phases: an
	2346	+ * inspection, then (if inspection indicates something needs to be
	2347	+ * done) an acquisition of appropriate locks followed by another pass
	2348	+ * to implement whatever link state changes are indicated.
	2349	+ */
	2350	+void bond_mii_monitor(struct work_struct *work)
	2351	+{
	2352	+ struct bonding *bond = container_of(work, struct bonding,
	2353	+ mii_work.work);
	2354	+ unsigned long delay;
	2355	+
	2356	+ read_lock(&bond->lock);
	2357	+ if (bond->kill_timers) {
	2358	+ read_unlock(&bond->lock);
	2359	+ return;
	2360	+ }
	2361	+ if (__bond_mii_monitor(bond, 0)) {
	2362	+ read_unlock(&bond->lock);
	2363	+ rtnl_lock();
	2364	+ read_lock(&bond->lock);
	2365	+ __bond_mii_monitor(bond, 1);
	2366	+ rtnl_unlock();
	2367	+ }
	2368	+
	2369	+ delay = ((bond->params.miimon * HZ) / 1000) ? : 1;
	2370	+ read_unlock(&bond->lock);
	2371	+ queue_delayed_work(bond->wq, &bond->mii_work, delay);
	2372	+}
2341	2373
2342	2374	static __be32 bond_glean_dev_ip(struct net_device *dev)
2343	2375	{