panic in 8.3-PRERELEASE

[Rick Macklem]

John Baldwin wrote:
> On Wednesday, February 22, 2012 2:24:14 pm 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.
> >
> > Anyway, you should use consistent outer lock for the same wchan,
> > i.e.
> > no lock (tsleep) or mtx (msleep), but not mix them.
> 
> Correct.
> 
> > > 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 ?
> 
> mtx_sleep() is preferred over msleep(), but I doubt I will remove
> msleep()
> anytime soon.
> 
Ok, I'll redo the patch with mtx_sleep() and get one of you guys to
review it.

One question. Do you think this is serious enough to worry about for
8.3? (Just wondering if I need to rush a patch into head with a 1 week
MFC. I realize it would still be up to re@, even if I rush it.)

Thanks for the useful comments, rick

> --
> John Baldwin
> _______________________________________________
> freebsd-stable at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to
> "freebsd-stable-unsubscribe at freebsd.org"