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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to