Subject: Re: simple_lock: uninitialized lock
To: Juergen Hannken-Illjes <email@example.com>
From: Antti Kantee <firstname.lastname@example.org>
Date: 07/24/2007 00:06:44
On Mon Jul 23 2007 at 22:52:06 +0200, Juergen Hannken-Illjes wrote:
> On Mon, Jul 23, 2007 at 10:18:28PM +0300, Antti Kantee wrote:
> > On Mon Jul 23 2007 at 19:33:41 +0200, Juergen Hannken-Illjes wrote:
> > > With rev. 1.289 of vfs_subr.c the lock type in function vclean()
> > > was changed from LK_DRAIN to LK_EXCLUSIVE. This seems wrong.
> > Care to explain why you think it's wrong? I had several theories on
> > why it could be wrong, but none of them held water.
> I always took LK_DRAIN as to leave with an exclusive lock and no more
> waiters. Using LK_EXCLUSIVE we may still have other threads waiting
> on this lock. If thread lwp_0 in the example below had used LK_DRAIN
> it had to wait for ioflush to return from its VOP_LOCK instead of allowing
> ioflush to wake up on a now freed lock.
I assumed that locks are granted out in the order they are requested.
If that is not the case, I think the situation you describe is possible
and we should do something akin of LK_DRAIN | LK_REENABLE, or if that's
not possible, LK_DRAIN, LK_REENABLE, LK_EXCLUSIVE.
Actually, if it's easy to repeat, just try this. It's stupid, yes,
but still non-racy because of VXLOCK and let's try to prove the theory.
RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
retrieving revision 1.291
diff -p -u -r1.291 vfs_subr.c
--- vfs_subr.c 17 Jul 2007 21:14:05 -0000 1.291
+++ vfs_subr.c 23 Jul 2007 21:04:41 -0000
@@ -1535,7 +1535,9 @@ vclean(struct vnode *vp, int flags, stru
* We don't drain the lock because it might have been exported
* to upper layers by this vnode and could still be in use.
- VOP_LOCK(vp, LK_EXCLUSIVE | LK_INTERLOCK);
+ VOP_LOCK(vp, LK_DRAIN | LK_INTERLOCK);
+ VOP_UNLOCK(vp, LK_REENABLE);
+ VOP_LOCK(vp, LK_EXCLUSIVE);
* Clean out any cached data associated with the vnode.
Antti Kantee <email@example.com> Of course he runs NetBSD
"la qualité la plus indispensable du cuisinier est l'exactitude"