panic in 8.3-PRERELEASE

[Konstantin Belousov]

On Wed, Feb 22, 2012 at 06:53:55PM -0500, Rick Macklem wrote:
> 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.
I do not see a need in the changing to mtx_sleep, esp. if other places
in nfsd use msleep(). There are more then 570 uses of msleep(9) in the
kernel, and undefined number of them in third-party modules.

> 
> 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.)
I think it is usual routine bugfix, which is as good to have in release
as any other bugfix. 8.3 is in the stabilization period, made exactly
for pushing bugfixes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20120223/384f48a3/attachment.pgp