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

[Brandon Gooch]

On Sat, Dec 5, 2009 at 2:50 AM, Benjamin Kaduk <kaduk at mit.edu> wrote:
> The following reply was made to PR kern/140036; it has been noted by GNATS.
>
> From: Benjamin Kaduk <kaduk at MIT.EDU>
> To: Bernhard Schmidt <bschmidt at techwires.net>, sam at freebsd.org
> Cc: bug-followup at freebsd.org
> Subject: Re: kern/140036: [iwn] [lor] lock order reversal with iwn0_com_lock
>  and iwn0 softc lock
> Date: Fri, 4 Dec 2009 21:49:42 -0500 (EST)
>
>  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
>
>  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.
>

Ben, have you tried Bernhard Schmidt's driver? He's recently updated
it with a reordering of locking code:

http://svn.techwires.net/viewvc/viewvc.cgi/svnrepos/projects/freebsd/sys/dev/iwn/if_iwn.c?view=log

I've been using the code from his repository for a while, and it's
much more stable than what's in the tree currently.

-Brandon