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:
[snip]
> > 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 managed to get this trace:
> > 
> > lwp_0	sys_munmap -> uvm_unmap_detach -> vrele ->
> > 	    vn_lock -> VOP_LOCK -> ufs_lock -> SUCCESS
> > 
> > ioflush	sched_sync -> VOP_FSYNC -> sync_fsync -> ffs_sync -> vget ->
> > 	    vn_lock -> VOP_LOCK -> ufs_lock -> EBUSY
> > 	...
> > 	    vrele -> vn_lock -> VOP_LOCK -> ufs_lock -> SLEEP
> 
> Is this the problematic thread?

Yes, when it wakes up after lwp_0 has run vrecycle -> vgonel -> pool_put.

> > lwp_0	sys_munmap -> uvm_unmap_detach -> vrele ->
> > 	    VOP_INACTIVE -> ufs_inactive -> VOP_UNLOCK -> ufs_unlock
> > 	    VOP_INACTIVE -> ufs_inactive -> vrecycle -> vgonel -> vclean ->
> > 		VOP_LOCK (*) -> ufs_lock -> SUCCESS
> > 	    VOP_INACTIVE -> ufs_inactive -> vrecycle -> vgonel -> pool_put
> > 
> > (*) This is the lock that changed.

-- 
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)