--- Bill Studenmund <wrstuden@netbsd.org> wrote: > On Sat, 6 Sep 2003,
Gary Thorpe wrote:
> 
> > I am having a problem with passing LK_SPIN as a flag to lockmgr().
> It
> > raises a panic in the following code section,
> src/sys/kern/kern_lock.c,
> > 1.72, in lockmgr() (current kernel is compiled with diagnostic):
> >
> > #ifdef DIAGNOSTIC /* { */
> > 	/*
> > 	 * Don't allow spins on sleep locks and don't allow sleeps
> > 	 * on spin locks.
> > 	 */
> > 	if ((flags ^ lkp->lk_flags) & LK_SPIN)
> > 		panic("lockmgr: sleep/spin mismatch");
> > #endif /* } */
> >
> >
> > The flag is LK_SPIN|LK_EXCLUSIVE|LK_RECURSEFAIL. lkp->lk_flags is
> equal
> > to 0.
> > (LK_SPIN|LK_...) ^ 0 = (LK_SPIN|LK_...)
> > (LK_SPIN|LK_...) & LK_SPIN = nonzero
> > And so the panic is called.
> 
> As well it should.
> 
> > I call lockinit using something similar to:
> > lockinit(lkp, prio, wmesg, (1 * hz), 0);
> 
> If it's going to be a spin lock, you should initialize it as such.
> 
> > Which is why lkp->lk_flags is 0. I then call lockmgr with something
> > similar to:
> > lockmgr(lkp, LK_SPIN|LK_EXCLUSIVE|LK_RECURSEFAIL, NULL);
> >
> > It seems perfectly legal to call lockinit() with a 0 flag. I need
> to do
> > this because the thread calling lockmgr() may need to sleep or spin
> > depending on the situation (the spin case is in a network ioctl for
> a
> > device: is sleeping safe?). Is this behaviour possible? I have not
> > actually found code in the tree that uses LK_SPIN though, so could
> this
> > be a genuine error that has not been found until now?
> 
> That's not what LK_SPIN is about, it seems. LK_SPIN is about making a
> spin
> lock, not a sleeper lock. A lock should be made one way or the other,
> then
> always used that same way. Thus the code is correct.
> 
> Why are you calling lockmgr() locks in case where sleeping is not
> possible? That's the bug...

LK_SPIN exists, but I made the mistake of trying to use it.

> 
> > Should the code be:
> >
> > #ifdef DIAGNOSTIC /* { */
> > 	/*
> > 	 * Don't allow spins on sleep locks and don't allow sleeps
> > 	 * on spin locks.
> > 	 */
> > 	if (((flags|lkp->lk_flags) & LK_SPIN) &&
> >                 ((flags|lkp->lk_flags) & LK_SLEEPFAIL)))
> > 		panic("lockmgr: sleep/spin mismatch");
> > #endif /* } */
> 
> Nope.
> 
> Take care,
> 
> Bill
> 

Ok, so basically, the only way the code won't panic is if both
lockinit() and lockmgr() specify LK_SPIN with each invocation for the
same lock. However, looking at the code it is possible to do this:

lockinit(lkp, prio, wmesg, 0, LK_SPIN|LK_SLEEPFAIL);