--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sun, Dec 03, 2006 at 01:06:59PM -0800, Bill Studenmund wrote:
> > wouldn't getcleanvnode() skip over this locked VLAYER vnode?
> 
> Hmm... It's supposed to.
> 
> Please see Darrin's analysis in the PR. If you can see something that's 
> not right in it, please comment!

I've figured out what's going wrong with the VOP_ISLOCKED() call.
lockstatus() can return 0 incorrectly if the lk_flags contains LK_WANT_EXCL
or LK_WANT_UPGRADE.  so if we have this sequence of events then we'll
hit the deadlock:

thread 1 locks the lower vnode with LK_SHARED and sleeps.
thread 2 sets LK_WANT_EXCL on the vnode lock and sleeps.
thread 1 wakes up and releases the vnode lock.
thread 3 tries to recycle the upper vp, set XLOCK but sleeps on VOP_LOCK()
         because LK_WANT_EXCL is set
thread 2 wakes up, clears LK_WANT_EXCL, finishes acquiring the lock,
         and then calls vget() on the upper vnode and sleeps on XLOCK.

now threads 2 and 3 are deadlocked.

the attached patch fixes this problem, and with both this patch and
my other patch for the vrele() issue applied, a stress test survives
much longer than it previously took to trigger a hang.

any objection to going with this approach?

-Chuck

--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.lockstatus"

Index: kern/kern_lock.c
===================================================================
RCS file: /home/chs/netbsd/cvs/src/sys/kern/kern_lock.c,v
retrieving revision 1.102
diff -u -p -r1.102 kern_lock.c
--- kern/kern_lock.c	1 Nov 2006 10:17:58 -0000	1.102
+++ kern/kern_lock.c	5 Dec 2006 16:34:32 -0000
@@ -460,6 +460,8 @@ lockstatus(struct lock *lkp)
 			lock_type = LK_EXCLOTHER;
 	} else if (lkp->lk_sharecount != 0)
 		lock_type = LK_SHARED;
+	else if (lkp->lk_flags & (LK_WANT_EXCL | LK_WANT_UPGRADE))
+		lock_type = LK_EXCLOTHER;
 	INTERLOCK_RELEASE(lkp, lkp->lk_flags, s);
 	return (lock_type);
 }

--HlL+5n6rz5pIUxbD--