From owner-freebsd-wireless@FreeBSD.ORG Thu Sep 29 05:55:48 2011 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DD93F106566C for ; Thu, 29 Sep 2011 05:55:48 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-yw0-f54.google.com (mail-yw0-f54.google.com [209.85.213.54]) by mx1.freebsd.org (Postfix) with ESMTP id 9E0B28FC14 for ; Thu, 29 Sep 2011 05:55:48 +0000 (UTC) Received: by ywp17 with SMTP id 17so281775ywp.13 for ; Wed, 28 Sep 2011 22:55:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=JHtFLSLdL6afmOcEAGk2q2iM5hT6dCRotCi8gtpVfRs=; b=KZoczgsjxwz2M90cMPgfVxAbt2lbDbXmBaQRrs3PWZ7fvLquZfW0BPdnXOaTXhXmON hZrKNJa8XIWp7D4ebUTWHbPV2oQAfrUTN3KLXLusTbNim+B2nggr/e4J8YQCyP0ioOiF x/+p5MYbo81w/3z3iynDM+k1oBR43ZBCVYK/E= MIME-Version: 1.0 Received: by 10.236.129.242 with SMTP id h78mr61240725yhi.89.1317275747948; Wed, 28 Sep 2011 22:55:47 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.236.111.42 with HTTP; Wed, 28 Sep 2011 22:55:47 -0700 (PDT) In-Reply-To: <957EB052144AA64AB39F7AB268783201022F835CCE@VA3DIAXVS881.RED001.local> References: <957EB052144AA64AB39F7AB268783201022F835CCE@VA3DIAXVS881.RED001.local> Date: Thu, 29 Sep 2011 13:55:47 +0800 X-Google-Sender-Auth: BGTJf4OTEzyRcqoJqci_UmKSCL8 Message-ID: From: Adrian Chadd To: Edgar Martinez Content-Type: text/plain; charset=ISO-8859-1 Cc: "freebsd-wireless@freebsd.org" Subject: Re: PANIC - SWBMISS (9.0-CURRENT) X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Sep 2011 05:55:48 -0000 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));