On 10 Sep, Don Lewis wrote: > On 10 Sep, Bruce Evans wrote: >> On Tue, 10 Sep 2002, Don Lewis wrote: >> >>> On 10 Sep, Bruce Evans wrote: > >>> > The changes are obviously just cleanups for leaf file systems, but I >>> > wonder why everything wasn't always locked at the top. Could it have >>> > been because locking all the way down is harmful?
>> I think this is because some places are still worrying about using >> link(2) on directories. I think there aren't any significant problems >> when vp is not a directory. Hard linking of directories was permitted >> (and perhaps even supported) until relatively recently: > > I suspect that you are right about this. If so, it should be able to > let namei() lock the first vnode since we check its type and bail out if > it is a directory before the second namei() call. I looked at namei() and ufs_lookup() and found a problem with telling the first namei() to grab the lock. It appears that the *_lookup() routines always lock the child vnode, and namei() later releases the lock if LOCKLEAF is not set. If we passed LOCKLEAF to the first namei(), then link("/foo/bar", "/foo/bar") would attempt to lock "bar" twice. Because we now bail out early if the first vnode turns out to be a pointer, I believe we can dispense with the vnode inequality test and always grab the lock. BTW, is it safe to call ASSERT_VOP_UNLOCKED() in the SMP case after the reference has been dropped with vput() or vrele()? Here's a replacement for the kern_link() patch in my previous patch file: Index: vfs_syscalls.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.285 diff -u -r1.285 vfs_syscalls.c --- vfs_syscalls.c 1 Sep 2002 20:37:28 -0000 1.285 +++ vfs_syscalls.c 10 Sep 2002 11:28:01 -0000 @@ -1027,10 +1027,12 @@ if (nd.ni_vp != NULL) { vrele(nd.ni_vp); error = EEXIST; - } else { + } else if ((error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td)) + == 0) { VOP_LEASE(nd.ni_dvp, td, td->td_ucred, LEASE_WRITE); VOP_LEASE(vp, td, td->td_ucred, LEASE_WRITE); error = VOP_LINK(nd.ni_dvp, vp, &nd.ni_cnd); + VOP_UNLOCK(vp, 0, td); } NDFREE(&nd, NDF_ONLY_PNBUF); vput(nd.ni_dvp); To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message