Date: Mon, 9 Sep 2002 21:19:29 -0700 (PDT) From: Don Lewis <dl-freebsd@catspoiler.org> To: rwatson@FreeBSD.org Cc: current@FreeBSD.org, jeff@FreeBSD.org Subject: Re: vnode lock assertion problem in nfs_link() Message-ID: <200209100419.g8A4JTwr093971@gw.catspoiler.org> In-Reply-To: <Pine.NEB.3.96L.1020909200649.4317A-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9 Sep, Robert Watson wrote:
> What I'd actually like to do is lock vp on going in to the VOP. I need to
> grab the lock in the link() code anyway to do the MAC check. UFS and
> others all immediately lock the vnode on entry anyway...
Here's a patch to implement this. It compiles and seems to work OK, but
needs review. There are a couple of issues that definitely need a look:
I believe the new vn_lock() call in kern_link() should use the
LK_RETRY flag.
The locking changes in union_link() need a thorough review,
though the light testing of that I performed didn't turn up any
glaring problems.
The VOP_LINK(9) man page needs would also need to be updated. I also
missed a change to the block comment before union_link() :-(
Index: fs/unionfs/union_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/unionfs/union_vnops.c,v
retrieving revision 1.89
diff -u -r1.89 union_vnops.c
--- fs/unionfs/union_vnops.c 4 Aug 2002 10:29:31 -0000 1.89
+++ fs/unionfs/union_vnops.c 10 Sep 2002 02:45:34 -0000
@@ -1250,7 +1250,6 @@
struct union_node *tun = VTOUNION(ap->a_vp);
if (tun->un_uppervp == NULLVP) {
- vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY, td);
#if 0
if (dun->un_uppervp == tun->un_dirvp) {
if (dun->un_flags & UN_ULOCK) {
@@ -1267,9 +1266,9 @@
dun->un_flags |= UN_ULOCK;
}
#endif
- VOP_UNLOCK(ap->a_vp, 0, td);
}
vp = tun->un_uppervp;
+ vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
}
if (error)
@@ -1288,6 +1287,8 @@
VOP_UNLOCK(ap->a_tdvp, 0, td); /* unlock calling node */
error = VOP_LINK(tdvp, vp, cnp); /* call link on upper */
+ if (ap->a_tdvp->v_op == ap->a_vp->v_op)
+ VOP_UNLOCK(vp, 0, td);
/*
* We have to unlock tdvp prior to relocking our calling node in
* order to avoid a deadlock.
Index: gnu/ext2fs/ext2_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/gnu/ext2fs/ext2_vnops.c,v
retrieving revision 1.68
diff -u -r1.68 ext2_vnops.c
--- gnu/ext2fs/ext2_vnops.c 15 Aug 2002 20:55:01 -0000 1.68
+++ gnu/ext2fs/ext2_vnops.c 10 Sep 2002 03:09:33 -0000
@@ -827,7 +827,6 @@
struct vnode *vp = ap->a_vp;
struct vnode *tdvp = ap->a_tdvp;
struct componentname *cnp = ap->a_cnp;
- struct thread *td = cnp->cn_thread;
struct inode *ip;
int error;
@@ -837,19 +836,16 @@
#endif
if (tdvp->v_mount != vp->v_mount) {
error = EXDEV;
- goto out2;
- }
- if (tdvp != vp && (error = vn_lock(vp, LK_EXCLUSIVE, td))) {
- goto out2;
+ goto out;
}
ip = VTOI(vp);
if ((nlink_t)ip->i_nlink >= LINK_MAX) {
error = EMLINK;
- goto out1;
+ goto out;
}
if (ip->i_flags & (IMMUTABLE | APPEND)) {
error = EPERM;
- goto out1;
+ goto out;
}
ip->i_nlink++;
ip->i_flag |= IN_CHANGE;
@@ -860,10 +856,7 @@
ip->i_nlink--;
ip->i_flag |= IN_CHANGE;
}
-out1:
- if (tdvp != vp)
- VOP_UNLOCK(vp, 0, td);
-out2:
+out:
return (error);
}
Index: kern/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
--- kern/vfs_syscalls.c 1 Sep 2002 20:37:28 -0000 1.285
+++ kern/vfs_syscalls.c 10 Sep 2002 03:00:50 -0000
@@ -1027,10 +1027,13 @@
if (nd.ni_vp != NULL) {
vrele(nd.ni_vp);
error = EEXIST;
- } else {
+ } else if (nd.ni_dvp == vp ||
+ (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);
+ if (nd.ni_dvp != vp)
+ VOP_UNLOCK(vp, 0, td);
}
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(nd.ni_dvp);
Index: kern/vnode_if.src
===================================================================
RCS file: /home/ncvs/src/sys/kern/vnode_if.src,v
retrieving revision 1.56
diff -u -r1.56 vnode_if.src
--- kern/vnode_if.src 5 Sep 2002 20:56:14 -0000 1.56
+++ kern/vnode_if.src 10 Sep 2002 02:29:56 -0000
@@ -261,7 +261,7 @@
#
#% link tdvp L L L
-#% link vp U U U
+#% link vp L L L
#
vop_link {
IN struct vnode *tdvp;
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.204
diff -u -r1.204 ufs_vnops.c
--- ufs/ufs/ufs_vnops.c 15 Aug 2002 20:55:08 -0000 1.204
+++ ufs/ufs/ufs_vnops.c 10 Sep 2002 03:08:48 -0000
@@ -814,7 +814,6 @@
struct vnode *vp = ap->a_vp;
struct vnode *tdvp = ap->a_tdvp;
struct componentname *cnp = ap->a_cnp;
- struct thread *td = cnp->cn_thread;
struct inode *ip;
struct direct newdir;
int error;
@@ -825,19 +824,16 @@
#endif
if (tdvp->v_mount != vp->v_mount) {
error = EXDEV;
- goto out2;
- }
- if (tdvp != vp && (error = vn_lock(vp, LK_EXCLUSIVE, td))) {
- goto out2;
+ goto out;
}
ip = VTOI(vp);
if ((nlink_t)ip->i_nlink >= LINK_MAX) {
error = EMLINK;
- goto out1;
+ goto out;
}
if (ip->i_flags & (IMMUTABLE | APPEND)) {
error = EPERM;
- goto out1;
+ goto out;
}
ip->i_effnlink++;
ip->i_nlink++;
@@ -859,10 +855,7 @@
if (DOINGSOFTDEP(vp))
softdep_change_linkcnt(ip);
}
-out1:
- if (tdvp != vp)
- VOP_UNLOCK(vp, 0, td);
-out2:
+out:
VN_KNOTE(vp, NOTE_LINK);
VN_KNOTE(tdvp, NOTE_WRITE);
return (error);
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?200209100419.g8A4JTwr093971>