PERFORCE change 140218 for review

[Sam Leffler]

http://perforce.freebsd.org/chv.cgi?CH=140218

Change 140218 by sam at sam_ebb on 2008/04/18 18:21:48

	Eliminate holding the com lock over the ioctl callback to
	auto-up the parent device by deferring to a task.  We mark
	the vap running so when the driver calls back to us through
	ieee80211_start_all we don't double count them in ic_nrunning.
	This eliminates the lock being held over the driver's init
	method which was problematic for drivers that must defer work
	(e.g. usb)

Affected files ...

.. //depot/projects/vap/sys/net80211/ieee80211_proto.c#28 edit
.. //depot/projects/vap/sys/net80211/ieee80211_var.h#39 edit

Differences ...

==== //depot/projects/vap/sys/net80211/ieee80211_proto.c#28 (text+ko) ====

@@ -37,6 +37,7 @@
 #include <sys/param.h>
 #include <sys/kernel.h>
 #include <sys/systm.h>
+#include <sys/taskqueue.h>
 
 #include <sys/socket.h>
 #include <sys/sockio.h>
@@ -95,6 +96,7 @@
 	"WME_UPSD",
 };
 
+static void parent_up(void *, int);
 static int ieee80211_new_state_locked(struct ieee80211vap *,
 	enum ieee80211_state, int);
 
@@ -128,6 +130,8 @@
 	}
 	ic->ic_protmode = IEEE80211_PROT_CTSONLY;
 
+	TASK_INIT(&ic->ic_parent_task, 0, parent_up, ifp);
+
 	ic->ic_wme.wme_hipri_switch_hysteresis =
 		AGGRESSIVE_MODE_SWITCH_HYSTERESIS;
 
@@ -1059,6 +1063,14 @@
 	}
 }
 
+static void
+parent_up(void *arg, int npending)
+{
+	struct ifnet *parent = arg;
+
+	parent->if_ioctl(parent, SIOCSIFFLAGS, NULL);
+}
+
 /*
  * Start a vap running.  If this is the first vap to be
  * set running on the underlying device then we
@@ -1079,6 +1091,15 @@
 
 	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 		/*
+		 * Mark us running.  Note that it's ok to do this first;
+		 * if we need to bring the parent device up we defer that
+		 * to avoid dropping the com lock.  We expect the device
+		 * to respond to being marked up by calling back into us
+		 * through ieee80211_start_all at which point we'll come
+		 * back in here and complete the work.
+		 */
+		ifp->if_drv_flags |= IFF_DRV_RUNNING;
+		/*
 		 * We are not running; if this we are the first vap
 		 * to be brought up auto-up the parent if necessary.
 		 */
@@ -1088,13 +1109,9 @@
 			    IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG,
 			    "%s: up parent %s\n", __func__, parent->if_xname);
 			parent->if_flags |= IFF_UP;
-			parent->if_ioctl(parent, SIOCSIFFLAGS, NULL);
+			taskqueue_enqueue(taskqueue_swi, &ic->ic_parent_task);
+			return;
 		}
-		/*
-		 * Mark us running.  Note that we do this after
-		 * bringing up the parent device to avoid recursion.
-		 */
-		ifp->if_drv_flags |= IFF_DRV_RUNNING;	/* mark us running */
 	}
 	/*
 	 * If the parent is up and running, then kick the
@@ -1231,7 +1248,6 @@
 
 /*
  * Stop all vap's running on a device.
- * XXX not currently used
  */
 void
 ieee80211_stop_all(struct ieee80211com *ic)
@@ -1240,12 +1256,18 @@
 	struct ieee80211vap *vap;
 
 	IEEE80211_LOCK(ic);
+	/* XXX why do we do this? */
 	/* XXX shouldn't touch driver state */
 	parent->if_drv_flags &= ~IFF_DRV_RUNNING;
 	TAILQ_FOREACH(vap, &ic->ic_vaps, iv_next) {
 		struct ifnet *ifp = vap->iv_ifp;
-		if (IFNET_IS_UP_RUNNING(ifp))	/* NB: avoid recursion */
+		if (IFNET_IS_UP_RUNNING(ifp)) {	/* NB: avoid recursion */
+			/*
+			 * NB: since parent is marked !RUNNING
+			 * this won't drop com lock
+			 */
 			ieee80211_stop_locked(vap);
+		}
 	}
 	IEEE80211_UNLOCK(ic);
 }

==== //depot/projects/vap/sys/net80211/ieee80211_var.h#39 (text+ko) ====

@@ -116,6 +116,7 @@
 	struct ifmedia		ic_media;	/* interface media config */
 	uint8_t			ic_myaddr[IEEE80211_ADDR_LEN];
 	struct callout		ic_inact;	/* inactivity processing */
+	struct task		ic_parent_task;	/* deferred parent processing */
 
 	uint32_t		ic_flags;	/* state flags */
 	uint32_t		ic_flags_ext;	/* extended state flags */