kern/140036: [iwn] [lor] lock order reversal with iwn0_com_lock and iwn0 softc lock

[Bernhard Schmidt]

The following reply was made to PR kern/140036; it has been noted by GNATS.

From: Bernhard Schmidt <bschmidt at techwires.net>
To: Benjamin Kaduk <kaduk at mit.edu>
Cc: sam at freebsd.org, bug-followup at freebsd.org
Subject: Re: kern/140036: [iwn] [lor] lock order reversal with iwn0_com_lock and iwn0 softc lock
Date: Sat, 5 Dec 2009 09:56:23 +0100

 On Saturday 05 December 2009 03:49:42 Benjamin Kaduk wrote:
 > Okay, I've been getting these lockups fairly frequently -- in fact,
 > there was this one room where I was getting them every five minutes
 > or so.  I hypothesized that dissociation/association events might
 > be triggers, so I set dev.iwn.0.debug=-1 and started wandering around
 > this building (which is chock full of APs).
 > I get:
 > panic: lock (sleep mutex) iwn0_com_lock not locked @
 > /usr/devel/iwn/freebsd/sys/modules/iwn/../../dev/iwn/if_iwn.c:3254
 
 Are you sure your code is in sync? there is nothing accessing the lock in 
 if_iwn.c:3254
 
 3251	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 3252		ieee80211_free_node(ni);
 3253		m_freem(m);
 3254		return ENETDOWN;
 3255	}
 
 > Unfortunately, it appears to have messed up my machine pretty badly,
 > as only numlock blinks the LED, and I don't have a db> prompt ...
 > 
 > Trying again without the debugging spew gets a useful debugger, though.
 > "show alllocks" produces empty output (?!).
 > backtrace is:
 > kdb_enter
 > panic
 > witness_unlock
 > _mtx_unlock_flags
 > iwn_raw_xmit
 > ieee80211_send_probereq
 > beacom_miss
 > taskqueue_run
 > taskqueue_thread_loop
 > fork_exit
 > 
 > Looking at the coredump, I'm inclined to suspect this block of
 > ieee80211_proto.c (in beacon_miss() ):
 > 1.46      sam   1379:        /* XXX locking */
 >                  1380:        TAILQ_FOREACH(vap, &ic->ic_vaps, iv_next) {
 > 1.38      sam   1381:                /*
 > 1.46      sam   1382:                 * We only pass events through for sta
 >  vap's in RUN state; 1383:                 * may be too restrictive but for
 >  now this saves all the 1384:                 * handlers duplicating these
 >  checks. 1.38      sam   1385:                 */
 > 1.46      sam   1386:                if (vap->iv_opmode == IEEE80211_M_STA
 >  && 1.64      sam   1387:                    vap->iv_state >=
 >  IEEE80211_S_RUN && 1.46      sam   1388:                    vap->iv_bmiss
 >  != NULL)
 >                  1389:                        vap->iv_bmiss(vap);
 > 
 > 
 > Sam, do you have any thoughts as to why throwing a
 > IEEE80211_LOCK(ic)/IEEE80211_UNLOCK(ic) block around the TAILQ_FOREACH
 > might not be a good idea?
 > I'm currently running with that, which gives me a new LOR
 > (iwn_com_lock @ /usr/src/sys/net80211/ieee80211_scan.c:683 and
 > iwn0 @ /usr/devel/iwn/freebsd/sys/modules/iwn/../../dev/iwn/if_iwn.c:3289)
 > but it hasn't locked up or panic()ed on me, yet.
 > 
 > 
 > I am happy to pull more information from the coredump if needed.
 > 
 > Thanks,
 > 
 > Ben Kaduk
 > 
 
 -- 
 Bernhard