[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: PR/38141 CVS commit: src/sys
The following reply was made to PR kern/38141; it has been noted by GNATS.
From: Andrew Doran <ad%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost, yamt%mwd.biglobe.ne.jp@localhost
Subject: Re: PR/38141 CVS commit: src/sys
Date: Sun, 4 May 2008 01:21:58 +0100
On Sun, May 04, 2008 at 12:00:12AM +0000, Andrew Doran wrote:
> The following reply was made to PR kern/38141; it has been noted by GNATS.
> From: Andrew Doran <ad%netbsd.org@localhost>
> To: YAMAMOTO Takashi <yamt%mwd.biglobe.ne.jp@localhost>
> Cc: gnats-bugs%NetBSD.org@localhost
> Subject: Re: PR/38141 CVS commit: src/sys
> Date: Sun, 4 May 2008 00:57:01 +0100
> On Sat, May 03, 2008 at 11:05:16PM +0900, YAMAMOTO Takashi wrote:
> > > Log Message:
> > > PR kern/38141 lookup/vfs_busy acquire rwlock recursively
> > >
> > > Until the code paths are fixed properly, put in place an ugly
> > > to make it safe to recursively acquire a read lock on a mount.
> > unfortunately, recursive lock was not necessary to triger
> > a deadlock. i saw the following while running "build.sh -j128".
> > (well, the actual deadlock i saw had 10 or more vnodes involved.
> > the following is a simplified version for explanation.)
> > LWP1 in vn_open()
> > 1. lock a directory vnode which is VV_ROOT.
> > LWP2 in lookup()
> > 1. vfs_busy(RW_READER)
> > 2. call VFS_ROOT, which tries to lock the VV_ROOT vnode which
> > is already locked by LWP1. => block
> > LWP3 (syncer) in sync_fsync()
> > 1. vfs_trybusy(RW_WRITER) => block
> > now, LWP1 again:
> > 2. call VOP_CREATE -> getnewvnode -> vfs_busy(RW_READER) => block
> Heh, lockmgr() strikes from beyond the grave. I guess we could make it safe
> in the short term by making vfs_busy() an even dumber kind of rwlock based
> on atomics, with no preference for writers.
> sched_fsync() could be a problem becase of VOP_LOCK -> vfs_busy(RW_WRITER).
> I suppose there is no need for it to be represented as a vnode, and we could
> make the syncer look over the list of file systems for ones that need a
> periodic VFS_SYNC().
> What do you think?
Actually, it's still not enough. Many of the existing users of vfs_busy()
would also need to be converted to vfs_trybusy(), for example in sys_fchdir:
chdir: vn_lock -> vfs_busy
unmount vfs_busy (mnt_unmounter != NULL) -> vn_lock
Main Index |
Thread Index |