Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Sep 2002 04:54:39 -0700 (PDT)
From:      Don Lewis <dl-freebsd@catspoiler.org>
To:        bde@zeta.org.au
Cc:        rwatson@FreeBSD.ORG, current@FreeBSD.ORG, jeff@FreeBSD.ORG
Subject:   Re: vnode lock assertion problem in nfs_link()
Message-ID:  <200209101154.g8ABsdwr094898@gw.catspoiler.org>
In-Reply-To: <200209101046.g8AAk7wr094789@gw.catspoiler.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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 majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200209101154.g8ABsdwr094898>