PERFORCE change 139292 for review
Sam Leffler
sam at FreeBSD.org
Thu Apr 3 21:04:26 UTC 2008
http://perforce.freebsd.org/chv.cgi?CH=139292
Change 139292 by sam at sam_ebb on 2008/04/03 21:03:33
Use the com lock to cover scan state changes instead of doing
some hoaky stuff that was race prone:
o hold the com lock across the scan timer using callout_init_mtx
o expand scope of various com locking to cover most all scan
work (still have to deal with adding scan entries from the rx
thread which is mostly ok as the scan policy module locks it's
own internal data structures but we the mindwell handling is
still safe only because rx processing is single-threaded and
covered by IEEE80211_F_SCAN)
Note these changes mean various callbacks into drivers are now
done with the com lock held. It also means that drivers that
call into the scan code because the device does scanning in
firmware may nee locking changes to avoid LOR's.
Affected files ...
.. //depot/projects/vap/sys/net80211/ieee80211_scan.c#19 edit
Differences ...
==== //depot/projects/vap/sys/net80211/ieee80211_scan.c#19 (text+ko) ====
@@ -102,7 +102,7 @@
ic->ic_scan = NULL;
return;
}
- callout_init(&ss->ss_scan_timer, CALLOUT_MPSAFE);
+ callout_init_mtx(&ss->ss_scan_timer, &ic->ic_comlock, 0);
ic->ic_scan = &ss->base;
ic->ic_scan_curchan = scan_curchan;
@@ -446,25 +446,17 @@
/*
* Start a scan unless one is already going.
*/
-int
-ieee80211_start_scan(struct ieee80211vap *vap, int flags,
- u_int duration, u_int mindwell, u_int maxdwell,
+static int
+start_scan_locked(const struct ieee80211_scanner *scan,
+ struct ieee80211vap *vap, int flags, u_int duration,
+ u_int mindwell, u_int maxdwell,
u_int nssid, const struct ieee80211_scan_ssid ssids[])
{
struct ieee80211com *ic = vap->iv_ic;
- const struct ieee80211_scanner *scan;
struct ieee80211_scan_state *ss = ic->ic_scan;
- scan = ieee80211_scanner_get(vap->iv_opmode);
- if (scan == NULL) {
- IEEE80211_DPRINTF(vap, IEEE80211_MSG_SCAN,
- "%s: no scanner support for %s mode\n",
- __func__, ieee80211_opmode_name[vap->iv_opmode]);
- /* XXX stat */
- return 0;
- }
+ IEEE80211_LOCK_ASSERT(ic);
- IEEE80211_LOCK(ic);
if (ic->ic_flags & IEEE80211_F_CSAPENDING) {
IEEE80211_DPRINTF(vap, IEEE80211_MSG_SCAN,
"%s: scan inhibited by pending channel change\n", __func__);
@@ -515,10 +507,36 @@
"%s: %s scan already in progress\n", __func__,
ss->ss_flags & IEEE80211_SCAN_ACTIVE ? "active" : "passive");
}
+ return (ic->ic_flags & IEEE80211_F_SCAN);
+}
+
+/*
+ * Start a scan unless one is already going.
+ */
+int
+ieee80211_start_scan(struct ieee80211vap *vap, int flags,
+ u_int duration, u_int mindwell, u_int maxdwell,
+ u_int nssid, const struct ieee80211_scan_ssid ssids[])
+{
+ struct ieee80211com *ic = vap->iv_ic;
+ const struct ieee80211_scanner *scan;
+ int result;
+
+ scan = ieee80211_scanner_get(vap->iv_opmode);
+ if (scan == NULL) {
+ IEEE80211_DPRINTF(vap, IEEE80211_MSG_SCAN,
+ "%s: no scanner support for %s mode\n",
+ __func__, ieee80211_opmode_name[vap->iv_opmode]);
+ /* XXX stat */
+ return 0;
+ }
+
+ IEEE80211_LOCK(ic);
+ result = start_scan_locked(scan, vap, flags, duration,
+ mindwell, maxdwell, nssid, ssids);
IEEE80211_UNLOCK(ic);
- /* NB: racey, does it matter? */
- return (ic->ic_flags & IEEE80211_F_SCAN);
+ return result;
}
/*
@@ -533,7 +551,7 @@
struct ieee80211com *ic = vap->iv_ic;
struct ieee80211_scan_state *ss = ic->ic_scan;
const struct ieee80211_scanner *scan;
- int checkscanlist = 0;
+ int checkscanlist = 0, result;
scan = ieee80211_scanner_get(vap->iv_opmode);
if (scan == NULL) {
@@ -594,19 +612,22 @@
checkscanlist = 1;
}
}
- IEEE80211_UNLOCK(ic);
if (checkscanlist) {
if (ss->ss_ops->scan_end(ss, vap)) {
/* found an ap, just clear the flag */
ic->ic_flags &= ~IEEE80211_F_SCAN;
ieee80211_notify_scan_done(vap);
+ IEEE80211_UNLOCK(ic);
return 1;
}
/* no ap, clear the flag before starting a scan */
ic->ic_flags &= ~IEEE80211_F_SCAN;
}
- return ieee80211_start_scan(vap, flags,
- duration, mindwell, maxdwell, nssid, ssids);
+ result = start_scan_locked(scan, vap, flags, duration,
+ mindwell, maxdwell, nssid, ssids);
+ IEEE80211_UNLOCK(ic);
+
+ return result;
}
/*
@@ -784,13 +805,17 @@
void
ieee80211_scan_next(struct ieee80211vap *vap)
{
+ struct ieee80211com *ic = vap->iv_ic;
+
/*
* XXX: We might need/want to decouple context here by either:
* callout_reset(&SCAN_PRIVATE(ss)->ss_scan_timer, 0, scan_next, ss);
* or using a taskqueue. Let's see what kind of problems direct
* dispatch has for now.
*/
- scan_next(vap->iv_ic->ic_scan);
+ IEEE80211_LOCK(ic);
+ scan_next(ic->ic_scan);
+ IEEE80211_UNLOCK(ic);
}
/*
@@ -800,10 +825,14 @@
void
ieee80211_scan_done(struct ieee80211vap *vap)
{
- struct ieee80211_scan_state *ss = vap->iv_ic->ic_scan;
+ struct ieee80211com *ic = vap->iv_ic;
+ struct ieee80211_scan_state *ss;
+ IEEE80211_LOCK(ic);
+ ss = ic->ic_scan;
ss->ss_next = ss->ss_last; /* all channels are complete */
scan_next(ss);
+ IEEE80211_UNLOCK(ic);
}
/*
@@ -880,14 +909,12 @@
struct ieee80211com *ic = vap->iv_ic;
struct ieee80211_channel *chan;
unsigned long maxdwell, scanend;
- int scanning, scandone;
+ int scandone;
+
+ IEEE80211_LOCK_ASSERT(ic);
- IEEE80211_LOCK(ic);
- scanning = (ic->ic_flags & IEEE80211_F_SCAN) != 0;
- IEEE80211_UNLOCK(ic);
- if (!scanning) /* canceled */
+ if ((ic->ic_flags & IEEE80211_F_SCAN) == 0)
return;
-
again:
scandone = (ss->ss_next >= ss->ss_last) ||
(SCAN_PRIVATE(ss)->ss_iflags & ISCAN_CANCEL) != 0;
@@ -1079,13 +1106,13 @@
struct ieee80211com *ic = vap->iv_ic;
struct ieee80211_scan_state *ss = ic->ic_scan;
+ /* XXX locking */
/*
* Frames received during startup are discarded to avoid
* using scan state setup on the initial entry to the timer
* callback. This can occur because the device may enable
* rx prior to our doing the initial channel change in the
- * timer routine (we defer the channel change to the timer
- * code to simplify locking on linux).
+ * timer routine.
*/
if (SCAN_PRIVATE(ss)->ss_iflags & ISCAN_DISCARD)
return;
More information about the p4-projects
mailing list