panic in 8.3-PRERELEASE

[Rick Macklem]

Konstantin Belousov wrote:
> On Wed, Feb 22, 2012 at 11:29:40AM -0500, Rick Macklem wrote:
> > Hiroki Sato wrote:
> > > Hi,
> > >
> > > Just a report, but I got the following panic on an NFS server
> > > running
> > > 8.3-PRERELEASE:
> > >
> > > ----(from here)----
> > > pool.allbsd.org dumped core - see /var/crash/vmcore.0
> > >
> > > Tue Feb 21 10:59:44 JST 2012
> > >
> > > FreeBSD pool.allbsd.org 8.3-PRERELEASE FreeBSD 8.3-PRERELEASE #7:
> > > Thu
> > > Feb 16 19:29:19 JST 2012
> > > hrs at pool.allbsd.org:/usr/obj/usr/src/sys/POOL
> > > amd64
> > >
> > > panic: Assertion lock == sq->sq_lock failed at
> > > /usr/src/sys/kern/subr_sleepqueue.c:335
> > >
> > Oops, I didn't know that mixing msleep() and tsleep() calls on the
> > same
> > event wasn't allowed.
> > There are two places in the code where it did a:
> >   mtx_unlock();
> >   tsleep();
> > left over from the days when it was written for OpenBSD.
> This sequence allows to lost the wakeup which is happen right after
> cache unlock (together with clearing the RC_WANTED flag) but before
> the thread enters sleep state. The tsleep has a timeout so thread
> should
> recover in 10 seconds, but still.
> 
Yes.

> Anyway, you should use consistent outer lock for the same wchan, i.e.
> no lock (tsleep) or mtx (msleep), but not mix them.
> >
> > I don't think the mix would actually break anything, except that the
> > MPASS() assertion fails, but I've cc'd jhb@ since he seems to have
> > been
> > the author of the sleep() stuff.
> >
> > Anyhow, please try the attached patch which replaces the
> > mtx_unlock(); tsleep(); with
> > msleep()s using PDROP. If the attachment gets lost, the patch is
> > also here:
> >   http://people.freebsd.org/~rmacklem/tsleep.patch
> >
> > Thanks for reporting this, rick
> > ps: Is mtx_lock() now preferred over msleep()?
> What do you mean ?
It appears jhb@ figured out the typo. I meant to type mtx_sleep(), not
mtx_lock().

rick