Date: Thu, 29 Sep 2011 13:55:47 +0800 From: Adrian Chadd <adrian@freebsd.org> To: Edgar Martinez <emartinez@kbcnetworks.com> Cc: "freebsd-wireless@freebsd.org" <freebsd-wireless@freebsd.org> Subject: Re: PANIC - SWBMISS (9.0-CURRENT) Message-ID: <CAJ-Vmom8WTWCjssChHQNYfphvkrbHO1tjKxPASUHNvHqzwJJew@mail.gmail.com> In-Reply-To: <957EB052144AA64AB39F7AB268783201022F835CCE@VA3DIAXVS881.RED001.local> References: <957EB052144AA64AB39F7AB268783201022F835CCE@VA3DIAXVS881.RED001.local>
next in thread | previous in thread | raw e-mail | index | archive | help
Hm, how are the interfaces configured? You're doing wds, I wonder how
the heck that works. :)
There's a locking problem with how software beacon miss is handled:
* sta_beacon_miss doesn't grab any lock, so I think its possible that
the swbmiss callout is being run on another CPU whilst
sta_becaon_miss() stops the callout and changes the state to S_ASSOC
or S_SCAN
* sta_newstate() grabs the ic lock, then it changes the state
(vap->iv_state = nstate) before it cancels the callout. Again, the
problem here is that the swbmiss callout may be scheduled during this
and there's no locking in sta_beacon_miss.
What should the solution be?
* should sta_beacon_miss (and the tdma one too?) grab the ic lock? i
think so, but I'd have to audit all the functions that it calls to
ensure it can be called with the ic lock held. In fact, I did a 30
second audit and it can't just hold the ic lock - as the calls to
ieeee80211_new_state() need the ic lock to be not held as it grabs the
lock itself. So we can't hold the lock for the whole function.
* should ieee80211_swbmiss() be called with the ic lock held? I think
so. That way if something external changes state, it should first grab
the lock, change the state, then cancel the swbmiss timer.
What I think should happen is that the beacon miss handler should be
called with the ic lock held. That way a state change can't occur
whilst its processing. That's going to take a bit of auditing though.
So here, try this patch; it may make things worse, it may make things
slightly better. I'm not sure. It just tries to eliminate a couple of
the race conditions that I found and makes sure ieee80211_swbmiss is
called with the ic lock held. I haven't tried to fix the more general
problem though.
Adrian
[adrian@pcbsd-2547] /data/freebsd/mips/if_ath_tx/src/sys/net80211> svn
diff ieee80211_sta.c ieee80211_tdma.c ieee80211_proto.c
Index: ieee80211_sta.c
===================================================================
--- ieee80211_sta.c (revision 225723)
+++ ieee80211_sta.c (working copy)
@@ -145,7 +145,9 @@
return;
}
+ IEEE80211_LOCK(ic);
callout_stop(&vap->iv_swbmiss);
+ IEEE80211_UNLOCK(ic);
vap->iv_bmiss_count = 0;
vap->iv_stats.is_beacon_miss++;
if (vap->iv_roaming == IEEE80211_ROAMING_AUTO) {
Index: ieee80211_tdma.c
===================================================================
--- ieee80211_tdma.c (revision 225610)
+++ ieee80211_tdma.c (working copy)
@@ -295,7 +295,9 @@
"beacon miss, mode %u state %s\n",
vap->iv_opmode, ieee80211_state_name[vap->iv_state]);
+ IEEE80211_LOCK(ic);
callout_stop(&vap->iv_swbmiss);
+ IEEE80211_UNLOCK(ic);
if (ts->tdma_peer != NULL) { /* XXX? can this be null? */
ieee80211_notify_node_leave(vap->iv_bss);
Index: ieee80211_proto.c
===================================================================
--- ieee80211_proto.c (revision 225610)
+++ ieee80211_proto.c (working copy)
@@ -193,7 +193,8 @@
vap->iv_rtsthreshold = IEEE80211_RTS_DEFAULT;
vap->iv_fragthreshold = IEEE80211_FRAG_DEFAULT;
vap->iv_bmiss_max = IEEE80211_BMISS_MAX;
- callout_init(&vap->iv_swbmiss, CALLOUT_MPSAFE);
+ callout_init_mtx(&vap->iv_swbmiss, IEEE80211_LOCK_OBJ(ic),
+ CALLOUT_MPSAFE);
callout_init(&vap->iv_mgtsend, CALLOUT_MPSAFE);
TASK_INIT(&vap->iv_nstate_task, 0, ieee80211_newstate_cb, vap);
TASK_INIT(&vap->iv_swbmiss_task, 0, beacon_swmiss, vap);
@@ -1448,6 +1449,8 @@
struct ieee80211vap *vap = arg;
struct ieee80211com *ic = vap->iv_ic;
+ IEEE80211_LOCK_ASSERT(ic);
+
/* XXX sleep state? */
KASSERT(vap->iv_state == IEEE80211_S_RUN,
("wrong state %d", vap->iv_state));
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmom8WTWCjssChHQNYfphvkrbHO1tjKxPASUHNvHqzwJJew>