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

Reply via email to